diff --git a/bindings/c/clay.h b/bindings/c/clay.h index 5ef81111..e9309f47 100644 --- a/bindings/c/clay.h +++ b/bindings/c/clay.h @@ -7549,7 +7549,12 @@ clay_result clay_sdf_move_preview_nodes(const clay_sdf_move_tx* tx, clay_node_id * the layer carries a mirror or a radial count, when it is one per IMAGE of the * drag that reaches the node — a straddler takes the ball's grab and its * reflection's, and a host that drew only the first would preview half the - * drag. CLAY_ERROR_NOT_FOUND for a node the drag does not reach. */ + * drag. Images that share ONE ball — a drag centred on the mirror plane or on + * the radial axis — are one brush (issue #663): a pull along the plane from on + * it is ONE grab, a pull across it two opposite ones, and an oblique pull + * three (the shared along-plane part once, then each image's remainder). So + * the count can differ from the number of reaching images; read it, never + * derive it. CLAY_ERROR_NOT_FOUND for a node the drag does not reach. */ clay_result clay_sdf_move_preview_grab_count(const clay_sdf_move_tx* tx, clay_node_id node, size_t* out_count); diff --git a/docs/05-claycore-library.md b/docs/05-claycore-library.md index 6821caa0..18d79f61 100644 --- a/docs/05-claycore-library.md +++ b/docs/05-claycore-library.md @@ -3119,7 +3119,7 @@ Two consequences worth knowing at the call. Both C transform setters write the W **And the live preview is incremental** (ABI 0.60.0): `clay_sdf_smooth_preview_item` copies the whole working volume, which is the right shape for a host joining mid-gesture and the wrong one for a per-frame loop — a dab moves a ball of bricks and the host re-uploads the model. `clay_sdf_smooth_preview_delta_info` and `clay_sdf_smooth_preview_delta_take` hand over exactly the bricks whose bytes are new: the ones a dab materialized and the ones its relax moved. Ask, size the buffers, take — the caller owns the memory it uploads from, which is the convention `clay_brick_cache_take_dirty` and `clay_voxel_take_dirty_chunks` already set. A short buffer takes **nothing** and reports what it needs, because taking is what clears the delta and a partial drain would strand bricks no later call reports. The delta ACCUMULATES until taken and is deduplicated by brick — a dab materializes a brick and then relaxes it, so the same coordinate arrives twice by construction — so a host that skips a frame loses nothing. `generation` is bumped by every update that changed the preview and by nothing else, which is how a host tells a duplicate read from a skipped frame and drops an upload begun against an older one; taking clears the delta and not the generation, because the generation names the state the caller now holds. `clay_sdf_preview_brick` carries no `struct_size`: it is an array element like `clay_brick_request`, received in hundreds and read rather than filled in. -**And a live field brush is a transaction** (ABI 0.59.0): `clay_sdf_smooth_*` and `clay_sdf_move_*` are two opaque handles with the same four verbs — `begin`, `update`, `commit`, `cancel` — plus a `destroy` that IS a cancel, so an error path which simply drops the handle leaves no half a stroke behind. They exist because two brushes cannot be spelled as edit-list stamps and so had no live form at all. Smooth BAKES (`field::relax` returns a volume, not an edit list), and a host with nowhere to keep that volume between pointer events must bake the layer again per dab — `sdf_consolidate`, 313 ms on the reference iPad — so the only affordable implementation ran once at pointer-up and the artist smoothed blind. Move warps every item it reaches, and written into the document per pointer event a drag churns revisions, tapes, caches and picking sixty times a second to produce one edit. A transaction holds the transient state instead: `clay_sdf_smooth_begin` samples the layer ONCE, each `clay_sdf_smooth_update` relaxes that retained volume in place, and `clay_sdf_smooth_commit` installs the volume the dabs were applied to rather than baking again; `clay_sdf_move_begin` walks the edit list once to prepare the items the drag reaches — through the ball or, under a layer mirror or radial count, through any image of it — each `clay_sdf_move_update` resolves the grabs of every *affected* item (one per image that reaches it) with no traversal at all, and the commit writes every final chain as one step. **Between begin and commit the document does not change** — no nodes, no deformers, no undo entries, and `clay_document_save_memory` taken mid-gesture returns the bytes it returned before it, which is what the ABI tests assert first. Each update fills a `clay_sculpt_dirty`: a conservative world box, the brick count, and `changed`, with the box and the count *geometric* (what the brush selected, not what happened to move) so they are reproducible however much unrelated model surrounds the brush, and `has_bounds` carrying emptiness as the third state the rest of this ABI already uses. Move's update takes the TOTAL displacement from the anchor, never an increment — 0.1, 0.2, 0.5 must end where a single fresh 0.5 lands, not past it — and `clay_sdf_move_preview_grab_count` / `clay_sdf_move_preview_grab` report the resolved grabs as the parameters `clay_item_add_deformer(CLAY_DEFORM_GRAB, ...)` already takes — one grab without symmetry, one per reaching image with it — so a host draws the preview through machinery it has. A commit **refuses a source that moved underneath it**: the layer is fingerprinted at begin and re-checked at commit, and an external edit stands rather than being written over by a preview computed against a document that no longer exists. `clay_sculpt_policy` carries the sampling (`clay_consolidation_params`' three numbers, with its meanings) and the budget — `min_safe_step_scale`, `max_deformer_chain`, `max_item_count`, where zero disables a criterion so an all-zero policy authorises nothing — plus `allow_consolidation`, which is the whole opt-in for the destructive half: over budget without it fills `clay_sculpt_budget.over_budget` and changes nothing, and with it the collapse happens inside the stroke's own undo step so one undo puts back both. The check runs only between completed strokes, never while the pointer is down. +**And a live field brush is a transaction** (ABI 0.59.0): `clay_sdf_smooth_*` and `clay_sdf_move_*` are two opaque handles with the same four verbs — `begin`, `update`, `commit`, `cancel` — plus a `destroy` that IS a cancel, so an error path which simply drops the handle leaves no half a stroke behind. They exist because two brushes cannot be spelled as edit-list stamps and so had no live form at all. Smooth BAKES (`field::relax` returns a volume, not an edit list), and a host with nowhere to keep that volume between pointer events must bake the layer again per dab — `sdf_consolidate`, 313 ms on the reference iPad — so the only affordable implementation ran once at pointer-up and the artist smoothed blind. Move warps every item it reaches, and written into the document per pointer event a drag churns revisions, tapes, caches and picking sixty times a second to produce one edit. A transaction holds the transient state instead: `clay_sdf_smooth_begin` samples the layer ONCE, each `clay_sdf_smooth_update` relaxes that retained volume in place, and `clay_sdf_smooth_commit` installs the volume the dabs were applied to rather than baking again; `clay_sdf_move_begin` walks the edit list once to prepare the items the drag reaches — through the ball or, under a layer mirror or radial count, through any image of it — each `clay_sdf_move_update` resolves the grabs of every *affected* item (one per image that reaches it, images sharing one ball counted as one brush — #663) with no traversal at all, and the commit writes every final chain as one step. **Between begin and commit the document does not change** — no nodes, no deformers, no undo entries, and `clay_document_save_memory` taken mid-gesture returns the bytes it returned before it, which is what the ABI tests assert first. Each update fills a `clay_sculpt_dirty`: a conservative world box, the brick count, and `changed`, with the box and the count *geometric* (what the brush selected, not what happened to move) so they are reproducible however much unrelated model surrounds the brush, and `has_bounds` carrying emptiness as the third state the rest of this ABI already uses. Move's update takes the TOTAL displacement from the anchor, never an increment — 0.1, 0.2, 0.5 must end where a single fresh 0.5 lands, not past it — and `clay_sdf_move_preview_grab_count` / `clay_sdf_move_preview_grab` report the resolved grabs as the parameters `clay_item_add_deformer(CLAY_DEFORM_GRAB, ...)` already takes — one grab without symmetry, one per reaching image with it, fewer where a drag on the plane makes two images one ball — so a host draws the preview through machinery it has. A commit **refuses a source that moved underneath it**: the layer is fingerprinted at begin and re-checked at commit, and an external edit stands rather than being written over by a preview computed against a document that no longer exists. `clay_sculpt_policy` carries the sampling (`clay_consolidation_params`' three numbers, with its meanings) and the budget — `min_safe_step_scale`, `max_deformer_chain`, `max_item_count`, where zero disables a criterion so an all-zero policy authorises nothing — plus `allow_consolidation`, which is the whole opt-in for the destructive half: over budget without it fills `clay_sculpt_budget.over_budget` and changes nothing, and with it the collapse happens inside the stroke's own undo step so one undo puts back both. The check runs only between completed strokes, never while the pointer is down. **And a deep layer's history has a cache a host can hold** (ABI 0.79.0): `clay_sdf_prefix_cache_*`. A Smooth gesture on a layer with a long edit list pays diff --git a/docs/07-brushes-and-features.md b/docs/07-brushes-and-features.md index ce6c750c..e1cc8507 100644 --- a/docs/07-brushes-and-features.md +++ b/docs/07-brushes-and-features.md @@ -326,11 +326,32 @@ 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 reach — one straddling the plane — takes both grabs in one warp, **composing -the two pulls as two brushes would**: a drag centred on the plane pulling along -it lifts a ball of radius .4 by 0.1645 against 0.0950 under one grab (1.73x), -continuously as the centre leaves the plane (0.1650 at x .01, 0.1600 at .05, -0.1460 at .1); pulling across the plane the two cancel to a pinch. This is the -mesh-sculpt default; an overlap reducer would be a kernel-level opt-in. With +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. + +**Except where two images are one ball** (#663). On the plane, the reflection +of a pull *along* the plane is the drag itself, and a grab composed with itself +is a second brush: a unit sphere dragged from (0,1,0) by (0,.25,0) at radius +.35 rose 0.2309 under mirror X against 0.1458 without (1.58x), through both +`clay_sdf_move_*` and `clay_layer_move_surface_regions`. Images whose centres +coincide — within a ten-thousandth of the radius plus a millionth of the +coordinates, which absorbs a placed layer's rounding — are resolved as one +group: the mean of their displacements is one grab, and each image keeps a grab +only for what it adds beyond the mean. A pull along the plane is then exactly +the unmirrored grab (0.1458); a pull across it has a zero mean and keeps its two +opposite grabs bit for bit; an oblique pull applies its along-plane part once +and pinches the rest. A radial drag on its axis, pulling along it, is the same +case with N images. + +This is **not continuous**, and it is chosen knowingly. Just off the plane the +images are two balls and the along-plane pull still composes where they +overlap — the same sphere lifts 0.2309 with the centre at x 1e-4, 0.2307 at +.01, 0.2266 at .05, 0.2135 at .1, 0.1597 at .2 — so there is a step at the +plane. The continuous rule (weight overlapping images by the max of their +falloffs instead of composing them) is not a composition of grabs at all: two +half-grabs composed lift that sphere 0.1600, not 0.1458. It needs a +multi-centre deformer in the kernel, on every backend, in the format and in the +Lipschitz and bound code, which is a feature rather than this fix. With an identity layer transform a drag and its mirror image produce the same field **bit for bit**, and a mirrored drag on late-history items states its own frontier and resumes where it used to take the base's ordinal 0 and drop. @@ -361,6 +382,11 @@ dimensionless factor and a reflection of a radial scale is a radial scale of equal strength, so the strength crosses every image untouched. And the region a host must invalidate is the ball itself with no dilation — outside the radius the weight is zero and the point is returned unchanged, for either sign. +Images that are one ball (#663) are one magnify: centred on the plane, the +reflection has the same centre and the same strength, and composing it with +itself scaled twice (a unit sphere at radius .35 rose 0.006 a little off the +centre under a hard mirror X, against 0.003 without). So the straddler takes one +magnify, reaching when any image of the group does. `pose` is in this section's title and does **not** have a resolver. It does not fit the shape: radial pose carries an axis, a direction that would have to be diff --git a/include/clay/brush/move.h b/include/clay/brush/move.h index 8b275b5c..96a7e0d8 100644 --- a/include/clay/brush/move.h +++ b/include/clay/brush/move.h @@ -57,7 +57,31 @@ // 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 // to a pinch, which is continuous as the centre approaches the plane and is -// what a mesh sculptor's mirror does. The grabs are ordered by their VALUES, +// what a mesh sculptor's mirror does. +// +// EXCEPT WHERE TWO IMAGES ARE ONE BALL (#663). On the plane, the reflection of +// a pull ALONG the plane is the drag itself — same centre, same displacement — +// and a grab composed with itself is not one brush but a second one: measured +// on a unit sphere dragged from (0,1,0) by (0,.25,0) at radius .35, the +// surface rose .2309 under mirror X against .1458 without (1.58x). Images +// whose centres coincide (within a tolerance that absorbs a placed layer's +// rounding, see `coincident`) are therefore resolved as ONE group: the mean of +// their displacements is applied once, and each image keeps a grab only for +// what it adds beyond that mean. A pull along the plane is then exactly the +// unmirrored grab; a pull across it has a zero mean and keeps the two +// opposite grabs, bit for bit, so the pinch above is unchanged; an oblique +// pull applies its along-plane part once and pinches the rest. A radial drag +// on its axis, pulling along it, is the same case N times over. +// +// NOT CONTINUOUS, and said so. Just off the plane the images are two balls and +// the along-plane pull still composes twice where the balls overlap; on it, +// once. The continuous rule — weighting overlapping images by the MAX of +// their falloffs rather than composing them — is not a composition of grabs +// at all (two composed half-grabs lift that sphere .160, not .1458), so it +// needs a multi-centre deformer in the kernel, on every backend, in the +// format and in the Lipschitz and bound code. That is a feature, not this fix. +// +// The grabs are ordered by their VALUES, // never by which image produced them: once the item is not itself // plane-symmetric the two orders are different fields, and the order has to // come out the same whichever side the drag was made from for the +x drag to @@ -117,7 +141,9 @@ std::vector drag_images(const scene::Layer& layer, kernel::cfloat3 wo // 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 -// for an item straddling a mirror plane. Already in the order the chain takes +// for an item straddling a mirror plane. Images that share one ball are one +// brush (#663): a pull along the plane from ON it is one grab, a pull across +// it two. Already in the order the chain takes // them. They belong at the FRONT of the node's chain — see `moved_chain`. struct MoveWarp { scene::NodeId node = scene::kNoNode; @@ -151,6 +177,19 @@ struct PreparedImage { // Whether this image's ball reaches the item's OWN bound. At least one // image of a prepared item does; an item no image reaches is not prepared. bool reaches = false; + // The first earlier image, in `drag_images` order, whose ball COINCIDES + // with this one — `kOwnBall` when none does (#663). A drag centred on a + // mirror plane, or on a radial axis, is its own image: the images that + // share a ball are one brush, and `resolve_prepared_move` builds their + // grabs once, at the leader's centre. Decided per drag, from world centres + // alone, so it holds for every frame of the gesture. + static constexpr std::size_t kOwnBall = static_cast(-1); + std::size_t leader = kOwnBall; + // The next image, in `drag_images` order, that shares this one's ball — + // `kOwnBall` for the last of a group and for an image alone in its ball. + // The group as a list from its leader, so resolving a drag stays + // O(images) however many groups a radial count makes. + std::size_t next = kOwnBall; }; // One affected item's share of a drag, resolved as far as it can be BEFORE the @@ -227,7 +266,8 @@ std::vector prepare_move(const scene::Layer& layer, kernel::cfloat MovePrepareStats* out_stats = nullptr); // The other half: the warp for a TOTAL world displacement — one grab per image -// that reaches the item, ordered by value, and the rest of the gesture's +// that reaches the item, coincident images resolved as one brush (see +// PreparedImage::leader), ordered by value, and the rest of the gesture's // identity in `gesture`. O(images), and no scene access at all — which is // what makes a live drag cost the items it moves. // diff --git a/openspec/changes/merge-coincident-drag-images/.openspec.yaml b/openspec/changes/merge-coincident-drag-images/.openspec.yaml new file mode 100644 index 00000000..29382a2c --- /dev/null +++ b/openspec/changes/merge-coincident-drag-images/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-29 diff --git a/openspec/changes/merge-coincident-drag-images/README.md b/openspec/changes/merge-coincident-drag-images/README.md new file mode 100644 index 00000000..9ab7d614 --- /dev/null +++ b/openspec/changes/merge-coincident-drag-images/README.md @@ -0,0 +1,3 @@ +# merge-coincident-drag-images + +Resolve a drag's coincident symmetry images as one brush, so a Move on a mirror plane is not applied twice (#663) diff --git a/openspec/changes/merge-coincident-drag-images/proposal.md b/openspec/changes/merge-coincident-drag-images/proposal.md new file mode 100644 index 00000000..b9ec4af6 --- /dev/null +++ b/openspec/changes/merge-coincident-drag-images/proposal.md @@ -0,0 +1,26 @@ +## Why +A Move drag centred on a layer mirror plane and pulling along it was applied twice to an item straddling the plane (#663). The reflection of that drag IS the drag — same centre, same displacement — and the resolver gave the straddler one grab per reaching image, so the same grab twice. Measured on a unit sphere dragged from (0, 1, 0) by (0, 0.25, 0) at radius 0.35, linear ease: + +| symmetry | surface lift at +y | +|---|---| +| none | 0.1458 | +| mirror X, before | 0.2309 (1.58x) | +| mirror X, after | 0.1458 | + +Identical through the live transaction (`clay_sdf_move_*`) and the held call (`clay_layer_move_surface_regions`), because both resolve through `resolve_prepared_move`. A radial drag on its axis, pulling along it, had the same defect N times over (four grabs for a count of four). The host has no lever: the images are resolved inside the engine. + +## What Changes +- Images of a drag whose centres coincide (within a ten-thousandth of the radius plus a millionth of the coordinates' magnitude, which absorbs a placed layer's rounding) are grouped when the drag is prepared. `PreparedImage::leader` names the first image of the group. +- A group resolves to the mean of its displacements as one grab, plus one grab per image for what it adds beyond that mean; components below a relative tolerance are dropped. A pull along the plane is exactly the unmirrored grab. A pull across it has a zero mean and keeps its two opposite grabs bit for bit, so the documented pinch is unchanged. An oblique pull applies the along-plane part once and pinches the rest (three grabs where there were two). +- Magnify resolves through the same prepared images and had the same defect: on the plane its reflection has the same centre and strength, so a straddler took the same magnify twice (a hard-seam mirror X lifted a unit sphere 0.006 a little off the centre, against 0.003 without). A group now resolves to one magnify, reaching when any member does. +- Grouping stays O(images) per frame: each group is a list threaded through `PreparedImage::next`, and the leaders are found through a projection-keyed index when the drag is prepared. A scan of every earlier image made a radial count of 4096 cost 19.5 ms to prepare and 5.1 ms per item per frame to resolve, against 0.2 ms and 0.52 ms before the merge; with the index they are 0.78 ms and 0.52 ms. +- `clay_sdf_move_preview_grab_count`'s header note, `docs/05`, `docs/07` and `include/clay/brush/move.h` state the rule, the measurement, and the discontinuity it leaves. + +## What building it found +- The issue's preferred continuous fix (weight overlapping images by the max of their falloffs) is not a composition of grabs. Two composed half-grabs lift the same sphere 0.1600, not 0.1458, so no scaling of the existing grabs is exact on the plane. The continuous rule needs a multi-centre deformer in the kernel, on every backend, in the format and in the Lipschitz and bound code. That is a feature, not this fix. +- The resulting step is real and is documented: with the centre at x 1e-5 the lift is 0.1458, at x 1e-4 it is 0.2309, and the doubling fades continuously as the balls separate (0.2266 at 0.05, 0.2135 at 0.1, 0.1597 at 0.2). +- Dropping coincident images outright (the issue's other suggestion) would have fixed only the pure along-plane pull. An oblique pull's images differ in their across-plane parts, so they are not duplicates, and the along-plane part would still have been applied twice. Keeping only the first image would have brought back the one-sided pull the pinch test already rejects. +- `test_sdf_sculpt`'s "a live Move on the plane gives a straddler both grabs" asserted the defect. Its oblique drag carried the along-plane pull on both grabs. It now asserts three grabs with that pull on one of them. + +## Impact +Only drags whose images coincide change: a drag on a mirror plane, or on a radial axis. Every other drag resolves bit-identically, because a group of one takes the old path unchanged. No ABI, format or version change. diff --git a/openspec/changes/merge-coincident-drag-images/specs/brush-engine/spec.md b/openspec/changes/merge-coincident-drag-images/specs/brush-engine/spec.md new file mode 100644 index 00000000..fb582a32 --- /dev/null +++ b/openspec/changes/merge-coincident-drag-images/specs/brush-engine/spec.md @@ -0,0 +1,133 @@ +## MODIFIED Requirements + +### Requirement: A world drag resolves into a field-level move +The library SHALL resolve a world-space centre, radius and displacement into the per-item warps that reproduce that drag on the ASSEMBLED surface of a layer, rather than on one item of it. The result SHALL be returned rather than applied, so a host can preview a drag before committing it and decide which commands carry it — the same rule the stroke engine's node consumer follows. + +Each warp SHALL be expressed in its item's OWN frame, mapping the world centre, radius and displacement through that item's world transform — which is the layer's transform composed with the item's, because a group's transform does not reach its children in this scene model. The resolver SHALL agree with the evaluator on that point rather than accumulating a chain the evaluator does not. + +Each warp SHALL be marked for the FRONT of its item's chain, because a warp appended behind an existing deformer has its region weight evaluated at a point that deformer already moved. + +Under the layer's symmetry the drag SHALL be stated as its IMAGES — the ball itself, one reflection per set mirror axis and one rotation per radial copy, additively and never as products, which is exactly the set of copies the compiler emits of an item — and each item SHALL be tested on its OWN influence bound, without the reflected or rotated copies, against each image. An image that reaches an item yields one grab at that image's centre with that image's displacement, so the reflected ball grabs the items whose reflections sit under the ball; an item no image reaches SHALL receive no warp at all, since a deformer with finite support, outside its own support, is a no-op that still costs a tape record on every evaluation. An item that does not participate in the symmetry SHALL see the drag alone. A node's grabs SHALL be ordered by their values, never by which image produced them, and a warp SHALL name every image the item can see so that a continuing gesture can recognise all of its earlier frames. + +Images whose centres COINCIDE — a drag centred on a mirror plane, or on a radial axis — SHALL be resolved as one brush rather than one grab each, since a grab composed with itself is a second brush and not the same one. Coincidence SHALL be decided per drag from the world centres, within a tolerance that absorbs the rounding a reflection picks up through a placed layer transform and no wider. A group of coincident images SHALL contribute the mean of their displacements as one grab and, for each image, one grab for what it adds beyond that mean, dropping components below a relative tolerance: a pull along the plane from on it SHALL then be exactly the unmirrored drag's grab, a pull straight across it SHALL keep its two opposite grabs, and an oblique pull SHALL apply its along-plane part once. Images that do not coincide SHALL keep one grab each, unchanged. The step this leaves between a centre on the plane and one just off it SHALL be documented, because the continuous rule is not a composition of grabs. + +Where the drag is resolved in two halves, the half that does not depend on the displacement SHALL carry the images the item sees — where each lands in the item's frame, and whether it reaches the item's own bound — so that the per-frame half maps the displacement through each image without the layer, and preparing once then resolving under symmetry is bit-identical to resolving in one step. + +#### Scenario: A blended form moves as one surface +- **WHEN** a drag centred between two smooth-unioned items is resolved and applied +- **THEN** both sides lift, symmetrically, and the lift peaks at the world centre + +#### Scenario: Grabbing one item is not the same thing +- **WHEN** the same drag is expressed as a grab on a single item instead +- **THEN** that item's side moves and the other is left behind + +#### Scenario: A nested item moves where the drag was aimed +- **WHEN** the layer's items sit under a group and a drag is resolved +- **THEN** the surface moves where the drag was aimed in WORLD space, matching what the evaluator does with those items + +#### Scenario: A transformed layer maps correctly +- **WHEN** the layer carries a transform and a drag is resolved in world space +- **THEN** the surface moves where the drag was aimed, not where it would have landed in layer space + +#### Scenario: Items out of reach are skipped +- **WHEN** a layer holds items far outside the drag's radius +- **THEN** no warp is produced for them + +#### Scenario: Nothing is written +- **WHEN** a drag is resolved +- **THEN** the document is unchanged until the caller applies the result + +#### Scenario: Under a mirror the drag selects what the ball or its reflection touches +- **GIVEN** an x-mirrored layer holding a base ball on the plane, an item under the ball and an item whose reflection sits under the ball +- **WHEN** the drag is resolved +- **THEN** the two items are selected and the base is not, and the item reached through its reflection takes a grab at the reflected centre with the reflected displacement — where selecting on the mirror-expanded bound took the base as well and gave that item a grab two diameters off its body + +#### Scenario: Material under the ball moves even when it is a copy +- **WHEN** a mirrored drag is applied over an item and over another item's reflection +- **THEN** the material under the ball moves by the same amount whether it is an item or a copy, and so do both of their reflections + +#### Scenario: A mirrored drag is the mirror image of its mirror image +- **GIVEN** a mirrored layer with an identity transform and no item opted out of the mirror +- **WHEN** a drag is applied to one document and its reflection across the plane to a fresh one +- **THEN** the two documents carry identical deformer chains and evaluate to identical fields at every sample, bit for bit + +#### Scenario: An item both images reach takes one grab per image, in a fixed order +- **WHEN** a drag's ball and its reflection, distinct balls, both reach an item straddling the plane +- **THEN** the item takes one warp carrying one grab per image, the two pulls compose as two brushes would, and the grabs are ordered by value so the +x drag and its mirror image produce the same field whether or not the item is itself plane-symmetric + +#### Scenario: A drag on the plane pulling along it is one brush +- **GIVEN** a unit sphere on an x-mirrored layer +- **WHEN** a drag from (0, 1, 0) by (0, 0.25, 0) at radius 0.35 and linear ease is applied, through the held call or the live transaction +- **THEN** the straddling item takes the unmirrored drag's single grab and the surface rises as far as it does with no mirror, where one grab per image lifted it 1.58x as far + +#### Scenario: A drag on the plane pulling across it still pinches +- **WHEN** a drag centred on the plane pulls straight across it +- **THEN** the coincident images' mean pull is zero and the straddler keeps its two opposite grabs, narrowing it symmetrically + +#### Scenario: An oblique drag on the plane shares its along-plane pull once +- **WHEN** a drag centred on the plane pulls obliquely +- **THEN** the straddler takes one grab carrying the along-plane part and two opposite grabs carrying the across-plane parts + +#### Scenario: Coincidence tolerates a placed layer and nothing wider +- **WHEN** a drag on the plane of a translated, rotated and scaled layer is resolved, and separately a drag 0.001 off the plane of an identity layer +- **THEN** the first straddler takes one grab despite the reflection's rounding, and the second takes two because its images are distinct balls + +#### Scenario: A drag on a radial axis along it is one brush +- **WHEN** a layer carries a radial count and a drag centred on the axis pulls along it +- **THEN** the item under it takes one grab rather than one per copy + +#### Scenario: An opted-out item sees the ball, not its reflection +- **WHEN** an item that does not participate in the mirror sits only where the reflected ball would reach it +- **THEN** it takes no warp, and the same item participating takes one through its copy + +#### Scenario: Two mirror axes make two reflections, not four quadrants +- **WHEN** a layer mirrors about two axes and a drag is resolved in one quadrant +- **THEN** the items in the two single-reflection quadrants are selected and the item in the diagonal quadrant is not + +#### Scenario: A prepared drag under symmetry is the one-step drag +- **WHEN** a drag on a mirrored, two-axis or radial layer is prepared once and resolved for a displacement +- **THEN** every item's grabs and the rest of its gesture identity are bit-identical to resolving the drag in one step, and an item reached only through its copy is prepared with that image marked as the one that reaches it + +#### Scenario: Radial symmetry rotates the brush the same way +- **WHEN** a layer carries a radial count and an item's rotated copy sits under the ball +- **THEN** that item takes a grab at the ball rotated by the copy's inverse angle, the copy under the ball moves, and the rotated-image drag on a fresh document matches the original to floating-point tolerance + +### Requirement: A world magnify resolves into a field-level radial scale +A magnify stated in WORLD space — a centre, a radius and a SIGNED strength — SHALL resolve into one `magnify` deformer per item the region reaches, each already in that item's own frame, so that the layer's ASSEMBLED surface swells or gathers rather than one item's share of it. + +`magnify` is per item and applied to that item's local point, exactly as `grab` is, so a magnify put on one item of a smooth-unioned form scales that item's field and leaves the others where they were. This is the hazard `move_brush` exists for, and it applies to the radial scale verbatim. + +A POSITIVE strength SHALL swell the surface away from the centre and a NEGATIVE one gather it toward. One signed parameter covers Magnify and Pinch, which are one deformation. + +The strength SHALL cross the layer's symmetry images unchanged: a reflection or a rotation of a radial scale is a radial scale of equal strength, unlike a drag's displacement, which has to be mapped per image. + +Images whose balls COINCIDE — a gesture centred on a mirror plane or on a radial axis — SHALL resolve to ONE magnify, reaching the item when any image of the group does: with the same centre and the same strength they are the same deformation, and composing it with itself would scale twice (#663). + +This SHALL follow the resolver pattern the Move brush established: the layer is READ and never written so a host can preview the gesture, the warps are RETURNED rather than applied so one command per node inside an undo group makes the gesture one undo step, and each warp SHALL belong at the FRONT of its node's chain. + +Items the region cannot reach SHALL take no deformer, a strength of zero SHALL produce nothing, and a non-positive radius SHALL produce nothing. + +#### Scenario: A blended form swells as one surface +- **WHEN** a magnify is resolved over a form smooth-unioned from two items and the warps are applied +- **THEN** both items take a share and the surface swells symmetrically about the gesture's centre + +#### Scenario: Magnifying one item is not the same thing +- **WHEN** the same deformation is expressed as a magnify on a single item instead +- **THEN** that item's side moves and the other is left behind + +#### Scenario: The sign chooses Magnify or Pinch +- **WHEN** the same region is resolved at a positive and then a negative strength +- **THEN** the surface swells away from the centre in the first case and gathers toward it in the second + +#### Scenario: A transformed layer maps correctly +- **WHEN** the layer carries a transform and a magnify is resolved in world space +- **THEN** the surface changes where the gesture was aimed, and the radius each item sees is the world radius through that layer's scale + +#### Scenario: Nothing is written +- **WHEN** a magnify is resolved +- **THEN** the document is unchanged until the caller applies the result + +#### Scenario: A magnify on the mirror plane is one magnify +- **GIVEN** a unit sphere on a layer mirrored about x with a hard seam +- **WHEN** a magnify centred at (0, 1, 0) at radius 0.35 is resolved and applied +- **THEN** the straddling item takes one magnify and the surface off the centre moves as far as with no mirror, where one magnify per image moved it twice as far diff --git a/openspec/changes/merge-coincident-drag-images/specs/c-abi/spec.md b/openspec/changes/merge-coincident-drag-images/specs/c-abi/spec.md new file mode 100644 index 00000000..9b80146f --- /dev/null +++ b/openspec/changes/merge-coincident-drag-images/specs/c-abi/spec.md @@ -0,0 +1,32 @@ +## MODIFIED Requirements + +### Requirement: A drag under symmetry reaches every image and keeps its history +`clay_layer_move_surface` SHALL state its reach as one box per image the layer's symmetry makes of the drag — the ball, one reflection per mirror axis, one rotation per radial copy — since the copies a mirrored or radial layer emits move where the images are. The boxes SHALL be taken as one invalidation, one revision, under one lock, and SHALL NOT be replaced by their union: the union of two balls a diameter apart is the slab between them, which under a mirror is the whole document. + +Under symmetry the drag SHALL state the frontier of the items it actually reaches, so a mirrored drag on late-history items keeps the prefix seeds it would keep unmirrored. A drag whose image does reach root ordinal 0 still takes the legacy drop, by design. + +`*out_applied` and the preview SHALL count ITEMS: an item that both the ball and an image reach takes its grabs in one command and is reported once. + +A live Move transaction SHALL report every grab the last update resolved for an affected node — `clay_sdf_move_preview_grab_count` gives how many, one without symmetry and one per image of the drag that reaches the node with it — images sharing one ball resolved as one brush, so a pull along a mirror plane from on it is one grab — and `clay_sdf_move_preview_grab` gives the grab at an index, refusing an index past the count — since a host that drew the first grab alone would preview half of a straddler's drag. + +#### Scenario: The reflected side is not stale +- **GIVEN** a mirrored layer and bricks warm on the side the ball's reflection covers +- **WHEN** a drag is applied on the other side and those bricks are refilled +- **THEN** the values match a fresh mirrored document's cold refill bit for bit, and differ from the undragged document's + +#### Scenario: A mirrored drag resumes +- **GIVEN** a mirrored layer whose dragged items were appended last, and a warm window of bricks over them +- **WHEN** a continuing drag is applied frame after frame +- **THEN** the frontier probe reports the dragged items' own root ordinal, the window resumes exactly as the unmirrored layer's does, and the refill matches a fresh mirrored oracle bit for bit + +#### Scenario: A transaction reports one grab per reaching image +- **WHEN** a live Move on an unmirrored layer is asked for an affected node's grabs +- **THEN** the count is one and index 1 is refused, while a node the drag does not reach is not found + +#### Scenario: A drag on the mirror plane reports one grab +- **WHEN** a live Move centred on a layer's mirror plane pulls along it over an item straddling the plane +- **THEN** the item's grab count is one, and the committed surface moves as far as the same drag on the unmirrored layer + +#### Scenario: A straddling item counts once +- **WHEN** a mirrored drag's ball and reflection both reach one item +- **THEN** the preview names it once and `*out_applied` counts it once, and the two agree diff --git a/openspec/changes/merge-coincident-drag-images/tasks.md b/openspec/changes/merge-coincident-drag-images/tasks.md new file mode 100644 index 00000000..3439a3ed --- /dev/null +++ b/openspec/changes/merge-coincident-drag-images/tasks.md @@ -0,0 +1,9 @@ +## Implementation +- [x] Group coincident drag images once per drag (`image_balls`, `PreparedImage::leader`). +- [x] Resolve a group as its mean pull once plus each image's remainder (`resolve_prepared_move`). +- [x] Regression tests: the issue's measurement through the held call and the live transaction; one grab for a pull along the plane, the three-grab oblique split, a placed layer's rounding, a radial axis, and two grabs for images 1e-3 apart. +- [x] Resolve a coincident group as one magnify (`resolve_prepared_magnify`), with its regression test. +- [x] Keep grouping O(images) per frame (`PreparedImage::next`, the leader index), with a 256-copy radial test. +- [x] Correct the live-transaction test that asserted the doubled along-plane pull. +- [x] Update `move.h`, the `clay_sdf_move_preview_grab_count` note, `docs/05` and `docs/07`. +- [x] No ABI, format or version change: ABI stays at its current line. diff --git a/src/brush/magnify.cpp b/src/brush/magnify.cpp index d9013273..21028ef9 100644 --- a/src/brush/magnify.cpp +++ b/src/brush/magnify.cpp @@ -44,6 +44,15 @@ void resolve_prepared_magnify(const PreparedMove& prepared, float strength, Move warp.deformers.clear(); warp.gesture.clear(); for (const PreparedImage& image : prepared.images) { + // Images that share one ball are ONE magnify (#663): with the same + // centre and the same strength they are the same deformation, and + // composing it with itself would scale twice. The leader carries the + // group, and reaches the item when any member does. + if (image.leader != PreparedImage::kOwnBall) continue; + bool reaches = image.reaches; + for (std::size_t j = image.next; j != PreparedImage::kOwnBall; + j = prepared.images[j].next) + reaches = reaches || prepared.images[j].reaches; // The strength is the SAME for every image. A reflection or a rotation // of a radial scale is a radial scale of equal strength, so unlike a // drag's displacement there is nothing per-image to map — and nothing @@ -55,7 +64,7 @@ void resolve_prepared_magnify(const PreparedMove& prepared, float strength, Move // Each one lands in exactly one of the two: the reaching images are the // warp, the others only its identity, so `moved_chain` recognises a // frame in which an image stopped reaching this item. - if (image.reaches) { + if (reaches) { warp.deformers.push_back(std::move(magnify)); } else { warp.gesture.push_back(std::move(magnify)); diff --git a/src/brush/move.cpp b/src/brush/move.cpp index 286d9f44..cdf18401 100644 --- a/src/brush/move.cpp +++ b/src/brush/move.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -73,18 +74,104 @@ struct ImageBall { cfloat3 centre; math::Aabb box; const math::cfloat4x4* linear; // null for the drag itself + std::size_t leader; // its group's leader; itself for a leader + std::size_t next; // see PreparedImage::next + std::size_t tail; // a leader's last member, where the next links }; +constexpr std::size_t kNoImage = PreparedImage::kOwnBall; + +float max_abs(cfloat3 v) { return std::max({std::fabs(v.x), std::fabs(v.y), std::fabs(v.z)}); } + +// Are two images the SAME ball (#663)? A centre on the plane reflects onto +// itself exactly on an identity layer (0 and -0 compare equal), but through a +// placed layer it goes out and back through the layer transform and misses by +// a few ulps of the coordinates involved. So the tolerance has two terms: a +// ten-thousandth of the radius, which no artist can place a drag inside of on +// purpose, and a millionth of the coordinates' magnitude, which covers that +// rounding however far from the origin the layer sits. Anything further apart +// is two balls, and keeps two grabs, exactly as before. +bool coincident(cfloat3 a, cfloat3 b, float radius, const math::Transform& xf) { + const float magnitude = std::max({max_abs(a), max_abs(b), max_abs(xf.position)}); + const float tolerance = 1e-4f * radius + 1e-6f * magnitude; + return max_abs(a - b) <= tolerance; +} + +// The earlier images that lead a group, keyed by where their centres fall +// along one fixed direction, so the leader a new image coincides with is a +// range query rather than a test against every earlier image. The scan was +// images^2 before the first frame: 19.5 ms at a radial count of 4096, against +// 0.2 ms with no merge at all. Centres within `tolerance` of each other on +// every axis project within 1.61 `tolerance` plus the projection's own +// rounding, which the doubled window covers; the direction is generic, so +// distinct images rarely share a window. A non-finite centre is never indexed +// (NaN has no place in an ordered map) and so leads its own group. +class LeaderIndex { +public: + explicit LeaderIndex(float tolerance) : window_(2.0f * tolerance) {} + + // The earliest leader whose ball `c` coincides with, or kNoImage. + std::size_t find(const std::vector& balls, cfloat3 c, float radius, + const math::Transform& xf) const { + const float key = project(c); + std::size_t found = kNoImage; + if (!std::isfinite(key)) return found; + for (auto it = leaders_.lower_bound(key - window_); + it != leaders_.end() && it->first <= key + window_; ++it) { + if (it->second < found && coincident(balls[it->second].centre, c, radius, xf)) + found = it->second; + } + return found; + } + + void add(cfloat3 c, std::size_t k) { + const float key = project(c); + if (std::isfinite(key)) leaders_.emplace(key, k); + } + +private: + static float project(cfloat3 c) { + return 0.26726124f * c.x + 0.53452248f * c.y + 0.80178373f * c.z; + } + + float window_; + std::multimap leaders_; +}; + +// Links each image to the earliest earlier LEADER whose ball it coincides +// with, and appends it to that leader's list; an image that coincides with no +// leader leads its own group. +void group_coincident(const math::Transform& xf, float radius, std::vector* balls) { + float magnitude = max_abs(xf.position); + for (const ImageBall& ball : *balls) magnitude = std::max(magnitude, max_abs(ball.centre)); + // Every pair `coincident` accepts is within this, on every axis. + LeaderIndex leaders(1e-4f * radius + 2e-6f * magnitude); + for (std::size_t k = 0; k < balls->size(); ++k) { + const std::size_t lead = leaders.find(*balls, (*balls)[k].centre, radius, xf); + if (lead == kNoImage) { + leaders.add((*balls)[k].centre, k); + continue; + } + ImageBall& head = (*balls)[lead]; + (*balls)[k].leader = lead; + (*balls)[head.tail].next = k; + head.tail = k; + } +} + std::vector image_balls(const math::Transform& xf, cfloat3 world_centre, const std::vector& linears, float radius) { const cfloat3 r = cf3(radius, radius, radius); std::vector balls; balls.reserve(linears.size() + 1); - balls.push_back({world_centre, math::Aabb{world_centre - r, world_centre + r}, nullptr}); + balls.push_back({world_centre, math::Aabb{world_centre - r, world_centre + r}, nullptr, 0u, + kNoImage, 0u}); for (const math::cfloat4x4& linear : linears) { const cfloat3 c = image_centre(xf, linear, world_centre); - balls.push_back({c, math::Aabb{c - r, c + r}, &linear}); + const std::size_t k = balls.size(); + balls.push_back({c, math::Aabb{c - r, c + r}, &linear, k, kNoImage, k}); } + if (balls.size() > 1) group_coincident(xf, radius, &balls); return balls; } @@ -176,6 +263,12 @@ PreparedMove prepare_item(const scene::Layer& layer, const scene::Node& n, scene image.copy = true; } image.reaches = reaches(own, balls[k].box); + // A leader always precedes its followers, so it is inside `seen` + // whenever they are. + image.leader = balls[k].leader == k ? PreparedImage::kOwnBall : balls[k].leader; + // ...and a follower is always after its leader, so an item that sees + // only the drag itself ends its list there. + image.next = balls[k].next < seen ? balls[k].next : PreparedImage::kOwnBall; prepared.images.push_back(image); } prepared.inverse_rotation = world.rotation.conjugate(); @@ -342,50 +435,128 @@ MoveWarp resolve_prepared_move(const PreparedMove& prepared, cfloat3 total_world return warp; } +namespace { + +// The world displacement one image pulls with: the drag's own, or its +// reflection or rotation for a copy. +cfloat3 image_world_displacement(const PreparedMove& prepared, const PreparedImage& image, + cfloat3 total_world_displacement) { + return image.copy + ? image_displacement(prepared.layer_xform, image.linear, total_world_displacement) + : total_world_displacement; +} + +// A displacement is a vector: the same map as a point, without the +// translation. Held to the same order and the same operations as +// `prepare_item`'s centre, on both branches, or a grab's centre and its +// displacement would describe different spaces. +cfloat3 local_displacement(const PreparedMove& prepared, cfloat3 world_displacement) { + if (prepared.layer_squashed) { + return scene::into_scaled_local( + prepared.node_rotation.conjugate().rotate(scene::into_scaled_local( + prepared.layer_rotation.conjugate().rotate(world_displacement) / + prepared.layer_uniform_scale, + prepared.layer_scale_axes)) / + prepared.node_uniform_scale, + prepared.scale_axes); + } + return scene::into_scaled_local( + prepared.inverse_rotation.rotate(world_displacement) / prepared.world_scale, + prepared.scale_axes); +} + +void emit_grab(const PreparedMove& prepared, const PreparedImage& image, bool reaching, + cfloat3 world_displacement, MoveWarp* warp) { + scene::Deformer grab = scene::Deformer::grab( + image.local_centre, prepared.local_radius, local_displacement(prepared, world_displacement), + prepared.ease, prepared.front_only); + // Stamped on every image, so a mirrored drag's copies belong to the + // same gesture as the one that produced them (#533). + grab.gesture_id = prepared.gesture_id; + // Each grab lands in exactly one of the two: the reaching ones are the + // warp, the others only its identity. With one image nothing lands in + // `gesture`, and a fresh warp costs one allocation; a reused one, none. + if (reaching) { + warp->deformers.push_back(std::move(grab)); + } else { + warp->gesture.push_back(std::move(grab)); + } +} + +// The images that share leader `lead`'s ball: how many, whether any reaches +// the item, and the mean of their displacements. +struct CoincidentGroup { + std::size_t count = 0; + bool reaches = false; + cfloat3 mean = cf3(0, 0, 0); +}; + +CoincidentGroup coincident_group(const PreparedMove& prepared, std::size_t lead, + cfloat3 total_world_displacement) { + CoincidentGroup group; + cfloat3 sum = cf3(0, 0, 0); + for (std::size_t j = lead; j != PreparedImage::kOwnBall; j = prepared.images[j].next) { + const PreparedImage& image = prepared.images[j]; + ++group.count; + group.reaches = group.reaches || image.reaches; + sum = sum + image_world_displacement(prepared, image, total_world_displacement); + } + group.mean = sum / static_cast(group.count); + return group; +} + +// Images sharing one ball are ONE brush for what they agree on and separate +// brushes only for what they do not (#663, and brush/move.h). `tolerance` +// absorbs the rounding a reflection picks up through a placed layer, so a +// pull along the plane on any layer resolves to the drag's own grab. +void emit_coincident(const PreparedMove& prepared, std::size_t lead, + cfloat3 total_world_displacement, MoveWarp* warp) { + const CoincidentGroup group = coincident_group(prepared, lead, total_world_displacement); + const PreparedImage& leader = prepared.images[lead]; + const float tolerance = 1e-5f * kernel::clength(total_world_displacement); + const auto residual = [&](const PreparedImage& image) { + return image_world_displacement(prepared, image, total_world_displacement) - group.mean; + }; + bool agree = true; + for (std::size_t j = lead; j != PreparedImage::kOwnBall && agree; j = prepared.images[j].next) + agree = kernel::clength(residual(prepared.images[j])) <= tolerance; + // All the same pull: the leader's own grab, exactly as without symmetry. + if (agree) { + emit_grab(prepared, leader, group.reaches, + image_world_displacement(prepared, leader, total_world_displacement), warp); + return; + } + // The shared part once — zero, and so absent, for a pull straight across a + // plane, which keeps that drag's two opposite grabs bit for bit — then + // what each image adds beyond it. + if (kernel::clength(group.mean) > tolerance) + emit_grab(prepared, leader, group.reaches, group.mean, warp); + for (std::size_t j = lead; j != PreparedImage::kOwnBall; j = prepared.images[j].next) { + const cfloat3 own = residual(prepared.images[j]); + if (kernel::clength(own) > tolerance) emit_grab(prepared, leader, group.reaches, own, warp); + } +} + +} // namespace + void resolve_prepared_move(const PreparedMove& prepared, cfloat3 total_world_displacement, MoveWarp* out) { MoveWarp& warp = *out; warp.node = prepared.node; warp.deformers.clear(); warp.gesture.clear(); - for (const PreparedImage& image : prepared.images) { - const cfloat3 world_displacement = - image.copy ? image_displacement(prepared.layer_xform, image.linear, - total_world_displacement) - : total_world_displacement; - // A displacement is a vector: the same map as a point, without the - // translation. Held to the same order and the same operations as - // `prepare_item`'s centre, on both branches, or a grab's centre and its - // displacement would describe different spaces. - const cfloat3 local_displacement = - prepared.layer_squashed - ? scene::into_scaled_local( - prepared.node_rotation.conjugate().rotate(scene::into_scaled_local( - prepared.layer_rotation.conjugate().rotate(world_displacement) / - prepared.layer_uniform_scale, - prepared.layer_scale_axes)) / - prepared.node_uniform_scale, - prepared.scale_axes) - : scene::into_scaled_local( - prepared.inverse_rotation.rotate(world_displacement) / prepared.world_scale, - prepared.scale_axes); - scene::Deformer grab = - scene::Deformer::grab(image.local_centre, prepared.local_radius, local_displacement, - prepared.ease, prepared.front_only); - // Stamped on every image, so a mirrored drag's copies belong to the - // same gesture as the one that produced them (#533). - grab.gesture_id = prepared.gesture_id; - // Each grab lands in exactly one of the two: the reaching ones are the - // warp, the others only its identity. With one image nothing lands in - // `gesture`, and a fresh warp costs one allocation; a reused one, none. - if (image.reaches) { - warp.deformers.push_back(std::move(grab)); - } else { - warp.gesture.push_back(std::move(grab)); + for (std::size_t k = 0; k < prepared.images.size(); ++k) { + const PreparedImage& image = prepared.images[k]; + if (image.leader != PreparedImage::kOwnBall) continue; // resolved with its leader + if (image.next != PreparedImage::kOwnBall) { // it leads a group + emit_coincident(prepared, k, total_world_displacement, &warp); + continue; } + emit_grab(prepared, image, image.reaches, + image_world_displacement(prepared, image, total_world_displacement), &warp); } - // A non-local item (infinite own bound) takes one grab per image it sees: - // its field reaches everywhere, so every image's ball is on it. + // A non-local item (infinite own bound) takes one grab per distinct image + // it sees: its field reaches everywhere, so every image's ball is on it. order_by_value(&warp.deformers); } diff --git a/tests/unit/test_c_move_brush.cpp b/tests/unit/test_c_move_brush.cpp index ded49a2b..877d9f38 100644 --- a/tests/unit/test_c_move_brush.cpp +++ b/tests/unit/test_c_move_brush.cpp @@ -1288,3 +1288,74 @@ TEST_CASE("a caller predating steady is unaffected") { const clay_layer_id lc = one_ball(c.doc); REQUIRE(clay_layer_move_surface(c.doc, lc, centre, disp, &mid, &n) == CLAY_OK); } + +// -- a drag ON a mirror plane is one brush (#663) ------------------------------- +// The reflection of a drag centred on the plane and pulling along it is the +// drag itself, and one grab per reaching image gave the straddling item that +// grab twice: measured on this exact fixture, 0.2307 of lift under mirror X +// against 0.1458 without. The images are resolved inside the engine, so a host +// has no lever; both the held call and the live transaction must agree. + +namespace { + +// The top of the surface above the origin, by bisection: a march step would +// be as large as the error it is meant to catch. +float top_above_origin(clay_document* doc, clay_layer_id layer) { + float lo = 0.5f, hi = 1.6f; // inside the sphere, outside the pulled tip + for (int i = 0; i < 40; ++i) { + const float mid = 0.5f * (lo + hi); + const float point[3] = {0.0f, mid, 0.0f}; + float d = 0.0f; + REQUIRE(clay_layer_eval_points(doc, layer, "cpu", point, 1, &d, nullptr) == CLAY_OK); + (d <= 0.0f ? lo : hi) = mid; + } + return 0.5f * (lo + hi); +} + +// The issue's measurement: a unit sphere, a drag from (0, 1, 0) by +// (0, 0.25, 0), radius 0.35, linear ease -- through the held call or the live +// transaction -- and the lift of the surface at +y. +float on_plane_lift(bool mirror_x, bool live) { + CDoc d; + const clay_layer_id layer = one_ball(d.doc); + if (mirror_x) REQUIRE(clay_set_layer_mirror(d.doc, layer, 1, 0, 0, 0.0f) == CLAY_OK); + const float centre[3] = {0.0f, 1.0f, 0.0f}; + const float disp[3] = {0.0f, 0.25f, 0.0f}; + clay_move_params p = move_params(0.35f); + p.ease = CLAY_EASE_LINEAR; + if (live) { + clay_sdf_move_tx* tx = clay_sdf_move_begin(d.doc, layer, centre, &p, nullptr); + REQUIRE(tx != nullptr); + REQUIRE(clay_sdf_move_update(tx, disp, nullptr) == CLAY_OK); + size_t grabs = 0; + std::vector nodes(4, 0); + size_t count = 0; + REQUIRE(clay_sdf_move_preview_nodes(tx, nodes.data(), nodes.size(), &count) == CLAY_OK); + REQUIRE(count == 1); + REQUIRE(clay_sdf_move_preview_grab_count(tx, nodes[0], &grabs) == CLAY_OK); + CHECK(grabs == 1); // one brush, whatever the mirror + REQUIRE(clay_sdf_move_commit(tx, nullptr) == CLAY_OK); + clay_sdf_move_destroy(tx); + } else { + std::vector boxes(6 * 8, 0.0f); + size_t applied = 0, count = 0; + REQUIRE(clay_layer_move_surface_regions(d.doc, layer, centre, disp, &p, &applied, + boxes.data(), 8, &count) == CLAY_OK); + REQUIRE(applied == 1); + } + return top_above_origin(d.doc, layer) - 1.0f; +} + +} // namespace + +TEST_CASE("a drag on the mirror plane moves the surface as far as with no mirror (#663)") { + for (bool live : {false, true}) { + CAPTURE(live); + const float plain = on_plane_lift(false, live); + const float mirrored = on_plane_lift(true, live); + CAPTURE(plain); + CAPTURE(mirrored); + CHECK(plain == doctest::Approx(0.1458f).epsilon(0.01)); + CHECK(mirrored == doctest::Approx(plain).epsilon(1e-4)); + } +} diff --git a/tests/unit/test_magnify_brush.cpp b/tests/unit/test_magnify_brush.cpp index a4919f97..a74d6fab 100644 --- a/tests/unit/test_magnify_brush.cpp +++ b/tests/unit/test_magnify_brush.cpp @@ -404,3 +404,52 @@ TEST_CASE("magnify: a straddler's deformers are ordered by their values") { CHECK(right[0].deformers[i].ext[0] == left[0].deformers[i].ext[0]); } } + +TEST_CASE("magnify: a gesture ON the mirror plane is one magnify, not two (#663)") { + // Centred on the plane, the reflection of a magnify IS the magnify — same + // centre, same strength — and composing it with itself scales twice: on a + // unit sphere at radius .35 the surface a little off the centre rose .006 + // under mirror X against .003 without. The seam is hard so the lift is the + // brush's alone, not the seam blend's. + const auto lift = [](bool mirrored, std::size_t* out_deformers) { + Document doc; + Layer layer; + layer.id = 1; + layer.kind = LayerKind::Sdf; + layer.sdf = std::make_shared(); + if (mirrored) layer.mirror_axes = 1u; // about x, k 0 + Node ball; + ball.prim = Prim::sphere(1.0f); + ball.op = Op::Add; + ball.blend = Blend{BlendProfile::Hard, 0.0f}; + layer.sdf->insert(ball); + doc.layers.push_back(layer); + const auto surface = [&doc] { + Tape t = compile_document(doc); + float last = 1.0f; + for (float y = 1.6f; y > 0.0f; y -= 0.0005f) { + const float d = t.eval(cf3(0, y, 0.15f)).d; + if (d <= 0.0f && last > 0.0f) return y; + last = d; + } + return 0.0f; + }; + const float before = surface(); + MagnifySettings s; + s.radius = 0.35f; + const std::vector warps = + brush::magnify_brush(doc.layers[0], cf3(0, 1, 0), 0.5f, s); + REQUIRE(warps.size() == 1); + *out_deformers = warps[0].deformers.size(); + REQUIRE(apply_warps(doc, 1, warps) == 1); + return surface() - before; + }; + std::size_t plain_count = 0; + std::size_t mirrored_count = 0; + const float plain = lift(false, &plain_count); + const float mirrored = lift(true, &mirrored_count); + REQUIRE(plain > 0.002f); // the gesture moved the probe at all + CHECK(plain_count == 1); + CHECK(mirrored_count == 1); + CHECK(mirrored == doctest::Approx(plain).epsilon(1e-3)); +} diff --git a/tests/unit/test_move_brush.cpp b/tests/unit/test_move_brush.cpp index 08fed6c8..65a83a5e 100644 --- a/tests/unit/test_move_brush.cpp +++ b/tests/unit/test_move_brush.cpp @@ -973,7 +973,9 @@ TEST_CASE("move: a straddler's grabs are ordered by their values, not by their i TEST_CASE("move: a drag centred on the plane pinches rather than picking a side") { // Both images share the centre and pull opposite ways. Two grabs, not - // one — deduplicating coincident images would keep today's one-sided + // one: coincident images are merged by their MEAN pull (#663), which is + // zero here, so the two opposite grabs survive bit for bit. Keeping only + // the first of two coincident images would instead keep today's one-sided // pull at exactly x = 0 and drop it .03 the moment the centre moves off // the plane (measured extents .2805 at x .05, .238 at .01, .2285 at // .001, .2275 at 0: continuous), so the rule is the same on the plane as @@ -1014,6 +1016,159 @@ TEST_CASE("move: a drag centred on the plane pinches rather than picking a side" CHECK(hi == doctest::Approx(-lo).epsilon(1e-3)); } +// -- coincident images are one brush (#663) ----------------------------------- +// A drag centred ON a mirror plane and pulling along it has a reflection that +// IS the drag: same centre, same displacement. One grab per reaching image +// gave a straddler the same grab twice, and a grab composed with itself moves +// the surface 1.58x as far (0.2307 against 0.1458 on the issue's sphere). +// Coincident images are merged before any grab is built: what they share is +// applied once, and only where they differ (a pull across the plane) does each +// keep a grab of its own. Reverting the merge in `resolve_prepared_move` fails +// every case below except the last, which pins the tolerance from the other +// side. + +namespace { + +// The top of the surface above x = 0, by bisection between a point inside and +// one outside. A march step would be the size of what is being measured. +float top_at_centre(const Document& doc, float inside_y, float outside_y) { + Tape t = compile_document(doc); + float lo = inside_y, hi = outside_y; + for (int i = 0; i < 40; ++i) { + const float mid = 0.5f * (lo + hi); + (t.eval(cf3(0, mid, 0)).d <= 0.0f ? lo : hi) = mid; + } + return 0.5f * (lo + hi); +} + +// S straddles the plane at y 1.5; the base sits out of reach below it. The +// mirror seam is hard, so S and its copy coincide exactly and the mirrored +// field above S is S's own. +Document straddler(bool mirror_on) { + Document doc = symmetric_layer(mirror_on); + doc.layers[0].mirror_k = 0.0f; + add_ball(doc, cf3(0, 0, 0), 0.4f); + add_ball(doc, cf3(0, 1.5f, 0), 0.2f); + return doc; +} + +} // namespace + +TEST_CASE("move: a drag ON the plane pulling along it is one brush, not two (#663)") { + const cfloat3 w = cf3(0, 1.5f, 0); + const cfloat3 d = cf3(0, 0.15f, 0); + const MoveSettings s{0.35f, 0, false}; + + Document plain = straddler(false), mirrored = straddler(true); + const std::vector wp = brush::move_brush(plain.layers[0], w, d, s); + const std::vector wm = brush::move_brush(mirrored.layers[0], w, d, s); + REQUIRE(wp.size() == 1); + REQUIRE(wm.size() == 1); + REQUIRE(wp[0].deformers.size() == 1); + // The mirrored straddler takes exactly the unmirrored drag's grab. + REQUIRE(wm[0].deformers.size() == 1); + CHECK(same_grab(wm[0].deformers[0], wp[0].deformers[0])); + + const float before = top_at_centre(plain, 1.5f, 2.2f); + apply_move(plain, 1, wp); + apply_move(mirrored, 1, wm); + const float lift_plain = top_at_centre(plain, 1.5f, 2.2f) - before; + const float lift_mirrored = top_at_centre(mirrored, 1.5f, 2.2f) - before; + CHECK(lift_plain > 0.02f); // teeth: the drag moved something + CHECK(lift_mirrored == doctest::Approx(lift_plain).epsilon(1e-4)); +} + +TEST_CASE("move: an oblique drag on the plane shares its along-plane pull once") { + // The images agree on the pull along the plane and disagree across it. The + // shared part is one grab; the across-plane parts stay two opposite grabs, + // which is the pinch the pure across-plane drag already gives. + Document mirrored = straddler(true); + const std::vector warps = brush::move_brush( + mirrored.layers[0], cf3(0, 1.5f, 0), cf3(0.1f, 0.15f, 0), {0.35f, 0, false}); + REQUIRE(warps.size() == 1); + REQUIRE(warps[0].deformers.size() == 3); + int along = 0, plus = 0, minus = 0; + for (const Deformer& g : warps[0].deformers) { + CHECK(g.k == 0.0f); // every grab sits at the drag's centre + if (g.ext[1] != 0.0f) { + ++along; + CHECK(g.ext[1] == doctest::Approx(0.15f)); + CHECK(g.ext[0] == doctest::Approx(0.0f)); + continue; + } + plus += g.ext[0] == doctest::Approx(0.1f) ? 1 : 0; + minus += g.ext[0] == doctest::Approx(-0.1f) ? 1 : 0; + } + CHECK(along == 1); + CHECK(plus == 1); + CHECK(minus == 1); +} + +TEST_CASE("move: coincident images merge through a placed layer's rounding") { + // Reflected in the layer's frame and mapped back, the image of a centre on + // the plane misses the drag by a rounding error, not by zero. The merge has + // to tolerate that or it only works on an identity layer. + Document mirrored = straddler(true); + Layer& layer = mirrored.layers[0]; + layer.xform.position = cf3(0.3f, -0.2f, 0.1f); + layer.xform.rotation = math::Quat::from_axis_angle(cf3(0, 1, 0), 0.7f); + layer.xform.scale = 1.4f; + const cfloat3 w = layer.xform.apply(cf3(0, 1.5f, 0)); + const cfloat3 d = layer.xform.rotation.rotate(cf3(0, 0.15f, 0.05f)); + const std::vector warps = brush::move_brush(layer, w, d, {0.35f, 0, false}); + REQUIRE(warps.size() == 1); + CHECK(warps[0].deformers.size() == 1); +} + +TEST_CASE("move: a drag on a radial axis, along it, is one brush and not one per copy") { + // Every rotation of a centre on the axis is that centre, and every rotation + // of a pull along the axis is that pull: four coincident images. + Document doc = symmetric_layer(false); + doc.layers[0].radial_count = 4; + doc.layers[0].radial_axis = 1; + const NodeId base = add_ball(doc, cf3(0, 0, 0), 0.4f); + const std::vector warps = + brush::move_brush(doc.layers[0], cf3(0, 0.4f, 0), cf3(0, 0.1f, 0), {0.3f, 0, false}); + const MoveWarp* b = warp_on(warps, base); + REQUIRE(b != nullptr); + CHECK(b->deformers.size() == 1); +} + +TEST_CASE("move: a radial count in the hundreds merges exactly the coincident pair") { + // Mirror X with 256 radial copies about Y, the drag on the plane but off + // the axis: its reflection is itself, and no two radial copies coincide. + // So 257 images are 256 balls. Grouping went through a projection-keyed + // index rather than an images^2 scan, and this is the case where every + // image but one leads its own group; the axis case below it is the other + // extreme, one leader and everything else following it. + Document doc = symmetric_layer(true); + doc.layers[0].radial_count = 256; + doc.layers[0].radial_axis = 1; + const NodeId base = add_ball(doc, cf3(0, 0, 0), 3.0f); + const std::vector off_axis = + brush::move_brush(doc.layers[0], cf3(0, 0.5f, 2.9f), cf3(0, 0.1f, 0), {0.3f, 0, false}); + const MoveWarp* a = warp_on(off_axis, base); + REQUIRE(a != nullptr); + CHECK(a->deformers.size() + a->gesture.size() == 256); + + const std::vector on_axis = + brush::move_brush(doc.layers[0], cf3(0, 3.0f, 0), cf3(0, 0.1f, 0), {0.3f, 0, false}); + const MoveWarp* b = warp_on(on_axis, base); + REQUIRE(b != nullptr); + CHECK(b->deformers.size() + b->gesture.size() == 1); +} + +TEST_CASE("move: images a hair apart are still two brushes") { + // The merge is for images that ARE one another, not for images that are + // close: off the plane by 1e-3 the two balls are distinct and each keeps + // its grab, as every off-plane drag always has. + Document mirrored = straddler(true); + const std::vector warps = brush::move_brush( + mirrored.layers[0], cf3(1e-3f, 1.5f, 0), cf3(0, 0.15f, 0), {0.35f, 0, false}); + REQUIRE(warps.size() == 1); + CHECK(warps[0].deformers.size() == 2); +} + TEST_CASE("move: an opted-out item sees the ball, not its reflection") { // C opted out of the mirror (Node::mirror = false, the C ABI's -1): the // compiler emits no copy of it, so the reflected ball has nothing of C diff --git a/tests/unit/test_sdf_sculpt.cpp b/tests/unit/test_sdf_sculpt.cpp index 979d4007..0a6f20a9 100644 --- a/tests/unit/test_sdf_sculpt.cpp +++ b/tests/unit/test_sdf_sculpt.cpp @@ -641,7 +641,12 @@ TEST_CASE("sculpt: a live Move under a mirror is the drag move_brush commits") { CHECK(moved_b == moved_a); } -TEST_CASE("sculpt: a live Move on the plane gives a straddler both grabs, every frame") { +TEST_CASE("sculpt: a live Move on the plane shares its along-plane pull once, every frame") { + // Centred on the plane, the drag's reflection shares its ball: the two + // images agree on the pull along the plane and oppose across it. The shared + // part is ONE grab (#663: it used to ride both images and move the surface + // up to twice as far as the unmirrored drag), the opposed parts stay two, + // and a continuing drag replaces all three rather than stacking them. Document doc; Layer& layer = doc.add_sdf_layer("mirrored"); layer.mirror_axes = kMirrorX; @@ -655,18 +660,30 @@ TEST_CASE("sculpt: a live Move on the plane gives a straddler both grabs, every REQUIRE(tx->affected_count() == 1); for (int i = 1; i <= 5; ++i) { tx->update(cf3(0.05f * static_cast(i), 0.02f, 0)); - // One grab per image, replaced frame to frame rather than stacked. - CHECK(tx->preview_layer().sdf->find(base)->deformers.size() == 2); + // Replaced frame to frame rather than stacked. + CHECK(tx->preview_layer().sdf->find(base)->deformers.size() == 3); } std::vector grabs; REQUIRE(tx->preview_grabs(base, &grabs)); - REQUIRE(grabs.size() == 2); - CHECK(grabs[0].ext[0] == -grabs[1].ext[0]); // opposite pulls across the plane - CHECK(grabs[0].ext[1] == grabs[1].ext[1]); // the same pull along it + REQUIRE(grabs.size() == 3); + int along = 0; + float across = 0.0f; + for (const Deformer& g : grabs) { + if (g.ext[1] != 0.0f) { + ++along; + CHECK(g.ext[1] == doctest::Approx(0.02f)); // the pull along it, once + CHECK(g.ext[0] == 0.0f); + } else { + CHECK(std::fabs(g.ext[0]) == doctest::Approx(0.25f)); + across += g.ext[0]; + } + } + CHECK(along == 1); + CHECK(across == 0.0f); // opposite pulls across the plane UndoStack undo; REQUIRE(tx->commit(&undo)); - CHECK(doc.layers.front().sdf->find(base)->deformers.size() == 2); + CHECK(doc.layers.front().sdf->find(base)->deformers.size() == 3); } TEST_CASE("sculpt: a Move commit refuses a layer that changed underneath it") {