Move annotation control points on the surface by haraldsteinlechner · Pull Request #681 · pro3d-space/PRo3D

haraldsteinlechner · 2026-08-14T15:14:18Z

Adds Interactions.EditAnnotation, in which the control points of the selected annotation appear as handles. Ctrl+click one to pick it up, it follows the preview cursor's live surface hit, ctrl+click again to drop it. Escape restores. Clicking an annotation body still re-selects, so you move between annotations without leaving the mode.

Part of #639. That issue asks for add/remove point; this is the move operation plus the infrastructure those need. Add and remove are not implemented.

Stacked on #679 — based on features/annotation-polygon-fill because it rewrites Picking.pickId and pickRenderTarget, which that PR introduced. Retarget to develop once #679 merges.

Why it needed new infrastructure

Nothing could edit a finished annotation's geometry before: SetSegment only reaches model.working, and finished leaves are only touched via Groups.updateLeaf.

Pick buffer sub-index. Alpha already carried the packed object id; red was unread except by OpcViewer's debug lens. pickId now writes -1 there and pickVertexId writes the control point index, so red >= 0 distinguishes a handle from the annotation body. One 1×1 download still serves both, no second render target, and handles take their ids from the same orderedAnnotations array as lines and fills.

Both pick systems live at once. PickAnnotation turns kd-tree picking off; here the live surface hit is the whole point. So the click that grabs a handle also reaches the surface and would read as a drop, with mailbox ordering deciding the outcome. VertexGrab.movedSinceGrab makes it order-independent: a drop requires a preview hit after the grab.

Handle geometry is six CPU-expanded vertices per control point. gl_PointSize/gl_PointCoord depend on GL_PROGRAM_POINT_SIZE, and DefaultSurfaces.pointSprite cannot be used because it does not carry ObjId/SubId. Constraints found the hard way are recorded in docs/AnnotationVertexEditing.md.

Commit behaviour

Re-samples only the segments either side of the moved point, via resampleSegment factored out of addPoint. Most annotations are Projection.Linear and carry no segments, so that branch is usually skipped — an edit never changes the shape an annotation was drawn with. Measurements are recomputed inline so a drop is one model update and one undo entry, pushed as the SnapshotDelta property edits already use.

A vertex may land on a surface other than the one recorded; that is allowed (surfaceName is advisory) and reported through the transient top-right overlay.

Two fixes that are not the feature

Packed geometry stopped updating in place. Drawing.Sg.getPolylinePoints returns an AVal.custom, and the packed draws called it from inside their own AVal.custom. Adaptive edges are forward-only and weak, so source -> intermediate -> outer hung off a node nothing held; once collected, an edited annotation only redrew when something unrelated marked the geometry dirty. getPolylinePointsAt flattens against the caller's token instead. Pre-existing and not specific to this feature — vertex editing is simply the first in-place geometry edit. src/Tests/AdaptiveNestingTests.fs demonstrates it in plain FSharp.Data.Adaptive and regression-tests the real flattening.

Preview picking forced on in edit mode. #669 made showPreviewIntersection default to true, but Json.tryRead only defaults an absent key, so scenes saved earlier carry an explicit false. The drop is gated on a live surface hit, which made edit mode inert on exactly those scenes. The config value is read, never written.

Testing

290 tests pass, 39 new across VertexEditingTests.fs and AdaptiveNestingTests.fs: resampleSegment, the touched-segment index arithmetic including polygon wrap, and grab/commit/cancel/undo through the headless Draw harness.

Verified in the viewer against the VictoriaCrater OPC scene, driven with Playwright: selecting shows 14 handles, the one under the cursor reads back as vertex 4 of annotation 0, grabbing turns it green and switches the hint line, and the drop moves the point and releases — including when dropping onto another handle rather than bare terrain.

Not covered

Ellipses (their points are the sampled outline, not control points), the in-progress working annotation, and press-drag-release.

🤖 Generated with Claude Code

Adds Interactions.EditAnnotation, in which the control points of the selected annotation appear as handles. Ctrl+click one to pick it up, move the cursor and it follows the preview cursor's live surface hit, ctrl+click again to drop it. Escape puts it back. Clicking the annotation body still re-selects, so you move between annotations without leaving the mode. Until now nothing could edit a finished annotation's geometry at all: SetSegment only reaches model.working, and finished leaves are only touched via Groups.updateLeaf. This is the first path that does, and the infrastructure that issue #639's add/remove point tools need. The pick buffer grows a sub-index. Its alpha channel already carried the packed object id; red was unread except by OpcViewer's debug lens, so pickId now writes -1 there and a new pickVertexId writes the control point index. Red >= 0 is what tells a handle apart from the annotation body, and one 1x1 download still serves both. No second render target, no change to the object-id space - handles take their ids from the same orderedAnnotations array as lines and fills. Both pick systems are live in this mode, which is new: PickAnnotation turns kd-tree picking off, but here the live surface hit is the whole point. That means the click which grabs a handle also reaches the surface and would read as a drop, with the mailbox order deciding the outcome. VertexGrab.movedSinceGrab makes it order-independent - a drop requires a preview hit after the grab, and "move it before you drop it" is what the gesture means anyway. Commit re-samples only the segments either side of the moved point, via resampleSegment factored out of addPoint. Most annotations are Projection.Linear and carry no segments at all, so that branch is usually skipped; the shape an annotation was drawn with is never changed. Measurements are recomputed inline so the drop is one model update and therefore one undo entry, pushed as the SnapshotDelta that property edits already use. A vertex may land on a surface other than the one the annotation records. That is allowed - surfaceName is advisory and nothing re-validates it - but it is reported through the transient top-right overlay rather than silently.

