Carry the paint across with transfer_facet_paint

The branch used TriangleSelector::remap_painting(), which pairs a source
paint triangle with a target triangle overlapping it to within
TriangleCursor::tolerance. That is right after a remesh, which leaves the
surface where it was, and wrong after a bake, which has moved it by the
relief depth - most of the paint finds no partner. transfer_facet_paint()
asks the other question: every triangle of the new mesh takes the state
painted at its foot point on the old one, which is exactly where it came
from however deep the relief.

It also does all four paint channels rather than colour alone, and it runs
where the volume is read, so the snapshot and the timestamp that guarded
against the user painting while the bake ran are no longer needed: the
annotation is read at commit time and is by definition current.

The function and its call site are byte for byte what the model-UV branch
carries, so the two merge in any order and in either order produce one
mechanism rather than two fighting over mmu_segmentation_facets. The
regression test is unchanged except for the call it makes.

src/libslic3r/TextureDisplacement.cpp

@@ -1570,6 +1570,61 @@ std::vector<TextureIsland> compute_connected_net(const PatchUnwrap &unwrap)
     return islands;
 }
 
+std::vector<uint8_t> transfer_facet_paint(const indexed_triangle_set                    &src_mesh,
+                                         const TriangleSelector::TriangleSplittingData &src_paint,
+                                         const indexed_triangle_set                    &dst_mesh)
+{
+    if (src_mesh.indices.empty() || dst_mesh.indices.empty() || src_paint.bitstream.empty())
+        return {};
+
+    const TriangleMesh src(src_mesh);
+    TriangleSelector   selector(src);
+    selector.deserialize(src_paint, false);
+
+    // Every sub-triangle of the source, each carrying its own state - NONE included, so the soup
+    // covers the whole surface and a foot point always lands on something. One soup rather than one
+    // tree per state: a foot point has one nearest triangle, and that triangle's state is the answer.
+    std::vector<indexed_triangle_set> per_state;
+    selector.get_facets(per_state);
+    indexed_triangle_set soup;
+    std::vector<uint8_t> label;
+    for (size_t st = 0; st < per_state.size(); ++st) {
+        const indexed_triangle_set &part = per_state[st];
+        if (part.indices.empty())
+            continue;
+        const int base = int(soup.vertices.size());
+        soup.vertices.insert(soup.vertices.end(), part.vertices.begin(), part.vertices.end());
+        for (const stl_triangle_vertex_indices &t : part.indices) {
+            soup.indices.emplace_back(t[0] + base, t[1] + base, t[2] + base);
+            label.push_back(uint8_t(st));
+        }
+    }
+    if (soup.indices.empty())
+        return {};
+
+    const AABBTreeIndirect::Tree3f tree =
+        AABBTreeIndirect::build_aabb_tree_over_indexed_triangle_set(soup.vertices, soup.indices);
+
+    std::vector<uint8_t> out(dst_mesh.indices.size(), 0);
+    tbb::parallel_for(tbb::blocked_range<size_t>(0, dst_mesh.indices.size()),
+                      [&](const tbb::blocked_range<size_t> &range) {
+        for (size_t j = range.begin(); j < range.end(); ++j) {
+            const stl_triangle_vertex_indices &t = dst_mesh.indices[j];
+            // The centroid, not a corner: a facet takes one state, and its middle is the point that
+            // is inside it whatever the triangle looks like.
+            const Vec3f centroid = (dst_mesh.vertices[size_t(t[0])] + dst_mesh.vertices[size_t(t[1])] +
+                                    dst_mesh.vertices[size_t(t[2])]) / 3.f;
+            size_t      hit = 0;
+            Vec3f       foot;
+            const float d2 = AABBTreeIndirect::squared_distance_to_indexed_triangle_set(soup.vertices, soup.indices,
+                                                                                        tree, centroid, hit, foot);
+            if (d2 >= 0.f && hit < label.size())
+                out[j] = label[hit];
+        }
+    });
+    return out;
+}
+
 void average_island_scales(std::vector<TextureIsland> &islands)
 {
     if (islands.empty())

src/libslic3r/TextureDisplacement.hpp

@@ -578,6 +578,31 @@ bool sample_layer_color(const DecodedHeightTexture &texture, const TextureDispla
                         const Vec3f &patch_center = Vec3f::Zero(), const Vec3f &patch_axis = Vec3f::UnitZ(),
                         const Vec2f *lscm_uv = nullptr);
 
+// Carries a per-facet paint annotation - the colour paint above all - from a mesh onto the mesh a
+// bake produced from it: every triangle of `dst_mesh` takes the state painted at its own foot point on
+// `src_mesh`, i.e. on the nearest point of the surface it was built from. Returned as one
+// EnforcerBlockerType per `dst_mesh` triangle, 0 (NONE) where that foot point carried no paint.
+//
+// ModelVolume::restore_painting() cannot do this. Its remap pairs a source paint triangle with a
+// target triangle that *overlaps* it to within 0.01 mm, which is right for a remesh or a
+// simplification - they leave the surface where it was - and useless after a bake, which moves the
+// surface by the relief depth. Measured on a model displaced by a tenth of its own size, that remap
+// kept a quarter of the colour and left the rest unpainted; this keeps essentially all of it, because
+// a foot point is exactly as far away as the relief is deep and the lookup does not care how far that
+// is.
+//
+// The source's *sub*-triangles are what is looked up, not its triangles, so the part of a triangle a
+// brush stroke only clipped a corner off keeps its own state rather than rounding to the whole
+// triangle's. No distance limit is needed or wanted: the nearest point on the source surface is where
+// a baked triangle came from however far the relief carried it, and an unpainted origin reports NONE
+// on its own.
+//
+// Returns an empty vector when `src_paint` is empty or either mesh is - the caller then has nothing to
+// carry, which is not the same as "carry nothing".
+std::vector<uint8_t> transfer_facet_paint(const indexed_triangle_set                    &src_mesh,
+                                         const TriangleSelector::TriangleSplittingData &src_paint,
+                                         const indexed_triangle_set                    &dst_mesh);
+
 // Area-weighted centroid and average normal of a layer's currently painted patch, in mesh-local
 // coordinates - the same measurements build_texture_displacement() uses to pick its dominant
 // projection axis. Used by the GUI to anchor the on-canvas "adjust texture placement" gizmo to

src/slic3r/GUI/Jobs/TextureDisplacementBakeJob.cpp

@@ -8,6 +8,7 @@
 #include "libslic3r/TextureDisplacement.hpp"
 #include <exception>
 #include <cstddef>
+#include <cstdint>
 #include "slic3r/GUI/Jobs/Worker.hpp"
 #include <memory>
 
@@ -26,6 +27,7 @@
 #include "slic3r/Utils/UndoRedo.hpp"
 #include "libslic3r/ObjectID.hpp"
 #include "libslic3r/TriangleMesh.hpp"
+#include <array>
 
 namespace Slic3r::GUI {
 
@@ -126,16 +128,71 @@ void TextureDisplacementBakeJob::finalize(bool canceled, std::exception_ptr &ept
                                                      { mix.a_percent(), 100 - mix.a_percent() });
             });
 
