Skip to content

Sketcher: experimental line-midpoint vertex - #143

Closed
brianmk wants to merge 1 commit into
gui/viewport-renderer-labelfrom
sketcher/line-midpoint-vertex
Closed

brianmk wants to merge 1 commit into
gui/viewport-renderer-labelfrom
sketcher/line-midpoint-vertex

Conversation

@brianmk

@brianmk brianmk commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Expose each line segment's midpoint as a real solver vertex so it can be snapped to and constrained like any other point:

  • Sketch::addLineSegment adds a free parameter for the midpoint, tied to the endpoints by an internal P2PSymmetric constraint. The constraint is solver-only (rebuilt from the geometry on load) and never serialized, so no file-format change is needed.
  • Vertex mapping (SketchObjectGeometry::rebuildVertexIndex, GeoList) and the point getters expose PointPos::mid for lines.
  • The Sketcher edit-mode coin converter draws start/end/mid for lines and keeps the midpoint hidden (per-vertex transparency) until it is hovered/selected.
  • SketcherGui: Vulkan-only hover-midpoint marker (gated by FREECAD_USE_VULKAN).

Revert note: this adds a vertex per line, so sketch "Vertex N" sub-element numbering shifts for every line after the first (a line's mid vertex is inserted between its end and the following geometry's vertices). Documents that reference line vertices externally by number may need review.


Part 2 of a stacked series; base is the Gui: viewport renderer label branch.

Expose each line segment's midpoint as a real solver vertex so it can be
snapped to and constrained like any other point:

- Sketch::addLineSegment adds a free parameter for the midpoint, tied to the
  endpoints by an internal P2PSymmetric constraint. The constraint is
  solver-only (rebuilt from the geometry on load) and never serialized, so no
  file-format change is needed.
- Vertex mapping (SketchObjectGeometry::rebuildVertexIndex, GeoList) and the
  point getters expose PointPos::mid for lines.
- The Sketcher edit-mode coin converter draws start/end/mid for lines and keeps
  the midpoint hidden (per-vertex transparency) until it is hovered/selected.
- SketcherGui: Vulkan-only hover-midpoint marker (gated by FREECAD_USE_VULKAN).

Validated with Sketcher_tests_run (107/107) and FreeCADCmd/probe checks:
Coincident(line.mid, root) moves the line so its midpoint lands on the root;
the midpoint is transparent until the cursor is near it, then preselected.

REVERT NOTE
-----------
This adds a vertex per line, so sketch "Vertex N" sub-element numbering shifts
for every line after the first (a line's mid vertex is inserted between its end
and the following geometry's vertices). Documents that reference line vertices
externally by number may therefore need review. To revert, drop this single
commit; there is no serialized state to migrate.
@brianmk

brianmk commented Oct 11, 2026

Copy link
Copy Markdown
Owner Author

CI investigation: the Ubuntu / Build failure on this branch is caused by this PR's own commit, so I have dropped it from the stack and I'm closing this PR rather than merging it.

Symptom (identical on #143, #144, #145, #146 — all stacked on this branch):

FAIL: testNegativeCone (PartDesignTests.TestHelix.TestHelix.testNegativeCone)
  AssertionError: -0.17596393545793754 != 6.0643 within 4 places
FAIL: testPartDesignTNPChamfer  AssertionError: 44 != 62
FAIL: testPartDesignTNPFillet   AssertionError: 44 != 62
Ran 3558 tests ... FAILED (failures=3, skipped=39, expected failures=5)

Bisect (locally, against an up-to-date build):

  • Gui: show the viewport renderer label from the active backend #142's tip af34c81031 passes the same 3558 tests (CI run 37736182463, Oct 8).
  • This PR is af34c81031 + one commit (c061658d1c), so the regression is in it.
  • Reverse-applying the App-side hunk (Sketch.cpp, GeoList.cpp, SketchGeometry.cpp, SketchObjectGeometry.cpp) makes all three tests pass, on the same build.
  • Keeping the extra midpoint parameters but removing only the internal rule (GCSsys.addConstraintP2PSymmetric(p1, p2, pmid)) also makes all three pass — the failure is the solver equation, not the extra unknown.
  • Keeping the vertex-index/getPoint exposure without any solver change is harmless.

Why it can't be fixed by tweaking the rule: the midpoint equations are linear and implied for any line endpoints, yet adding them changes which point DogLeg converges to on underdetermined sketches. That silently changes existing PartDesign results (helix volume, TNP element maps) and can even fail the solve (Invalid solution from DogLeg solver, Both points are equal). The commit's own revert note also flags the sub-element renumbering. A future attempt should keep the midpoint out of the solver (e.g. translate Coincident(line.mid, x) into constraints on the line's endpoints, or create the midpoint unknown lazily only when a constraint references it) rather than adding an equation to every line.

The commit is preserved at c061658d1c on branch sketcher/line-midpoint-vertex (untouched) if you want to revive it as a standalone, opt-in feature with its own tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant