Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 105 additions & 6 deletions bindings/c/clay_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,50 @@ clay_result from_io(const io::IoStatus& s) {
}
}

// The append log's reaches as a binary tree of unions over index ranges: "does
// any append from index `from` on reach this box?" in about log(n) box tests
// instead of n (#665). A region invalidation asks it once per lagging seed, and
// a straight scan made that seeds x appends: 43 ms at 3,000 dabs against 0.3 ms
// before the check existed, growing with the length of the stroke. Consecutive
// dabs lie close together, so a range's union stays tight and prunes well.
//
// Built once per region invalidation, from a log that invalidation then
// forgets, so nothing keeps it in step with later appends.
class ReachTree {
public:
explicit ReachTree(std::span<const math::Aabb> reaches) : count_(reaches.size()) {
while (width_ < count_) width_ *= 2;
nodes_.resize(2 * width_); // the padding leaves stay empty: they reach nothing
std::copy(reaches.begin(), reaches.end(),
nodes_.begin() + static_cast<std::ptrdiff_t>(width_));
for (std::size_t k = width_ - 1; k >= 1; --k) {
nodes_[k] = nodes_[2 * k];
nodes_[k].expand(nodes_[2 * k + 1]);
}
}

// Whether any reach at index `from` or later intersects `box`.
bool any_from(std::size_t from, const math::Aabb& box) const {
return from < count_ && any_in(1, 0, width_, from, box);
}

private:
// A node's union also holds entries before `from`, so it can only PRUNE;
// the answer is decided at a leaf, and every leaf reached here is at or
// after `from`.
bool any_in(std::size_t node, std::size_t lo, std::size_t hi, std::size_t from,
const math::Aabb& box) const {
if (hi <= from || !nodes_[node].intersects(box)) return false;
if (node >= width_) return true;
const std::size_t mid = lo + (hi - lo) / 2;
return any_in(2 * node, lo, mid, from, box) || any_in(2 * node + 1, mid, hi, from, box);
}

std::size_t count_ = 0;
std::size_t width_ = 1;
std::vector<math::Aabb> nodes_;
};

} // namespace

