Skip to content

Make a voxel sculpt layer's creation, and each edit inside it, an undo step - #651

Merged
leonardoaraujosantos merged 1 commit into
mainfrom
fix/642-voxel-sculpt-layer-creation-is-a-step
Sep 23, 2026
Merged

leonardoaraujosantos merged 1 commit into
mainfrom
fix/642-voxel-sculpt-layer-creation-is-a-step

Conversation

@leonardoaraujosantos

Copy link
Copy Markdown
Contributor

Defect

clay_voxel_begin_sculpt_layer / VoxelGrid::begin_sculpt_layer added a record to the grid's sculpt-layer stack that the session history never saw. The edits made inside the layer were recorded as plain Voxel steps: their cells went into the history, but not what they did to the layer's record. Reproduced on main through the C ABI before any change:

probe main this PR
begin a layer, one inflate, undo: layer cell count 110 0
then dial the layer to 0.5: occupied cells vs before the inflate 2251 vs 2197 (54 undone cells put back) 2197 vs 2197
document bytes after that undo vs before the inflate differ identical
snapshot, two passes and a dial, journal replayed onto the snapshot refused after 2 of 3 events 5 of 5 applied, bytes identical

Cause

The history had no event for the layer's creation, and none for the record half of an edit inside it. So an undo reverted the cells and left changes listing them, and a later recompose (set_sculpt_layer_strength, _visible, and so on) re-applied them. A journal had nothing to rebuild the layer from, so apply_sculpt_layer_op refused the first VoxelLayerProperty event that named it.

Fix

This follows the mesh stack's approach, where MultiresSurface::add_sculpt_layer fills a Structural SculptLayerProperty:

  • SculptLayerOp gets two new kinds, appended after the existing ones.
    • Begin is the creation. Undo removes the top layer (it must have an empty record) and ends recording. Redo pushes the layer back with its name and seed, and leaves it closed.
    • Pass is one edit made while a layer is recording. It carries the edit's cells plus the record half: the record's length before the edit, the entries it rewrote with their old and new after, and the entries it appended. The field names come from merge-down, which already restores a record by truncating it and putting back overwritten afters. pass_afters is the one new field.
  • begin_sculpt_layer(name, record) takes the same optional record out-parameter as its five siblings. The C binding and both pyclay entry points (begin_sculpt_layer, with grid.sculpt_layer()) record the creation.
  • VoxelGrid::begin_pass_capture / end_pass_capture add a third channel at the set choke point, fed by the recording hook's rewrite branch. History::begin_voxel_step opens it next to the change sink. When a step closes and the record changed, the step is recorded as a VoxelLayerProperty Pass instead of a Voxel step. Undo, redo, the budget and the journal all use the existing VoxelLayerProperty path, so there is no new step or event kind and no new resolver.
  • An edit inside a layer that changed no cell is still dropped, like every no-op edit, and its entries in the record are rolled back with it. My first version left them in place. That broke a later case: after undoing an earlier pass and redoing it, every following pass found a record of a different length from the one it recorded, so its redo was refused. Its journal replay was refused too, because the journal never saw the dropped edit. The regression test for this fails when the rollback is removed. With undo off, nothing is captured and the record behaves as before.
  • Replay is strict. A Pass only applies to a record of exactly the length it recorded. Anything else is refused before a cell moves, like the other layer operations.

What a host will see: a pass inside a new layer now takes two undos (the edit, then the layer). Undoing past the begin ends the recording, and redoing it brings the layer back closed. Recording is not saved, and end_sculpt_layer is not a step, so reopening the layer on redo would get a host that had already ended the pass refused at its next begin.

