diff --git a/CMakeLists.txt b/CMakeLists.txt index 98e92f2c..8fab8a65 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.24) project(claycore - VERSION 0.120.1 + VERSION 0.121.0 DESCRIPTION "Portable C++20 SDF + voxel engine library" LANGUAGES C CXX) # C: ufbx diff --git a/bindings/c/clay.h b/bindings/c/clay.h index e9309f47..0b879a68 100644 --- a/bindings/c/clay.h +++ b/bindings/c/clay.h @@ -24,8 +24,8 @@ extern "C" { #endif #define CLAY_ABI_MAJOR 0 -#define CLAY_ABI_MINOR 120 -#define CLAY_ABI_PATCH 1 +#define CLAY_ABI_MINOR 121 +#define CLAY_ABI_PATCH 0 /* Upper bound on the element count of any batch call: points, rays, cells, * selected node ids, stroke points, polygon vertices. A count above it is @@ -1817,7 +1817,12 @@ clay_result clay_document_layer_composition(const clay_document* doc, clay_layer * line every other minor draws: subdivision is deterministic, so a hierarchy * without detail rebuilds from its cage and losing it is the ordinary "smaller * or plainer" degrade. Detail is something an artist made and cannot be - * rebuilt. */ + * rebuilt. + * + * Minor 20 (ABI 0.121.0) adds a third: an item carrying its OWN mirror axes + * (clay_item_set_mirror_axes) blocks every minor below 20, since an older + * build would hand it the layer's twins instead. Put such items back on the + * layer's mirror (CLAY_MIRROR_AXES_INHERIT) to write for an older build. */ clay_result clay_document_writable_at_minor(const clay_document* doc, uint32_t minor, clay_layer_id* out_blocking_layer); @@ -1848,9 +1853,10 @@ clay_result clay_document_writable_at_minor(const clay_document* doc, uint32_t m * WHAT AN OLDER MINOR ACTUALLY LOSES, said once rather than per call: 14 comes * back unsquashed, 15's instances come back as copies, 17 writes each payload * once per node, and below 19 a hierarchy carrying no detail is absent and - * rebuilds from its cage. Every one of those is smaller or plainer. The two + * rebuilds from its cage. Every one of those is smaller or plainer. The three * that would be a DIFFERENT SCULPTURE — a composition below 18, a hierarchy - * with detail below 19 — are refused instead. */ + * with detail below 19, an item carrying its own mirror axes below 20 (ABI + * 0.121.0) — are refused instead. */ clay_result clay_document_save_at_minor(const clay_document* doc, const char* path, uint32_t minor, clay_layer_id* out_blocking_layer); @@ -2229,6 +2235,12 @@ clay_result clay_layer_zero_to_origin(clay_document* doc, clay_layer_id layer); * mirror = 1 (#60); 1 is still accepted and means what it meant. Layers with * no mirror axes evaluate exactly as before, whatever the items' flags. * + * THE LAYER'S AXES ARE A DEFAULT, since ABI 0.121.0 (#664): an item that + * carries its own (clay_item_set_mirror_axes, clay_layer_set_node_mirror) is + * reflected through those instead, and this call no longer changes it. Items + * that never set any — every item in every document saved before — follow + * this call exactly as they always did. + * * SETTING WHAT THE LAYER ALREADY CARRIES DOES NOTHING, since ABI 0.103.0 * (#536): identical axes and an identical mirror_k return CLAY_OK without * recording an undo step and without invalidating anything. Through 0.102.0 @@ -2469,9 +2481,43 @@ clay_result clay_item_set_rounding(clay_item* item, float rounding); /* >= 0 clay_result clay_item_set_color(clay_item* item, const float rgb[3]); /* Layer-mirror participation, the builder's spelling of clay_item_desc.mirror: * -1 excludes the item from the layer's mirror, 0 and 1 follow it (the - * default). See clay_set_layer_mirror. */ + * default). See clay_set_layer_mirror. Once the item carries its own axes + * (clay_item_set_mirror_axes) this decides only its RADIAL participation. */ clay_result clay_item_set_mirror(clay_item* item, int32_t mirror); +/* "Take the layer's mirror axes": the default own-axes value of every item, + * and what every document saved before ABI 0.121.0 loads with. */ +#define CLAY_MIRROR_AXES_INHERIT 255 + +/* The item's OWN mirror axes, since ABI 0.121.0 (#664): CLAY_MIRROR_X|Y|Z OR'd + * together, 0 for none, or CLAY_MIRROR_AXES_INHERIT to follow the layer again. + * Anything else is CLAY_ERROR_INVALID_ARGUMENT. + * + * Set, the item's axes REPLACE the layer's for this item outright — whatever + * the layer's mirror is now or is changed to later, and whatever the item's + * participation flag says. That is what lets a host keep what was made under + * a symmetry as it was made: give each new item the axes symmetry had when it + * was made, and turning the layer's mirror off or pointing it at Y no longer + * moves any of them. An item set to 0 has no mirror copy on any layer. + * + * WHAT STAYS THE LAYER'S: the seam (mirror_k), the planes (the layer's local + * frame), and the radial mode, which still follows clay_item_set_mirror. A + * feathered volume replace takes no mirror copies whatever it carries, as it + * never has (clay_volume_params.feather). + * + * A Move or magnify drag reaches such an item through ITS reflections: both + * sides of an item that kept X move together, and an item held at 0 moves on + * the side the drag touched only (clay_layer_move_surface). + * + * NOT FREE ON THE WAY DOWN: a document holding an item with its own axes + * cannot be written at a format minor below 20 — clay_document_save_at_minor + * refuses it and names the layer — because dropping them would hand the item + * the layer's twins instead of its own. */ +clay_result clay_item_set_mirror_axes(clay_item* item, uint8_t axes); +/* What clay_item_set_mirror_axes last set: CLAY_MIRROR_AXES_INHERIT for a + * builder that never set any. */ +clay_result clay_item_mirror_axes(const clay_item* item, uint8_t* out_axes); + /* Appends one domain warp (clay_deform) to the item's chain: the local point * is warped by the first one added first. params/param_count are the kind's * own, listed on clay_deform; ease indexes an easing curve and is read by the @@ -3001,6 +3047,33 @@ clay_result clay_layer_node_op_blend(const clay_document* doc, clay_layer_id lay clay_result clay_layer_node_color(const clay_document* doc, clay_layer_id layer, clay_node_id node, float out_rgb[3]); +/* A PLACED item's mirror participation and own axes, set as one undoable edit + * (#664) — the same two values clay_item_set_mirror and + * clay_item_set_mirror_axes give a builder, for an item already in the + * document. `mirror` takes clay_item_set_mirror's rule (negative excludes, 0 + * and 1 follow); `axes` takes clay_item_set_mirror_axes's, so + * CLAY_MIRROR_AXES_INHERIT puts the item back on the layer's mirror. Before + * this a host could correct neither without removing and re-adding the item, + * which changes its id and its place in the chain. + * + * One command, one undo step: undo puts back both values the item had. + * Refused on a protected layer as every node edit is. A GROUP is + * CLAY_ERROR_INVALID_ARGUMENT: evaluation never reads a group's mirror, so a + * value there would be a control that does not act. Setting the values the + * item already holds is still recorded — it is not the layer mirror's no-op + * short-circuit — and invalidates only the item's own influence. */ +clay_result clay_layer_set_node_mirror(clay_document* doc, clay_layer_id layer, + clay_node_id node, int32_t mirror, uint8_t axes); +/* The reader: `*out_mirror` 1 when the item follows the layer's mirror and -1 + * when it opted out; `*out_axes` its own axes or CLAY_MIRROR_AXES_INHERIT; + * `*out_effective_axes` the axes it is ACTUALLY reflected through on this + * layer right now — its own, else the layer's when it participates, else 0, + * and 0 for a feathered volume replace. Any out pointer may be null. A group + * is CLAY_ERROR_INVALID_ARGUMENT, as its setter is. */ +clay_result clay_layer_node_mirror(const clay_document* doc, clay_layer_id layer, + clay_node_id node, int32_t* out_mirror, uint8_t* out_axes, + uint8_t* out_effective_axes); + /* The layer's TOP-LEVEL nodes, count-then-index, in the layer's EVALUATION * order — index 0 is the node evaluated first, and the index is the one * clay_layer_add_group and clay_add_item_in_group place against. An index of diff --git a/bindings/c/clay_c.cpp b/bindings/c/clay_c.cpp index 8624fdc5..6382607a 100644 --- a/bindings/c/clay_c.cpp +++ b/bindings/c/clay_c.cpp @@ -784,6 +784,20 @@ clay_result check_palette_index(std::int32_t index, std::uint8_t* out) { return CLAY_OK; } +// An item's own mirror axes (#664): CLAY_MIRROR_X|Y|Z OR'd, 0, or the inherit +// sentinel. Kept apart from check_mirror_axes because the sentinel is only +// meaningful here, and accepting it for a voxel edit would be a control that +// means nothing there. +clay_result check_own_mirror_axes(std::uint8_t axes) { + static_assert(CLAY_MIRROR_AXES_INHERIT == scene::kMirrorAxesInherit, + "the C sentinel and the engine's must be one value"); + constexpr unsigned kAll = CLAY_MIRROR_X | CLAY_MIRROR_Y | CLAY_MIRROR_Z; + if (axes == scene::kMirrorAxesInherit || (axes & ~kAll) == 0) return CLAY_OK; + return fail(CLAY_ERROR_INVALID_ARGUMENT, + "mirror axes outside CLAY_MIRROR_X|Y|Z and not CLAY_MIRROR_AXES_INHERIT: " + + std::to_string(axes)); +} + clay_result check_mirror_axes(std::int32_t axes, std::uint8_t* out) { constexpr std::int32_t kAll = CLAY_MIRROR_X | CLAY_MIRROR_Y | CLAY_MIRROR_Z; if (axes < 0 || (axes & ~kAll) != 0) @@ -5337,6 +5351,11 @@ eval::DeviceBuffer brick_slot(const eval::DeviceBuffer& whole, std::size_t at, s // SENTENCES: a host reading "a layer carries a composition" about a layer that // carries a hierarchy would go and look at the wrong thing. std::string minor_refusal(const clay_document* doc, std::uint16_t at, clay_layer_id blocking) { + // At 19 the one scene field that blocks is an item's own mirror axes + // (#664), which layer_blocking_minor checks first at every minor below 20. + if (at < 20 && scene::layer_blocking_minor(doc->doc.document, 19) == blocking) + return "an item carries its own mirror axes, which this format minor cannot say: " + "writing it there would hand the item the layer's mirror copies instead"; if (scene::layer_blocking_minor(doc->doc.document, at) == blocking) return "a layer carries a composition that this format minor cannot say: writing it " "there would turn a cutting layer into a unioning one"; @@ -6256,6 +6275,42 @@ clay_result clay_layer_node_color(const clay_document* doc, clay_layer_id layer, return CLAY_OK; } +clay_result clay_layer_set_node_mirror(clay_document* doc, clay_layer_id layer, + clay_node_id node, int32_t mirror, uint8_t axes) { + clay_result r = check_own_mirror_axes(axes); + if (r != CLAY_OK) return r; + // A group's flag is never read by evaluation (emit_item copies items), so + // a value there would be a control that does not act. A miss falls + // through to apply_edit's NOT_FOUND, as the op/blend setter's does. + const scene::Node* target = peek_node(doc, layer, node); + if (target && target->is_group) + return fail(CLAY_ERROR_INVALID_ARGUMENT, + "a group takes no mirror: set it on the items inside"); + return apply_edit(doc, + scene::Command{scene::SetNodeMirrorCmd{layer, node, mirror >= 0, axes}}, + "node not found"); +} + +clay_result clay_layer_node_mirror(const clay_document* doc, clay_layer_id layer, + clay_node_id node, int32_t* out_mirror, uint8_t* out_axes, + uint8_t* out_effective_axes) { + const scene::Node* n = nullptr; + clay_result r = find_node(doc, layer, node, &n); + if (r != CLAY_OK) return r; + if (n->is_group) + return fail(CLAY_ERROR_INVALID_ARGUMENT, "a group takes no mirror: ask the items inside"); + if (out_mirror) *out_mirror = n->mirror ? 1 : -1; + if (out_axes) *out_axes = n->own_mirror_axes; + if (out_effective_axes) { + // find_node resolved the layer, so it is there. + const scene::Layer* l = doc->doc.document.find_layer(layer); + *out_effective_axes = scene::item_is_feathered_replace(*n) + ? std::uint8_t{0} + : scene::effective_mirror_axes(*n, *l); + } + return CLAY_OK; +} + clay_result clay_layer_node_count(const clay_document* doc, clay_layer_id layer, size_t* out_count) { if (!doc || !out_count) return fail(CLAY_ERROR_INVALID_ARGUMENT, "null document or count"); @@ -7149,6 +7204,20 @@ clay_result clay_item_set_mirror(clay_item* item, int32_t mirror) { return CLAY_OK; } +clay_result clay_item_set_mirror_axes(clay_item* item, uint8_t axes) { + if (!item) return fail(CLAY_ERROR_INVALID_ARGUMENT, "null item"); + clay_result r = check_own_mirror_axes(axes); + if (r != CLAY_OK) return r; + item->node.own_mirror_axes = axes; + return CLAY_OK; +} + +clay_result clay_item_mirror_axes(const clay_item* item, uint8_t* out_axes) { + if (!item || !out_axes) return fail(CLAY_ERROR_INVALID_ARGUMENT, "null item or out_axes"); + *out_axes = item->node.own_mirror_axes; + return CLAY_OK; +} + clay_result clay_item_add_deformer(clay_item* item, int32_t deform, const float* params, size_t param_count, int32_t ease) { if (!item) return fail(CLAY_ERROR_INVALID_ARGUMENT, "null item"); @@ -8859,10 +8928,13 @@ clay_result move_surface_impl(clay_document* doc, clay_layer_id layer, const flo const float pull = std::sqrt(displacement[0] * displacement[0] + displacement[1] * displacement[1] + displacement[2] * displacement[2]); + // The dragged items' OWN mirror axes join the layer's (#664): an item that + // kept a twin the layer's mirror no longer names moves it too. std::vector reach; for (const brush::DragImage& image : brush::drag_images(*l, kernel::cf3(centre[0], centre[1], centre[2]), - kernel::cf3(displacement[0], displacement[1], displacement[2]))) + kernel::cf3(displacement[0], displacement[1], displacement[2]), + brush::prepared_own_mirror_axes(prepared))) reach.push_back(math::Aabb{image.centre, image.centre}.dilated(radius + pull)); const kernel::cfloat3 world_pull = @@ -9014,7 +9086,8 @@ clay_result clay_layer_magnify_surface(clay_document* doc, clay_layer_id layer, std::vector reach; for (const brush::DragImage& image : brush::drag_images(*l, kernel::cf3(centre[0], centre[1], centre[2]), - kernel::cf3(0.0f, 0.0f, 0.0f))) + kernel::cf3(0.0f, 0.0f, 0.0f), + brush::prepared_own_mirror_axes(prepared))) reach.push_back(math::Aabb{image.centre, image.centre}.dilated(radius)); GestureResolver resolver; diff --git a/bindings/python/pyclay_module.cpp b/bindings/python/pyclay_module.cpp index d8a4cc9f..ef191a0e 100644 --- a/bindings/python/pyclay_module.cpp +++ b/bindings/python/pyclay_module.cpp @@ -154,6 +154,26 @@ int parse_axis(const std::string& axis) { throw std::invalid_argument("axis must be 'x', 'y' or 'z'"); } +// An item's own mirror axes (#664), spelled as the letters it reflects across: +// "x", "xz", "" for none — or None to take the layer's (the inherit sentinel). +std::uint8_t parse_own_mirror_axes(nb::handle axes) { + if (axes.is_none()) return scene::kMirrorAxesInherit; + std::uint8_t mask = 0; + for (char c : nb::cast(axes)) + mask = static_cast(mask | (1u << parse_axis(std::string(1, c)))); + return mask; +} + +// The inverse: the letters, or None for an item that takes the layer's axes. +nb::object own_mirror_axes_object(std::uint8_t axes) { + if (axes == scene::kMirrorAxesInherit) return nb::none(); + std::string out; + if (axes & scene::kMirrorX) out += 'x'; + if (axes & scene::kMirrorY) out += 'y'; + if (axes & scene::kMirrorZ) out += 'z'; + return nb::str(out.c_str()); +} + voxel::BrushShape parse_brush_shape(const std::string& shape) { if (shape == "cube") return voxel::BrushShape::Cube; if (shape == "sphere") return voxel::BrushShape::Sphere; @@ -6040,7 +6060,7 @@ NB_MODULE(pyclay, m) { .def("add", [](PyLayer& l, const PyPrim& prim, scene::Op op, nb::handle blend, nb::handle color, nb::handle rounding, bool mirror, nb::handle transition, nb::handle parent, - int index) { + int index, nb::handle mirror_axes) { if (op == scene::Op::None) throw std::invalid_argument( "op must be a combine operator, not Op.INLINE — that one is for " @@ -6079,6 +6099,7 @@ NB_MODULE(pyclay, m) { n.rounding = rounding.is_none() ? prim.rounding : nb::cast(rounding); if (n.rounding < 0.0f) throw std::invalid_argument("rounding must be >= 0"); n.mirror = mirror; + n.own_mirror_axes = parse_own_mirror_axes(mirror_axes); if (scene::op_is_transition(op)) { if (transition.is_none()) throw std::invalid_argument( @@ -6099,11 +6120,14 @@ NB_MODULE(pyclay, m) { }, "prim"_a, "op"_a = scene::Op::Add, "blend"_a = nb::none(), "color"_a = nb::none(), "rounding"_a = nb::none(), "mirror"_a = true, "transition"_a = nb::none(), - "parent"_a = nb::none(), "index"_a = -1, + "parent"_a = nb::none(), "index"_a = -1, "mirror_axes"_a = nb::none(), "Append an edit to the layer; returns the node id. parent= " "puts it inside that group instead of at the layer root, and index<0 " "appends. mirror=False keeps the item out of the layer's mirror " - "(items follow it by default).") + "(items follow it by default). mirror_axes='x' (or 'xz', or '' for " + "none) gives the item its OWN mirror axes, which replace the layer's " + "for it whatever the layer's mirror is later set to; None, the " + "default, follows the layer.") .def("add_group", [](PyLayer& l, scene::Op op, nb::handle blend, nb::handle color, float rounding, nb::handle parent, int index) { @@ -6587,6 +6611,40 @@ NB_MODULE(pyclay, m) { "set_color", l.undo.get()); }, "node"_a, "color"_a) + .def("set_node_mirror", + [](PyLayer& l, scene::NodeId node, bool mirror, nb::handle axes) { + const scene::Node* n = l.layer().sdf->find(node); + if (!n) throw std::invalid_argument("no node with that id in this layer"); + if (n->is_group) + throw std::invalid_argument("a group takes no mirror: set it on the items"); + apply_or_throw(l.doc->document, + scene::Command{scene::SetNodeMirrorCmd{ + l.id, node, mirror, parse_own_mirror_axes(axes)}}, + "set_node_mirror", l.undo.get()); + }, + "node"_a, "mirror"_a = true, "axes"_a = nb::none(), + "Set a PLACED item's mirror participation and its own axes as one undoable " + "edit (clay_layer_set_node_mirror). axes='x'/'xz'/'' gives the item its " + "own axes; None puts it back on the layer's mirror.") + .def("node_mirror", + [](const PyLayer& l, scene::NodeId node) { + const scene::Node* n = l.layer().sdf->find(node); + if (!n) throw std::invalid_argument("no node with that id in this layer"); + if (n->is_group) + throw std::invalid_argument("a group takes no mirror: ask the items"); + const std::uint8_t effective = scene::item_is_feathered_replace(*n) + ? std::uint8_t{0} + : scene::effective_mirror_axes(*n, l.layer()); + nb::dict d; + d["mirror"] = n->mirror; + d["axes"] = own_mirror_axes_object(n->own_mirror_axes); + d["effective_axes"] = own_mirror_axes_object(effective); + return d; + }, + "node"_a, + "A placed item's mirror state (clay_layer_node_mirror): 'mirror' its " + "participation flag, 'axes' its own axes or None when it follows the " + "layer, and 'effective_axes' the axes it is actually reflected through.") .def("set_op_blend", [](PyLayer& l, scene::NodeId node, nb::handle op, nb::handle blend, nb::handle rounding) { diff --git a/bindings/python/tests/test_pyclay.py b/bindings/python/tests/test_pyclay.py index 06a037a8..0918caaf 100644 --- a/bindings/python/tests/test_pyclay.py +++ b/bindings/python/tests/test_pyclay.py @@ -8710,3 +8710,34 @@ def test_a_refused_save_leaves_the_file_that_was_there_alone(tmp_path): # A save that cannot represent the document must not first destroy the last # one that could. assert path.read_bytes() == before + + +# --- per-item mirror axes (issue #664) --------------------------------------- + + +def test_an_item_keeps_its_own_mirror_axes_when_the_layer_mirror_changes(): + doc = clay.Document() + doc.enable_undo() + layer = doc.add_sdf_layer("body") + layer.mirror(axis="x") + lump = clay.Sphere(r=0.25).at((0.6, 0.4, 0.0)) + kept = layer.add(lump, mirror_axes="x") + twin_x = np.array([[-0.6, 0.4, 0.0]], np.float32) + twin_y = np.array([[0.6, -0.4, 0.0]], np.float32) + assert doc.eval(twin_x)[0] < 0 + assert layer.node_mirror(kept) == {"mirror": True, "axes": "x", "effective_axes": "x"} + + # A placed item corrected in place, as one undoable step. + layer.set_node_mirror(kept, mirror=True, axes="y") + assert doc.eval(twin_x)[0] > 0 and doc.eval(twin_y)[0] < 0 + assert doc.undo() + assert doc.eval(twin_x)[0] < 0 and doc.eval(twin_y)[0] > 0 + assert layer.node_mirror(kept)["axes"] == "x" + assert doc.redo() + assert layer.node_mirror(kept)["axes"] == "y" + + # None puts it back on the layer's mirror. + layer.set_node_mirror(kept, axes=None) + assert layer.node_mirror(kept) == {"mirror": True, "axes": None, "effective_axes": "x"} + with pytest.raises(ValueError): + layer.set_node_mirror(kept, axes="w") diff --git a/docs/05-claycore-library.md b/docs/05-claycore-library.md index 3d205677..18b175b0 100644 --- a/docs/05-claycore-library.md +++ b/docs/05-claycore-library.md @@ -916,6 +916,15 @@ names the layer that blocks. pyclay has the same question as id rather than only a flag, so a script can say which subtool to change instead of leaving a person to find it. +Scene minor 20 (ABI 0.121.0, #664) is the second field refused this way: an +item's OWN mirror axes (`clay_item_set_mirror_axes`, +`clay_layer_set_node_mirror`; see docs/07, "Symmetry: the layer mirror"). The +node record gains one byte, and an older document loads with every item on its +layer's mirror, which is exactly what it meant. A document where every item +inherits still writes minor 19's bytes. One holding an item with its own axes +is refused below 20 with its layer named, because dropping the byte would give +that item the layer's copies instead of its own. + ### An intersect is bounded by its layer `item_influence_bound` reported `Everything` for any op that is not local, and diff --git a/docs/07-brushes-and-features.md b/docs/07-brushes-and-features.md index e1cc8507..472ffa61 100644 --- a/docs/07-brushes-and-features.md +++ b/docs/07-brushes-and-features.md @@ -152,6 +152,26 @@ the same as an add mirrors the deposit. Until 0.27.3 this flag was an **opt-in that defaulted to excluded**, which read as the layer mirror doing nothing at all (#60). Layers with no mirror axes cost nothing either way. +**An item can carry its own axes** (#664, ABI 0.121.0, format minor 20). The +flag above can only say "follow the layer or don't", 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 (the field at the twin's centre went from -0.25 +to 0.95). `Node::own_mirror_axes` (C: `clay_item_set_mirror_axes` on a builder, +`clay_layer_set_node_mirror` on a placed item; Python: `mirror_axes="x"` on +`add`, `Layer.set_node_mirror`) holds the item's own `x|y|z`, `0` for none, or +inherit, which is the default and what every older document loads as. Own axes +replace the layer's for that item outright, whatever the participation flag +says. A host that stamps each new item with the axes symmetry has at the time +gets both halves: switching the layer's mirror changes only what inherits. +The seam (`mirror_k`), the planes and the radial mode stay the layer's; radial +participation is still the flag's. `scene::effective_mirror_axes` is the one +definition the compiler, the bounds, the cull pad, picking and the Move brush +read. A placed item's pair is one undoable command (`SetNodeMirrorCmd`), and +`clay_layer_node_mirror` reports it together with the axes the item is actually +reflected through. A document holding own axes cannot be written below minor 20: +the write is refused and the layer named, because an older build would give +the item the layer's copies. + **Setting the symmetry to what the layer already carries is not an edit** (#536, ABI 0.103.0). Identical axes and an identical `mirror_k` — or, for the radial mode below, an identical count, axis and `radial_k` — leave the document @@ -324,7 +344,12 @@ its **own** bound against each image. An image that reaches an item gives it a grab at that image's centre with that image's displacement, so the reflected ball grabs the items whose reflections sit under the ball. Items that opted out of the mirror (`mirror=False`, `-1` in C) and feathered volume replaces see the -ball alone, because the compiler emits no copy of them. An item both images +ball alone, because the compiler emits no copy of them. An item carrying its +OWN mirror axes (#664) is tested against ITS reflections instead of the +layer's. That is intended: an item that kept X after the layer's mirror went +off moves on both sides under a drag, because both sides are the item, and one +held at 0 moves on the touched side only. The gesture's reach adds the dragged +items' own axes (`brush::prepared_own_mirror_axes`), so that twin is dirtied too. An item both images reach — one straddling the plane — takes both grabs in one warp, **composing the two pulls as two brushes would** when the two balls are distinct; pulling across the plane from on it, the two cancel to a pinch. diff --git a/examples/output/08_combined.clayspace b/examples/output/08_combined.clayspace index 9c40b9cc..9b76b495 100644 Binary files a/examples/output/08_combined.clayspace and b/examples/output/08_combined.clayspace differ diff --git a/examples/output/08_scene.clayspace b/examples/output/08_scene.clayspace index b7fbb85a..5fbca34f 100644 Binary files a/examples/output/08_scene.clayspace and b/examples/output/08_scene.clayspace differ diff --git a/examples/output/11_masked.clayspace b/examples/output/11_masked.clayspace index 6cee9b7a..d672b280 100644 Binary files a/examples/output/11_masked.clayspace and b/examples/output/11_masked.clayspace differ diff --git a/examples/output/18_sampled.clayspace b/examples/output/18_sampled.clayspace index adb732e1..85807bfe 100644 Binary files a/examples/output/18_sampled.clayspace and b/examples/output/18_sampled.clayspace differ diff --git a/examples/output/37_groups.clayspace b/examples/output/37_groups.clayspace index 3da0d15f..cf68f2e4 100644 Binary files a/examples/output/37_groups.clayspace and b/examples/output/37_groups.clayspace differ diff --git a/examples/output/52_sculpt_layers.clayspace b/examples/output/52_sculpt_layers.clayspace index bbf88d13..18cd489a 100644 Binary files a/examples/output/52_sculpt_layers.clayspace and b/examples/output/52_sculpt_layers.clayspace differ diff --git a/examples/output/53_sdf_alphas.clayspace b/examples/output/53_sdf_alphas.clayspace index 0fd08ed7..5a638248 100644 Binary files a/examples/output/53_sdf_alphas.clayspace and b/examples/output/53_sdf_alphas.clayspace differ diff --git a/examples/output/54_masked_operations.clayspace b/examples/output/54_masked_operations.clayspace index a40b4cfa..46598a3b 100644 Binary files a/examples/output/54_masked_operations.clayspace and b/examples/output/54_masked_operations.clayspace differ diff --git a/examples/output/63_groups.clayspace b/examples/output/63_groups.clayspace index 614a364d..39a76f47 100644 Binary files a/examples/output/63_groups.clayspace and b/examples/output/63_groups.clayspace differ diff --git a/include/clay/brush/move.h b/include/clay/brush/move.h index 96a7e0d8..d456f6c8 100644 --- a/include/clay/brush/move.h +++ b/include/clay/brush/move.h @@ -53,6 +53,18 @@ // Items that do not participate in the symmetry see the ball alone. With no // symmetry there is one image and this is byte-for-byte the rule it replaces. // +// AN ITEM CARRYING ITS OWN MIRROR AXES (#664) IS REACHED THROUGH ITS OWN +// REFLECTIONS, not the layer's: the drag, one reflection per axis the ITEM +// sets, then the layer's rotations if the item takes the radial mode. That is +// the same rule — the images are the copies the compiler emits of THIS item — +// and it is the intended behaviour, stated because the alternative reads +// plausibly: an item that kept its X twin after the layer's mirror was turned +// off still moves on both sides under a drag, since both sides ARE the item; +// one whose own axes are 0 moves on the touched side alone whatever the +// layer's mirror says. A host that wants a formerly mirrored item to stop +// following its twin sets that item's axes to 0 — the per-item spelling of +// switching symmetry off for what was already made. +// // ONE WARP PER NODE, CARRYING ONE GRAB PER REACHING IMAGE. An item both images // reach — one straddling the plane — takes both, as two brushes would, and a // drag centred on the plane gives it two grabs of opposite pull that compose @@ -136,8 +148,15 @@ struct DragImage { // Exposed because the gesture's REACH is the union of these balls: the copies // move where the images are, and a host that invalidates the ball alone // serves the reflected side stale. +// +// `item_axes` adds reflections for axes the DRAGGED items carry as their own +// (#664) — `prepared_own_mirror_axes` of the drag's prepared items — so the +// reach covers the twin of an item whose axes the layer's mirror does not +// set. The reflections are unioned with the layer's, in axis order; zero, the +// default, is exactly the layer's images. std::vector drag_images(const scene::Layer& layer, kernel::cfloat3 world_centre, - kernel::cfloat3 world_displacement); + kernel::cfloat3 world_displacement, + std::uint8_t item_axes = 0); // One item's share of the drag: the grabs that reproduce it in that item's own // frame, one per image of the drag that reaches the item — usually one, two @@ -215,8 +234,14 @@ struct PreparedMove { // The images of the drag this item can see, in `drag_images` order: the // drag itself first, then the copies the layer's symmetry emits. One, // reaching, under no symmetry; one for an item the compiler emits once. + // An item carrying its own mirror axes sees its own reflections in place + // of the layer's (#664). std::vector images; + // The item's own mirror axes when it carries them, else 0 — what + // `prepared_own_mirror_axes` folds into a drag's reach (#664). + std::uint8_t own_mirror_axes = 0; + // The reach in this item's own scaled-local frame, shared by every image. float local_radius = 0.0f; @@ -265,6 +290,11 @@ std::vector prepare_move(const scene::Layer& layer, kernel::cfloat const MoveSettings& settings = {}, MovePrepareStats* out_stats = nullptr); +// The union of the own mirror axes the prepared items carry (#664): what a +// caller passes `drag_images` so the reach covers every copy the drag moves. +// Zero on a layer where no item overrides the mirror. +std::uint8_t prepared_own_mirror_axes(const std::vector& prepared); + // The other half: the warp for a TOTAL world displacement — one grab per image // that reaches the item, coincident images resolved as one brush (see // PreparedImage::leader), ordered by value, and the rest of the gesture's diff --git a/include/clay/io/clayspace.h b/include/clay/io/clayspace.h index 8463b68e..81c56867 100644 --- a/include/clay/io/clayspace.h +++ b/include/clay/io/clayspace.h @@ -215,7 +215,14 @@ inline constexpr std::uint16_t kClaySpaceMajor = 1; // hardest to avoid. Nothing here can stop an older build; what this can do is // say so, and `multires_carries_detail` is what a host asks to know whether a // given document has anything to lose. -inline constexpr std::uint16_t kClaySpaceMinor = 19; +// Minor 20 adds an item's OWN MIRROR AXES (issue #664), an APPENDED scene field +// like 11 and 14; see scene::kSceneMinor for the byte and its meaning. A build +// that predates 20 FAILS on the first node rather than misreading it. Writing +// AT 19 follows 18's rule, not 14's: a document whose items all take the +// layer's mirror writes the bytes it always did, and one holding an item with +// its own axes is REFUSED, because dropping them gives an item the layer's +// twins instead of its own. scene::layer_blocking_minor names the layer. +inline constexpr std::uint16_t kClaySpaceMinor = 20; // The document bundle a .clayspace file holds. Voxel layer content is keyed // by layer id (the scene module stays voxel-agnostic by layering rule). diff --git a/include/clay/scene/bounds.h b/include/clay/scene/bounds.h index 0013fd86..2c8d5b3d 100644 --- a/include/clay/scene/bounds.h +++ b/include/clay/scene/bounds.h @@ -328,7 +328,11 @@ float chain_pad_envelope(BlendProfile profile, std::size_t nodes); // count by this over-counts items that opted out of the mirror, groups and // feathered replaces, which is safe: the chain-pad envelope is monotone and // every per-item term stays clamped at its own support. -std::size_t layer_symmetry_multiplicity(const Layer& layer); +// +// `item_axes` adds the axes items carry as their OWN (issue #664), unioned +// with the layer's — CullPadTerms::own_mirror_axes is where a caller gets it. +// Omitting it is only right for a layer no item overrides. +std::size_t layer_symmetry_multiplicity(const Layer& layer, std::uint8_t item_axes = 0); // How far this node can drag a CHAIN's running value, which is the quantity the // pad below is the maximum of — and NOT the same question as how far the node's @@ -412,6 +416,11 @@ struct CullPadTerms { // so the pre-#335 pad remains derivable from them alone as the ceiling // blend_total clamps the seam term to. float blend_k_seam = 0.0f; + // Union of the mirror axes the layer's items carry as their OWN (#664), + // which layer_symmetry_multiplicity adds to the layer's: an item keeping + // an X twin on a layer whose mirror is off still doubles its place in the + // chain. Raise-only (a union), so an append keeps the incremental contract. + std::uint8_t own_mirror_axes = 0; void raise(const CullPadTerms& o) { feather = kernel::cmax(feather, o.feather); @@ -420,6 +429,7 @@ struct CullPadTerms { blend_k_cubic = kernel::cmax(blend_k_cubic, o.blend_k_cubic); blend_k_circular = kernel::cmax(blend_k_circular, o.blend_k_circular); blend_k_seam = kernel::cmax(blend_k_seam, o.blend_k_seam); + own_mirror_axes = static_cast(own_mirror_axes | o.own_mirror_axes); } // The blend term resolved for a layer whose chain holds `n_eff` EFFECTIVE // contributors — node-map size times layer_symmetry_multiplicity, both diff --git a/include/clay/scene/commands.h b/include/clay/scene/commands.h index 24a71fff..b03f3244 100644 --- a/include/clay/scene/commands.h +++ b/include/clay/scene/commands.h @@ -55,6 +55,17 @@ struct SetColorCmd { NodeId node = kNoNode; kernel::cfloat3 color = kernel::cf3(0, 0, 0); }; +// A placed item's mirror participation AND its own axes, as one command +// (issue #664): they are one decision — which copies of this item the mirror +// emits — so one undo step puts back both. `own_mirror_axes` takes +// kMirrorAxesInherit to follow the layer again. Without this a host could fix +// neither on an item already placed, only re-add it. +struct SetNodeMirrorCmd { + LayerId layer = 0; + NodeId node = kNoNode; + bool mirror = true; + std::uint8_t own_mirror_axes = kMirrorAxesInherit; +}; struct SetOpBlendCmd { LayerId layer = 0; NodeId node = kNoNode; @@ -211,7 +222,7 @@ using Command = RemoveLayerCmd, SetLayerVisibleCmd, SetLayerTransformCmd, SetLayerProtectionCmd, SetStrokePointsCmd, SetDeformersCmd, SetLayerMirrorCmd, SetLayerRadialCmd, SetArmatureCmd, SetLayerNameCmd, - SetLayerCompositionCmd>; + SetLayerCompositionCmd, SetNodeMirrorCmd>; // The layer a command would edit, or 0 for one that edits no existing layer // (adding a layer creates its target; changing protection is how a protected @@ -460,7 +471,22 @@ LayerId content_sharer_of(const Document& doc, LayerId layer); // bytes minor 18 always did, and `layer_blocking_minor` answers for 19 exactly // as it answers for 18, because the composition is still the only field whose // absence changes the model. This is minor 10's case, for minor 10's reason. -inline constexpr std::uint16_t kSceneMinor = 19; +// +// Minor 20 adds an item's OWN MIRROR AXES (issue #664): one byte appended last +// in the node record, kMirrorAxesInherit meaning "the layer's", which is what +// every earlier document loads as — so each evaluates exactly as it was saved. +// Same shape as minors 7, 8, 11 and 14, so a build that predates 20 reading a +// 20 document is one byte long on the first node and FAILS on the reader's own +// bounds and count checks rather than misreading it. +// +// ON THE WAY DOWN it takes minor 18's departure, not minor 14's degradation. +// Dropping the byte would hand an item that kept its X twin after the layer's +// mirror went off back to the layer — the twin gone, in a file that opens +// cleanly — and hand an item held at 0 a twin it never had. That is a +// different sculpture, so `layer_blocking_minor` names the first layer holding +// an item with its own axes below 20 and serialize_document refuses; a +// document whose items all inherit writes exactly the bytes 19 always did. +inline constexpr std::uint16_t kSceneMinor = 20; // Apply a command; returns its inverse, or nullopt if the target does not // exist or is protected (ghosted or locked). The document is unchanged in @@ -495,6 +521,12 @@ std::optional deserialize(const std::uint8_t* data, std::size_t size); // per-axis scale comes back unsquashed, 15's instances come back as copies, 16 // the same one level up, and 17 writes each payload once per node. Every one of // those is smaller or plainer and none of them is a different sculpture. +// +// Every minor below 20 also refuses a layer holding an item that carries its +// OWN mirror axes (#664), and names that layer first: dropping the byte would +// hand the item the layer's twins in place of its own. A host that must write +// for an older build puts such items back on the layer's mirror first +// (SetNodeMirrorCmd with kMirrorAxesInherit). LayerId layer_blocking_minor(const Document& doc, std::uint16_t minor); // Whole-document snapshot (used by tests for bit-identity checks and by the @@ -508,7 +540,8 @@ LayerId layer_blocking_minor(const Document& doc, std::uint16_t minor); // REFUSES — returns an EMPTY vector, which is never a valid stream since even // an empty document writes its layer count — when `minor` cannot express what // this document says. Today that is exactly `layer_blocking_minor(doc, minor)` -// being non-zero: a layer carrying a composition, written below minor 18. +// being non-zero: a layer carrying a composition, written below minor 18, or +// an item carrying its own mirror axes, written below minor 20. // // Refusing rather than degrading is a departure from every earlier minor, and // it is deliberate. The rule this format has followed is "writable at the diff --git a/include/clay/scene/document.h b/include/clay/scene/document.h index 52ee0076..f2e3d692 100644 --- a/include/clay/scene/document.h +++ b/include/clay/scene/document.h @@ -281,6 +281,21 @@ struct Layer { // document evaluates to" structural rather than a rule to maintain. }; +// The mirror axes this item is actually reflected through on this layer: its +// own when it carries them, else the layer's when it participates, else none +// (issue #664). THE ONE DEFINITION — the compiler (emit_item), the bounds, the +// cull pad, picking and the Move brush's drag images all read it, so an item +// cannot be emitted with a copy that a bound or a drag does not know about. +// A feathered volume replace is excluded on top of this by each consumer, as +// it always was (item_is_feathered_replace, bounds.h). +// +// The RADIAL mode is not here: it still follows `Node::mirror` alone. +inline std::uint8_t effective_mirror_axes(const Node& item, const Layer& layer) { + if (item.own_mirror_axes != kMirrorAxesInherit) + return static_cast(item.own_mirror_axes & (kMirrorX | kMirrorY | kMirrorZ)); + return item.mirror ? layer.mirror_axes : std::uint8_t{0}; +} + // -- an item's PLACED frame, both per-axis scales composed -------------------- // // One place each, so tape_build, bounds, pick and the two brushes cannot drift diff --git a/include/clay/scene/types.h b/include/clay/scene/types.h index 6305e04e..dca16092 100644 --- a/include/clay/scene/types.h +++ b/include/clay/scene/types.h @@ -21,6 +21,9 @@ namespace scene { using NodeId = std::uint32_t; using LayerId = std::uint32_t; inline constexpr NodeId kNoNode = 0; +// Node::own_mirror_axes' "take the layer's axes" — the default (issue #664). +// Not a combination of kMirrorX|Y|Z (document.h), which use the low 3 bits. +inline constexpr std::uint8_t kMirrorAxesInherit = 0xFF; // 1:1 with the tape primitive opcodes (kernel/tape.h). enum class PrimType : std::uint8_t { @@ -982,7 +985,23 @@ struct Node { // in (issue #60 — a layer mirror that mirrored nothing by default). A // layer with no mirror axes evaluates identically either way, and a // document loads with whatever each node had when it was saved. + // + // Once `own_mirror_axes` is set this flag no longer decides the MIRROR — + // the item's own axes do — and it keeps deciding the layer's RADIAL + // participation alone (effective_mirror_axes in document.h). bool mirror = true; + // The item's OWN mirror axes (kMirrorX|Y|Z), or kMirrorAxesInherit to take + // the layer's (issue #664). Inherit is the default and what every document + // saved before scene minor 20 loads as, so each of them evaluates exactly + // as it was saved. Set, it replaces the layer's axes for this item + // outright — `0` is "no mirror copies whatever the layer says", `kMirrorX` + // keeps an X twin after the layer's mirror is turned off or pointed + // elsewhere. The seam still blends with the LAYER's mirror_k. + // + // Named apart from Layer::mirror_axes on purpose: the two mean different + // things (a default for every item, and this item's override), and one + // name for both is how a caller reads the wrong one. + std::uint8_t own_mirror_axes = kMirrorAxesInherit; // Control points, for a Stroke and for a Swept guide alike — a guide is an // ordinary curve, so it uses the same list rather than one of its own. std::vector stroke; diff --git a/include/clay/session/sdf_sculpt.h b/include/clay/session/sdf_sculpt.h index 6f76ce80..c45e0e2b 100644 --- a/include/clay/session/sdf_sculpt.h +++ b/include/clay/session/sdf_sculpt.h @@ -429,6 +429,9 @@ class SdfMoveTransaction { scene::BakePointEval point_eval_; brush::MoveSettings settings_; kernel::cfloat3 anchor_ = kernel::cf3(0, 0, 0); + // The affected items' own mirror axes (#664), which the dirty report's + // images add to the layer's so an item's own twin is not left stale. + std::uint8_t own_mirror_axes_ = 0; kernel::cfloat3 displacement_ = kernel::cf3(0, 0, 0); std::uint64_t source_ = 0; std::vector affected_; diff --git a/openspec/changes/add-per-item-mirror-axes/.openspec.yaml b/openspec/changes/add-per-item-mirror-axes/.openspec.yaml new file mode 100644 index 00000000..1aca8b91 --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-30 diff --git a/openspec/changes/add-per-item-mirror-axes/README.md b/openspec/changes/add-per-item-mirror-axes/README.md new file mode 100644 index 00000000..7084b23b --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/README.md @@ -0,0 +1,3 @@ +# add-per-item-mirror-axes + +Let an item carry its own mirror axes, so a host can turn symmetry off or change its axis without changing what was made under the old mirror (#664) diff --git a/openspec/changes/add-per-item-mirror-axes/design.md b/openspec/changes/add-per-item-mirror-axes/design.md new file mode 100644 index 00000000..3b4b364c --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/design.md @@ -0,0 +1,27 @@ +## Context +Issue #664 offered two asks: per-item axes, or an undoable bake of the layer mirror into reflected copies. This change takes the first. A bake materializes copies, which multiplies what evaluation and every later edit pay. Per-item axes keep the mirror as something evaluation reads, which is what the layer mirror already is. + +## Decisions + +### One field, one sentinel, a separate name +`Node::own_mirror_axes` is a `uint8_t`: the `kMirrorX|Y|Z` bits, or `kMirrorAxesInherit` (0xFF). Inherit and "own axes 0" must be distinct: 0 is "no copies whatever the layer says", which is how an item made with symmetry off is kept single after symmetry is turned on. It is deliberately not called `mirror_axes`. `Layer::mirror_axes` means a default for every item, and one name for both is how a caller reads the wrong one. + +### Own axes replace the layer's outright +`effective_mirror_axes(item, layer)` returns the item's own axes when set, else `layer.mirror_axes` when `item.mirror`, else 0. The alternative was to AND own axes with participation, so that an opted-out item ignores its own axes. That makes setting axes on an opted-out item a silent no-op, and it gives a host two switches to set for one decision. The participation flag keeps its meaning for an inheriting item and for the radial mode, which this change does not touch. + +The seam blend and the planes stay the layer's. A per-item seam would be a second knob for something the issue did not ask about. + +### The drag images are the item's own copies +Intended behaviour, written into `brush/move.h` and the brush-engine spec: an item carrying its own axes is reached through its own reflections (the drag, one reflection per own axis, then the layer's rotations if it participates). This is the existing rule, "the images are the copies the compiler emits of this item", applied to an item whose copies differ from the layer's. So with the layer's mirror off, a drag moves both sides of an item that kept X. A host that wants a formerly mirrored item to move on one side only sets that item's axes to 0, which is the per-item spelling of the bake. + +The resolver builds the layer's image set once per drag exactly as before. An item that inherits reads a prefix of it, so its drag is bit-identical. An item with its own axes reads a set keyed on (axes, radial), built lazily. There are at most 16 per drag, and none on a layer where no item overrides. #663's coincident-image grouping runs per set, so it applies unchanged. + +### Format: refuse, don't degrade +Minor 20 appends one byte to the node record and is gated on both sides. Writing at 19 drops the byte, and an item with its own axes would come back taking the layer's copies. That is a different sculpture in a file that opens cleanly, which is minor 18's case. So `layer_blocking_minor` refuses it and names the first layer holding such an item. A document where every item inherits still writes 19's exact bytes, and the test checks those bytes. + +### Cull pad +Own axes lengthen the chain. `CullPadTerms::own_mirror_axes` is a raise-only union, and `layer_symmetry_multiplicity(layer, item_axes)` counts `popcount(layer | item_axes)`. It stays conservative in the same way the old count was: it overcounts items that do not carry the union. + +## Risks +- A host that keeps passing `clay_item_set_mirror(-1)` to opt an item out, and also sets own axes, gets the own axes. The header states it. +- A journal holding a `SetNodeMirror` command does not replay on a build that predates the tag. Such a build refuses it in `deserialize`'s default arm rather than misreading it, which is the same trade `SetLayerComposition` made. diff --git a/openspec/changes/add-per-item-mirror-axes/proposal.md b/openspec/changes/add-per-item-mirror-axes/proposal.md new file mode 100644 index 00000000..36d0b648 --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/proposal.md @@ -0,0 +1,34 @@ +## Why +An item's mirror participation was a bool (`Node::mirror`, `clay_item_set_mirror`) and the axes lived on the layer (`clay_set_layer_mirror`). A host that mirrors what is made while symmetry is on, and leaves existing items as they were made, could only do half of it (#664, ClaySpaceDesktop #170 / #277). Turning symmetry on worked: items made with it off opt out. Turning it OFF, or switching X to Y, had to change the layer mirror, and every item made under the old axes changed with it. + +Measured through the C ABI on a lone lump of radius 0.25 at (0.6, 0.4, 0), made under a layer mirror X, reading the field at its X twin's centre (-0.6, 0.4, 0) and at its Y reflection's (0.6, -0.4, 0). A copy there reads -0.25: + +| item made under X, then the layer mirror is set to | X twin | Y reflection | +|---|---|---| +| X (as made) | -0.25 | 0.55 | +| off, before | 0.95 (twin gone) | 0.55 | +| Y, before | 0.95 (twin gone) | -0.25 (wrong plane) | +| off, item carrying own axes X | -0.25 | 0.55 | +| Y, item carrying own axes X | -0.25 | 0.55 | + +The host cannot bake the old mirror either. A reflection has determinant -1, and neither a node transform nor `scale_axes` can express one. + +There was also no setter or reader for a PLACED node's participation, so a host could not correct an item saved with the default `mirror = true` without re-adding it, which changes its id and its place in the chain. + +## What Changes +- `scene::Node::own_mirror_axes`: the item's own mirror axes (`kMirrorX|Y|Z`, `0` for none) or `kMirrorAxesInherit` (0xFF, the default) to take the layer's. `scene::effective_mirror_axes(item, layer)` is the one definition of which axes an item is reflected through: its own when set, else the layer's when it participates, else none. The compiler (`emit_item`), the geometry and influence bounds, the cull pad (seam term and symmetry multiplicity), picking's selection bound, consolidation's patch test and the Move brush's drag images all read it. +- Own axes replace the layer's outright, whatever the participation flag says. The flag keeps deciding what an INHERITING item takes from the layer, and it alone still decides radial participation. The seam blend (`mirror_k`) and the planes stay the layer's. +- `SetNodeMirrorCmd {layer, node, mirror, own_mirror_axes}`: one undoable command for both values, with a new command tag in the journal. +- C ABI 0.120.1 -> 0.121.0: `CLAY_MIRROR_AXES_INHERIT`, `clay_item_set_mirror_axes` / `clay_item_mirror_axes` for the builder, `clay_layer_set_node_mirror` / `clay_layer_node_mirror` for a placed item. The reader also reports the effective axes. A group is refused, since evaluation never reads a group's mirror. +- Scene / `.clayspace` minor 19 -> 20: one byte appended to the node record. Older documents load with every item inheriting, so they evaluate as saved. Writing at a minor below 20 is refused for a document holding an item with its own axes, following minor 18's precedent: dropping the byte would give the item the layer's copies instead of its own. `layer_blocking_minor` names the layer, and `clay_document_writable_at_minor` / `_save_*_at_minor` give a distinct message for it. +- The Move and magnify drag images for an item carrying its own axes are that item's own reflections. This is the intended behaviour: both sides of an item that kept X move under a drag even with the layer's mirror off, because both sides are the item. An item held at 0 moves on the touched side only. The reach a host is told (`drag_images`, `clay_layer_move_surface`'s invalidation, the live transaction's dirty box) adds the dragged items' own axes. +- pyclay: `Layer.add(..., mirror_axes=)`, `Layer.set_node_mirror`, `Layer.node_mirror`. + +## What building it found +- The layer's symmetry multiplicity feeds the cull chain pad, and a per-layer count cannot see an item's own axes: an item with its own X|Y on an unmirrored layer is three contributors, not one. `CullPadTerms` now carries the union of own axes, which only grows, so the incremental cull index keeps its raise-only contract on append. A placed-node change is not an `AddNodeCmd` and takes the general invalidation, as the layer-symmetry setters do. +- `drag_images` is a layer-level function, but the reach it answers now depends on items. Walking the layer on every frame to find the union would add an O(items) pass to the live drag's dirty report. The union is taken from the prepared items instead (`prepared_own_mirror_axes`), since only a reached item can move a copy. +- The downgrade cannot keep the exact cases. An item whose own axes equal what it would inherit could be written at 19 as an inheriting item, but the node writer has no layer: content is shared by instance layers whose mirrors differ. Refusing whenever any item carries its own axes is the rule the writer can apply without being wrong. +- `test_layer_composition`'s byte-count test compared minor 17 against the current minor and assumed only minor 18's block lay between them. It now compares against 18. + +## Impact +Every existing document evaluates bit-identically: the default is inherit, and an inheriting item takes the same code path, the same drag image set and the same bounds it took before. Only items that set their own axes behave differently. ABI minor and format minor move. No descriptor struct changes, so `check_c_abi.py`'s struct mirror is untouched. diff --git a/openspec/changes/add-per-item-mirror-axes/specs/brush-engine/spec.md b/openspec/changes/add-per-item-mirror-axes/specs/brush-engine/spec.md new file mode 100644 index 00000000..d4bf947f --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/specs/brush-engine/spec.md @@ -0,0 +1,16 @@ +## ADDED Requirements + +### Requirement: A drag reaches an item through the item's own mirror copies +An item that carries its own mirror axes SHALL be tested against the images its OWN copies make of a drag: the drag, one reflection per axis the item sets, then the layer's radial rotations if the item participates in them. It SHALL NOT be tested against the layer's images. An item that inherits SHALL see the layer's images exactly as before, bit for bit. + +This is the intended behaviour. A Move or magnify drag SHALL move both sides of an item that kept its own X twin, even with the layer's mirror off, because both sides are that item. It SHALL move only the touched side of an item whose own axes are none, whatever the layer's mirror is. A host that wants a formerly mirrored item to move on one side only sets that item's own axes to none. + +The reach a gesture reports for invalidation SHALL include the reflections for the dragged items' own axes, so a twin that the layer's mirror does not name is not served stale. + +#### Scenario: An own-axes twin is reachable with the layer mirror off +- **WHEN** an item carries its own axes X on a layer with no mirror, and a drag is prepared at its twin +- **THEN** the item is reached through two images, and the reported drag images include the reflected ball + +#### Scenario: An item held at none moves on one side +- **WHEN** an item carries its own axes of none on a layer mirroring X, and a drag is prepared at the layer's reflection of it +- **THEN** the item is not reached, and a drag on the item itself reaches it through one image diff --git a/openspec/changes/add-per-item-mirror-axes/specs/c-abi/spec.md b/openspec/changes/add-per-item-mirror-axes/specs/c-abi/spec.md new file mode 100644 index 00000000..5e0cb11b --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/specs/c-abi/spec.md @@ -0,0 +1,25 @@ +## ADDED Requirements + +### Requirement: A host can set and read an item's own mirror axes +The C ABI SHALL expose an item's own mirror axes on the builder (`clay_item_set_mirror_axes`, `clay_item_mirror_axes`) and on a placed item (`clay_layer_set_node_mirror`, `clay_layer_node_mirror`), with `CLAY_MIRROR_AXES_INHERIT` meaning "follow the layer". Axes outside `CLAY_MIRROR_X|Y|Z` that are not the inherit value SHALL be refused with `CLAY_ERROR_INVALID_ARGUMENT`. + +`clay_layer_set_node_mirror` SHALL set the participation flag and the own axes as one undoable edit. It SHALL refuse a group with `CLAY_ERROR_INVALID_ARGUMENT`, since evaluation never reads a group's mirror, and SHALL answer `CLAY_ERROR_NOT_FOUND` for a node that is not there. `clay_layer_node_mirror` SHALL report the participation flag, the own axes, and the axes the item is actually reflected through on its layer. + +#### Scenario: A builder starts inheriting +- **WHEN** a new item builder's own axes are read +- **THEN** they are `CLAY_MIRROR_AXES_INHERIT` + +#### Scenario: A placed item is corrected in place +- **WHEN** a host sets a placed item's own axes and participation, then undoes +- **THEN** the reader reports the new values and the effective axes follow them, and after the undo it reports the old ones + +#### Scenario: Invalid axes and groups are refused +- **WHEN** axes of 8 are passed, or a group is named +- **THEN** the call returns `CLAY_ERROR_INVALID_ARGUMENT` and nothing changes + +### Requirement: Writing below minor 20 refuses an item's own mirror axes +`clay_document_writable_at_minor`, `clay_document_save_at_minor` and `clay_document_save_memory_at_minor` SHALL refuse any minor below 20 for a document holding an item with its own mirror axes. They SHALL return `CLAY_ERROR_UNSUPPORTED`, name the first such layer, and give a message that names the own axes as the reason. Writing below 20 would give the item the layer's copies instead of its own. + +#### Scenario: The blocking layer is named +- **WHEN** a document holding an item with its own axes X is asked whether it can be written at minor 19 +- **THEN** the answer is `CLAY_ERROR_UNSUPPORTED`, the item's layer is named, and a save at 19 writes nothing diff --git a/openspec/changes/add-per-item-mirror-axes/specs/file-io/spec.md b/openspec/changes/add-per-item-mirror-axes/specs/file-io/spec.md new file mode 100644 index 00000000..5433fb8c --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/specs/file-io/spec.md @@ -0,0 +1,14 @@ +## ADDED Requirements + +### Requirement: Minor 20 carries an item's own mirror axes +Scene and container minor 20 SHALL append one byte per node record: the item's own mirror axes, or the inherit value. It SHALL be written and read only at minor 20 and above. A document read at an older minor SHALL load every item inheriting. + +A document whose items all inherit SHALL write, at minor 19, exactly the bytes minor 19 always wrote. A document holding any item with its own axes SHALL NOT be written below minor 20. The writer SHALL refuse it rather than drop the byte, and `layer_blocking_minor` SHALL name the first layer holding such an item. + +#### Scenario: Own axes round-trip +- **WHEN** a document holding an item with its own axes X is written and read at minor 20 +- **THEN** the item carries X again, evaluates with its twin, and re-writing it reproduces the stream + +#### Scenario: An inheriting document still writes minor 19's bytes +- **WHEN** a document whose items all inherit is written at 19, read back, and written at 19 again +- **THEN** the two streams are identical and the write is not refused diff --git a/openspec/changes/add-per-item-mirror-axes/specs/python-bindings/spec.md b/openspec/changes/add-per-item-mirror-axes/specs/python-bindings/spec.md new file mode 100644 index 00000000..5edec4dc --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/specs/python-bindings/spec.md @@ -0,0 +1,8 @@ +## ADDED Requirements + +### Requirement: An item's own mirror axes are reachable from Python +pyclay SHALL let a caller give an item its own mirror axes when adding it (`Layer.add(..., mirror_axes=)`), set a placed item's participation and own axes as one undoable edit (`Layer.set_node_mirror`), and read them back (`Layer.node_mirror`). Axes are spelled as the letters of the axes they reflect across, `""` for none, and `None` for "follow the layer". + +#### Scenario: A placed item's axes are one undo step +- **WHEN** an item added with `mirror_axes="x"` is set to `axes="y"` and the document is undone +- **THEN** the item reads `"x"` again and evaluates with its X twin diff --git a/openspec/changes/add-per-item-mirror-axes/specs/scene-model/spec.md b/openspec/changes/add-per-item-mirror-axes/specs/scene-model/spec.md new file mode 100644 index 00000000..bcef329c --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/specs/scene-model/spec.md @@ -0,0 +1,36 @@ +## ADDED Requirements + +### Requirement: An item may carry its own mirror axes +An item SHALL carry its own mirror axes, which are any of x/y/z, none, or "inherit". Inherit SHALL be the default. An item with its own axes SHALL be reflected through exactly those axes, whatever the layer's mirror axes are now or are later set to, and whatever its participation flag says. An item that inherits SHALL follow the layer's mirror under its participation flag exactly as before. The seam blend and the mirror planes SHALL remain the layer's, and radial participation SHALL remain governed by the participation flag alone. + +Every consumer that reasons about an item's mirror copies SHALL read one definition of the axes an item is reflected through. That includes evaluation, the geometry and influence bounds, the cull pad, picking and the Move brush. Otherwise a bound or a drag could disagree with the copies evaluation emits. + +A document saved before this field existed SHALL load with every item inheriting, and SHALL evaluate as it did when it was saved. + +#### Scenario: Own axes override the layer +- **WHEN** a layer mirrors X and an item carries its own axes Y +- **THEN** the item has a reflection across Y and none across X + +#### Scenario: Own axes of none keep an item single +- **WHEN** a layer mirrors X and an item carries its own axes of none +- **THEN** the item has no reflection + +#### Scenario: Switching the layer mirror leaves an item's own axes alone +- **GIVEN** an item made under a layer mirror X that carries its own axes X +- **WHEN** the layer's mirror is turned off, and then set to Y +- **THEN** the item keeps its X twin and gains no Y reflection, while an item that inherits and is added after the switch takes Y + +#### Scenario: An older document is unchanged +- **WHEN** a document written at scene minor 19, holding a participating item and an opted-out item under a layer mirror, is loaded +- **THEN** every item inherits and the field matches the saved document sample for sample + +#### Scenario: The brick cache sees an item's own copy +- **WHEN** an item carries its own axes X on a layer with no mirror and a brick cache is filled over the model +- **THEN** a ray toward the twin hits it where the document tape puts it + +### Requirement: A placed item's mirror is an undoable edit +Setting a placed item's participation and its own axes SHALL be one command that goes through the command vocabulary. It SHALL respect layer protection, SHALL land on the undo stack as one step, and SHALL be invertible exactly: undo restores both values the item had. + +#### Scenario: Undo restores both values +- **WHEN** an item saved with the default participation is set to opt out with its own axes Y, then undone +- **THEN** the item participates and inherits again, and the field is the one before the edit; redo applies both values again diff --git a/openspec/changes/add-per-item-mirror-axes/tasks.md b/openspec/changes/add-per-item-mirror-axes/tasks.md new file mode 100644 index 00000000..1d46731b --- /dev/null +++ b/openspec/changes/add-per-item-mirror-axes/tasks.md @@ -0,0 +1,11 @@ +## Implementation +- [x] `Node::own_mirror_axes` with the `kMirrorAxesInherit` default, and `effective_mirror_axes` as the one definition. +- [x] Compiler (`emit_item`), bounds (`expand_by_mirror_copies`), cull pad (seam term, `CullPadTerms::own_mirror_axes`, `layer_symmetry_multiplicity`), cull index, picking's selection bound, consolidation's patch test and the layer digest read it. +- [x] `SetNodeMirrorCmd` with its applier, journal tag, and entry in `command_edited_item`. +- [x] Scene / container minor 19 -> 20: node byte appended and gated; `layer_blocking_minor` refuses below 20; distinct C ABI refusal message. +- [x] C ABI 0.120.1 -> 0.121.0: `CLAY_MIRROR_AXES_INHERIT`, `clay_item_set_mirror_axes`, `clay_item_mirror_axes`, `clay_layer_set_node_mirror`, `clay_layer_node_mirror`. Version moved in `CMakeLists.txt`, `bindings/c/clay.h` and `pyproject.toml`. +- [x] Move / magnify: per-item image sets (`DragImageSets`), `PreparedMove::own_mirror_axes`, `prepared_own_mirror_axes`; reach in `clay_layer_move_surface`, `clay_layer_magnify_surface` and the live move transaction. +- [x] pyclay: `Layer.add(mirror_axes=)`, `Layer.set_node_mirror`, `Layer.node_mirror`. +- [x] Tests (`tests/unit/test_item_mirror_axes.cpp`, `test_pyclay.py`): old documents load inheriting and evaluate unchanged, and write 19's bytes; own axes round-trip and block every minor below 20; per-item axes override the layer; switching the layer mirror leaves own-axes items alone; the brick cache sees an own-axes twin; undo/redo of the placed-node setter; refusals; drag images; chain-pad multiplicity. +- [x] Mutation check: reverting each of `effective_mirror_axes`, the bound, the drag image selection, the node byte and the cull term to the old behaviour fails the matching tests. +- [x] `docs/05`, `docs/07`, the `clay.h` notes and `brush/move.h` state the rule and the drag behaviour. diff --git a/pyproject.toml b/pyproject.toml index addae3d6..04d63523 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "scikit_build_core.build" [project] name = "pyclay" -version = "0.120.1" +version = "0.121.0" description = "Python bindings for claycore, the portable C++20 SDF + voxel engine library" requires-python = ">=3.9" license = { text = "MIT" } diff --git a/src/brush/move.cpp b/src/brush/move.cpp index cdf18401..54c9574b 100644 --- a/src/brush/move.cpp +++ b/src/brush/move.cpp @@ -6,8 +6,10 @@ #include "clay/brush/move.h" #include +#include #include #include +#include #include #include @@ -24,18 +26,23 @@ namespace { // -- the images of a drag ----------------------------------------------------- -// The linear maps, in the LAYER's frame, of the copies the compiler emits of a -// participating item: one reflection per set mirror axis, then one rotation -// per radial copy. Empty without symmetry. Copy k of a radial item is the item +// The linear maps, in the LAYER's frame, of the copies the compiler emits of an +// item reflected through `mirror_axes` and, when `radial`, arrayed by the +// layer's radial mode: one reflection per set axis, then one rotation per +// radial copy. Empty without symmetry. Copy k of a radial item is the item // rotated by +angle, so the item that sits under the ball THROUGH copy k has // its body at the ball rotated by -angle — the inverse emit_item composes for // that copy. -std::vector copy_linears(const scene::Layer& layer) { +// +// The axes are a parameter rather than the layer's because an item may carry +// its OWN (#664); the layer's are what every inheriting item reads. +std::vector copy_linears(const scene::Layer& layer, std::uint8_t mirror_axes, + bool radial) { std::vector out; for (int axis = 0; axis < 3; ++axis) { - if (layer.mirror_axes & (1u << axis)) out.push_back(math::reflection_matrix(axis)); + if (mirror_axes & (1u << axis)) out.push_back(math::reflection_matrix(axis)); } - if (layer.radial_count > 1) { + if (radial && layer.radial_count > 1) { const int axis = layer.radial_axis < 3 ? layer.radial_axis : 1; const int count = static_cast(layer.radial_count); for (int k = 1; k < count; ++k) { @@ -188,16 +195,84 @@ bool reaches(const math::Aabb& influence, const math::Aabb& drag) { influence.max.z < drag.min.z || influence.min.z > drag.max.z); } -// How many of the drag's images this item sees. EXACTLY the compiler's -// participation gate (tape_build.cpp, emit_item): an item that opted out of -// the mirror, or a feathered volume replace, is emitted once and has no copy -// for a reflected image to reach. The two tests must stay identical, or the -// brush warps an item where the compiler put no geometry. -std::size_t images_seen(const scene::Node& n, std::size_t images) { - if (images == 1) return 1u; // no symmetry: nothing to gate, and no call per item - const bool participates = n.mirror && !scene::item_is_feathered_replace(n); - return participates ? images : 1u; -} +// The images one symmetry makes of a drag: the copies' maps and their balls. +// Held behind a pointer so `ImageBall::linear` keeps pointing at `linears`. +struct ImageSet { + std::vector linears; + std::vector balls; +}; + +// The drag's images PER SYMMETRY an item can carry, built once per drag. +// +// Every item that inherits the layer's mirror sees a prefix of ONE set — the +// layer's, all of it or the drag alone — which is the whole story for every +// document written before items could carry their own axes, and it is built +// and read exactly as it always was, so those drags stay bit-identical. +// +// An item carrying its OWN axes (#664) is emitted with ITS reflections, and a +// drag has to reach it through those or the brush warps the item where no copy +// is (or misses the copy that is): an item that kept its X twin after the +// layer's mirror went off still moves on both sides, and one whose axes are 0 +// moves on the side the drag touched only. Its set is keyed on (axes, radial) +// and built the first time an item asks — at most sixteen per drag, and none +// at all on a layer no item overrides. +class DragImageSets { +public: + DragImageSets(const scene::Layer& layer, cfloat3 centre, float radius) + : layer_(layer), centre_(centre), radius_(radius), + layer_key_(key(layer.mirror_axes, layer.radial_count > 1)), + layer_set_(build(layer.mirror_axes, true)) {} + + // The images `n` can be reached through, and how many of them it sees. + // EXACTLY the compiler's gate (tape_build.cpp, emit_item): an item with no + // effective axes that takes no radial copy, or a feathered volume replace, + // is emitted once and has no copy for a reflected image to reach. The two + // tests must stay identical, or the brush warps an item where the compiler + // put no geometry. + const std::vector& for_item(const scene::Node& n, std::size_t* seen) { + const std::vector& layer_balls = layer_set_->balls; + *seen = 1u; + // No symmetry anywhere: nothing to gate, and no call per item. + if (layer_balls.size() == 1 && n.own_mirror_axes == scene::kMirrorAxesInherit) + return layer_balls; + if (scene::item_is_feathered_replace(n)) return layer_balls; // ball 0 is the drag + if (n.own_mirror_axes == scene::kMirrorAxesInherit) { + if (n.mirror) *seen = layer_balls.size(); + return layer_balls; + } + const std::uint8_t axes = scene::effective_mirror_axes(n, layer_); + const bool radial = n.mirror && layer_.radial_count > 1; + const unsigned k = key(axes, radial); + const ImageSet& set = k == layer_key_ ? *layer_set_ : variant(k, axes, radial); + *seen = set.balls.size(); + return set.balls; + } + +private: + static unsigned key(std::uint8_t axes, bool radial) { + return (axes & 0x7u) | (radial ? 0x8u : 0u); + } + + std::unique_ptr build(std::uint8_t axes, bool radial) const { + auto set = std::make_unique(); + set->linears = copy_linears(layer_, axes, radial); + set->balls = image_balls(layer_.xform, centre_, set->linears, radius_); + return set; + } + + const ImageSet& variant(unsigned k, std::uint8_t axes, bool radial) { + std::unique_ptr& slot = variants_[k]; + if (!slot) slot = build(axes, radial); + return *slot; + } + + const scene::Layer& layer_; + cfloat3 centre_; + float radius_; + unsigned layer_key_; + std::unique_ptr layer_set_; + std::array, 16> variants_{}; +}; bool any_reaches(const math::Aabb& own, const std::vector& balls, std::size_t seen) { for (std::size_t k = 0; k < seen; ++k) { @@ -307,7 +382,7 @@ PreparedMove prepare_item(const scene::Layer& layer, const scene::Node& n, scene // anchor and the radius and NOT on the displacement, which is what lets a live // drag pay for it once — see brush/move.h. void collect(const scene::SdfContent& content, const scene::Layer& layer, - const std::vector& ids, const std::vector& balls, + const std::vector& ids, DragImageSets& images, const MoveSettings& settings, std::vector* out, MovePrepareStats* stats) { for (scene::NodeId id : ids) { @@ -318,7 +393,7 @@ void collect(const scene::SdfContent& content, const scene::Layer& layer, if (n->is_group) { // A group takes no warp of its own: its transform does not reach // its children here, so the children are what carry the drag. - collect(content, layer, n->children, balls, settings, out, stats); + collect(content, layer, n->children, images, settings, out, stats); continue; } // Reach is decided here, before anything is prepared: the resolver @@ -328,10 +403,13 @@ void collect(const scene::SdfContent& content, const scene::Layer& layer, // -- 1.4-1.6x on BM_MoveDrag1000/10000 for output that was // byte-identical. const math::Aabb own = scene::item_own_influence_bound(*n, layer); - const std::size_t seen = images_seen(*n, balls.size()); + std::size_t seen = 1u; + const std::vector& balls = images.for_item(*n, &seen); if (!any_reaches(own, balls, seen)) continue; if (stats) ++stats->reached; out->push_back(prepare_item(layer, *n, id, own, balls, seen, settings)); + if (n->own_mirror_axes != scene::kMirrorAxesInherit) + out->back().own_mirror_axes = scene::effective_mirror_axes(*n, layer); } } @@ -405,10 +483,11 @@ void order_by_value(std::vector* deformers) { } std::vector drag_images(const scene::Layer& layer, cfloat3 world_centre, - cfloat3 world_displacement) { + cfloat3 world_displacement, std::uint8_t item_axes) { std::vector out; out.push_back({world_centre, world_displacement}); - for (const math::cfloat4x4& linear : copy_linears(layer)) { + const auto axes = static_cast((layer.mirror_axes | item_axes) & 0x7u); + for (const math::cfloat4x4& linear : copy_linears(layer, axes, true)) { out.push_back({image_centre(layer.xform, linear, world_centre), image_displacement(layer.xform, linear, world_displacement)}); } @@ -422,13 +501,17 @@ std::vector prepare_move(const scene::Layer& layer, cfloat3 world_ if (out_stats) *out_stats = MovePrepareStats{}; if (!(settings.radius > 0.0f)) return out; if (!layer.sdf) return out; - const std::vector linears = copy_linears(layer); - const std::vector balls = - image_balls(layer.xform, world_centre, linears, settings.radius); - collect(*layer.sdf, layer, layer.sdf->roots, balls, settings, &out, out_stats); + DragImageSets images(layer, world_centre, settings.radius); + collect(*layer.sdf, layer, layer.sdf->roots, images, settings, &out, out_stats); return out; } +std::uint8_t prepared_own_mirror_axes(const std::vector& prepared) { + std::uint8_t axes = 0; + for (const PreparedMove& p : prepared) axes = static_cast(axes | p.own_mirror_axes); + return axes; +} + MoveWarp resolve_prepared_move(const PreparedMove& prepared, cfloat3 total_world_displacement) { MoveWarp warp; resolve_prepared_move(prepared, total_world_displacement, &warp); diff --git a/src/pick/pick.cpp b/src/pick/pick.cpp index 76f5535d..64146abd 100644 --- a/src/pick/pick.cpp +++ b/src/pick/pick.cpp @@ -1209,9 +1209,11 @@ math::Aabb node_shape_bounds(const scene::SdfContent& content, const scene::Node // the item IS, not around the one its primitive was authored as. const math::cfloat4x4 axes = math::scale_matrix(n.scale_axes); math::Aabb bound = local.transformed(scene::placed_matrix(layer, n)); - if (n.mirror && layer.mirror_axes != 0) { + // The item's EFFECTIVE axes (#664): its own when it carries them. + const std::uint8_t mirror_axes = scene::effective_mirror_axes(n, layer); + if (mirror_axes != 0) { for (int axis = 0; axis < 3; ++axis) { - if (!(layer.mirror_axes & (1u << axis))) continue; + if (!(mirror_axes & (1u << axis))) continue; bound.expand(local.transformed(math::mul( scene::layer_matrix(layer), math::mul(math::reflection_matrix(axis), math::mul(n.xform.matrix(), axes))))); diff --git a/src/scene/bounds.cpp b/src/scene/bounds.cpp index dd68dbe3..fc0a9312 100644 --- a/src/scene/bounds.cpp +++ b/src/scene/bounds.cpp @@ -942,12 +942,18 @@ namespace { // The copy's map is on the order emit_item uses: the reflection acts in the // layer's LOCAL space, so the layer's per-axis scale is outside it, and the // item's per-axis scale is innermost. +// +// The axes are the item's EFFECTIVE ones (#664): its own when it carries them, +// the layer's when it participates. A bound that read the layer's alone would +// miss the twin of an item that kept its own X after the layer's mirror went +// off, and the cull would drop a copy that is on screen. void expand_by_mirror_copies(const Node& item, const Layer& layer, const Aabb& local, const math::cfloat4x4& lm, Aabb* bound) { - if (layer.mirror_axes == 0) return; + const std::uint8_t mirror_axes = effective_mirror_axes(item, layer); + if (mirror_axes == 0) return; const math::cfloat4x4 axes = math::scale_matrix(item.scale_axes); for (int axis = 0; axis < 3; ++axis) { - if (!(layer.mirror_axes & (1u << axis))) continue; + if (!(mirror_axes & (1u << axis))) continue; math::cfloat4x4 m = math::mul( lm, math::mul(math::reflection_matrix(axis), math::mul(item.xform.matrix(), axes))); bound->expand(local.transformed(m)); @@ -1001,9 +1007,11 @@ Aabb placed_local_bound(const Node& item, const Layer& layer, const Aabb& local, // missed either would be tight around a shape the item no longer is, and // the cull would drop a squashed cylinder that is on screen. Aabb bound = local.transformed(math::mul(lm, item_matrix(item))); - if (with_copies && item.mirror) { + if (with_copies) { + // The mirror reads the item's effective axes, which already fold in + // its participation; the radial mode still follows the flag alone. expand_by_mirror_copies(item, layer, local, lm, &bound); - expand_by_radial_copies(item, layer, local, lm, &bound); + if (item.mirror) expand_by_radial_copies(item, layer, local, lm, &bound); } // Rounding is authored in item-local units (tape emits round*scale); // erosion (negative rounding) shrinks the surface, never the bound. @@ -1154,9 +1162,15 @@ float chain_pad_envelope(BlendProfile profile, std::size_t nodes) { // counts groups, invisible nodes and items that opted out of the mirror, // which only raises the envelope, and every term blend_total resolves stays // clamped at its own support, so the pad never exceeds the pre-#335 one. -std::size_t layer_symmetry_multiplicity(const Layer& layer) { +// +// `item_axes` is the union of the axes the layer's items carry as their OWN +// (CullPadTerms::own_mirror_axes, #664): an item can reflect through an axis +// the layer's mirror does not set, and its copies lengthen the chain exactly +// as the layer's would. Unioned rather than added, because one item emits at +// most one copy per axis whichever of the two named it. +std::size_t layer_symmetry_multiplicity(const Layer& layer, std::uint8_t item_axes) { std::size_t m = 1 + static_cast(std::popcount( - static_cast(layer.mirror_axes & 0x7u))); + static_cast((layer.mirror_axes | item_axes) & 0x7u))); if (layer.radial_count > 1) m += static_cast(layer.radial_count) - 1; return m; } @@ -1301,8 +1315,9 @@ float chain_drag_reach(const Node& item, std::size_t effective_nodes) { // since the drag grows with length -- and tape.h says so rather than // implying otherwise. float blend_cull_pad(const SdfContent& content, const Layer& layer) { - return cull_pad_terms(content, layer) - .blend_total(content.nodes().size() * layer_symmetry_multiplicity(layer)); + const CullPadTerms terms = cull_pad_terms(content, layer); + return terms.blend_total(content.nodes().size() * + layer_symmetry_multiplicity(layer, terms.own_mirror_axes)); } // ONE node's contribution to both pads, and the ONE definition of either: the @@ -1318,13 +1333,20 @@ CullPadTerms cull_pad_terms(const Node& n, const Layer& layer) { t.feather = n.volume->band() * placed_distance_scale(layer, n); raise_blend_term(n, &t); // The SEAM blends this node's symmetry copies enter the chain through. - // Exactly the nodes emit_item copies: items participating in the mirror, - // minus feathered replaces, which skip both symmetry blocks. A zero seam - // k is a HARD seam and drags nothing — its copies still lengthen the - // chain, which the multiplicity counts. - if (!n.is_group && n.mirror && !feathered) { - if (layer.mirror_axes != 0) t.blend_k_seam = kernel::cmax(t.blend_k_seam, layer.mirror_k); - if (layer.radial_count > 1) t.blend_k_seam = kernel::cmax(t.blend_k_seam, layer.radial_k); + // Exactly the nodes emit_item copies: items with effective mirror axes + // for the mirror seam and participating items for the radial one, minus + // feathered replaces, which skip both symmetry blocks. A zero seam k is a + // HARD seam and drags nothing — its copies still lengthen the chain, which + // the multiplicity counts. + if (!n.is_group && !feathered) { + if (effective_mirror_axes(n, layer) != 0) + t.blend_k_seam = kernel::cmax(t.blend_k_seam, layer.mirror_k); + if (n.mirror && layer.radial_count > 1) + t.blend_k_seam = kernel::cmax(t.blend_k_seam, layer.radial_k); + // The axes the item names ITSELF, which the multiplicity cannot read + // off the layer (#664). Raise-only, like every other term here. + if (n.own_mirror_axes != kMirrorAxesInherit) + t.own_mirror_axes = static_cast(n.own_mirror_axes & 0x7u); } return t; } @@ -1361,8 +1383,9 @@ CullPadTerms cull_pad_terms(const SdfContent& content, const Layer& layer) { } float cull_pad(const SdfContent& content, const Layer& layer) { - return cull_pad_terms(content, layer) - .total(content.nodes().size() * layer_symmetry_multiplicity(layer)); + const CullPadTerms terms = cull_pad_terms(content, layer); + return terms.total(content.nodes().size() * + layer_symmetry_multiplicity(layer, terms.own_mirror_axes)); } bool item_influence_is_local(const Node& item) { diff --git a/src/scene/commands.cpp b/src/scene/commands.cpp index 4632c529..90adfab7 100644 --- a/src/scene/commands.cpp +++ b/src/scene/commands.cpp @@ -91,6 +91,19 @@ std::optional apply_one(Document& doc, const SetColorCmd& c) { [](Node& n, const SetColorCmd& cc) { n.color = cc.color; }); } +std::optional apply_one(Document& doc, const SetNodeMirrorCmd& c) { + return apply_field( + doc, c, + [](SetNodeMirrorCmd& inv, const Node& n) { + inv.mirror = n.mirror; + inv.own_mirror_axes = n.own_mirror_axes; + }, + [](Node& n, const SetNodeMirrorCmd& cc) { + n.mirror = cc.mirror; + n.own_mirror_axes = cc.own_mirror_axes; + }); +} + std::optional apply_one(Document& doc, const SetOpBlendCmd& c) { return apply_field( doc, c, @@ -514,6 +527,7 @@ EditedItem command_edited_item(const Command& cmd) { if constexpr (std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v || + std::is_same_v || std::is_same_v || std::is_same_v || std::is_same_v || @@ -996,6 +1010,11 @@ void write_node(Writer& w, const Node& n) { // the item degrades to its UNIFORM scale — a squashed cylinder comes back // round rather than missing, which is the recoverable direction. if (w.minor >= 14) w.pod(n.scale_axes); + // The item's OWN MIRROR AXES, from minor 20 (#664), appended last for the + // same reason. Below 20 there is nothing to drop: serialize_document has + // already refused a document holding an item that carries its own axes + // (layer_blocking_minor), so every node reaching here inherits. + if (w.minor >= 20) w.pod(n.own_mirror_axes); } std::vector read_deformers(Reader& r) { @@ -1179,6 +1198,10 @@ Node read_node(Reader& r) { // exactly what those documents already meant, so an older file's field is // unchanged rather than reinterpreted. if (r.minor >= 14) n.scale_axes = r.pod(); + // Minor 19 and below had no per-item axes: every item followed the layer, + // which is exactly what the inherit default means, so an older document + // evaluates as it was saved rather than being reinterpreted. + if (r.minor >= 20) n.own_mirror_axes = r.pod(); return n; } @@ -1366,6 +1389,7 @@ enum class Tag : std::uint8_t { // value, and inserting one would renumber every tag after it. SetLayerRadial, SetLayerComposition, + SetNodeMirror, }; struct SerializeVisitor { @@ -1412,6 +1436,16 @@ struct SerializeVisitor { w.pod(c.node); w.pod(c.color); } + void operator()(const SetNodeMirrorCmd& c) { + // Ungated: a whole new TAG, which a build that predates it refuses in + // deserialize's default arm rather than misreads (see + // SetLayerComposition below). + w.pod(Tag::SetNodeMirror); + w.pod(c.layer); + w.pod(c.node); + w.pod(c.mirror); + w.pod(c.own_mirror_axes); + } void operator()(const SetOpBlendCmd& c) { w.pod(Tag::SetOpBlend); w.pod(c.layer); @@ -1710,6 +1744,15 @@ std::optional deserialize(const std::uint8_t* data, std::size_t size) { cmd = c; break; } + case Tag::SetNodeMirror: { + SetNodeMirrorCmd c; + c.layer = r.pod(); + c.node = r.pod(); + c.mirror = r.pod(); + c.own_mirror_axes = r.pod(); + cmd = c; + break; + } case Tag::SetLayerName: { SetLayerNameCmd c; c.id = r.pod(); @@ -1729,7 +1772,30 @@ std::optional deserialize(const std::uint8_t* data, std::size_t size) { return cmd; } +namespace { + +// Does this layer hold an item carrying its OWN mirror axes — the one node +// field minor 19 and below cannot say (#664)? Groups are included: a group's +// flag is not read by evaluation, but it is stored, and a writer that dropped +// it would load something other than what was saved. +bool holds_own_mirror_axes(const Layer& l) { + if (l.kind != LayerKind::Sdf || !l.sdf) return false; + for (const auto& [id, n] : l.sdf->nodes()) { + (void)id; + if (n.own_mirror_axes != kMirrorAxesInherit) return true; + } + return false; +} + +} // namespace + LayerId layer_blocking_minor(const Document& doc, std::uint16_t minor) { + if (minor >= 20) return 0; + // Minor 20's field first, since it blocks every minor below it; the first + // layer in stack order holding one is named, for the reason a composition + // names its layer. + for (const Layer& l : doc.layers) + if (holds_own_mirror_axes(l)) return l.id; if (minor >= 18) return 0; for (const Layer& l : doc.layers) { if (l.kind != LayerKind::Sdf) continue; // a non-SDF layer carries none diff --git a/src/scene/consolidate.cpp b/src/scene/consolidate.cpp index 2f908f9e..12abb01c 100644 --- a/src/scene/consolidate.cpp +++ b/src/scene/consolidate.cpp @@ -545,7 +545,7 @@ bool patchable_volume(const Node& n, const Layer& layer) { n.rounding == 0.0f && !n.gated() && n.repeat.type == kernel::crepeat_none && identity_transform(n.xform) && n.scale_axes.x == 1.0f && n.scale_axes.y == 1.0f && n.scale_axes.z == 1.0f && - (!n.mirror || (layer.mirror_axes == 0 && layer.radial_count < 2)); + effective_mirror_axes(n, layer) == 0 && (!n.mirror || layer.radial_count < 2); } bool include_local_warps(const Node& n, math::Aabb& core) { diff --git a/src/scene/cull_index.cpp b/src/scene/cull_index.cpp index 661c2180..5df28961 100644 --- a/src/scene/cull_index.cpp +++ b/src/scene/cull_index.cpp @@ -34,6 +34,12 @@ CullIndex::CullIndex(const Document& doc) : doc_(&doc) { // symmetry is constant; appends only grow `nodes`, and both factors of the // product only rise, keeping the raise-only append contract exact. // +// An item's OWN mirror axes (#664) keep that true by riding the terms rather +// than the layer: an appended item carrying them raises `own_mirror_axes` +// with the rest of its terms, and a later change to a placed item's axes is a +// SetNodeMirrorCmd — not an AddNodeCmd, so it takes the general invalidation +// the layer-symmetry setters take. +// // PLUS THE FOLDS ABOVE EACH LAYER, which is the term no per-layer walk can // produce: a smooth layer composition drags the value of every layer beneath // it, and a stack of them composes, so the items of the bottom layer need the @@ -51,7 +57,8 @@ void CullIndex::refresh_pad() { pad_ = 0.0f; for (const LayerPad& p : pads_) pad_ = kernel::cmax(pad_, - p.terms.total(p.nodes * layer_symmetry_multiplicity(*p.layer)) + + p.terms.total(p.nodes * layer_symmetry_multiplicity( + *p.layer, p.terms.own_mirror_axes)) + folds_from_layer_support(*doc_, p.layer->id)); } diff --git a/src/scene/tape_build.cpp b/src/scene/tape_build.cpp index 82b1294f..a4093695 100644 --- a/src/scene/tape_build.cpp +++ b/src/scene/tape_build.cpp @@ -969,16 +969,19 @@ struct Compiler { // strokes it was baked from, or bake each side. Documented on // clay_volume_params.feather. // - // This gate -- `item.mirror && !is_feathered_replace(item)` -- is the - // one the move brush repeats to decide which images of a drag an item - // can see (brush/move.cpp, warp_for). Change one and change the other, - // or the brush warps an item where no copy was emitted. - if (item.mirror && layer.mirror_axes != 0 && !is_feathered_replace(item)) { + // This gate -- `effective_mirror_axes(item, layer)` minus a feathered + // replace -- is the one the move brush repeats to decide which images + // of a drag an item can see (brush/move.cpp, DragImageSets::for_item). Change one + // and change the other, or the brush warps an item where no copy was + // emitted. The axes are the ITEM's own when it carries them (#664), + // and the layer's otherwise; the seam is the layer's either way. + const std::uint8_t mirror_axes = effective_mirror_axes(item, layer); + if (mirror_axes != 0 && !is_feathered_replace(item)) { Blend mirror_blend{layer.mirror_k > 0.0f ? BlendProfile::Quadratic : BlendProfile::Hard, layer.mirror_k}; for (int axis = 0; axis < 3; ++axis) { - if (!(layer.mirror_axes & (1u << axis))) continue; + if (!(mirror_axes & (1u << axis))) continue; // inv of (layer * diag(L) * R * item) = item^-1 * R * diag(1/L) * layer^-1. // The reflection acts in the layer's LOCAL space, so the // layer's per-axis scale is OUTSIDE it — and for a reflection diff --git a/src/session/layer_digest.h b/src/session/layer_digest.h index 1f0eb54b..7c28793c 100644 --- a/src/session/layer_digest.h +++ b/src/session/layer_digest.h @@ -177,6 +177,7 @@ inline void mix_node(std::uint64_t& h, const scene::SdfContent& content, scene:: mix_f(h, n->rounding); mix_v3(h, n->color); mix(h, n->mirror); + mix(h, n->own_mirror_axes); mix_points(h, n->stroke); mix_f(h, n->stroke_blend_k); mix(h, n->stroke_closed); diff --git a/src/session/sdf_sculpt.cpp b/src/session/sdf_sculpt.cpp index bfb4fd44..748111e9 100644 --- a/src/session/sdf_sculpt.cpp +++ b/src/session/sdf_sculpt.cpp @@ -401,6 +401,7 @@ std::optional SdfMoveTransaction::begin(scene::Document& doc // displacement does, and that is `resolve_prepared_move`. const std::vector prepared = brush::prepare_move(*layer, world_centre, settings, &tx.prepare_stats_); + tx.own_mirror_axes_ = brush::prepared_own_mirror_axes(prepared); tx.affected_.reserve(prepared.size()); for (const brush::PreparedMove& p : prepared) { const scene::Node* n = layer->sdf->find(p.node); @@ -458,7 +459,7 @@ SdfSculptDirty SdfMoveTransaction::update(cfloat3 total_world_displacement) { // drag began under, so this reads no document state. const cfloat3 r = cf3(settings_.radius, settings_.radius, settings_.radius); for (const brush::DragImage& image : - brush::drag_images(preview_, anchor_, total_world_displacement)) { + brush::drag_images(preview_, anchor_, total_world_displacement, own_mirror_axes_)) { dirty.bounds.expand(math::Aabb{image.centre - r, image.centre + r}); const cfloat3 moved = image.centre + image.displacement; dirty.bounds.expand(math::Aabb{moved - r, moved + r}); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3c4ad51d..526d6724 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -132,6 +132,7 @@ add_executable(clay_unit_tests unit/test_c_layer_scale_axes.cpp unit/test_c_layer_rename.cpp unit/test_c_layer_mirror.cpp + unit/test_item_mirror_axes.cpp unit/test_c_layer_radial.cpp unit/test_c_voxel.cpp unit/test_c_voxel_incremental.cpp diff --git a/tests/unit/test_item_mirror_axes.cpp b/tests/unit/test_item_mirror_axes.cpp new file mode 100644 index 00000000..1a563fb2 --- /dev/null +++ b/tests/unit/test_item_mirror_axes.cpp @@ -0,0 +1,447 @@ +#include + +#include +#include +#include +#include + +#include "clay.h" +#include "clay/brush/move.h" +#include "clay/scene/bounds.h" +#include "clay/scene/commands.h" +#include "clay/scene/document.h" +#include "clay/scene/tape.h" + +// Per-item mirror axes (issue #664). An item's mirror participation was a bool +// and the axes lived on the layer, so a host that turned symmetry off, or +// switched it from X to Y, changed every item made under the old axes with it: +// a lump sculpted on +x under X symmetry lost its -x twin. An item can now +// carry its OWN axes, which replace the layer's for it; the layer's stay the +// default for every item that sets none, which is every item in every document +// saved before. +// +// The probe throughout: a lone lump of radius 0.25 at (0.6, 0.4, 0), and the +// field at its centre's reflections. A copy there reads about -0.25; no copy +// reads the distance to the lump itself, far positive. + +using namespace clay; + +namespace { + +constexpr float kLump[3] = {0.6f, 0.4f, 0.0f}; +constexpr float kR = 0.25f; + +// ---- scene level ------------------------------------------------------------- + +scene::Document lump_doc(std::uint8_t layer_axes, std::uint8_t own_axes, bool mirror = true) { + scene::Document doc; + scene::Layer& l = doc.add_sdf_layer("body"); + l.mirror_axes = layer_axes; + scene::Node n; + n.prim = scene::Prim::sphere(kR); + n.xform.position = kernel::cf3(kLump[0], kLump[1], kLump[2]); + n.mirror = mirror; + n.own_mirror_axes = own_axes; + l.sdf->insert(n); + return doc; +} + +float field_at(const scene::Document& doc, float x, float y, float z) { + return scene::compile_document(doc).eval(kernel::cf3(x, y, z)).d; +} + +bool twin_x(const scene::Document& doc) { return field_at(doc, -kLump[0], kLump[1], 0) < 0.0f; } +bool twin_y(const scene::Document& doc) { return field_at(doc, kLump[0], -kLump[1], 0) < 0.0f; } + +scene::NodeId only_node(const scene::Document& doc) { + return doc.layers.front().sdf->roots.front(); +} + +// ---- C ABI ------------------------------------------------------------------- + +struct CDoc { + clay_document* d = nullptr; + clay_layer_id layer = 0; + CDoc() { + d = clay_document_create(); + REQUIRE(d != nullptr); + REQUIRE(clay_add_sdf_layer(d, "body", &layer) == CLAY_OK); + } + ~CDoc() { clay_document_destroy(d); } + CDoc(const CDoc&) = delete; + CDoc& operator=(const CDoc&) = delete; +}; + +clay_node_id add_lump(CDoc& doc, int own_axes /* -1: never set */) { + const float params[1] = {kR}; + clay_item* item = clay_item_create(CLAY_PRIM_SPHERE, params, 1); + REQUIRE(item != nullptr); + REQUIRE(clay_item_set_position(item, kLump) == CLAY_OK); + if (own_axes >= 0) + REQUIRE(clay_item_set_mirror_axes(item, static_cast(own_axes)) == CLAY_OK); + clay_node_id node = 0; + REQUIRE(clay_layer_add_item(doc.d, doc.layer, item, &node) == CLAY_OK); + clay_item_destroy(item); + return node; +} + +float c_field(const clay_document* doc, float x, float y, float z) { + const float p[3] = {x, y, z}; + float d = 0.0f; + REQUIRE(clay_eval_points(doc, nullptr, p, 1, &d, nullptr) == CLAY_OK); + return d; +} + +bool c_twin_x(const clay_document* doc) { return c_field(doc, -kLump[0], kLump[1], 0) < 0.0f; } +bool c_twin_y(const clay_document* doc) { return c_field(doc, kLump[0], -kLump[1], 0) < 0.0f; } + +void set_layer_mirror(CDoc& doc, int x, int y, int z) { + REQUIRE(clay_set_layer_mirror(doc.d, doc.layer, x, y, z, 0.0f) == CLAY_OK); +} + +} // namespace + +// ---- old documents ----------------------------------------------------------- + +TEST_CASE("a document saved before per-item axes loads inheriting and evaluates as saved") { + // Written at minor 19 — the layout every earlier build writes — with a + // participating item and an opted-out one under a layer mirror X. + scene::Document doc = lump_doc(scene::kMirrorX, scene::kMirrorAxesInherit); + { + scene::Node out; + out.prim = scene::Prim::sphere(kR); + out.xform.position = kernel::cf3(0.6f, -0.9f, 0.0f); + out.mirror = false; + doc.layers.front().sdf->insert(out); + } + const std::vector old = scene::serialize_document(doc, 19); + REQUIRE_FALSE(old.empty()); + const std::optional back = + scene::deserialize_document(old.data(), old.size(), 19); + REQUIRE(back.has_value()); + for (const auto& [id, n] : back->layers.front().sdf->nodes()) { + (void)id; + CHECK(n.own_mirror_axes == scene::kMirrorAxesInherit); + } + // Same field, sample for sample, as the document that was saved. + for (float x : {-0.6f, 0.6f, 0.0f}) + for (float y : {-0.9f, -0.4f, 0.4f}) + CHECK(field_at(*back, x, y, 0.0f) == field_at(doc, x, y, 0.0f)); + CHECK(twin_x(*back)); + CHECK(field_at(*back, -0.6f, -0.9f, 0.0f) > 0.0f); // the opted-out one stays single + + SUBCASE("and a document whose items all inherit still writes the bytes 19 wrote") { + CHECK(scene::layer_blocking_minor(*back, 19) == 0); + CHECK(scene::serialize_document(*back, 19) == old); + } +} + +TEST_CASE("an item's own axes round-trip at minor 20 and block every minor below") { + scene::Document doc = lump_doc(0, scene::kMirrorX); + const std::vector bytes = scene::serialize_document(doc, scene::kSceneMinor); + const std::optional back = + scene::deserialize_document(bytes.data(), bytes.size(), scene::kSceneMinor); + REQUIRE(back.has_value()); + CHECK(back->layers.front().sdf->find(only_node(*back))->own_mirror_axes == scene::kMirrorX); + CHECK(twin_x(*back)); + CHECK(scene::serialize_document(*back, scene::kSceneMinor) == bytes); + + // Dropping the byte would hand the item the layer's (absent) mirror and + // lose its twin in a file that opens cleanly, so the write is refused. + const scene::LayerId layer = doc.layers.front().id; + CHECK(scene::layer_blocking_minor(doc, 19) == layer); + CHECK(scene::layer_blocking_minor(doc, 14) == layer); + CHECK(scene::layer_blocking_minor(doc, scene::kSceneMinor) == 0); + CHECK(scene::serialize_document(doc, 19).empty()); + + SUBCASE("own axes of 0 block too: 19 would give the item the layer's twin") { + scene::Document zero = lump_doc(scene::kMirrorX, 0); + CHECK_FALSE(twin_x(zero)); + CHECK(scene::layer_blocking_minor(zero, 19) == zero.layers.front().id); + } +} + +// ---- evaluation -------------------------------------------------------------- + +TEST_CASE("an item's own axes override the layer's mirror") { + CHECK(twin_x(lump_doc(scene::kMirrorX, scene::kMirrorAxesInherit))); // the default + CHECK_FALSE(twin_y(lump_doc(scene::kMirrorX, scene::kMirrorAxesInherit))); + + const scene::Document own_y = lump_doc(scene::kMirrorX, scene::kMirrorY); + CHECK_FALSE(twin_x(own_y)); + CHECK(twin_y(own_y)); + + const scene::Document own_none = lump_doc(scene::kMirrorX, 0); + CHECK_FALSE(twin_x(own_none)); + CHECK_FALSE(twin_y(own_none)); + + // Own axes win over the participation flag too: the flag governs only + // what an INHERITING item takes from the layer. + CHECK(twin_x(lump_doc(0, scene::kMirrorX, /*mirror=*/false))); +} + +TEST_CASE("switching the layer mirror leaves an item with its own axes as it was made") { + CDoc doc; + set_layer_mirror(doc, 1, 0, 0); + const clay_node_id made_under_x = add_lump(doc, CLAY_MIRROR_X); + REQUIRE(c_twin_x(doc.d)); + + // The host turns symmetry OFF: the layer's default goes, the item's twin + // stays. + set_layer_mirror(doc, 0, 0, 0); + CHECK(c_twin_x(doc.d)); + CHECK_FALSE(c_twin_y(doc.d)); + + // ...and points it at Y: an inheriting item made now takes Y, the old one + // keeps X and gains nothing. + set_layer_mirror(doc, 0, 1, 0); + CHECK(c_twin_x(doc.d)); + CHECK_FALSE(c_twin_y(doc.d)); + const clay_node_id inheriting = add_lump(doc, -1); + (void)inheriting; + CHECK(c_twin_y(doc.d)); + + int32_t mirror = 0; + uint8_t axes = 0, effective = 0; + REQUIRE(clay_layer_node_mirror(doc.d, doc.layer, made_under_x, &mirror, &axes, &effective) == + CLAY_OK); + CHECK(mirror == 1); + CHECK(axes == CLAY_MIRROR_X); + CHECK(effective == CLAY_MIRROR_X); + REQUIRE(clay_layer_node_mirror(doc.d, doc.layer, inheriting, &mirror, &axes, &effective) == + CLAY_OK); + CHECK(axes == CLAY_MIRROR_AXES_INHERIT); + CHECK(effective == CLAY_MIRROR_Y); +} + +TEST_CASE("the brick cache sees an item's own twin where the layer has no mirror") { + // The cull reads the item's bound: a bound built from the LAYER's axes + // alone would stop at the +x lump and a brick holding only the twin would + // compile without it. Compared against the full document tape. + CDoc doc; + add_lump(doc, CLAY_MIRROR_X); + clay_brick_config cfg; + cfg.struct_size = sizeof(cfg); + REQUIRE(clay_brick_config_defaults(&cfg) == CLAY_OK); + clay_brick_cache* cache = clay_brick_cache_create(&cfg); + REQUIRE(cache != nullptr); + const float lo[3] = {-1.5f, -1.5f, -1.5f}, hi[3] = {1.5f, 1.5f, 1.5f}; + REQUIRE(clay_brick_cache_mark_dirty(cache, lo, hi) == CLAY_OK); + const std::size_t samples = static_cast(cfg.dim) * cfg.dim * cfg.dim; + std::vector reqs(64); + std::vector values(64 * samples); + std::vector results(64); + for (;;) { + std::size_t count = reqs.size(), remaining = 0; + REQUIRE(clay_brick_cache_take_dirty(cache, reqs.data(), &count, &remaining) == CLAY_OK); + if (count == 0) break; + REQUIRE(clay_brick_cache_eval_requests(doc.d, nullptr, reqs.data(), count, values.data(), + count * samples, nullptr, 0) == CLAY_OK); + std::size_t accepted = 0; + REQUIRE(clay_brick_cache_submit(cache, reqs.data(), count, values.data(), + count * samples, nullptr, 0, results.data(), + &accepted) == CLAY_OK); + if (remaining == 0) break; + } + // A ray along -x at the lump's height, from beyond the twin. + const float origin[3] = {-2.0f, kLump[1], 0.0f}; + const float dir[3] = {1.0f, 0.0f, 0.0f}; + int32_t hit = 0; + float t = 0.0f; + REQUIRE(clay_brick_cache_raycast(cache, origin, dir, &hit, &t, nullptr, nullptr) == CLAY_OK); + CHECK(hit == 1); + CHECK(t == doctest::Approx(2.0f - kLump[0] - kR).epsilon(0.02)); + clay_brick_cache_destroy(cache); +} + +// ---- the placed-node setter -------------------------------------------------- + +TEST_CASE("a placed item's mirror is one undoable edit") { + CDoc doc; + REQUIRE(clay_document_enable_undo(doc.d) == CLAY_OK); + set_layer_mirror(doc, 1, 0, 0); + const clay_node_id node = add_lump(doc, -1); // saved with the default: follows X + REQUIRE(c_twin_x(doc.d)); + + // Correct it in place: own axes Y, participation off (radial only). + REQUIRE(clay_layer_set_node_mirror(doc.d, doc.layer, node, -1, CLAY_MIRROR_Y) == CLAY_OK); + int32_t mirror = 0; + uint8_t axes = 0, effective = 0; + REQUIRE(clay_layer_node_mirror(doc.d, doc.layer, node, &mirror, &axes, &effective) == CLAY_OK); + CHECK(mirror == -1); + CHECK(axes == CLAY_MIRROR_Y); + CHECK(effective == CLAY_MIRROR_Y); + CHECK_FALSE(c_twin_x(doc.d)); + CHECK(c_twin_y(doc.d)); + + // One undo puts back BOTH values. + int32_t undone = 0; + REQUIRE(clay_document_undo(doc.d, &undone) == CLAY_OK); + CHECK(undone == 1); + REQUIRE(clay_layer_node_mirror(doc.d, doc.layer, node, &mirror, &axes, &effective) == CLAY_OK); + CHECK(mirror == 1); + CHECK(axes == CLAY_MIRROR_AXES_INHERIT); + CHECK(effective == CLAY_MIRROR_X); + CHECK(c_twin_x(doc.d)); + CHECK_FALSE(c_twin_y(doc.d)); + + int32_t redone = 0; + REQUIRE(clay_document_redo(doc.d, &redone) == CLAY_OK); + CHECK(redone == 1); + REQUIRE(clay_layer_node_mirror(doc.d, doc.layer, node, &mirror, &axes, nullptr) == CLAY_OK); + CHECK(mirror == -1); + CHECK(axes == CLAY_MIRROR_Y); + CHECK(c_twin_y(doc.d)); + + SUBCASE("inherit puts the item back on the layer") { + REQUIRE(clay_layer_set_node_mirror(doc.d, doc.layer, node, 1, CLAY_MIRROR_AXES_INHERIT) == + CLAY_OK); + CHECK(c_twin_x(doc.d)); + CHECK_FALSE(c_twin_y(doc.d)); + } +} + +TEST_CASE("the mirror-axes entry points refuse what they cannot mean") { + CDoc doc; + const clay_node_id node = add_lump(doc, -1); + CHECK(clay_layer_set_node_mirror(doc.d, doc.layer, node, 1, 8) == CLAY_ERROR_INVALID_ARGUMENT); + CHECK(clay_layer_set_node_mirror(doc.d, doc.layer, 999, 1, 0) == CLAY_ERROR_NOT_FOUND); + clay_node_id group = 0; + REQUIRE(clay_layer_add_group(doc.d, doc.layer, 0, -1, CLAY_OP_ADD, CLAY_BLEND_HARD, 0.0f, 0.0f, + &group) == CLAY_OK); + CHECK(clay_layer_set_node_mirror(doc.d, doc.layer, group, 1, 0) == + CLAY_ERROR_INVALID_ARGUMENT); + CHECK(clay_layer_node_mirror(doc.d, doc.layer, group, nullptr, nullptr, nullptr) == + CLAY_ERROR_INVALID_ARGUMENT); + + const float params[1] = {kR}; + clay_item* item = clay_item_create(CLAY_PRIM_SPHERE, params, 1); + REQUIRE(item != nullptr); + uint8_t axes = 0; + REQUIRE(clay_item_mirror_axes(item, &axes) == CLAY_OK); + CHECK(axes == CLAY_MIRROR_AXES_INHERIT); + CHECK(clay_item_set_mirror_axes(item, 0x10) == CLAY_ERROR_INVALID_ARGUMENT); + REQUIRE(clay_item_set_mirror_axes(item, CLAY_MIRROR_X | CLAY_MIRROR_Z) == CLAY_OK); + REQUIRE(clay_item_mirror_axes(item, &axes) == CLAY_OK); + CHECK(axes == (CLAY_MIRROR_X | CLAY_MIRROR_Z)); + clay_item_destroy(item); +} + +TEST_CASE("saving at an older minor names the layer an item's own axes block") { + CDoc doc; + add_lump(doc, CLAY_MIRROR_X); + clay_layer_id blocking = 0; + CHECK(clay_document_writable_at_minor(doc.d, 19, &blocking) == CLAY_ERROR_UNSUPPORTED); + CHECK(blocking == doc.layer); + clay_blob* blob = nullptr; + CHECK(clay_document_save_memory_at_minor(doc.d, 19, &blob, &blocking) == + CLAY_ERROR_UNSUPPORTED); + CHECK(blob == nullptr); + + // At the current minor it saves, and loads with the axes and the twin. + REQUIRE(clay_document_save_memory(doc.d, &blob) == CLAY_OK); + clay_document* back = nullptr; + REQUIRE(clay_document_load_memory(clay_blob_data(blob), clay_blob_size(blob), &back) == + CLAY_OK); + clay_blob_destroy(blob); + CHECK(c_twin_x(back)); + clay_document_destroy(back); +} + +// ---- the Move brush ------------------------------------------------------------ + +TEST_CASE("a drag reaches an item through the item's own reflections") { + // The intended behaviour, stated in brush/move.h: the images of a drag are + // the copies the compiler emits of THIS item. Dragging at the -x twin of an + // item that kept X on a layer with no mirror moves it (both sides are the + // item); dragging at the layer's reflection of an item held at 0 does not. + const brush::MoveSettings settings{0.2f, 0, false, 0}; + const kernel::cfloat3 at_twin = kernel::cf3(-kLump[0], kLump[1] + kR, 0.0f); + + SUBCASE("own X on an unmirrored layer: the twin is reachable") { + const scene::Document doc = lump_doc(0, scene::kMirrorX); + const std::vector prepared = + brush::prepare_move(doc.layers.front(), at_twin, settings); + REQUIRE(prepared.size() == 1); + CHECK(prepared.front().images.size() == 2); + CHECK(prepared.front().own_mirror_axes == scene::kMirrorX); + // And the reach the host is told covers the twin's ball. + CHECK(brush::drag_images(doc.layers.front(), at_twin, kernel::cf3(0, 0.1f, 0), + brush::prepared_own_mirror_axes(prepared)) + .size() == 2); + } + SUBCASE("own 0 on a mirrored layer: only the touched side moves") { + const scene::Document doc = lump_doc(scene::kMirrorX, 0); + CHECK(brush::prepare_move(doc.layers.front(), at_twin, settings).empty()); + const kernel::cfloat3 on_item = kernel::cf3(kLump[0], kLump[1] + kR, 0.0f); + const std::vector prepared = + brush::prepare_move(doc.layers.front(), on_item, settings); + REQUIRE(prepared.size() == 1); + CHECK(prepared.front().images.size() == 1); + } + SUBCASE("an inheriting item still sees the layer's images, exactly as before") { + const scene::Document doc = lump_doc(scene::kMirrorX, scene::kMirrorAxesInherit); + const std::vector prepared = + brush::prepare_move(doc.layers.front(), at_twin, settings); + REQUIRE(prepared.size() == 1); + CHECK(prepared.front().images.size() == 2); + CHECK(prepared.front().own_mirror_axes == 0); + } +} + +TEST_CASE("the chain pad counts an item's own copies") { + // An item's own axes lengthen the layer's chain exactly as the layer's + // would, so the multiplicity the envelope resolves against includes them. + const scene::Document doc = lump_doc(0, scene::kMirrorX | scene::kMirrorY); + const scene::Layer& l = doc.layers.front(); + const scene::CullPadTerms terms = scene::cull_pad_terms(*l.sdf, l); + CHECK(terms.own_mirror_axes == (scene::kMirrorX | scene::kMirrorY)); + CHECK(scene::layer_symmetry_multiplicity(l, terms.own_mirror_axes) == 3); + CHECK(scene::layer_symmetry_multiplicity(l) == 1); +} + +TEST_CASE("a drag states the region of an item's own twin, not only the layer's images") { + // The reach a host invalidates is one box per image. On a layer with no + // mirror an item that kept X moves on both sides, so a drag on +x must + // also name the -x ball, both through the one-shot call's regions and + // through a live drag's dirty bounds; a host holding the +x box alone + // serves the moved twin stale. + CDoc doc; + add_lump(doc, CLAY_MIRROR_X); + const float on_item[3] = {kLump[0], kLump[1] + kR, 0.0f}; + const float pull[3] = {0.0f, 0.05f, 0.0f}; + clay_move_params params; + std::memset(¶ms, 0, sizeof params); + params.struct_size = static_cast(sizeof params); + params.radius = 0.2f; + + // A box reaches the twin when it holds the twin's grab point. + const auto holds_twin = [&](const float* box) { + return box[0] <= -on_item[0] && box[3] >= -on_item[0] && box[1] <= on_item[1] && + box[4] >= on_item[1]; + }; + + SUBCASE("clay_layer_move_surface_regions") { + std::vector boxes(6 * 8); + size_t applied = 0, count = 0; + REQUIRE(clay_layer_move_surface_regions(doc.d, doc.layer, on_item, pull, ¶ms, &applied, + boxes.data(), 8, &count) == CLAY_OK); + CHECK(applied == 1); + bool twin = false; + for (size_t i = 0; i < count; ++i) twin = twin || holds_twin(&boxes[6 * i]); + CHECK(twin); + } + SUBCASE("a live drag's dirty bounds") { + clay_sdf_move_tx* tx = clay_sdf_move_begin(doc.d, doc.layer, on_item, ¶ms, nullptr); + REQUIRE(tx != nullptr); + clay_sculpt_dirty dirty; + std::memset(&dirty, 0, sizeof dirty); + dirty.struct_size = static_cast(sizeof dirty); + REQUIRE(clay_sdf_move_update(tx, pull, &dirty) == CLAY_OK); + REQUIRE(dirty.has_bounds == 1); + const float box[6] = {dirty.bounds_min[0], dirty.bounds_min[1], dirty.bounds_min[2], + dirty.bounds_max[0], dirty.bounds_max[1], dirty.bounds_max[2]}; + CHECK(holds_twin(box)); + clay_sdf_move_destroy(tx); + } +} diff --git a/tests/unit/test_layer_composition.cpp b/tests/unit/test_layer_composition.cpp index 53211367..ab896908 100644 --- a/tests/unit/test_layer_composition.cpp +++ b/tests/unit/test_layer_composition.cpp @@ -284,7 +284,9 @@ TEST_CASE("a document where every layer unions still writes at minor 17, byte fo // A one-sided gate — a writer that emits it at 17, or a reader that consumes // it at 17 — fails here, and it has to, because layer records carry no // length and every record after the first would desynchronise. - const std::vector now = scene::serialize_document(doc, scene::kSceneMinor); + // Compared at 18, the minor that added the block: a later minor appends + // fields of its own (20 gives every node a byte), which is not this rule. + const std::vector now = scene::serialize_document(doc, 18); CHECK(now.size() == old_bytes.size() + 10 * doc.layers.size()); CHECK(now != old_bytes);