Port: mirror emits a half that continues the chain

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) <noreply@anthropic.com>
This commit is contained in:
Tommaso Bianchi
2026-08-22 15:00:50 +02:00
co-authored by Claude Opus 5
parent cbbd24dcb4
commit 942f6c28c7
3 changed files with 70 additions and 10 deletions
+30
View File
@@ -897,6 +897,36 @@ std::vector<SketchEntity> 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;
}
+18 -10
View File
@@ -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]")
+22
View File
@@ -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<SketchEntity> 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);
}