struct clay_document;
Expand Down Expand Up @@ -1379,7 +1423,13 @@ struct clay_document {
// sitting at revision R wants entries from `R - append_base_` onward, and
// several readers at different revisions can each take their own tail of
// the same log. `appends_since` is that lookup.
void touch_appended(scene::LayerId layer, scene::NodeId node) {
//
// `reach` is what the append can change -- the command_influence_bound
// apply_edit took on both sides of it. The log keeps one per entry so that
// a region invalidation arriving before any refill can tell which of the
// seeds this append left behind it may still carry forward (#665; see
// `carried_across_appends`).
void touch_appended(scene::LayerId layer, scene::NodeId node, const math::Aabb& reach) {
std::lock_guard<std::mutex> lock(cache_mutex_);
const std::uint64_t now = revision.load(std::memory_order_relaxed);
// The log is only usable while it is CONTIGUOUS: it describes the
Expand All @@ -1391,8 +1441,13 @@ struct clay_document {
append_layer_ = layer;
append_base_ = now;
append_log_.clear();
append_reach_.clear();
}
append_log_.push_back(node);
// Widened to the whole of space when infinite at all, so the union
// tree a region edit builds over these (ReachTree) can never prune an
// entry the per-entry test would have counted as reaching.
append_reach_.push_back(reach.is_infinite() ? math::Aabb::infinite() : reach);
// An append IS an order change: it grows the root list, so every
// ordinal taken before it counts a different tail. This does not gate
// the append fast path -- plan_resume never reads structure_revision_
Expand Down Expand Up @@ -2348,9 +2403,9 @@ struct clay_document {
// against 0.16x for the same drag unmirrored).
void touch_regions(std::span<const math::Aabb> changed) {
std::lock_guard<std::mutex> lock(cache_mutex_);
forget_appends(); // not an append; no prefix may be reused
const std::uint64_t next = revision.fetch_add(1, std::memory_order_relaxed) + 1;
touch_region_locked(changed, kFrontierDrop, next);
forget_appends(); // not an append; no prefix may be reused
}

// The region invalidation for an edit that MOVES ordinals -- node
Expand All @@ -2361,10 +2416,10 @@ struct clay_document {
// over-invalidating costs one full walk (the rule at touch(), restated).
void touch_region_structural(const math::Aabb& changed) {
std::lock_guard<std::mutex> lock(cache_mutex_);
forget_appends();
++structure_revision_;
const std::uint64_t next = revision.fetch_add(1, std::memory_order_relaxed) + 1;
touch_region_locked({&changed, 1}, kFrontierDrop, next);
forget_appends();
}

// The frontier invalidation (#360): a parameter edit inside root ordinal
Expand All @@ -2378,9 +2433,9 @@ struct clay_document {
// touch_region_from over several boxes, as touch_regions is to touch_region.
void touch_regions_from(std::span<const math::Aabb> changed, std::uint32_t frontier) {
std::lock_guard<std::mutex> lock(cache_mutex_);
forget_appends(); // not an append either; the log's contiguity is broken
const std::uint64_t next = revision.fetch_add(1, std::memory_order_relaxed) + 1;
touch_region_locked(changed, frontier, next);
forget_appends(); // not an append either; the log's contiguity is broken
}

std::uint64_t current_revision() const { return revision.load(std::memory_order_relaxed); }
Expand Down Expand Up @@ -2776,6 +2831,40 @@ struct clay_document {
void forget_appends() const {
append_valid_ = false;
append_log_.clear();
append_reach_.clear();
}

// May a seed at `rev` that THIS region edit does not reach be carried
// forward to `next`? Only if nothing between `rev` and this edit reached
// it either (#665). Caller holds cache_mutex_, before forget_appends.
//
// A region invalidation advances the seeds it misses, and that is only
// sound for a seed that was current just before it: the claim is "this
// edit did not change the brick", not "nothing did". A seed can lag the
// document by more than one revision because an append re-stamps no seed
// -- the log is what carries a seed across appends, and this edit is about
// to forget the log. So a lagging seed is carried only when the log covers
// every revision it lags by and no append in that span reaches its cull
// region. Anything else stays at its old revision, which no fast path can
// serve: the next refill of that brick walks it in full.
//
// The case that found it: append an intersect and move it before a
// refill. The append changes the whole layer, the move's swept surface
// delta (#471) is far smaller, and every seed outside the sweep was
// advanced past an append that had changed it -- after which `rev == now`
// handed the pre-append field back on every refill, however often the host
// dirtied the brick.
//
// `reaches` is append_reach_ as a union tree, built once by the caller:
// one scan of the log per lagging seed was seeds x appends (see ReachTree).
bool carried_across_appends(std::uint64_t rev, const math::Aabb& cull, std::uint64_t next,
const std::optional<ReachTree>& reaches) const {
if (rev + 1 == next) return true;
if (!reaches || !append_valid_ || append_at_ + 1 != next || rev < append_base_)
return false;
const std::size_t from = static_cast<std::size_t>(rev - append_base_);
if (from > append_reach_.size()) return false;
return !reaches->any_from(from, cull);
}

// The one loop behind the three touch_region fronts; caller holds
Expand All @@ -2798,6 +2887,8 @@ struct clay_document {
forget_resume();
return;
}
std::optional<ReachTree> reaches;
if (append_valid_) reaches.emplace(append_reach_);
for (auto it = resume_.begin(); it != resume_.end();) {
const ResumeKey& k = it->first;
ResumeEntry& e = it->second;
Expand Down Expand Up @@ -2849,7 +2940,11 @@ struct clay_document {
// until an accepted submit clears it in store_active. For a store
// that holds no dirty entries -- every path before #360 -- this is
// byte-for-byte the old behaviour.
if (e.dirty_from == kFrontierClean) e.revision = next;
//
// And only while nothing ELSE since the seed was taken reached the
// brick: see carried_across_appends (#665).
if (e.dirty_from == kFrontierClean && carried_across_appends(e.revision, cull, next, reaches))
e.revision = next;
++it;
}
}
Expand All @@ -2876,6 +2971,10 @@ struct clay_document {
// brings the document to append_at_.
mutable scene::TapeCheckpoint tape_checkpoint_;
mutable std::vector<scene::NodeId> append_log_;
// What each logged append could change, index for index with append_log_:
// what a region invalidation reads to decide which lagging seeds it may
// carry forward (carried_across_appends).
mutable std::vector<math::Aabb> append_reach_;
mutable scene::LayerId append_layer_ = 0;
mutable std::uint64_t append_base_ = 0;
mutable std::uint64_t append_at_ = 0;
Expand Down Expand Up @@ -4432,7 +4531,7 @@ clay_result apply_edit(clay_document* doc, const scene::Command& cmd, const char
// and what lets the next compile reuse its prefix. Everything else, here
// and at every other call site, keeps the general invalidation.
if (const TailAppend appended = tail_append(doc->doc.document, cmd); appended.layer != 0) {
doc->touch_appended(appended.layer, appended.node);
doc->touch_appended(appended.layer, appended.node, reach);
return CLAY_OK;
}
// Not an append, but its reach is known: a brick the edit cannot touch keeps
Expand Down
2 changes: 1 addition & 1 deletion docs/05-claycore-library.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-09-29
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# refill-a-dirtied-brick-against-the-current-document

A region invalidation carries a brick's seed forward only when nothing since the seed was taken reached the brick (#665).
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
## Where the reach comes from
`apply_edit` already takes `command_influence_bound` on both sides of every command before deciding it was a `TailAppend`. That box goes into `touch_appended` and is stored index for index with the log. Nothing is recomputed under `cache_mutex_`, and an intersect's box is the layer extent, which is exactly the region its append changes.

## Why not simply drop a lagging seed
Dropping is correct, and it was measured as the mutant `return rev + 1 == next`. After a stroke most seeds on the model sit at the revision before the stroke, because a refill re-stamps only the bricks it filled. Dropping them sends every later edit there down the full walk. On the regression fixture the resumed count for a re-dirtied underside went from 108 bricks to 0 (all 144 refilled). The per-append check keeps those seeds.

## Cost
The check runs once per untouched clean seed that lags. The first version scanned the log from each seed's revision, which made the region edit cost seeds x appends. On a filled unit sphere (dim 8, voxel 0.02) with N dabs on its cap, each refilled on its own, the first far move afterwards took:

| dabs | main (unsound) | linear scan | range-union tree |
|---|---|---|---|
| 200 | 0.21 ms | 3.98 ms | 0.52 ms |
| 1000 | 0.21 ms | 14.5 ms | 0.89 ms |
| 3000 | 0.30 ms | 42.9 ms | 1.27 ms |

The scan grew with the length of the stroke, under `cache_mutex_`. So the region edit now builds a binary tree of unions over the log's index ranges (`ReachTree`, O(n)) once, and each seed asks it whether any reach from its own index on meets its cull region. A node's union only prunes; the answer is decided at a leaf, so the result is exactly the scan's. Consecutive dabs lie close together, which keeps a range's union tight. An infinite reach is widened to the whole of space when it is logged, so no union can prune it.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## Why
Issue #665: appending an `Op::Intersect` item and then moving that same item with a uniform transform, with no refill between, left the brick cache holding the un-intersected layer. Marking the item's whole influence bound dirty and refilling did not repair it. The document was right (`clay_raycast` missed outside the cutter) and the bricks were not.

Measured on the issue's fixture (unit sphere, cylinder r 0.25 / half-height 1.6 moved to y 0.9, dim 8, voxel 0.02, band 3 voxels):

| variant | surface bricks before | after |
|---|---|---|
| cutter built at y 0.9 in one add (reference) | 192 | 192 |
| add at origin, uniform move, refill | **1171** (the sphere) | 192, bit-identical to the reference |
| the same, refilled a second time | **1171** | 192 |
| add a local item on the sphere, move a DIFFERENT item far away, refill the layer | **1123** vs 1116 | 1116, bit-identical |

The last row is not in the issue. It is the same defect without an intersect, and it shows that the cause is not the #471 swept bound.

## What the issue's reading got right and wrong
The issue guessed that the append's pending state is lost when the move calls `touch_region_from` at the appended ordinal. The loss is real, but the ordinal and the frontier path are not the cause. Instrumenting `touch_region_locked` showed every stale brick's seed at revision 3 (before the append) advanced straight to revision 5 (the move). An append re-stamps no seed, because the append log is what carries a seed across appends. The region invalidation that followed forgot the log and then advanced every seed its box missed to the new revision, including seeds still waiting to be carried across the append. After that, the refill's `rev == now` shortcut returned the pre-append value on every refill, however often the host dirtied the brick. All three region fronts share this loop (`touch_regions`, `touch_region_structural`, `touch_regions_from`), so any edit kind can do it. An intersect exposes it because its append changes the whole layer, and the swept move box is intentionally much smaller than that.

## What Changes
- The append log keeps each append's reach (the `command_influence_bound` union `apply_edit` already computes) beside the node it names.
- A region invalidation advances an unreached clean seed only if (a) it was current just before this edit, or (b) the log covers every revision the seed lags by and no append in that span reached the seed's cull region. The reaches are queried through a tree of range unions built once per region edit, so each seed costs about log(n) box tests, not n. Any other seed stays at its old revision, so the next refill of that brick walks it in full.
- All three region fronts now read the log before forgetting it.

## Impact
No ABI, format or version change. A brick the host dirties is refilled against the current document whatever edits came before. The carry-forward that makes an edit after a stroke cheap is kept: a seed that no append reached is still advanced and still resumed. A regression test pins this with `clay_document_resume_stats`. A mutant that drops every lagging seed turns the resumed count to zero.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
## ADDED Requirements

### Requirement: A dirtied brick is refilled against the current document
A brick the host marks dirty SHALL be refilled to the value the CURRENT document gives it, whatever sequence of edits preceded the mark and whether or not a refill ran between them. A stored seed SHALL be carried forward across an edit only when neither that edit nor any earlier edit the seed has not yet absorbed can reach the brick's cull region. A seed that cannot be shown current SHALL NOT answer a refill as if it were. The next refill of that brick SHALL walk it in full.

Carrying a seed forward is a performance cache and SHALL NOT be removed where it is sound: after appends that did not reach a brick, a later region edit that does not reach it either SHALL leave the brick answerable from its seed.

#### Scenario: Append an intersect, move it, refill
- **WHEN** an intersect item is appended to a layer and then moved with a uniform transform, with no refill between, and the item's influence bound is then marked dirty and refilled
- **THEN** the cache holds exactly the bricks, bit for bit, that a document built with the item at its final place holds

#### Scenario: A seed no append reached is still resumed
- **WHEN** a filled cache sees an append on one side of the model and a region edit elsewhere, and a region that neither reached is then re-dirtied and refilled
- **THEN** bricks of that region are answered from their seeds, and the result is bit-identical to a document built without the edit history
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
## Implementation
- [x] Reproduce the issue's table in a C ABI test and confirm it fails on main (1171 vs 192 surface bricks).
- [x] Instrument `touch_region_locked` to confirm pre-append seeds are advanced across the append (rev 3 -> 5).
- [x] Record each append's reach in the log; carry a lagging seed only across appends that miss it.
- [x] Read the log before forgetting it in all three region fronts.
- [x] Pin that a seed no append reached is still resumed (`clay_document_resume_stats`).
- [x] Mutation-check the gate: main-like carry, drop-all, and carry-without-scan each fail at least one case.
- [x] Replace the per-seed log scan with a range-union tree (42.9 ms -> 1.27 ms for the first region edit after 3,000 dabs).
- [x] Pin the log offset across several appends: a later append that reaches a brick holds it back, and an append the brick was refilled after does not. Mutants that read the log from its start, skip one entry, or test only the first entry each fail.
- [x] Update `docs/05-claycore-library.md`.
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ add_executable(clay_unit_tests
unit/test_cull_pad_stability.cpp
unit/test_node_reach_bound.cpp
unit/test_intersect_delta_bound.cpp
unit/test_c_intersect_append_move.cpp
unit/test_intersect_delta_oracle.cpp
unit/test_c_measure_reach.cpp
unit/test_worker_config.cpp
Expand Down
Loading
Loading