ABI: no change, stays at 0.120.0. No entry point is added and no signature changes. The SculptLayerOp journal encoding gains pass_afters. That encoding arrived after v0.120.0 (#647), so no shipped journal uses the old layout.

Docs: the undo notes in bindings/c/clay.h, the "What is NOT covered" section of docs/05, the ROADMAP row, and the OpenSpec change make-voxel-sculpt-layer-creation-a-step. The change ADDs a scene-model requirement rather than modifying the shared undo requirement, to stay clear of #641.

Tests

  • tests/unit/test_c_voxel_layer_history.cpp: three new cases. They cover undoing an edit inside a layer (record emptied, bytes restored, a dial moves nothing, the layer is still recording), undo and redo of a creation, and a journal replayed onto a snapshot older than the layer (exactly 5 events, recovered bytes identical). All three fail on main with the numbers in the table above. Existing depth expectations went up by one for the creation step.
  • tests/unit/test_voxel_layer_history.cpp: five new cases plus a round trip of the Pass encoding. They cover an edit that rewrites the pass's own entries (bit-exact both ways), the no-op rollback (undo/redo/journal after a no-cell edit), undoing a creation ending the recording, and an undo of a creation that still holds a pass being refused. The pinned "dialled pass replays from the journal" case now snapshots before the layers exist, as the issue asked, and undoes all the way back to the snapshot.
  • bindings/python/tests/test_voxel_layer_history.py: one new case through pyclay. Existing depths updated.
  • Full unit suite: 2941 / 2941 cases pass.
  • python3 tools/release_check.py --skip-slow: version, configure, build, parity, layering, dialect, licenses, task-symbols, bindings (imported this build's pyclay), kernels, abi and openspec all pass. The tests row failed on the first run because of a mistake in my new pytest case (it expected a redo past a new edit), since fixed: pyclay_pytest passes (781 passed, 1 skipped). The device row fails with "engine changed since the gate ran at 704f2d4". That is a release-time hardware gate, and it is already stale on main (29 engine files changed since that commit).

Closes #642

…o step

begin_sculpt_layer added a record to the grid's stack that the session
history never saw, and an edit made inside the layer recorded its cells as a
plain Voxel step without what it did to that record. Undoing a dab therefore
left the cells listed in the layer, and the next dial recomposed and put them
back (54 cells in the C ABI probe); a journal replayed onto a snapshot older
than the layer rebuilt the cells but not the stack and was refused at the
first dial.

SculptLayerOp gains two appended kinds. Begin is the creation: undo pops the
top layer and ends recording, redo pushes it back closed. Pass is an edit made
while a layer records: the cells plus the record half (length before,
rewritten afters old and new, appended entries), captured at the recording
hook through a pass capture the history opens beside its change sink. Both
replay through apply_sculpt_layer_op and the existing VoxelLayerProperty step
and journal event. An edit inside a layer that changed no cell is still
dropped, and its record entries are rolled back with it, so no later pass
record names a length the record does not have.

Both bindings record the creation. No entry point is added; the op's journal
encoding gains pass_afters and has not shipped in a release.

Closes #642
@leonardoaraujosantos
leonardoaraujosantos merged commit 4d92b8e into main Sep 23, 2026
16 checks passed
SummerTree pushed a commit to SummerTree/ClayCore that referenced this pull request Oct 7, 2026
Compared against v0.120.0. What moves under a caller: an unconfined
infinite grid reports an unbounded tape.bounds and meshing refuses it
without a region (CyberdyneCorp#645), bounds narrow per operator (CyberdyneCorp#637), the undo
bound reports a grab rather than its node (CyberdyneCorp#648), the node influence
bound widens behind a smooth sibling (CyberdyneCorp#653), the brick build keeps a
grab the cull cannot judge (CyberdyneCorp#652), voxel sculpt-layer operations and
creation are undo steps (CyberdyneCorp#647, CyberdyneCorp#651), and pyclay's multires Layer
brushes gain layer_height (CyberdyneCorp#636, CyberdyneCorp#646).

The ABI section is diffed against the tag: zero symbols added or
removed, no '-' line inside a typedef struct. The device gate result
is a marked placeholder until the iOS 27.0 same-OS run lands.
SummerTree pushed a commit to SummerTree/ClayCore that referenced this pull request Oct 7, 2026
v0.120.0's notes said its hardware and device gates were stale: that
section was written before the gates ran and not updated before the tag.
As tagged, the four hardware gates were waived at 59e42cc and the device
gate passed at 704f2d4 on iOS 27.0 -- the re-baseline run, which is why
v0.120.0 had no regression coverage. Corrected with a dated note above the
original text, which is kept.

make-voxel-sculpt-layer-creation-a-step (CyberdyneCorp#651) had all 11 tasks ticked
and was not archived by the PR that finished it; archived here, with its
scene-model delta synced. The roadmap header is recounted: 21
capabilities, 240 archived, 41 open.

The v0.120.1 notes now say why this is a patch although pyclay gained a
keyword argument, citing the 0.24.2 precedent.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Creating a voxel sculpt layer is not a history event: an undone pass stays in its record, and a journal loses the stack

1 participant