+        // Whether the bake replaced the triangles or only moved the vertices it was given. The classic
+        // path is topology-preserving by contract, so every per-triangle annotation on the volume still
+        // means what it did; the default pipeline refines, cuts and simplifies, so none of them do.
+        // Both are checked: the flag is what the bake promised, the count is what it delivered. Only
+        // the triangles matter - the annotations are per facet and reference no vertex index - so a
+        // vertex count that changed on its own (the bake compacts unreferenced vertices away) is not a
+        // reason to remap anything.
+        const bool topology_changed = m_input.options.pipeline_v2 ||
+                                      m_result.its.indices.size() != m_input.base_mesh.indices.size();
+
+        // Carry the volume's existing paint - the model's own colours above all - across that rebuild.
+        // Without it a bake left the mmu_segmentation_facets bitstream pointing at a triangle set that
+        // no longer existed, which is what scrambled the colours of an imported coloured model even
+        // with colour switched off on every layer and nothing in the bake that was meant to touch them.
+        //
+        // transfer_facet_paint() rather than ModelVolume::restore_painting(): the latter's remap wants
+        // a target triangle overlapping the source paint to within 0.01 mm, which the baked surface is
+        // nowhere near - it has moved by the relief depth. Nothing to do when the topology is intact:
+        // triangle i is still triangle i, so the paint is already exactly right and carrying it at all
+        // could only lose detail.
+        std::array<TriangleSelector::TriangleSplittingData, 4> before;
+        if (topology_changed) {
+            before[0] = volume->supported_facets.get_data();
+            before[1] = volume->seam_facets.get_data();
+            before[2] = volume->mmu_segmentation_facets.get_data();
+            before[3] = volume->fuzzy_skin_facets.get_data();
+        }
+
         volume->set_mesh(std::move(m_result));
         volume->set_new_unique_id();
         volume->calculate_convex_hull();
 
