Keep the paint highlight off the texture colour preview between strokes

The green highlight and tint no longer cover a colour preview, and return while a stroke is
painted. The Fast view also keeps the other parts of a multi-part object.

src/libslic3r/TextureDisplacement.cpp

@@ -247,6 +247,20 @@ void smooth_height_pixels(std::vector<uint8_t> &pixels, int width, int height, f
 }
 } // namespace
 
+bool height_texture_has_color(const TextureDisplacementLayer &layer)
+{
+    if (layer.empty())
+        return false;
+    {
+        std::lock_guard<std::mutex> lock(g_decoded_texture_cache.mutex);
+        const auto it = g_decoded_texture_cache.entries.find(layer.image_data.get());
+        if (it != g_decoded_texture_cache.entries.end() && it->second.first.lock() == layer.image_data)
+            return it->second.second.has_color();
+    }
+    // Not decoded yet. Decoding caches the raw image, so this happens once per image.
+    return decode_height_texture(layer).has_color();
+}
+
 DecodedHeightTexture decode_height_texture(const TextureDisplacementLayer &layer)
 {
     DecodedHeightTexture result;

src/libslic3r/TextureDisplacement.hpp

@@ -485,6 +485,10 @@ struct DecodedHeightTexture
 // DecodedHeightTexture if image_data is empty or is not a PNG at all.
 DecodedHeightTexture decode_height_texture(const TextureDisplacementLayer &layer);
 
+// decode_height_texture(layer).has_color(), answered from the decode cache rather than from a copy of the
+// texture - cheap enough to ask every frame. Smoothing does not change it, so the raw decode is what is read.
+bool height_texture_has_color(const TextureDisplacementLayer &layer);
+
 // Maps a linear RGB colour in [0, 1] to an index into the caller's palette, or -1 for "no colour".
 //
 // Deliberately a callback rather than a function here: matching a colour to a filament is a

src/slic3r/GUI/Gizmos/GLGizmoPainterBase.cpp

@@ -113,7 +113,7 @@ Vec3f GLGizmoPainterBase::get_tilt_up_direction() const
     return build_plate_tilt_up_direction().cast<float>();
 }
 
