Repository navigation
Sketcher: Symmetric constraint from two edges and a center edge - #144
Merged
brianmk merged 8 commits intoOct 11, 2026
Merged
Conversation
github-actions
Bot
added
Status: Needs triage
Language: C++
Mod: Sketcher
Language: Python
labels
The Symmetric constraint only accepted two points (or an element) plus a symmetry line or point. Add the analogous "two elements and a center edge" selection, where two elements are made mirror images of each other about a third, center edge. The solver has no element-pair symmetry primitive, so the selection is represented by two standard Symmetric constraints sharing the center edge, one per corresponding endpoint pair (start<->start and end<->end). Together they make one element the mirror image of the other about the axis. Axis identification, mirroring the swap logic already used for points: - an H/V axis is always the symmetry line, wherever it is selected; - otherwise, if exactly one of the three selected elements is a line and the other two are not, that line is the symmetry line, wherever it is selected; - otherwise (three plain sketch lines) the third-selected element is the symmetry line, following the "two elements and a symmetry line, in that order" convention. Selections that cannot form the constraint are rejected: a center edge that is not a line, an element without endpoints, an element used as its own symmetry line, and all-fixed geometry. The new option is added to allowedSelSequences, to the tooltip/status text and to the context-aware hints, and the "wrong selection" message now mentions it. The new three-edge sequences are reachable in run-once/preselection mode; the interactive continuous handler cannot reach them because the existing two-selection "element + symmetry line" sequences complete and apply on the second pick. The handler cases are added for completeness but cannot be reached without regressing that existing behaviour. Tests: - App level: two new SketchObjectTest cases check that the two constraints solve to an exact mirror of both endpoints about the (solved) center edge, for a user-drawn line and for the vertical sketch axis. - GUI: 11 new cases cover the three-edge selection (constraint form and solved mirror), the axis selected first/middle/last with an H/V axis, with a unique line among arcs and with an external edge, the rejection paths, and the pre-existing point-based and two-selection forms.
…enter edge The Symmetric constraint only ties the corresponding endpoints of the two selected elements together. When both elements are free and lie on the same side of the symmetry line, the closest solution the solver can find is the degenerate one in which both collapse onto the line, so applying the constraint to two edges and a center edge could merge them onto the center edge instead of mirroring them. Displace the moving element onto the exact mirror image of the reference element before adding the two Symmetric constraints, so the elements end up on opposite sides of the symmetry line. The reference element is the first selected one; when it is fixed or is its own mirror image, the other element becomes the reference instead. When both elements are their own mirror image there is no non-degenerate solution, so the selection is rejected. Non-line elements (arcs, open B-splines) keep the previous constraint-only behaviour. Tests: - Same-side line elements are displaced to opposite sides while the reference element stays in place. - A self-symmetric first element makes the second one the reference. - Two self-symmetric elements are rejected.
Making two edges symmetric about a center edge emitted one Symmetric constraint per corresponding endpoint pair. When the two edges share a vertex (the usual V / roof) that pair is coincident, and a point-pair Symmetric constraint on two coincident points has a perpendicularity equation that is identically satisfied: the pair is rank-deficient, so the solver reported it redundant and the two Symmetric constraints conflicting - an over-constrained sketch (solve() = -3). Such a pair only needs its shared point on the symmetry line, so it now gets a PointOnObject constraint instead - the rank-1 remainder of the pair - while the other pair keeps the full Symmetric constraint. The shared vertex also decides the endpoint correspondence. When it is shared cross-wise (reference start with moving end), keeping the start/start, end/end pairing satisfied the solver but collapsed both edges onto the same segment. The coincident endpoints are now paired together, so the apex lands on the line and the two free tips are mirrored; the displacement that pre-positions the moving element follows the same correspondence. Two elements sharing both endpoints cannot express a symmetry and are rejected. Tests: shared-apex V about an axis and about a free internal center line (no redundant/conflicting constraints, apex on the line, edges mirrored), a cross-wise shared vertex (non-degenerate result), both endpoints shared rejected, disjoint edges still get exactly two Symmetric constraints and no redundancy, plus a no-redundancy regression for the existing forms.
Symmetric(geo, pos, geo, pos, line) asks for a point to be the mirror of itself about a line. Both operands resolve to the same GCS point, so the perpendicularity half of the point-pair symmetry is identically satisfied and the constraint is rank-deficient: the solver reports it redundant and conflicting, leaving the sketch over-constrained (solve() = -3). Add only the midpoint-on-line half in that case, exactly as this function already does for an arc whose centre lies on the symmetry line. The point is then simply constrained to lie on the line, which is the rank-1 remainder of the request. Scope: two references to the same sketch point. Two distinct points that merely coincide (tied by a Coincident constraint) keep the full constraint and remain rank-deficient; the two-edge GUI path guards its own shared vertex separately via arePointsCoincident. Test: testSymmetricPointWithItselfOnLineIsNotOverconstrained (solve succeeds, no conflicts, the point ends up on the line while the element is not dragged onto it).
The two-points-plus-a-symmetry-line form emitted a point-pair Symmetric constraint unconditionally. When the two selected vertices are coincident (the same sketch point, or two distinct points tied by a Coincident constraint) the pair's perpendicularity equation is identically satisfied, so the constraint is rank-deficient: the solver reports it redundant and the sketch over-constrained (solve() = -3, RedundantConstraints = [1], ConflictingConstraints = [2]). Such a pair only needs the shared point to lie on the symmetry line, so the command now adds a PointOnObject constraint for it, mirroring what the two-edges form does for a shared vertex. The two-points-plus-a-center-point form is unaffected: with coincident points its midpoint equation is still rank 2. Tests: coincident vertices tied by a Coincident constraint, made symmetric about a line, now add PointOnObject with solve() == 0 and no redundant or conflicting constraints; the existing point-based and two-selection forms are unchanged.
Commit 2104add06a added the on-line remainder for the two-points-plus-a-line
form, but only on the run-once path, and it assumed the centre-point form was
unaffected. Both are wrong:
- The two-points-plus-a-centre-point form is degenerate too. It is PointOnLine +
PointOnPerpBisector of the segment between the two points, and a segment of
zero length has no perpendicular bisector, so both equations are identically
satisfied: solve() was -3 with conflicting constraints. The request really
means "the centre is that shared point", so a Coincident constraint is added
instead.
- The same remainder is needed on the other point-pair emission paths: the
continuous-mode sequences ({SelVertex..., SelEdgeOrAxis, SelVertex...} and the
three-vertex case) and the handler helper createSymmetryConstrain().
All point-pair emissions now go through one helper,
addPointPairSymmetricConstraint(), which picks the remainder from the selected
form: PointOnObject about a line, Coincident about a centre point, and the
unchanged Symmetric when the two points are not coincident.
At the App level, Sketch::addSymmetricConstraint() now applies the analogous
remainder when both operands resolve to the same GCS point in the 6-argument
form (Coincident between the centre and the point), mirroring the 5-argument
case added in e9d66813ee. Distinct points that merely coincide stay the GUI's
concern: coincidence is decomposed into two equality constraints, so the Sketch
layer has no query for it.
Tests: coincident vertices about a line and about a centre point (both solve
cleanly with no redundant or conflicting constraints, the centre landing on the
shared point), plus a C++ test for the 6-argument same-point case.
brianmk
force-pushed
the
sketcher/symmetric-two-edges
branch
2 times, most recently
from
October 11, 2026 05:38
a6a1306 to
2fbb031
Compare
Owner
Author
|
Branch rebuilt (force-pushed) to drop the experimental line-midpoint vertex commit that was inherited from #143.
Nothing else on this branch changed — the diff against its base is exactly the commits listed in this PR. Verified locally on the rebuilt tree: |
This was referenced Oct 11, 2026
The symmetric-constraint GUI tests picked the shifted "Vertex N" numbers that the experimental line-midpoint vertex introduced (a line exposed start, end and midpoint). That commit is gone from the stack, so a line exposes start and end again; retarget the three selections that relied on the extra per-line vertex. Verified: xvfb-run FreeCAD -t SketcherTests.TestConstraintCommandsGui -> 36/36 OK.
Owner
Author
|
Follow-up commit added: Dropping the experimental line-midpoint commit also removed the extra per-line "mid" vertex, and three selections in Verified with the CI GUI runner: |
brianmk
changed the base branch from
sketcher/line-midpoint-vertex
to
gui/viewport-renderer-label
October 11, 2026 06:24
The base of this PR moved from the dropped line-midpoint branch to gui/viewport-renderer-label. GitHub kept serving the cached refs/pull/144/merge (which still merged into the old base), so the previous run tested a tree that still contained the experimental midpoint commit and failed the same PartDesign tests. A synchronize event refreshes the test merge. No tree change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the two-edges-plus-a-center-edge form to the Symmetric constraint, and the four follow-up fixes it needed. The App layer only has point-pair symmetry, so the form is emitted as one constraint per corresponding endpoint pair (start↔start, end↔end) about the selected center edge.
Feature (
Sketcher: Symmetric constraint from two edges and a center edge)allowedSelSequencesandapplyConstraintcases for three edges, plus the run-once path, with axis identification: an H/V axis always wins; otherwise a uniquely-line element among the three is the axis; otherwise the third selected element is the axis.Fixes (each its own commit)
solve() = -3). A shared pair now getsPointOnObjectinstead (the rank-1 remainder), and the endpoint correspondence is pairing-aware, so a cross-wise shared vertex no longer collapses both edges onto one segment.Sketch::addSymmetricConstraint, both the line and the centre-point forms, when both operands are the same GCS point) and for the point-based GUI selections, where a coincident pair about a centre point now emitsCoincident(centre, sharedPoint).Tests
test_symmetric_*cases (feature, axis order, external axis, displacement, shared-vertex over-constraint for both forms, cross pairing, coincident remainders, rejection paths, and regressions for every pre-existing form).SketchObjectSymmetric.cppcases for the constraint pairs and both same-point remainders.Part 3 of a stacked series; base is the line-midpoint-vertex branch.