+        if (topology_changed) {
+            const std::array<FacetsAnnotation *, 4> target{ &volume->supported_facets, &volume->seam_facets,
+                                                            &volume->mmu_segmentation_facets,
+                                                            &volume->fuzzy_skin_facets };
+            for (size_t ch = 0; ch < target.size(); ++ch) {
+                const std::vector<uint8_t> state =
+                    transfer_facet_paint(m_input.base_mesh, before[ch], volume->mesh().its);
+                // Nothing was painted in this channel, or there is nothing to paint it onto: either
+                // way the annotation must not keep describing the mesh that is gone.
+                if (state.empty()) {
+                    target[ch]->reset();
+                    continue;
+                }
+                TriangleSelector selector(volume->mesh());
+                bool             any = false;
+                for (size_t i = 0; i < state.size(); ++i)
+                    if (state[i] != 0) {
+                        selector.set_facet(int(i), EnforcerBlockerType(state[i]));
+                        any = true;
+                    }
+                if (any)
+                    target[ch]->set(selector);
+                else
+                    target[ch]->reset();
+            }
+        }
+
         // Colour lands in mmu_segmentation_facets, merged *over* whatever is already painted there
         // rather than replacing it: a triangle the texture does not colour keeps its existing filament,
         // and one the user never painted at all stays at NONE, which already means "the volume's own
         // filament". That is what confines the effect to the painted area without having to invent a