-void GLGizmoPainterBase::render_triangles(const Selection& selection) const
+void GLGizmoPainterBase::render_triangles(const Selection& selection, const ModelVolume* skip) const
 {
     auto* shader = wxGetApp().get_shader("mm_gouraud");
     if (!shader)
@@ -135,6 +135,8 @@ void GLGizmoPainterBase::render_triangles(const Selection& selection) const
             continue;
 
         ++mesh_id;
+        if (mv == skip)
+            continue;
 
         Transform3d trafo_matrix;
         if (m_parent.get_canvas_type() == GLCanvas3D::CanvasAssembleView) {

src/slic3r/GUI/Gizmos/GLGizmoPainterBase.hpp

@@ -231,7 +231,8 @@ public:
     bool on_mouse(const wxMouseEvent &mouse_event) override;
 
 protected:
-    virtual void render_triangles(const Selection& selection) const;
+    // Draws every model part's selector, except `skip`'s when given.
+    virtual void render_triangles(const Selection& selection, const ModelVolume* skip = nullptr) const;
     void render_cursor();
     void render_cursor_circle();
     void render_cursor_sphere(const Transform3d& trafo) const;
@@ -328,6 +329,9 @@ protected:
 
     TriangleSelector::ClippingPlane get_clipping_plane_in_volume_coordinates(const Transform3d &trafo) const;
 
+    // True while a paint or erase stroke is under way.
+    bool is_painting() const { return m_button_down != Button::None; }
+
 private:
     std::vector<std::vector<ProjectedMousePosition>> get_projected_mouse_positions(const Vec2d &mouse_position, double resolution, const std::vector<Transform3d> &trafo_matrices) const;
 

src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.cpp

@@ -592,11 +592,15 @@ void GLGizmoTextureDisplacement::render_painter_gizmo()
     // The shaded preview is different: it never actually moves geometry (it only shades), so
     // its depth is identical to the overlay's *everywhere*, not just in the unpainted area - the
     // depth-biased opaque overlay would win the depth test across the whole surface and hide the relief
-    // shading entirely. So render_triangles() is skipped for it. What is *not* skipped is
+    // shading entirely. So render_triangles() leaves the textured volume out there. What is *not* skipped is
     // render_paint_overlay(): leaving the shading as the only paint feedback meant a stroke that
     // erased paint, or added it with no texture picked, changed nothing on screen until the whole
     // preview rebuilt at stroke end - and in the true-displacement view the opaque overlay is hidden
     // by the raised surface for the same reason. The translucent tint covers both cases.
+    //
+    // A colour preview is the exception to both: the opaque overlay buries its colours under a flat plane
+    // wherever the relief does not rise, and the tint washes them green. Both are what the user steers by
+    // while painting, though, so they give way only between strokes.
     // Coalesced shaded-preview rebuild from an in-progress UV island drag (see on_island_edited): done here, at
     // most once per drawn frame, rather than synchronously in the UV canvas's mouse-move handler.
     if (m_use_shaded_preview && m_shaded_preview_dirty) {
@@ -633,22 +637,33 @@ void GLGizmoTextureDisplacement::render_painter_gizmo()
     // Hide the real volume only when something is actually going to be drawn in its place; otherwise
     // put it back. Getting this wrong leaves an invisible model, so it is decided once, here, rather
     // than per branch below.
+    ModelVolume *mv = texture_volume();
     m_parent.toggle_model_objects_visibility(true);
-    if (use_shaded || use_true_preview) {
-        if (ModelVolume *mv = texture_volume())
-            m_parent.toggle_model_objects_visibility(false, m_c->selection_info()->model_object(),
-                                                      m_c->selection_info()->get_active_instance(), mv);
-    }
+    if ((use_shaded || use_true_preview) && mv != nullptr)
+        m_parent.toggle_model_objects_visibility(false, m_c->selection_info()->model_object(),
+                                                  m_c->selection_info()->get_active_instance(), mv);
+
+    // Which colour preview is on screen decides what the highlight gives way to: the Normal mesh colours
+    // from every layer, the shaded one from the active layer only. Only a stroke brings it back - a fill
+    // tool's hover does not, so the colours stay on screen until the click. The debug view shows a captured
+    // stage, not a colour preview, so it keeps its highlight.
+    const TextureDisplacementLayer *al         = active_layer();
+    const bool                      color_view = !is_painting() && m_debug_stage < 0;
+    const bool stack_colors  = color_view && mv != nullptr && any_layer_colors(*mv);
+    const bool active_colors = color_view && al != nullptr && layer_shows_color(*al);
 
     if (use_shaded) {
         render_shaded_preview_mesh();
+        // The shaded mesh is the textured volume alone, so the other model parts are still the selectors' to draw.
+        render_triangles(selection, mv);
     } else if (use_true_preview) {
         render_preview_mesh();
 
         if (show_paint_overlay) {
             glsafe(::glEnable(GL_POLYGON_OFFSET_FILL));
             glsafe(::glPolygonOffset(-1.0f, -1.0f));
-            render_triangles(selection);
+            // Over a colour preview only the other model parts: render_preview_mesh() draws the textured one.
+            render_triangles(selection, stack_colors ? mv : nullptr);
             glsafe(::glDisable(GL_POLYGON_OFFSET_FILL));
         }
     } else {
@@ -670,14 +685,13 @@ void GLGizmoTextureDisplacement::render_painter_gizmo()
     // skipped there, and in the true-displacement view because the displaced surface rises *above*
     // the undisplaced overlay geometry and hides it exactly where the relief is strongest - in both
     // cases leaving an erase stroke with no visible effect until the next full preview rebuild.
-    if (show_paint_overlay && (use_shaded || use_true_preview))
+    if (show_paint_overlay && (use_shaded || use_true_preview) && !active_colors)
         render_paint_overlay(m_paint_overlay_glmodel);
 
     // The UV editor's island selection, shown on the model. Polled here rather than pushed: the pane
     // changes its selection in its own mouse handling, and a compare of a few ints per frame is free.
     {
-        const TextureDisplacementLayer *al = active_layer();
-        const UVEditorCanvas           *uv_canvas = wxGetApp().plater()->get_uv_editor_canvas();
+        const UVEditorCanvas *uv_canvas = wxGetApp().plater()->get_uv_editor_canvas();
         if (m_show_uv_editor && al != nullptr && al->projection_method == TextureProjectionMethod::LSCM &&
             uv_canvas != nullptr && !m_uv_editor_unwrap.empty()) {
             if (uv_canvas->selected_islands() != m_island_overlay_selection)
@@ -1717,11 +1731,12 @@ void GLGizmoTextureDisplacement::render_paint_overlay(GLModel &overlay)
     shader->set_uniform("view_model_matrix", camera.get_view_matrix() * trafo_matrix);
     shader->set_uniform("projection_matrix", camera.get_projection_matrix());
     // Translucent, and pulled toward the camera so it wins the depth test against the coincident
-    // shaded surface. Depth writes are off: this is a tint, and letting it own the depth buffer would
-    // make the wireframe and seam overlays drawn after it fight with geometry that is not really
-    // there. Blending is already enabled by render_painter_gizmo().
+    // shaded surface - and against the selectors' highlight at -1 in the Normal view, by the full depth
+    // unit OpenGL guarantees to tell apart. Depth writes are off: this is a tint, and letting it own the
+    // depth buffer would make the wireframe and seam overlays drawn after it fight with geometry that is
+    // not really there. Blending is already enabled by render_painter_gizmo().
     glsafe(::glEnable(GL_POLYGON_OFFSET_FILL));
-    glsafe(::glPolygonOffset(-1.5f, -1.5f));
+    glsafe(::glPolygonOffset(-2.f, -2.f));
     glsafe(::glDepthMask(GL_FALSE));
     overlay.render();
     glsafe(::glDepthMask(GL_TRUE));
@@ -4305,12 +4320,14 @@ TextureDisplacementFacetsData GLGizmoTextureDisplacement::facets_data_of(const M
     return out;
 }
 
+bool GLGizmoTextureDisplacement::layer_shows_color(const TextureDisplacementLayer &layer)
+{
+    return layer.color_enabled && !layer.empty() && height_texture_has_color(layer);
+}
+
 bool GLGizmoTextureDisplacement::any_layer_colors(const ModelVolume &mv)
 {
-    for (const TextureDisplacementLayer &layer : mv.texture_displacement_layers)
-        if (layer.color_enabled && !layer.empty() && decode_height_texture(layer).has_color())
-            return true;
-    return false;
+    return std::any_of(mv.texture_displacement_layers.begin(), mv.texture_displacement_layers.end(), layer_shows_color);
 }
 
 TextureColorSettings GLGizmoTextureDisplacement::color_settings_for(const ModelVolume &mv)

src/slic3r/GUI/Gizmos/GLGizmoTextureDisplacement.hpp

@@ -175,9 +175,11 @@ public:
         ColorRGBA                 color;
     };
     std::vector<PreviewColorRun> m_preview_color_runs;
-    // True if any of the volume's layers would actually colour something: colour turned on, and a
-    // texture that has colour to give. What decides whether a palette is captured into a job at all,
-    // and so whether the colour criterion and the mmu write ever run.
+    // True if the layer would actually colour something: colour turned on, and a texture that has colour
+    // to give.
+    static bool layer_shows_color(const TextureDisplacementLayer &layer);
+    // True if any of the volume's layers would. What decides whether a palette is captured into a job at
+    // all, and so whether the colour criterion and the mmu write ever run.
     static bool any_layer_colors(const ModelVolume &mv);
 
     void render_painter_gizmo() override;
@@ -708,12 +710,13 @@ private:
     GLModel m_shaded_preview_glmodel;
 
     // Translucent tint over the active layer's painted triangles, drawn on top of whichever preview
-    // is showing. The base painter's own opaque paint highlight (render_triangles()) cannot be used
-    // in either preview mode - it is coincident with the surface and simply covers it - so the only
-    // paint feedback the gizmo had was the relief itself, which meant erasing showed nothing at all
-    // until the stroke ended and the whole preview rebuilt. This is that feedback: cheap (the painted
-    // patch only), translucent (the preview stays visible through it) and rebuilt live during a
-    // stroke.
+    // is showing - except, between strokes, the active layer's own colour preview, which it would wash
+    // green (see render_painter_gizmo()). The base painter's own opaque paint highlight
+    // (render_triangles()) cannot be used in either preview mode - it is coincident with the surface
+    // and simply covers it - so the only paint feedback the gizmo had was the relief itself, which
+    // meant erasing showed nothing at all until the stroke ended and the whole preview rebuilt. This
+    // is that feedback: cheap (the painted patch only), translucent (the preview stays visible through
+    // it) and rebuilt live during a stroke.
     GLModel m_paint_overlay_glmodel;
     // The islands selected in the UV editor, tinted on the model so the pane's selection can be seen
     // in place. Rebuilt whenever the pane's selection differs from the one it was built for.

tests/libslic3r/test_texture_displacement.cpp

@@ -68,6 +68,28 @@ static std::shared_ptr<std::vector<unsigned char>> make_flat_gray_png(uint8_t va
     return std::make_shared<std::vector<unsigned char>>(std::move(bytes));
 }
 
+// The same round trip for a flat colour image, which decode_height_texture() reads through its colour path.
+static std::shared_ptr<std::vector<unsigned char>> make_flat_rgb_png(uint8_t r, uint8_t g, uint8_t b, size_t w = 4, size_t h = 4)
+{
+    std::vector<uint8_t> rgb;
+    for (size_t i = 0; i < w * h; ++i)
+        rgb.insert(rgb.end(), { r, g, b });
+    const boost::filesystem::path tmp_path = boost::filesystem::temp_directory_path()
+        / boost::filesystem::unique_path("texdisp_test_%%%%%%%%.png");
+    REQUIRE(Slic3r::png::write_rgb_to_file(tmp_path.string(), w, h, rgb));
+
+    std::vector<unsigned char> bytes;
+    {
+        std::ifstream ifs(tmp_path.string(), std::ios::binary);
+        bytes.assign(std::istreambuf_iterator<char>(ifs), std::istreambuf_iterator<char>());
+    }
+    boost::system::error_code ec;
+    boost::filesystem::remove(tmp_path, ec);
+
+    REQUIRE_FALSE(bytes.empty());
+    return std::make_shared<std::vector<unsigned char>>(std::move(bytes));
+}
+
 // A hard-edged black/white checkerboard, the worst case for a height map: every texel boundary is a
 // step, which is precisely the relief the post-process smoothing exists to round off.
 static std::shared_ptr<std::vector<unsigned char>> make_checkerboard_png(size_t w = 16, size_t h = 16)
@@ -2135,3 +2157,20 @@ TEST_CASE("A second bake beside a first comes out as fine as a single bake", "[T
     CHECK(second <= single * 5 / 4);
 }
 
+TEST_CASE("whether a layer's texture has colour agrees with its decode", "[TextureDisplacement]")
+{
+    TextureDisplacementLayer gray;
+    gray.image_data = make_flat_gray_png(128);
+    CHECK_FALSE(height_texture_has_color(gray));
+
+    TextureDisplacementLayer color;
+    color.image_data = make_flat_rgb_png(200, 40, 10);
+    // Before the image is decoded and after, and whatever the smoothing.
+    CHECK(height_texture_has_color(color));
+    CHECK(decode_height_texture(color).has_color());
+    CHECK(height_texture_has_color(color));
+    color.smoothing = 0.5f;
+    CHECK(height_texture_has_color(color));
+
+    CHECK_FALSE(height_texture_has_color(TextureDisplacementLayer{}));
+}