From 942f6c28c781998bafdb74008e87473fd75b8c74 Mon Sep 17 00:00:00 2001 From: Tommaso Bianchi Date: Sat, 22 Aug 2026 15:00:50 +0200 Subject: [PATCH] Port: mirror emits a half that continues the chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carries snaporca 0231bd5b68. Parity holds: 17 files identical, 8 diverging as expected. A reflection reverses orientation, so mirror_entities now hands the reflected half back reversed in ORDER and flipped per ENTITY — an arc swapping its angles as well as its ends, a spline reversing its control points. Appending it to the source then yields one walkable chain instead of two halves meeting head-to-head, and a mirrored CCW loop stays CCW. This is the producer half of the confusion that cost three defects; the consumers (offset, and the exact loop area) keep their defensive handling, because that is what makes them correct for hand-built and imported sketches rather than only for geometry this function produced. Contract change, carried with the reason: a mirrored line's p0 is the reflection of the SOURCE's p1, and a mirrored CCW arc keeps a POSITIVE sweep — the reflection negates it, walking it the other way negates it again. Both [SketchEdit] cases updated, and a new [SketchProfile] case "a mirrored half continues the original chain" pins the property directly. Kernel here: all tests passed, 2687 assertions in 232 test cases. GUI target builds and links. Co-Authored-By: Claude Opus 5 (1M context) --- src/libslic3r/CAD/SketchEngine.cpp | 30 ++++++++++++++++++++++++++ tests/libslic3r/test_sketchedit.cpp | 28 +++++++++++++++--------- tests/libslic3r/test_sketchprofile.cpp | 22 +++++++++++++++++++ 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/libslic3r/CAD/SketchEngine.cpp b/src/libslic3r/CAD/SketchEngine.cpp index 62ecc4c97d..f7d2d10764 100644 --- a/src/libslic3r/CAD/SketchEngine.cpp +++ b/src/libslic3r/CAD/SketchEngine.cpp @@ -897,6 +897,36 @@ std::vector SketchEngine::mirror_entities( out.push_back(m); } + // A REFLECTION REVERSES ORIENTATION, so the reflected half is handed back reversed — in + // order, and each entity flipped — or it does not CONTINUE the chain it was made from. + // + // Draw half a stadium left-to-right along the bottom, round the cap, right-to-left along the + // top, ending at (0, R). Reflecting each entity in place gives a half whose top run STARTS at + // (-L, R) and ENDS at (0, R): it meets the original head-to-head, not head-to-tail. Every + // consumer that walks the loop then has to cope, and two already had to be taught — the loop + // area cancelled its own arc correction against the negated sweep, and offset put the + // reflected half on the wrong side because it read each entity's STORED direction. Reversed + // here, the two halves are one walkable chain and a mirrored CCW loop stays CCW. + std::reverse(out.begin(), out.end()); + for (SketchEntity& m : out) { + switch (m.type) { + case SketchEntity::Type::Line: + std::swap(m.p0, m.p1); + break; + case SketchEntity::Type::Arc: + case SketchEntity::Type::EllipseArc: + std::swap(m.p0, m.p1); + std::swap(m.start_angle, m.end_angle); // what "walked the other way" means + break; + case SketchEntity::Type::BSpline: + std::swap(m.p0, m.p1); + std::reverse(m.ctrl.begin(), m.ctrl.end()); + break; + default: + break; // circle, ellipse, point: no direction to reverse + } + } + return out; } diff --git a/tests/libslic3r/test_sketchedit.cpp b/tests/libslic3r/test_sketchedit.cpp index cad84e9f6c..5af9705d03 100644 --- a/tests/libslic3r/test_sketchedit.cpp +++ b/tests/libslic3r/test_sketchedit.cpp @@ -6,7 +6,11 @@ using namespace Slic3r; using Catch::Matchers::WithinAbs; -TEST_CASE("Mirror Line across Y axis", "[SketchEdit]") +// CONTRACT: mirror_entities hands the reflected half back REVERSED — the order of the entities +// and the direction of each — because a reflection reverses orientation and the result has to +// CONTINUE the chain it was made from. So a mirrored line's p0 is the reflection of the source's +// p1, not its p0. See [SketchProfile] "a mirrored half continues the original chain". +TEST_CASE("Mirror Line across Y axis (reversed: p0 is the reflection of the source p1)", "[SketchEdit]") { SketchEntity e; e.type = SketchEntity::Type::Line; @@ -21,10 +25,10 @@ TEST_CASE("Mirror Line across Y axis", "[SketchEdit]") const auto& m = result[0]; REQUIRE(m.type == SketchEntity::Type::Line); - REQUIRE_THAT(m.p0.x(), WithinAbs(-3.0, 1e-9)); - REQUIRE_THAT(m.p0.y(), WithinAbs(2.0, 1e-9)); - REQUIRE_THAT(m.p1.x(), WithinAbs(-5.0, 1e-9)); - REQUIRE_THAT(m.p1.y(), WithinAbs(4.0, 1e-9)); + REQUIRE_THAT(m.p0.x(), WithinAbs(-5.0, 1e-9)); // reflection of the SOURCE p1 + REQUIRE_THAT(m.p0.y(), WithinAbs(4.0, 1e-9)); + REQUIRE_THAT(m.p1.x(), WithinAbs(-3.0, 1e-9)); // reflection of the SOURCE p0 + REQUIRE_THAT(m.p1.y(), WithinAbs(2.0, 1e-9)); } TEST_CASE("Mirror Circle across Y axis", "[SketchEdit]") @@ -70,15 +74,19 @@ TEST_CASE("Mirror Arc across X axis", "[SketchEdit]") const auto& m = result[0]; REQUIRE(m.type == SketchEntity::Type::Arc); - REQUIRE_THAT(m.p0.x(), WithinAbs(1.0, 1e-9)); - REQUIRE_THAT(m.p0.y(), WithinAbs(0.0, 1e-9)); - REQUIRE_THAT(m.p1.x(), WithinAbs(0.0, 1e-9)); - REQUIRE_THAT(m.p1.y(), WithinAbs(-1.0, 1e-9)); + // Reversed with the rest of the half: the mirrored arc STARTS where the reflection of the + // source's end is, and finishes at the reflection of its start. + REQUIRE_THAT(m.p0.x(), WithinAbs(0.0, 1e-9)); + REQUIRE_THAT(m.p0.y(), WithinAbs(-1.0, 1e-9)); + REQUIRE_THAT(m.p1.x(), WithinAbs(1.0, 1e-9)); + REQUIRE_THAT(m.p1.y(), WithinAbs(0.0, 1e-9)); + // The reflection alone would negate the sweep; walking the arc the other way negates it + // again, so a mirrored CCW arc is CCW once more and a mirrored CCW loop stays CCW. double sweep = m.end_angle - m.start_angle; double orig_sweep = e.end_angle - e.start_angle; REQUIRE(orig_sweep > 0.0); - REQUIRE(sweep < 0.0); + REQUIRE(sweep > 0.0); } TEST_CASE("Offset Line by positive d", "[SketchEdit]") diff --git a/tests/libslic3r/test_sketchprofile.cpp b/tests/libslic3r/test_sketchprofile.cpp index 2bbe0cd71e..7f455eb936 100644 --- a/tests/libslic3r/test_sketchprofile.cpp +++ b/tests/libslic3r/test_sketchprofile.cpp @@ -185,3 +185,25 @@ TEST_CASE("profile: a mirrored half offsets as one loop, not two", "[SketchProfi if (o.type == SketchEntity::Type::Arc) REQUIRE_THAT(o.radius, WithinAbs(R + d, 1e-9)); } + +TEST_CASE("profile: a mirrored half continues the original chain", "[SketchProfile]") +{ + // The point of emitting the reflected half reversed: appending it to the source must give a + // chain you can WALK, head-to-tail, with no consumer having to notice that half of it came + // from a mirror. The end of the last source entity must be the start of the first mirrored + // one, and the end of the last mirrored one must close back to the very first start. + const std::vector half = { + line({0, -15}, {50, -15}), line({50, -15}, {50, 15}), line({50, 15}, {0, 15}) }; + const auto m = SketchEngine::mirror_entities(half, Vec2d(0, 0), Vec2d(0, 1)); + REQUIRE(m.size() == 3); + + REQUIRE_THAT((half.back().p1 - m.front().p0).norm(), WithinAbs(0.0, 1e-9)); + REQUIRE_THAT((m.back().p1 - half.front().p0).norm(), WithinAbs(0.0, 1e-9)); + + for (size_t i = 0; i + 1 < m.size(); ++i) + REQUIRE_THAT((m[i].p1 - m[i + 1].p0).norm(), WithinAbs(0.0, 1e-9)); + + auto all = half; + for (const auto& e : m) all.push_back(e); + REQUIRE(closed_wires(all) == 1); +}