-        // colour for everything outside it. Safe to index straight onto the new mesh - the bake is
+        // colour for everything outside it. What was already painted is whatever the transfer above
-        // topology-preserving, so triangle i is still triangle i.
+        // left in the annotation, so this bake's colour lands on top of it rather than instead of it.
         if (!m_triangle_color.empty() && m_triangle_color.size() == volume->mesh().its.indices.size()) {
             TriangleSelector selector(volume->mesh());
             const TriangleSelector::TriangleSplittingData &existing = volume->mmu_segmentation_facets.get_data();
@@ -159,7 +216,6 @@ void TextureDisplacementBakeJob::finalize(bool canceled, std::exception_ptr &ept
         // pipeline, which rebuilds and then simplifies the mesh. A mask left behind against the old
         // topology is exactly what makes the gizmo reload an empty selector over a non-empty mask and
         // then erase it on the next flush - see GLGizmoTextureDisplacement::update_from_model_object().
-        const bool topology_changed = m_input.base_mesh.indices.size() != volume->mesh().its.indices.size();
         for (int slot = 0; slot < int(TEXTURE_DISPLACEMENT_MAX_LAYERS); ++slot) {
             const auto it = std::find_if(m_input.layers.begin(), m_input.layers.end(),
                                          [slot](const TextureDisplacementLayer &l) { return l.slot == slot; });

tests/libslic3r/test_texture_displacement.cpp

@@ -35,10 +35,12 @@
 #include <boost/filesystem.hpp>
 
 #include "libslic3r/AABBMesh.hpp"
+#include "libslic3r/AABBTreeIndirect.hpp"
 #include "libslic3r/TextureDisplacement.hpp"
 #include "libslic3r/TextureBake/TextureBakeDecimate.hpp"
 #include "libslic3r/TextureBake/TextureBakeFlip.hpp"
 #include "libslic3r/TextureBake/TextureBakeMesh.hpp"
+#include "libslic3r/TextureBake/TextureBakeRepair.hpp"
 #include "libslic3r/TriangleMesh.hpp"
 #include "libslic3r/TriangleSelector.hpp"
 #include "libslic3r/PNGReadWrite.hpp"
@@ -2103,6 +2105,157 @@ TEST_CASE("Cancelling while flat faces are harvested past the budget stops the d
     CHECK(canceled > full);
 }
 
+TEST_CASE("Baking twice keeps the mesh closed and carries the first bake's colours across",
+          "[TextureDisplacement][Regression]")
+{
+    // Bake, colour part of what came back, bake again. The one-run pipeline rebuilds the topology, so
+    // the colour recorded against the first bake's mesh indexes triangles that no longer mean the same
+    // thing. Taken across by triangle id it lands on unrelated faces - a bake almost always grows the
+    // triangle count, so every stale id passes the only bound check deserialize() makes - which both
+    // scatters colour over surfaces nothing painted and destroys what the first pass laid down.
+    // transfer_facet_paint() carries it across by position instead - every triangle of the new mesh
+    // takes the state painted at its foot point on the old one - which is what makes successive bakes
+    // accumulate. See TextureDisplacementBakeJob, which is where the two meet.
+    constexpr float            side    = 20.f;
+    constexpr float            refine  = 0.5f;
+    constexpr float            radius  = 4.f;
+    const indexed_triangle_set cube    = its_make_cube(side, side, side);
+    const Vec2f                first_stroke(6.f, 6.f), second_stroke(14.f, 14.f);
+
+    TextureDisplacementLayer layer;
+    layer.slot          = 0;
+    layer.image_data    = make_rgb_png_2x2();
+    layer.tiling_scale  = 5.f;
+    layer.depth_mm      = 0.5f;
+    layer.color_enabled = true;
+
+    TextureDisplacementOptions options;
+    options.pipeline_v2        = true;
+    options.v2_refine_mm       = refine;
+    options.v2_max_triangles_k = 750;
+
+    // Three primaries, so a palette entry is legible in the assertions below.
+    const std::array<Vec3f, 3> palette = { Vec3f(1, 0, 0), Vec3f(0, 1, 0), Vec3f(0, 0, 1) };
+    TextureColorRequest        request;
+    request.quantize = [&palette](const Vec3f &rgb) {
+        int   best = 0;
+        float bd   = std::numeric_limits<float>::max();
+        for (int i = 0; i < 3; ++i)
+            if (const float d = (palette[size_t(i)] - rgb).squaredNorm(); d < bd) { bd = d; best = i; }
+        return best;
+    };
+
+    const auto bake = [&](const indexed_triangle_set &mesh, const Vec2f &stroke, std::vector<uint8_t> &color) {
+        TextureDisplacementFacetsData facets;
+        facets[0] = paint_top_disc(mesh, side, stroke, radius);
+        TextureColorRequest req = request;
+        req.out_triangle        = &color;
+        return build_texture_displacement(mesh, { layer }, facets, options, {}, &req);
+    };
+    // The pipeline's own counters, over welded positions, so a mesh watertight only after welding
+    // still reads as closed.
+    const auto defects = [](const indexed_triangle_set &mesh) {
+        return TextureBake::count_edge_defects(TextureBake::to_soup(mesh));
+    };
+
+    std::vector<uint8_t>       color1;
+    const indexed_triangle_set first = bake(cube, first_stroke, color1);
+    REQUIRE(first.indices.size() > cube.indices.size());
+    REQUIRE(color1.size() == first.indices.size());
+    const TextureBake::EdgeDefects d1 = defects(first);
+    INFO("first bake: " << d1.triangles << " triangles, " << d1.open << " open, " << d1.non_manifold << " non-manifold");
+    CHECK(d1.open == 0);
+    CHECK(d1.non_manifold == 0);
+
+    // The colour the first bake committed, as TextureDisplacementBakeJob writes it.
+    TriangleSelector::TriangleSplittingData painted_first;
+    {
+        const TriangleMesh mesh(first);
+        TriangleSelector   selector(mesh);
+        for (size_t i = 0; i < color1.size(); ++i)
+            if (color1[i] != 0)
+                selector.set_facet(int(i), EnforcerBlockerType(color1[i]));
+        painted_first = selector.serialize();
+    }
+    REQUIRE_FALSE(painted_first.bitstream.empty());
+
+    std::vector<uint8_t>       color2;
+    const indexed_triangle_set second = bake(first, second_stroke, color2);
+    REQUIRE(color2.size() == second.indices.size());
+    REQUIRE(second.indices.size() != first.indices.size()); // the topology really did change
+    const TextureBake::EdgeDefects d2 = defects(second);
+    INFO("second bake: " << d2.triangles << " triangles, " << d2.open << " open, " << d2.non_manifold << " non-manifold");
+    CHECK(d2.open == 0);
+    CHECK(d2.non_manifold == 0);
+
+    // The commit: the earlier colour carried onto the new topology, this bake's own colour over it.
+    std::vector<uint8_t> committed(second.indices.size(), 0);
+    {
+        const TriangleMesh mesh(second);
+        TriangleSelector   selector(mesh);
+        const std::vector<uint8_t> carried = transfer_facet_paint(first, painted_first, second);
+        REQUIRE(carried.size() == second.indices.size());
+        for (size_t i = 0; i < carried.size(); ++i)
+            if (carried[i] != 0)
+                selector.set_facet(int(i), EnforcerBlockerType(carried[i]));
+        for (size_t i = 0; i < color2.size(); ++i)
+            if (color2[i] != 0)
+                selector.set_facet(int(i), EnforcerBlockerType(color2[i]));
+        for (int state = 1; state <= int(palette.size()); ++state) {
+            std::vector<int> src;
+            if (selector.get_facets_strict(EnforcerBlockerType(state), &src).indices.empty())
+                continue;
+            for (const int s : src)
+                if (s >= 0 && size_t(s) < committed.size() && committed[size_t(s)] == 0)
+                    committed[size_t(s)] = uint8_t(state);
+        }
+    }
+
+    // Just the triangles the first bake coloured, to ask "how far is this from colour that was really
+    // laid down" of anything the commit painted.
+    indexed_triangle_set coloured_first;
+    coloured_first.vertices = first.vertices;
+    for (size_t i = 0; i < color1.size(); ++i)
+        if (color1[i] != 0)
+            coloured_first.indices.push_back(first.indices[i]);
+    const AABBTreeIndirect::Tree3f tree = AABBTreeIndirect::build_aabb_tree_over_indexed_triangle_set(
+        coloured_first.vertices, coloured_first.indices);
+
+    // A triangle may carry colour the second bake did not produce only where the first bake's colour
+    // actually was. A foot point sits on the surface the bake started from, which the relief has since
+    // moved, and the transfer can widen the region by a triangle, so the margin is twice the edge
+    // length the bake was told to refine to; landing anywhere else means the mask was read against a
+    // mesh it no longer matches.
+    size_t own = 0, carried_over = 0, stranded = 0;
+    float  worst = 0.f;
+    for (size_t i = 0; i < second.indices.size(); ++i) {
+        if (committed[i] == 0)
+            continue;
+        if (i < color2.size() && color2[i] != 0) {
+            CHECK(committed[i] == color2[i]); // this bake's own colour wins where it painted
+            ++own;
+            continue;
+        }
+        const stl_triangle_vertex_indices &t = second.indices[i];
+        const Vec3f g = (second.vertices[size_t(t[0])] + second.vertices[size_t(t[1])] +
+                         second.vertices[size_t(t[2])]) / 3.f;
+        size_t      hit = 0;
+        Vec3f       foot;
+        const float d = std::sqrt(std::max(0.f, AABBTreeIndirect::squared_distance_to_indexed_triangle_set(
+                                                    coloured_first.vertices, coloured_first.indices, tree, g, hit, foot)));
+        worst = std::max(worst, d);
+        if (d <= 2.f * refine)
+            ++carried_over;
+        else
+            ++stranded;
+    }
+    INFO("committed: " << own << " from this bake, " << carried_over << " carried over, " << stranded
+                       << " stranded, worst distance from the first bake's colour " << worst << " mm");
+    CHECK(own > 0);
+    CHECK(carried_over > 0);   // the first bake's colour survived the second
+    CHECK(stranded == 0);      // and nothing was painted where neither bake put colour
+}
+
 TEST_CASE("A second bake beside a first comes out as fine as a single bake", "[TextureDisplacement]")
 {
     // A stroke in each of the cube's two top triangles, baked one after the other. The first bake used