-
Notifications
You must be signed in to change notification settings - Fork 114
zone_painter_patch_edit_plan
Status: working plan (2026-07-28). Companion to zone_painter_ui_stories.md and
pipeline_max_design.md §14-paint. Written as a resume point: the first milestone of the
patch-editing story is complete and gated, one defect is open, and the next piece of work is
a model change rather than a feature.
Patch editing is reachable, does real work, and is byte-gated end to end.
Mode and levels. M enters patch mode (CPaintMouseListener::ModePatch, mode 4). Digits
1–5 pick the sub-object level, using the legacy plugin's EP_OBJECT / VERTEX / EDGE
/ PATCH / TILE values from rpo.h, so a level means the same thing in the tool as in the
file it came from. Entering the mode lands on Object level.
The digit row is shared with tile-set selection and only one binding is ever live, because
TileSetDigits is scoped ZPKS_PAINT and SubObjectDigits is ZPKS_PATCH. This is what
the mode-scope mask in the key table was built for — no ordering trick, no "and not in patch
mode" guard.
SRPatchMesh::SelLevel is deliberately not written back. Looking at a zone in vertex mode
is not an edit, and writing it would dirty every file the artist opened.
Cage and markers. The control cage draws from the display CPatchInfo, not from the
evaluated SPatchMesh: that data is already world space and already carries a shared-corner
identity in BaseVertices[], which is NeL's own per-zone vertex index. A corner touched by
four patches is one vertex, one marker, one selection entry. Nothing about the .max write
target is needed to draw, and keeping those apart is what let display land before the write
path existed.
Vertex markers are small solid squares (solid squares; crosses read as hatching at cage density), snapped to the pixel grid in both size and centre so they cannot shimmer between pixel counts as the camera moves. White is free, black is bound, red is selected.
Selection. Click selects, Ctrl adds, Alt removes, a click on nothing clears. Identity is
(zone id, BaseVertices index). Patch mode intercepts mouse-down ahead of the existing
Button == leftButton exact-equality test, which otherwise drops Ctrl+left and Alt+left —
precisely the pair add/remove needs.
Gizmo. Move gizmo on the selection centroid (counted once per unique vertex), with three axes, three plane handles whose inner corners meet on the origin, and an invisible screen handle that lights all three. Sized by the fit-at-rest model: it holds a world length, re-fitted only between interactions, hidden entirely while the view moves. Everything else is in pixels so arrowheads and pick radii are unaffected by axis foreshortening.
Move and write. Dragging commits through the geometry write-target policy, with undo, and the live landscape follows without a rebuild.
These cost real time to establish. Changing any of them needs a reason.
Overlays run after editorUI->draw(), which leaves the driver in NLGUI's 2D setup. World
-space CDRU lines silently draw nothing there. Every patch overlay projects through the
camera by hand and emits 2D lines, exactly as zpDrawZoneOutline already did. Also note
CDRU::drawLine's 2D overload takes the driver before the colour while the 3D one takes it
after — the world-space version compiles fine and draws nothing.
There are two render paths, the interactive loop and --screenshot, each with its own
overlay pass. The prop outline was already duplicated across both. Anything new goes in one
helper both call.
Bound vertices are derived, not authored. RPatchMesh::UpdateBindingPos recomputes
BIND_25, BIND_50, BIND_75 and BIND_SINGLE alike by interpolating the target edge's
Bézier, so the test is Binded, not the bind type. Keying on "not BIND_SINGLE" shows nothing
at all on real data: zonematerial-bassin-ilot_croix has 94 bound vertices and every one is a
single. A bound vertex must never accept a move — it is recomputed on load, so the write
could not survive a round trip. patch_eval.h replicates this evaluation, so the tool and Max
agree and a bound vertex visibly does not move.
The geometry write target is not the tile carrier. A tile record lives in the RPO; a vertex position lives in the PatchMesh and may not be the authoritative copy of itself. Per output vertex of the topmost edit-patch modifier (its output is displayed, so the policy is local to it and never recurses):
| case | write |
|---|---|
mapper record with Vert >= 0
|
the record's Delta (0x1130) |
| unmapped | the stored 0x1140 position |
| no modifier stack | the base RklPatch PatchMesh position |
All three are 12-byte in-place overwrites; the mapper record is a flat 32-byte stride
(OriginalStored, Vert, Original[3], Delta[3]), so its Delta has a computable offset. No
chunk resizes, no re-encode — an untouched file stays byte-identical. Tier A covers every
path.
Survey, 40 lacustre ligo files / 72 zone nodes: 67 modifier slots all carrying a 0x1140; 46
nodes with a mapper; 2752 mapped vertices; 26 nodes with no modifier stack at all. Stacks
go deep — transition-fond-bassin has 16 slots, 3 with mappers. Neither the mapper path nor
the base path is an edge case.
Geometry dirtiness needs its own flag. isZoneDirty re-encodes the RPO pristine copy; a
vertex move writes the PatchMesh or a mapper delta, which no RPO encode can see. Without
markGeomDirty a moved zone reports clean and the save skips the file.
Undo re-resolves its target rather than caching a CStorageRaw*, which would not survive
a working-set rebuild — and re-running the policy is the only way to be sure undo cannot drift
from the forward path. A whole selection lands as one stroke.
Live surface updates need no rebuild. CZone::refreshTesselationGeometry re-derives every
tessellation vertex from computeVertex, which evaluates the patch Bézier, so writing control
points and refreshing is the whole mechanism — the same door tile painting uses via
applyChanges. A rebuild would clear undo. Bind neighbours must be refreshed with the moved
patches or the surface cracks along the seam.
Control points are 16-bit fixed point (CVector3s) around PatchBias/PatchScale, one
LSB per PatchScale — a few millimetres on a normal zone. The live surface therefore sits on
a coarser lattice than the .max, which keeps full float precision. Display-only, but real.
The range check belongs to CZone::setPatchGeometry and must test what pack() can
represent (sint16), not the 32760 computeBBScaleBias divides by: the bbox is sized from
vertices plus a noise margin while tangents may lie outside that hull, so corpus control
points legitimately sit between the two. Checking at 32760 declines zones nobody edited.
Byte identity is measured against the --null-edit output, never the source .max — the
OLE container is rewritten on any save. This is how M31 measures cross-path identity, and
sessions must run --no-hint-stamp --no-thumbnail or hint/thumbnail bytes drown the signal.
e2e/m34_patch_move_byte_gates.sh, three files chosen so all three write targets are covered
and none can pass by luck (the gate asserts which target the log reports):
| file | target |
|---|---|
material-fond |
modifier PatchMesh |
material-bassin |
mapper delta |
zonematerial-bassin-1 |
base PatchMesh (no modifier stack) |
Each asserts: patch mode entered with nothing moved is byte-identical to the baseline; one vertex moved 1.5 along Z changes exactly 2 bytes; two vertices moved then one undo returns to the baseline byte for byte.
Dev hooks, alongside the existing ZONE_PAINTER_ZOOM_EXTENTS:
ZONE_PAINTER_GIZMO_HOVER=<handle> forces a handle hot, ZONE_PAINTER_GIZMO_DRAG="h:x,y,z"
forces a live drag. A --screenshot run has no pointer, so without these the hover and drag
states cannot be seen, let alone gated. The drag hook pins state every frame, so an
interactive session with it set cannot drag normally.
Multi-file live surface push is refused. On a working set with more than one editable
file, zpPatchPushLive hands CZone::setPatchGeometry control points far outside the target
zone's packed range and the refresh declines. Reported, never written wrong. The file write
is unaffected and gated; single-file sessions are unaffected.
Evidence gathered, so the next pass need not repeat it:
- Observed on zone 1000, patch 4: tangent
T[1]at 69026 against a limit of 32767 — about 170 world units from the centre of a zone whose radius is 81. -
V[0]of the same patch is comfortably inside (19414 / −32356 / 0). So this is not a marginal boundary case. -
buildDisplayZonecallszone.build(pz.ZoneId, pz.Patches, …), so zone id and patch order do correspond by construction, and the wrong-zone-id hypothesis is eliminated. - A Bézier tangent cannot legitimately sit that far from its own vertex, so the patch data at
that index is inconsistent — pointing at the weld pass (
weldPaintZonesruns before build) or atg_PaintCtx.Zonesnot being the vector the landscape was built from.
Both remaining hypotheses are cheap to test: dump pz.Patches[4] for zone 1000 immediately
after weldPaintZones and again at push time, and compare identity of the vector.
Note this defect may simply dissolve under Part 5 — it is a display-zone versus landscape-zone correspondence problem, which is exactly the confusion the model change removes.
The change. Today "instance" is encoded as an id range: ZoneId >= kInstanceZoneIdBase
(10000), produced by cloneInstanceZone copying the whole SPaintZone and transforming its
display geometry. That gives three problems at once. The copy carries its source's ObjectTM
unchanged, so an instance cannot do a world→object conversion and geometry refuses it
outright. "Editable" is decided by an integer comparison rather than by a property of the
thing being edited. And the display and the storage disagree about what a zone is.
The target model is the usual node/object split: objects, and nodes that reference them with a transform. Several nodes may reference one object; editing the object changes every node showing it.
This shape already exists in the tool for tiles. The paint carrier is keyed by RPO pointer
and setTile fans out to every zone sharing it, remapping through each zone's transformDesc.
The zone list simply does not use the same shape.
Work items.
-
SPaintZonebecomes a node: its own correctly-computedObjectTM, its own display transform (Rotate/Symmetry), and a reference to a shared object/carrier.ZoneIdbecomes nothing but a landscape id. - Retire
kInstanceZoneIdBase. 15 references acrosssession_ops.cpp,board_session.cpp,script_and_ui.cpp,zp_state.h,main.cpp,scene_paint.cpp— but they do not all mean the same thing (some mean "is editable", some "is a display copy", some "skip on save"), so this is a careful refactor, not a mechanical one. Classify each before touching any. - "Editable" becomes a property of the object, not of an id range.
- Selection identity becomes (object, vertex). The alias guard added in
a7b367942already implements exactly this rule keyed onNode; it should key on the carrier instead, which is strictly more correct since two distinct nodes could share one object. - Geometry fan-out follows the tile carrier pattern: one write, every node showing that
object refreshed through its own transform.
zpGeomVertChangedcurrently matches on zone id and would leave sibling nodes stale. - Drop the rotated/mirrored refusal in
zpApplyPatchMove— with a correct per-nodeObjectTMthe inverse is right by construction.
Why the selection rule matters. Selecting one underlying vertex through two nodes of the same object would apply the drag twice to a single storage location, and since each node has its own transform the two object-space deltas would not even agree. There is no correct answer, so the add is refused. Different vertices of the same object through different nodes share nothing and stay allowed — authoring an edge from whichever node shows it best is the point, and is why the guard is per-vertex rather than per-object.
Sequencing. Do this before adding the Edge / Patch / Tile sub-object levels, which would otherwise be written against the model that is about to change.
-
Out-of-range moves: currently reported, not handled. A move past the zone's packed
bounds needs a per-zone rebuild to recompute
PatchBias/PatchScale. - Edge / Patch / Tile sub-object levels: they select a level and draw the cage; they do not edit.
-
Transform keys and gizmos:
W/E/R.Wis free;Rcurrently means ModeProp, which isZPKS_ANY, so a scopedRfor scale would collide — resolve before binding. -
Tier B (
encodePatchMesh) for anything topological. Everything so far is Tier A. - Weld groups and cross-file seams: a moved vertex on a shared border must move its welded partners, or the seam opens. Untouched so far.
-
Unbind as an explicit op with group semantics:
BIND_25/BIND_75reference theBIND_50vertex throughPrimVert, so unbinding one of a group leaves the siblings evaluating against a vertex that is no longer their anchor. Auto-unbind on drag was considered and rejected: distance cannot discriminate "dragged off deliberately" from "followed along in a multi-selection", the discriminator has to be whether the bind's target edge is itself moving, and a silent unbind opens a crack in the surface. -
Cursors: the manifest above
zpSelectModelists what is needed; several need art rather than code.