diff --git a/src/libslic3r/TextureDisplacement.hpp b/src/libslic3r/TextureDisplacement.hpp index d61739c7d8..46097f1207 100644 --- a/src/libslic3r/TextureDisplacement.hpp +++ b/src/libslic3r/TextureDisplacement.hpp @@ -771,9 +771,12 @@ struct TextureColorRequest // single facets; an image detail a few facets wide still leaves thousands of pinhead islands // that the slicer's multi-material segmentation cannot digest. 0 turns it off. float min_color_region_mm2 = 0.5f; - // Filled per *base mesh* triangle (the bake is topology-preserving, so this indexes the returned - // mesh too): the quantize callback's index plus one, or 0 for "this triangle takes no colour from - // the texture". The +1 lines up with EnforcerBlockerType, where 0 is NONE ("use the volume's own + // Filled per triangle of the *returned* mesh: the quantize callback's index plus one, or 0 for + // "this triangle takes no colour from the texture". That is the same indexing as the base mesh + // only on the classic path, which moves vertices and leaves the topology alone; the one-run + // pipeline rebuilds the mesh, so nothing recorded against the base mesh - an existing paint mask, + // say - may be carried over by triangle id (see TriangleSelector::remap_painting()). + // The +1 lines up with EnforcerBlockerType, where 0 is NONE ("use the volume's own // filament"): where every palette entry is a filament, these go straight to a TriangleSelector. A // palette with mixes maps each index to the mix's filament slot first (see // GLGizmoTextureDisplacement::palette_filaments()). diff --git a/src/slic3r/GUI/Jobs/TextureDisplacementBakeJob.cpp b/src/slic3r/GUI/Jobs/TextureDisplacementBakeJob.cpp index e769800749..465c5b6386 100644 --- a/src/slic3r/GUI/Jobs/TextureDisplacementBakeJob.cpp +++ b/src/slic3r/GUI/Jobs/TextureDisplacementBakeJob.cpp @@ -13,6 +13,7 @@ #include "libslic3r/Model.hpp" #include "libslic3r/format.hpp" +#include "libslic3r/Point.hpp" #include "libslic3r/TriangleSelector.hpp" #include "slic3r/GUI/GLCanvas3D.hpp" @@ -76,6 +77,23 @@ void TextureDisplacementBakeJob::process(Ctl &ctl) }, color, m_input.volume_to_world, nullptr, &m_stats)); + // Bring the colour that was already painted onto the mesh this bake just built, while still on + // the worker thread. Triangle ids only mean the same thing on both meshes when the bake preserved + // the topology - the classic path moves vertices and nothing else - and the one-run pipeline + // rebuilds the mesh from scratch. Deserializing the old mask against the new one only checks that + // the ids are in range, and a bake almost always *grows* the triangle count, so every stale id + // passes that check and lands on an unrelated face: colour debris where nothing was painted, and + // the earlier pass's colour destroyed rather than kept. remap_painting() transfers it by position + // instead, which is what lets successive bakes accumulate. It is spatial work over both meshes, + // which is why it belongs here and not in finalize(). + if (!m_input.mmu_segmentation.bitstream.empty() && !m_result.empty() && !ctl.was_canceled()) { + if (m_result.its.indices.size() == m_input.base_mesh.indices.size()) + m_carried_mmu = m_input.mmu_segmentation; + else + m_carried_mmu = TriangleSelector::remap_painting(m_input.base_mesh, m_input.mmu_segmentation, + m_result.its, Transform3d::Identity(), {}); + } + // Always finish at 100: this is what closes the notification. Reported even on cancel, where // build_texture_displacement() returns an empty mesh and finalize() commits nothing. ctl.update_status(100, status); @@ -126,6 +144,18 @@ void TextureDisplacementBakeJob::finalize(bool canceled, std::exception_ptr &ept { mix.a_percent(), 100 - mix.a_percent() }); }); + // Read while the volume still holds the mesh the paint was recorded against, and after the + // filament slots above - adding one can clamp the paint, and that clamp has to survive. + // Normally process() has already carried the colour onto the baked topology; it only has to + // be redone here when the paint changed after the job was queued, because the snapshot + // process() worked from is then not what the user is looking at. + const bool mmu_changed = volume->mmu_segmentation_facets.timestamp() != m_input.mmu_segmentation_timestamp; + const indexed_triangle_set pre_bake_mesh = mmu_changed ? volume->mesh().its + : indexed_triangle_set{}; + const TriangleSelector::TriangleSplittingData live_mmu = mmu_changed + ? volume->mmu_segmentation_facets.get_data() + : TriangleSelector::TriangleSplittingData{}; + volume->set_mesh(std::move(m_result)); volume->set_new_unique_id(); volume->calculate_convex_hull(); @@ -134,13 +164,19 @@ void TextureDisplacementBakeJob::finalize(bool canceled, std::exception_ptr &ept // 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 - // topology-preserving, so triangle i is still triangle i. + // colour for everything outside it. What was already painted comes from m_carried_mmu, which + // process() brought onto this mesh's topology; this bake's own colour goes on top of it, so it + // wins wherever it painted and leaves the rest alone. if (!m_triangle_color.empty() && m_triangle_color.size() == volume->mesh().its.indices.size()) { + if (mmu_changed && !live_mmu.bitstream.empty()) + m_carried_mmu = pre_bake_mesh.indices.size() == volume->mesh().its.indices.size() + ? live_mmu + : TriangleSelector::remap_painting(pre_bake_mesh, live_mmu, + volume->mesh().its, + Transform3d::Identity(), {}); TriangleSelector selector(volume->mesh()); - const TriangleSelector::TriangleSplittingData &existing = volume->mmu_segmentation_facets.get_data(); - if (!existing.bitstream.empty()) - selector.deserialize(existing, false); + if (!m_carried_mmu.bitstream.empty()) + selector.deserialize(m_carried_mmu, false); for (size_t i = 0; i < m_triangle_color.size(); ++i) { const size_t entry = size_t(m_triangle_color[i]); if (entry > 0 && entry <= filament_of.size() && filament_of[entry - 1] >= 0) @@ -222,6 +258,8 @@ void queue_texture_displacement_bake(const ModelVolume &volume, const TextureCol input.layers = volume.texture_displacement_layers; input.options = volume.texture_displacement_options; input.volume_to_world = texture_displacement_volume_to_world(volume); + input.mmu_segmentation = volume.mmu_segmentation_facets.get_data(); + input.mmu_segmentation_timestamp = volume.mmu_segmentation_facets.timestamp(); for (int i = 0; i < int(TEXTURE_DISPLACEMENT_MAX_LAYERS); ++i) input.facets_data[size_t(i)] = volume.texture_displacement_facet(i).get_data(); diff --git a/src/slic3r/GUI/Jobs/TextureDisplacementBakeJob.hpp b/src/slic3r/GUI/Jobs/TextureDisplacementBakeJob.hpp index 40d7ab7b2b..9abf26743d 100644 --- a/src/slic3r/GUI/Jobs/TextureDisplacementBakeJob.hpp +++ b/src/slic3r/GUI/Jobs/TextureDisplacementBakeJob.hpp @@ -10,6 +10,7 @@ #include "libslic3r/ObjectID.hpp" #include "libslic3r/TextureDisplacement.hpp" #include "libslic3r/TriangleMesh.hpp" +#include "libslic3r/TriangleSelector.hpp" #include "Job.hpp" @@ -34,6 +35,13 @@ struct TextureDisplacementBakeInput // writes the volume's mmu_segmentation_facets - the same per-triangle filament assignment the MMU // paint gizmo writes - alongside the displaced geometry. TextureColorSettings color; + // The colour already painted on the volume, indexed against `base_mesh`. Carried here rather than + // read at commit time because the one-run pipeline rebuilds the mesh, and bringing this across to + // the new topology is spatial work (see process()) that has no business on the UI thread. + TriangleSelector::TriangleSplittingData mmu_segmentation; + // What `mmu_segmentation` was taken from, so the commit can tell whether the volume was painted + // while the bake ran and fall back to the live data if it was. + ObjectBase::Timestamp mmu_segmentation_timestamp = 0; // Whether this job pushes its own undo step when it commits. False when the caller has already // taken one that is meant to cover the displacement as well - Standard mode's Bake, which remeshes // and subdivides first and has to undo as a single action. @@ -60,6 +68,9 @@ private: // Empty unless a layer asked for colour. See TextureColorRequest; finalize() turns these into // filaments. std::vector m_triangle_color; + // m_input.mmu_segmentation brought onto m_result's topology, so finalize() can lay this bake's + // colour over the colour that was already there. Indexed against m_result, not base_mesh. + TriangleSelector::TriangleSplittingData m_carried_mmu; std::function m_on_finished; }; diff --git a/tests/libslic3r/test_texture_displacement.cpp b/tests/libslic3r/test_texture_displacement.cpp index 485f4f2a0f..94285b2ac6 100644 --- a/tests/libslic3r/test_texture_displacement.cpp +++ b/tests/libslic3r/test_texture_displacement.cpp @@ -35,10 +35,12 @@ #include #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" @@ -2174,3 +2176,151 @@ TEST_CASE("whether a layer's texture has colour agrees with its decode", "[Textu CHECK_FALSE(height_texture_has_color(TextureDisplacementLayer{})); } + +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. + // TriangleSelector::remap_painting() carries it by position instead, 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 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::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 &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 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 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 committed(second.indices.size(), 0); + { + const TriangleMesh mesh(second); + TriangleSelector selector(mesh); + const TriangleSelector::TriangleSplittingData carried = TriangleSelector::remap_painting( + first, painted_first, second, Transform3d::Identity(), {}); + REQUIRE_FALSE(carried.bitstream.empty()); + selector.deserialize(carried, false); + 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 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. Carrying the mask by position can widen that region by a triangle, so the margin + // is 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 +}