26 Expecto tests. The index arithmetic and resampleSegment are pure and tested directly; the move itself goes through the real DrawingApp.update via the Draw harness, which also pins down that a drop is one undo entry and that undo puts the point back. Two cases worth naming: a zero-length segment, reachable by dropping a control point onto its neighbour, used to normalize a zero vector and fill the segment with NaN; and an annotation drawn with Projection.Linear must not grow segments it never had. Full suite 286 passed, 0 failed. ai/RENDERING.md gains the annotation pick buffer's channel layout, which was undocumented even before the red channel took on the vertex sub-index.

Everything except the handle geometry is verified working in the viewer: EditAnnotation appears in the interaction dropdown, the hint text switches with the mode, selection produces the right handle set (14 control points, 84 vertices, correct pivot and local positions), and the pick buffer's channel encoding reads back exactly as designed for line fragments (R=-1, A=objId). What does not work: the handle quads reach neither the visible pass nor the pick pass, so red is never >= 0 and every click falls through to PickDirectly - which toggles the selection off, the symptom that started this. Eliminated so far, each confirmed in the running viewer against an OPC scene: - depth: no gl_FragDepth write, depth test off in both passes - point sprites: gl_PointSize/gl_PointCoord replaced by CPU-expanded quads - geometry shader: removed entirely, six vertices per handle instead - attributes: Colors added, all six arrays confirmed 84 long - shader plumbing: [<ReflectedDefinition>] helper and module-level colour constants inlined, so the shaders are self-contained - the fragment shader: solid colour with no discard still draws nothing - the data: pivot, local positions and sizes all logged and correct - composition: packedVertexHandles sits in the same Sg.ofSeq as packedLines, which does render Diagnostics are left in ([VertexHandles] and [VertexPick] log lines) since they are what makes this tractable; they come out once it works. Also fixed here, independent of the rendering: a body click on the annotation being edited no longer reaches PickDirectly. addSingleSelectedLeaf toggles, so clicking the annotation you are editing deselected it and took its handles away - indistinguishable from a slightly missed handle click.

Verified end to end in the viewer against the VictoriaCrater OPC scene: select an annotation in EditAnnotation, its 14 control points appear as white discs, the one under the cursor reads back as vertex 4 of annotation 0, ctrl+click grabs it (it turns green, the hint switches to "click to drop, ESC to cancel"), and the second click moves the point and releases the grab. Four separate causes, each of which alone produced exactly "nothing renders": - FShade could not inline the shader bodies. A [<ReflectedDefinition>] billboard helper and module-level V4f colour constants both had to go; the shaders are written out in full instead. - The sprite expansion. gl_PointSize/gl_PointCoord depend on GL_PROGRAM_POINT_SIZE and every working point draw in the codebase composes DefaultSurfaces.pointSprite - which cannot be used here because it does not carry ObjId/SubId. Expanded on the CPU instead: six vertices per handle with a corner offset, no geometry shader at all. - The pick pass chained two fragment shaders (handleFragment then Picking.pickVertexId), which makes FShade carry HandleVertex's whole record including its [<Position>] between them. Collapsed into handlePickFragment. - uniform.ViewportSize is not the pick target's size, so the quads collapsed to nothing there. The viewport is now passed explicitly per pass. Id and SubId are also declared flat, as integers must be and as Picking.SubVertex already had them. Separately: EditAnnotation now forces preview picking on. #669 made it the default, but Json.tryRead only defaults an absent key, so scenes saved earlier carry an explicit false - and the drop is gated on a live surface hit, which made edit mode inert on exactly the scenes most likely to be edited. The user's config value is read, never written.

Four FShade/aardvark constraints cost a lot of time to find and would cost it again: inline shader bodies, one fragment shader in the pick pass, an explicit viewport uniform, and flat integer varyings. Also notes that edit mode forces the 3D cursor on, since the drop depends on it.

An edited annotation only redrew once something unrelated marked the packed geometry dirty - in practice, drawing the next annotation. Drawing.Sg.getPolylinePoints returns an AVal.custom, and the packed draws called it from inside their own AVal.custom and read it immediately. In FSharp.Data.Adaptive dependency edges point forwards only and are weak: IAdaptiveObject has Outputs : IWeakOutputSet and no Inputs, and AVal.custom retains only its compute function. So source -> intermediate -> outer is a chain of weak edges whose middle node is a local that nothing holds. Once collected, the source can no longer reach the outer, which keeps serving its cached value. getPolylinePointsAt does the same flattening against the caller's token, so the dependency is on the annotation's own long-lived cvals. getPolylinePoints is now a thin wrapper for callers that genuinely want an aval. Pre-existing, and not specific to vertex editing - any in-place edit of an existing annotation's geometry could hit it. Vertex editing is simply the first feature that does that, which is why it surfaced now. fastDns already carried a hand-rolled workaround for the same hazard: two bindings whose values are never used, present only to register the dependency. src/Tests/AdaptiveNestingTests.fs demonstrates it in eleven lines of plain FSharp.Data.Adaptive - AVal.custom and AVal.map both lose the edge, a direct read does not - and regression-tests the real flattening. The two demo tests skip rather than fail if the intermediate happens to survive the collection.

haraldsteinlechner added 6 commits August 14, 2026 14:28

haraldsteinlechner merged commit 8732691 into features/annotation-polygon-fill Aug 19, 2026
8 checks passed

Navigation Menu

Move annotation control points on the surface - #681

Move annotation control points on the surface#681
haraldsteinlechner merged 6 commits into
features/annotation-polygon-fillfrom
features/639_annotation-vertex-editing

haraldsteinlechner commented Aug 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

haraldsteinlechner commented Aug 14, 2026

Why it needed new infrastructure

Commit behaviour

Two fixes that are not the feature

Testing

Not covered

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant