Skip to content

Let an item carry its own mirror axes (#664) - #673

Merged
leonardoaraujosantos merged 2 commits into
mainfrom
feat/664-per-item-mirror-axes
Oct 4, 2026
Merged

leonardoaraujosantos merged 2 commits into
mainfrom
feat/664-per-item-mirror-axes

Conversation

@leonardoaraujosantos

Copy link
Copy Markdown
Contributor

Fixes #664

Why

An item's mirror participation was a bool, and the axes lived on the layer. A host that mirrors what is made while symmetry is on, and wants existing items left as they were made, could only get half of that. Turning symmetry off, or switching X to Y, changed every item made under the old axes. This PR takes ask 1 from the issue: per-item axes, plus a setter and a reader for a placed node.

I measured this through the C ABI. The fixture is a lump of radius 0.25 at (0.6, 0.4, 0), made under a layer mirror X. The table reads the field at the centre of the X twin, (-0.6, 0.4, 0), and at the Y reflection, (0.6, -0.4, 0). A value of -0.25 means a copy is there.

layer mirror after the item was made X twin Y reflection
X (as made) -0.25 0.55
off, before 0.95 (twin gone) 0.55
Y, before 0.95 -0.25 (reflected across the wrong plane)
off, item has own axes X -0.25 0.55
Y, item has own axes X -0.25 0.55

What lands

  • Model. New field scene::Node::own_mirror_axes: kMirrorX|Y|Z, 0 for none, or kMirrorAxesInherit (0xFF, the default).
    • scene::effective_mirror_axes(item, layer) returns the item's own axes if it has them. Otherwise it returns the layer's axes when the item participates, and none when it doesn't.
    • Every consumer reads that one function: emit_item, the geometry and influence bounds, the cull pad (the seam term, plus a raise-only CullPadTerms::own_mirror_axes union in layer_symmetry_multiplicity), picking's selection bound, consolidation's patch test, and the layer digest.
    • An item's own axes replace the layer's for that item, whatever the participation flag says. The flag still decides what an inheriting item takes from the layer, and it still decides radial participation. The seam (mirror_k) and the mirror planes stay the layer's.
  • Undo. New SetNodeMirrorCmd {layer, node, mirror, own_mirror_axes} sets both values in one undo step. It has a new journal tag, and it is listed in command_edited_item.
  • C ABI. New macro CLAY_MIRROR_AXES_INHERIT, and four new functions:
    • clay_item_set_mirror_axes and clay_item_mirror_axes for the item builder.
    • clay_layer_set_node_mirror (undoable) and clay_layer_node_mirror for a placed node. The reader also reports the axes the item is actually reflected through.
    • Groups are refused. The header documents what the calls do and what they don't promise.
  • Format. Scene and .clayspace minor goes from 19 to 20: one byte is appended to each node record, gated on both the write and read sides.
    • Documents written at older minors load with every item inheriting, so they evaluate exactly as saved.
    • Writing below minor 20 is refused for a document that holds an item with its own axes. This follows minor 18's precedent, because dropping the byte would give the item the layer's copies instead of its own.
    • layer_blocking_minor names the layer that blocks. The C ABI gives a distinct refusal message for this case.
    • A document whose items all inherit still writes 19's exact bytes, and a test checks that.
  • Move and magnify: the intended behaviour. An item that carries its own axes is reached through its own reflections, not the layer's. This follows brush/move.h's existing rule: the drag images are the copies the compiler emits of this item.
    • With the layer mirror off, a drag moves both sides of an item that kept X, because both sides are that item.
    • A drag moves only the touched side of an item held at 0.
    • A host that wants a formerly mirrored item to move on one side sets that item's axes to 0.
    • Inheriting items keep reading the layer's image set unchanged. Items with their own axes get a lazily built set keyed on (axes, radial), so A Move drag on a mirror plane is applied twice: its reflected image coincides with the drag #663's coincident-image grouping applies per set.
    • The reach used for invalidation (clay_layer_move_surface, clay_layer_magnify_surface, the live move transaction) adds the dragged items' own axes via prepared_own_mirror_axes.
  • pyclay. Layer.add(..., mirror_axes=), Layer.set_node_mirror, Layer.node_mirror.
  • Specs and docs. OpenSpec change add-per-item-mirror-axes, with ADDED requirements in scene-model, c-abi, brush-engine, file-io and python-bindings. Also updated: docs/05, docs/07, brush/move.h and the clay.h notes.

ABI 0.120.1 -> 0.121.0. Version moved in all three files. Format minor 19 -> 20. The nine gallery .clayspace documents are regenerated at minor 20; renders, .ply and .obj outputs are discarded, as the verify skill says.

What building it found

  • The chain-pad multiplicity is per layer, so it could not see item axes. An item with its own X|Y on an unmirrored layer counts as three contributors, not one. The own-axes union rides the pad terms, so the incremental cull index keeps its raise-only append contract. A placed-node change is not an AddNodeCmd, so it takes the general invalidation.
  • The reach is taken from the prepared items, not the layer. A layer walk would add an O(items) pass to every frame of a live drag. Only a reached item can move a copy, so the prepared items are enough.
  • The downgrade can't keep the "exact" cases. An item whose own axes equal what it would inherit could in principle be written at 19. But the node writer has no layer context, and instance layers that share content can carry different mirrors. So the writer refuses any document that holds own axes.
  • One existing test assumed nothing was added after minor 18. test_layer_composition's byte-count test compared minor 17 with the current minor. It now compares against 18.

Verification

  • Regression tests. tests/unit/test_item_mirror_axes.cpp: 10 cases, 209 assertions. It covers:

    • old documents load inheriting, evaluate sample-for-sample unchanged, and rewrite 19's bytes
    • own axes round-trip at 20 and block every minor below
    • own axes override the layer
    • switching the layer mirror leaves own-axes items alone
    • the brick cache sees an own-axes twin
    • undo/redo of clay_layer_set_node_mirror
    • refusals, and saving below 20 names the blocking layer
    • drag images
    • chain-pad multiplicity

    Plus test_pyclay.py::test_an_item_keeps_its_own_mirror_axes_when_the_layer_mirror_changes.

  • Proof that the tests fail without the fix. The tests call the new API, so they cannot compile against main as-is. Instead I put main's behaviour back one mechanism at a time and ran the file:

    mechanism reverted to main's behaviour test cases failing
    effective_mirror_axes ignores own axes 7 of 10 (16 assertions)
    bound ignores own axes the brick-cache case
    drag images ignore own axes the drag case
    node byte not serialized round-trip and save cases (2)
    cull term ignores own axes the chain-pad case
  • Gates, all run on macOS arm64:

    • ctest --preset cpu-only (tests and pyclay on): 11/11 passed. That covers all four unit shards and pyclay_pytest.
    • tools/release_check.py --skip-slow: every row passes except device. The passing rows are version, configure, build, tests (11/11), cpu parity, layering, dialect, licenses, task-symbols, bindings (it imported the built pyclay, so the parity comparison is real), kernels, abi, openspec, and the waived hardware rows.
    • device FAILS: "engine changed since the gate ran at 7f6cf38". That record predates Carry a seed across a region edit only if no append since reached it #668 and Resolve coincident drag images as one brush (#663) #669, and main already differs from it in 9 engine files, so main fails this row too. It is a release-time gate on the reference iPad, and I did not run it.
    • Standalone runs, all OK: check_layering, check_kernel_dialect, check_licenses, check_test_shards --binary, check_doc_latency, check_c_abi against libclay_shared.dylib, check_binding_parity --require-import, and npx @fission-ai/openspec@1.12.0 validate --all --strict (67 passed).
    • check_gallery: I regenerated the gallery first with CLAY_EXAMPLES_FAST=1 examples/run_all.py (76/76 examples succeeded) and committed the nine .clayspace files at minor 20. The gate then passes.
    • GCC: g++-16 -fsyntax-only -Wall -Wextra -Wpedantic -Wshadow -Werror is clean on every changed engine file, clay_c.cpp and the new test. This stands in for the Ubuntu job, which AppleClang cannot reproduce.

Concerns

  • Pre-existing complexity. read_node and deserialize in src/scene/commands.cpp are over the parser target on main already: 40 and 37. They are 41 and 37 here, since the node byte adds one gated read and the new case adds nothing. Every function added or touched in this PR scores 12 or less.
  • 18_sampled.clayspace shrank by 9 KB (314,720 -> 305,397 bytes) when regenerated on this machine. The example uses no mirror or drag code, and the node byte adds bytes rather than removing them. So this looks like the platform float variation check_gallery.py describes for band-sampled output. I did not confirm that by regenerating on main.
  • Not measured. I did not run the drag benchmarks (BM_MoveDrag*) or the device gate. The inheriting path builds and reads the same image set it did before, and costs one extra branch per item.

An item's mirror participation was a bool and the axes lived on the layer,
so turning symmetry off or moving it from X to Y changed every item made
under the old axes: a lump made on +x under X lost its -x twin.

Node::own_mirror_axes holds the item's own x|y|z, 0 for none, or inherit
(the default, and what every older document loads as). Own axes replace
the layer's for that item outright; the participation flag keeps deciding
what an inheriting item takes and the radial mode. effective_mirror_axes is
the one definition the compiler, bounds, cull pad (seam term and a raise-only
own-axes union in the symmetry multiplicity), picking, consolidation and the
Move brush read.

The Move/magnify drag reaches an item with its own axes through its own
reflections, built per (axes, radial) set; inheriting items keep reading the
layer's set unchanged. The reported reach adds the dragged items' own axes.

SetNodeMirrorCmd sets a placed item's participation and axes as one undo
step. C ABI 0.120.1 -> 0.121.0 adds CLAY_MIRROR_AXES_INHERIT,
clay_item_set_mirror_axes / clay_item_mirror_axes and
clay_layer_set_node_mirror / clay_layer_node_mirror; pyclay gains
add(mirror_axes=), set_node_mirror and node_mirror.

Scene and container minor 19 -> 20 append one byte per node. Writing below
20 is refused for a document holding own axes, as minor 18 refuses a
composition, and a document whose items all inherit still writes 19's bytes.
The gallery documents are regenerated at minor 20.
The existing cases checked drag_images directly, but nothing covered the
two callers that fold the dragged items' own axes into the reach: dropping
prepared_own_mirror_axes from SdfMoveTransaction::begin left every case
green. Assert the twin's box through clay_layer_move_surface_regions and a
live drag's dirty bounds; each fails when its caller passes 0.

Also fix two comments naming functions that do not exist
(SetItemMirrorCmd, images_for).
@leonardoaraujosantos
leonardoaraujosantos merged commit 0f45417 into main Oct 4, 2026
16 checks passed
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.

Mirror participation is a bool per item, so a host cannot turn symmetry off or change axis without changing what was made under the old mirror

1 participant