From 280395e3fe1a159ecca71620d9f670f01c090b01 Mon Sep 17 00:00:00 2001 From: Leonardo Araujo dos Santos Date: Sun, 4 Oct 2026 02:43:34 -0400 Subject: [PATCH 1/2] Add clay_voxel_grid_clone and clay_voxel_get_occupied (ABI 0.122.0 -> 0.123.0) A document's voxel grid was reachable only as a borrow, so a host moving grid-to-field to a worker rebuilt it from one clay_voxel_get per cell of the occupied box: empty cells included, the active level only, sculpt layers lost (#658). VoxelGrid::clone() copies every level, the palette, the active level and the sculpt layers, and drops what belongs to the source's session: the member-wise copy also carries change_sink_ and pass_capture_, pointers into the owner's undo journal, and the recording flag, so an edit to a plain copy lands in the source's history. The clone starts undrawn (every occupied chunk dirty), with a zero change count and a cold bounds cache. VoxelGrid::occupied_cells() walks the material chunks of a level, inherited ones included, sorted by z, y, x. clay_voxel_get_occupied exposes it on the active level with the size-query pattern and CLAY_ERROR_BUFFER_TOO_SMALL for a short buffer. clay.h now states the threading footing of the clone, the bulk read, clay_item_volume_from_voxels and clay_voxel_to_layer, including the race two readers of a cold grid hit in the lazily filled bounds cache. --- CMakeLists.txt | 2 +- bindings/c/clay.h | 101 +++++++++++++- bindings/c/clay_c.cpp | 41 ++++++ include/clay/voxel/grid.h | 40 ++++++ pyproject.toml | 2 +- src/voxel/grid.cpp | 65 +++++++++ tests/unit/test_c_voxel.cpp | 256 ++++++++++++++++++++++++++++++++++++ tests/unit/test_voxel.cpp | 77 +++++++++++ tools/check_c_abi.py | 24 ++++ 9 files changed, 604 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 67e0d7fa..164f2f7d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.24) project(claycore - VERSION 0.122.0 + VERSION 0.123.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 3f9cbc3a..9cf1df6d 100644 --- a/bindings/c/clay.h +++ b/bindings/c/clay.h @@ -24,7 +24,7 @@ extern "C" { #endif #define CLAY_ABI_MAJOR 0 -#define CLAY_ABI_MINOR 122 +#define CLAY_ABI_MINOR 123 #define CLAY_ABI_PATCH 0 /* Upper bound on the element count of any batch call: points, rays, cells, @@ -5395,6 +5395,50 @@ clay_voxel_grid* clay_voxel_grid_create(float voxel_size); * document layer is owned by that document: this returns * CLAY_ERROR_INVALID_ARGUMENT and leaves the document untouched. */ clay_result clay_voxel_grid_destroy(clay_voxel_grid* grid); +/* A deep copy of `src` into a grid the CALLER owns, from an owned or a borrowed + * handle alike: every resolution level, the active level, the palette and the + * sculpt layers. Free it with clay_voxel_grid_destroy — on the clone that is + * obeyed even when the source was a document layer's, whose own destroy is + * still refused. Added at ABI 0.123.0 (issue #658). + * + * This is how a borrowed grid leaves the interface thread. Before it, the only + * route was a clay_voxel_get per cell of the occupied BOX — empty cells + * included, the active level only, the sculpt layers dropped — rebuilt on the + * worker with clay_voxel_set. The clone is one container copy, sized by the + * chunks that hold material rather than by the box. + * + * WHAT IS NOT COPIED, deliberately. The clone is in no document: edits to it + * reach neither the source nor the document's undo history, and + * clay_document_save does not write it. A sculpt layer that is RECORDING in + * the source arrives closed (clay_voxel_recording_sculpt_layer reads 0), since + * "the next edit belongs to this pass" describes the source's session rather + * than its cells; its record, strength and visibility arrive intact. A dirty + * chunk drain in progress on the source handle stays there: the clone's dirty + * set is EVERY occupied chunk, as a grid read from a file has, because nothing + * has drawn it yet. clay_voxel_change_count starts at 0. + * + * THREADING. Cloning is a READ of the source, on the footing + * clay_mesh_sculptor_create documents: it touches no document state, and + * clay_last_error is per-thread. A clone of an OWNED grid may run on any + * thread. A clone of a BORROWED grid may run on a worker against a const + * document, and is NOT safe concurrently with a mutating clay_document_* / + * clay_voxel_* call on that document — the host owes that serialization, and + * a grid edited while it is copied produces a copy of neither state rather + * than a refused call. Calls on ONE handle must be serialized by the host, as + * with every handle here. + * + * ONE MORE HAZARD, and it is easy to miss: two READERS of the same grid can + * race. clay_voxel_bounds (and the raycast and repair calls that read the + * bounds) fill a cache on the grid lazily, so a const read on a grid whose + * cache is cold WRITES it, and a clone running concurrently copies those + * fields while they are written. Before a worker clones a borrowed grid, + * either warm the cache — one clay_voxel_bounds on the source, on the thread + * that owns the document — after which those reads are pure reads, or keep + * every other read of the source off the interface thread until the clone + * returns. Cloning on the interface thread itself has neither problem, and at + * a copy of the material chunks it is cheap enough to (0.07 ms for 89k + * cells). The clone's own cache is cold and belongs to the clone. */ +clay_result clay_voxel_grid_clone(const clay_voxel_grid* src, clay_voxel_grid** out_owned); /* Adds a voxel layer and borrows its grid. A voxel layer carries no SDF * content, so clay_add_item and clay_layer_add_item do not apply to it. @@ -11428,6 +11472,36 @@ clay_result clay_voxel_repair_fill_voids(clay_voxel_grid* grid, const clay_mask* clay_result clay_voxel_occupied_count(const clay_voxel_grid* grid, size_t* out_count); +/* Every occupied cell of the ACTIVE level and its palette index — the read + * counterpart to clay_voxel_set_many. Added at ABI 0.123.0 (issue #658). + * + * out_xyz receives count*3 int32 values and out_index count values, in the + * same ORDER: ascending z, then y, then x, which is the order a box walk with + * x innermost visits them. The order is a promise, so two reads of an + * unchanged grid agree element for element. + * + * Size-query pattern: with both buffers NULL, *out_count receives the count + * (clay_voxel_occupied_count's) and no cell is walked. Either buffer may be + * NULL on its own to read only that half. `capacity` is in CELLS; a capacity + * below the count is CLAY_ERROR_BUFFER_TOO_SMALL with the needed count in + * *out_count and nothing written. out_count is required. + * + * The cost follows the chunks, not the box: it walks the 32^3-cell chunks that + * hold material, so two cells far apart are two chunks rather than every empty + * cell between them, then sorts into the order above. Measured on an M-series + * host, a 3-cell-thick sphere shell: 31k cells in 1.1 ms, 89k in 4.1 ms, about + * two thirds of it the sort. A clone of the same 89k-cell grid is 0.07 ms, so + * to take a grid OFF the interface thread, clone it there and read the clone + * on the worker. On a partially refined level the cells the level + * INHERITS from its parent are reported where they are, exactly as + * clay_voxel_get reads them. Other levels are not read: switch the active + * level, or clone the grid and switch the clone's. + * + * THREADING as clay_voxel_grid_clone above: a read, safe on a worker while the + * document is const, and touching no cache. */ +clay_result clay_voxel_get_occupied(const clay_voxel_grid* grid, int32_t* out_xyz, + int32_t* out_index, size_t capacity, size_t* out_count); + /* Cell writes that actually CHANGED a cell, since the grid was constructed. * * Why this exists: an edit that is entirely legal and entirely without effect @@ -11601,10 +11675,33 @@ clay_result clay_voxel_mesh_quads(const clay_voxel_grid* grid, const clay_quad_p * how a caller assembles a sculpt one entry at a time by hand. clay_voxel_to_layer does NOT need that: it converts at index 0, whose * volume already carries every entry's colour per sample. * - * Free with clay_item_destroy; placing it copies it, as every item does. */ + * Free with clay_item_destroy; placing it copies it, as every item does. + * + * THREADING, on the footing clay_mesh_sculptor_create documents. The + * conversion is a READ of the grid and writes nothing but the new item: it + * reads cells, never the bounds cache clay_voxel_grid_clone warns about. + * - On an OWNED grid it touches no document, so it may run on any thread. + * This is the shape to use for a responsive grid-to-field crossing: + * clay_voxel_grid_clone on the interface thread, this on a worker against + * the clone, then clay_add_item / clay_layer_add_item back on the interface + * thread. The clone costs a copy of the material chunks; the conversion is + * the expensive half and it is the half that leaves. + * - On a BORROWED grid it is a read of the document: it may run on a worker + * against a const document, and is NOT safe concurrently with a mutating + * clay_document_* / clay_voxel_* call. The host owes that serialization, + * and a grid edited mid-conversion yields a volume of neither state rather + * than a refused call — which is why the clone is the recommended route. + * Calls on ONE grid handle must be serialized by the host, as everywhere. */ clay_result clay_item_volume_from_voxels(const clay_voxel_grid* grid, int32_t blur, int32_t index, clay_item** out_item); +/* THREADING: this ADDS A LAYER, so it mutates the document and belongs on the + * thread that owns it, serialized with every other clay_document_* call. The + * grid argument is only read, and may be a clone (clay_voxel_grid_clone): a + * host that wants the conversion itself off that thread converts the clone with + * clay_item_volume_from_voxels at index 0 on a worker, then on this thread + * adds the layer (clay_add_sdf_layer) and places the item, bracketed with + * clay_document_begin_undo_group so it stays one undo as this call is. */ clay_result clay_voxel_to_layer(clay_document* doc, const clay_voxel_grid* grid, const char* name, int32_t blur, clay_layer_id* out_layer); diff --git a/bindings/c/clay_c.cpp b/bindings/c/clay_c.cpp index 090a7cb3..d4325d5f 100644 --- a/bindings/c/clay_c.cpp +++ b/bindings/c/clay_c.cpp @@ -11384,6 +11384,21 @@ clay_result clay_voxel_grid_destroy(clay_voxel_grid* grid) { return CLAY_OK; } +// VoxelGrid::clone rather than the copy constructor: a borrowed grid's sink and +// pass capture point into its document's history, and a member-wise copy would +// carry them (#658). The new handle is an owner, whatever the source was, and +// carries no staged drain — that belongs to the source handle's conversation. +clay_result clay_voxel_grid_clone(const clay_voxel_grid* src, clay_voxel_grid** out_owned) { + if (!out_owned) return fail(CLAY_ERROR_INVALID_ARGUMENT, "null out pointer"); + voxel::VoxelGrid* g = nullptr; + clay_result r = resolve(src, &g); + if (r != CLAY_OK) return r; + auto* handle = new clay_voxel_grid(); + handle->owned = new voxel::VoxelGrid(g->clone()); + *out_owned = handle; + return CLAY_OK; +} + clay_result clay_document_add_voxel_layer(clay_document* doc, const char* name, float voxel_size, clay_layer_id* out_layer, clay_voxel_grid** out_grid) { if (!doc || !name) return fail(CLAY_ERROR_INVALID_ARGUMENT, "null document or name"); @@ -13841,6 +13856,32 @@ clay_result clay_voxel_occupied_count(const clay_voxel_grid* grid, size_t* out_c return CLAY_OK; } +clay_result clay_voxel_get_occupied(const clay_voxel_grid* grid, int32_t* out_xyz, + int32_t* out_index, size_t capacity, size_t* out_count) { + if (!out_count) return fail(CLAY_ERROR_INVALID_ARGUMENT, "null out_count"); + voxel::VoxelGrid* g = nullptr; + clay_result r = resolve(grid, &g); + if (r != CLAY_OK) return r; + // A count query answers from the per-chunk counters and walks no cell. + if (!out_xyz && !out_index) { + *out_count = g->occupied_count(); + return CLAY_OK; + } + const std::vector cells = g->occupied_cells(); + *out_count = cells.size(); + if (capacity < cells.size()) + return fail(CLAY_ERROR_BUFFER_TOO_SMALL, "capacity is below the occupied cell count"); + for (std::size_t i = 0; i < cells.size(); ++i) { + if (out_xyz) { + out_xyz[i * 3] = cells[i].cell.x; + out_xyz[i * 3 + 1] = cells[i].cell.y; + out_xyz[i * 3 + 2] = cells[i].cell.z; + } + if (out_index) out_index[i] = cells[i].index; + } + return CLAY_OK; +} + clay_result clay_voxel_change_count(const clay_voxel_grid* grid, uint64_t* out_count) { voxel::VoxelGrid* g = nullptr; clay_result r = resolve(grid, &g); diff --git a/include/clay/voxel/grid.h b/include/clay/voxel/grid.h index 55c43441..693a452e 100644 --- a/include/clay/voxel/grid.h +++ b/include/clay/voxel/grid.h @@ -135,6 +135,24 @@ class VoxelGrid { palette_.resize(1, kernel::cf3(0, 0, 0)); // index 0 = empty, unused } + // A deep copy of the GRID and none of the SESSION around it (#658): every + // level, the palette, the active level and the sculpt layers, with their + // strengths and records. + // + // Use this rather than the copy constructor whenever the source may belong + // to a document. The implicit copy is a member-wise copy, so it also takes + // the change sink and the pass capture — two pointers into the OWNER's undo + // journal — and the recording flag, and an edit to such a copy is written + // into the source's history. A clone has no sink and no capture, and no + // sculpt layer is recording in it: the layer that was open in the source + // is carried as a closed one, because "the next edit belongs to this pass" + // is a statement about the source's session, not about its cells. + // + // It is also a grid nothing has displayed yet: every occupied chunk is + // dirty, exactly as a grid read back from a file is, the change counter + // starts at zero, and the bounds cache is cold. + VoxelGrid clone() const; + // Cell size of the ACTIVE level. float voxel_size() const { return levels_[active_].voxel_size; } @@ -586,6 +604,24 @@ class VoxelGrid { // -- queries ------------------------------------------------------------- std::size_t occupied_count() const; + // Every occupied cell of a level with its palette index, sorted by z, then + // y, then x — the order a box walk with x innermost visits them, so a + // caller replacing one with this sees the same sequence. + // + // Walks the chunks that hold material rather than the bounding box, so the + // cost follows the cells and not the box: two cells far apart are two + // chunks, not the volume between them. A partially refined level reports + // the cells it INHERITS from its parent as well as the ones it stores, + // which is the same solid level_occupied_count counts. Empty for a level + // the grid does not have. + struct OccupiedCell { + VoxelCoord cell; + std::uint8_t index = 0; + bool operator==(const OccupiedCell&) const = default; + }; + std::vector occupied_cells() const { return occupied_cells(active_); } + std::vector occupied_cells(std::size_t level) const; + // Cell writes that actually changed a cell, since this grid was // constructed. Monotone; only the DIFFERENCE between two reads means // anything. Every verb writes through set(), so this is how a caller tells @@ -1110,6 +1146,10 @@ class VoxelGrid { void seed_refined_chunks(std::size_t fine); const Chunk* inherited_chunk(std::size_t level, VoxelCoord key, int* out_up) const; void ensure_bounds() const; + // The occupied cells of one material chunk of a level, stored or + // inherited, appended to `out` in the chunk's own z, y, x order. + void append_chunk_cells(std::size_t level, VoxelCoord key, + std::vector& out) const; // Chunk keys where a level HAS material, which is not the chunks it // STORES: an unrefined chunk reads its parent, so the parent's material is // at this level too. Every enumeration over a level goes through here. diff --git a/pyproject.toml b/pyproject.toml index e50ee52a..f251e3df 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ build-backend = "scikit_build_core.build" [project] name = "pyclay" -version = "0.122.0" +version = "0.123.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/voxel/grid.cpp b/src/voxel/grid.cpp index eedbafed..4e3d50b7 100644 --- a/src/voxel/grid.cpp +++ b/src/voxel/grid.cpp @@ -1041,6 +1041,53 @@ std::optional VoxelGrid::bounds_max() const { return lv.bounds_empty ? std::nullopt : std::optional(lv.bounds_hi); } +// Chunk by chunk over the keys that hold material, then one sort into the +// order a box walk would have produced. The sort is what makes the order a +// promise: the keys come out of a hash map whose order is its own. +std::vector VoxelGrid::occupied_cells(std::size_t level) const { + std::vector out; + if (level >= levels_.size()) return out; + out.reserve(level_occupied_count(level)); + for (const VoxelCoord& key : material_chunk_keys(level)) append_chunk_cells(level, key, out); + std::sort(out.begin(), out.end(), [](const OccupiedCell& a, const OccupiedCell& b) { + if (a.cell.z != b.cell.z) return a.cell.z < b.cell.z; + if (a.cell.y != b.cell.y) return a.cell.y < b.cell.y; + return a.cell.x < b.cell.x; + }); + return out; +} + +namespace { +// A cell of an inherited chunk: the ancestor's data at the shifted coordinate, +// the same read ensure_bounds makes. +std::uint8_t ancestor_index(const std::vector& data, int up, VoxelCoord c) { + const std::size_t ox = static_cast(fmod_pos(c.x >> up, kChunkDim)); + const std::size_t oy = static_cast(fmod_pos(c.y >> up, kChunkDim)); + const std::size_t oz = static_cast(fmod_pos(c.z >> up, kChunkDim)); + return data[(oz * kChunkDim + oy) * kChunkDim + ox]; +} +} // namespace + +// The same chunk resolution ensure_bounds uses: one lookup for the chunk. A +// stored chunk is read straight through its flat array, whose layout is the +// z, y, x walk below; an inherited one through its ancestor. +void VoxelGrid::append_chunk_cells(std::size_t level, VoxelCoord key, + std::vector& out) const { + const ChunkMap& chunks = levels_[level].chunks; + auto it = chunks.find(key); + int up = 0; + const Chunk* src = it != chunks.end() ? &it->second : inherited_chunk(level, key, &up); + if (!src) return; + constexpr int kDim = kChunkDim; + const VoxelCoord base{key.x * kDim, key.y * kDim, key.z * kDim}; + for (int i = 0; i < kDim * kDim * kDim; ++i) { + const VoxelCoord c{base.x + i % kDim, base.y + i / kDim % kDim, base.z + i / (kDim * kDim)}; + const std::uint8_t index = + up == 0 ? src->data[static_cast(i)] : ancestor_index(src->data, up, c); + if (index != 0) out.push_back({c, index}); + } +} + std::optional VoxelGrid::build_plane_pick(const math::Ray& ray, std::int32_t plane_cell) const { const float vs = voxel_size(); @@ -1383,6 +1430,24 @@ void VoxelGrid::mark_every_chunk_dirty() { } } +// The member-wise copy, then everything that described the SOURCE's session +// rather than its cells put back to what a fresh grid has. See the header for +// why each one has to go. +VoxelGrid VoxelGrid::clone() const { + VoxelGrid copy(*this); + copy.change_sink_ = nullptr; + copy.pass_capture_ = nullptr; + copy.recording_ = false; + copy.change_count_ = 0; + for (Level& lv : copy.levels_) { + lv.bounds_valid = false; + lv.dirty.clear(); + } + // Like a grid read from a file, a clone has never been drawn. + copy.mark_every_chunk_dirty(); + return copy; +} + bool VoxelGrid::read_level_tail(const std::uint8_t* data, std::size_t size, std::size_t* pos, std::uint32_t palette_count, bool has_regions) { auto get32 = [&](std::uint32_t* v) { diff --git a/tests/unit/test_c_voxel.cpp b/tests/unit/test_c_voxel.cpp index 32a58314..64a238b7 100644 --- a/tests/unit/test_c_voxel.cpp +++ b/tests/unit/test_c_voxel.cpp @@ -1,5 +1,6 @@ #include +#include #include #include @@ -1320,3 +1321,258 @@ TEST_CASE("a voxel grab's steady shares the stroke path's ceiling") { clay_voxel_grab_destroy(tx); } + +// -- a clone and a bulk read (issue #658) -------------------------------------- +// +// A layer's grid was reachable only as a borrow of its document, so a host that +// wanted to convert it off the interface thread rebuilt it on the worker from a +// clay_voxel_get per cell of the occupied BOX — empty cells included, the +// active level only, the sculpt layers lost. + +namespace { + +using CellRow = std::array; // x, y, z, palette index + +// The occupied cells of the active level the only way there was: every cell of +// the bounding box, in z-major order with x fastest. +std::vector box_walk(const clay_voxel_grid* grid) { + std::vector rows; + std::int32_t lo[3] = {0, 0, 0}, hi[3] = {0, 0, 0}; + std::int32_t has = 0; + REQUIRE(clay_voxel_bounds(grid, lo, hi, &has) == CLAY_OK); + if (!has) return rows; + for (std::int32_t z = lo[2]; z <= hi[2]; ++z) + for (std::int32_t y = lo[1]; y <= hi[1]; ++y) + for (std::int32_t x = lo[0]; x <= hi[0]; ++x) { + const std::int32_t cell[3] = {x, y, z}; + std::int32_t index = 0; + REQUIRE(clay_voxel_get(grid, cell, &index) == CLAY_OK); + if (index != 0) rows.push_back({x, y, z, index}); + } + return rows; +} + +std::vector bulk_read(const clay_voxel_grid* grid) { + size_t count = 0; + REQUIRE(clay_voxel_get_occupied(grid, nullptr, nullptr, 0, &count) == CLAY_OK); + std::vector xyz(count * 3), index(count); + size_t written = 0; + REQUIRE(clay_voxel_get_occupied(grid, xyz.data(), index.data(), count, &written) == CLAY_OK); + REQUIRE(written == count); + std::vector rows(count); + for (size_t i = 0; i < count; ++i) + rows[i] = {xyz[i * 3], xyz[i * 3 + 1], xyz[i * 3 + 2], index[i]}; + return rows; +} + +void check_bulk_read_is_the_box_walk(const clay_voxel_grid* grid) { + const std::vector walked = box_walk(grid); + const std::vector read = bulk_read(grid); + size_t occupied = 0; + REQUIRE(clay_voxel_occupied_count(grid, &occupied) == CLAY_OK); + CHECK(read.size() == occupied); + CHECK(walked.size() == occupied); + // Same cells, same indices, and the same ORDER: z, then y, then x fastest. + CHECK(read == walked); +} + +void set_cell(clay_voxel_grid* grid, std::int32_t x, std::int32_t y, std::int32_t z, + std::int32_t index) { + const std::int32_t cell[3] = {x, y, z}; + REQUIRE(clay_voxel_set(grid, cell, index) == CLAY_OK); +} + +std::int32_t cell_of(const clay_voxel_grid* grid, std::int32_t x, std::int32_t y, std::int32_t z) { + const std::int32_t cell[3] = {x, y, z}; + std::int32_t index = -1; + REQUIRE(clay_voxel_get(grid, cell, &index) == CLAY_OK); + return index; +} + +size_t level_count_of(const clay_voxel_grid* grid, size_t level) { + size_t n = 0; + REQUIRE(clay_voxel_level_occupied_count(grid, level, &n) == CLAY_OK); + return n; +} + +size_t undo_depth(const clay_document* doc) { + std::int32_t enabled = 0; + size_t depth = 0, redo = 0; + REQUIRE(clay_document_undo_state(doc, &enabled, &depth, &redo) == CLAY_OK); + REQUIRE(enabled == 1); + return depth; +} + +} // namespace + +TEST_CASE("c voxel: a clone of a borrowed layer is the whole grid and none of its document") { + clay_document* doc = clay_document_create(); + REQUIRE(doc != nullptr); + REQUIRE(clay_document_enable_undo(doc) == CLAY_OK); + clay_layer_id layer = 0; + clay_voxel_grid* grid = nullptr; + REQUIRE(clay_document_add_voxel_layer(doc, "sculpt", 0.1f, &layer, &grid) == CLAY_OK); + + // A multi-entry palette, material at two levels, the finer one active, and + // a sculpt layer left OPEN — everything a host's grid can be holding. + const float red[3] = {1.0f, 0.0f, 0.0f}, green[3] = {0.0f, 1.0f, 0.0f}, + blue[3] = {0.0f, 0.0f, 1.0f}; + std::int32_t r = 0, g = 0, b = 0; + REQUIRE(clay_voxel_palette_add(grid, red, &r) == CLAY_OK); + REQUIRE(clay_voxel_palette_add(grid, green, &g) == CLAY_OK); + REQUIRE(clay_voxel_palette_add(grid, blue, &b) == CLAY_OK); + set_cell(grid, 0, 0, 0, r); + set_cell(grid, 1, 0, 0, g); + set_cell(grid, 40, 2, -5, b); + size_t fine = 0; + REQUIRE(clay_voxel_add_level(grid, &fine) == CLAY_OK); + REQUIRE(fine == 1); + REQUIRE(clay_voxel_set_active_level(grid, fine) == CLAY_OK); + set_cell(grid, 9, 9, 9, g); + size_t pass = 0; + REQUIRE(clay_voxel_begin_sculpt_layer(grid, "open pass", &pass) == CLAY_OK); + set_cell(grid, 12, 3, 3, b); + + clay_voxel_grid* copy = nullptr; + REQUIRE(clay_voxel_grid_clone(grid, ©) == CLAY_OK); + REQUIRE(copy != nullptr); + CHECK(copy != grid); + + // Every level, the active one, and per level the same cells. + size_t levels = 0, active = 0; + REQUIRE(clay_voxel_level_count(copy, &levels) == CLAY_OK); + CHECK(levels == 2); + REQUIRE(clay_voxel_active_level(copy, &active) == CLAY_OK); + CHECK(active == fine); + for (size_t level = 0; level < 2; ++level) { + CHECK(level_count_of(copy, level) == level_count_of(grid, level)); + float want = 0.0f, got = 0.0f; + REQUIRE(clay_voxel_level_voxel_size(grid, level, &want) == CLAY_OK); + REQUIRE(clay_voxel_level_voxel_size(copy, level, &got) == CLAY_OK); + CHECK(got == want); + } + CHECK(bulk_read(copy) == bulk_read(grid)); + CHECK(cell_of(copy, 12, 3, 3) == b); + + // The palette, entry for entry. + size_t entries = 0, copied_entries = 0; + REQUIRE(clay_voxel_palette_size(grid, &entries) == CLAY_OK); + REQUIRE(clay_voxel_palette_size(copy, &copied_entries) == CLAY_OK); + CHECK(entries == 4); + CHECK(copied_entries == entries); + for (std::int32_t i = 1; i < static_cast(entries); ++i) { + float want[3] = {0, 0, 0}, got[3] = {-1, -1, -1}; + REQUIRE(clay_voxel_palette_color(grid, i, want) == CLAY_OK); + REQUIRE(clay_voxel_palette_color(copy, i, got) == CLAY_OK); + CHECK(std::memcmp(want, got, sizeof want) == 0); + } + + // The sculpt layer travels; the open RECORDING does not. A recording is + // the document's session — the next edit belongs to the source's pass. + size_t layers = 0, cells = 0; + REQUIRE(clay_voxel_sculpt_layer_count(copy, &layers) == CLAY_OK); + CHECK(layers == 1); + REQUIRE(clay_voxel_sculpt_layer_cell_count(copy, 0, &cells) == CLAY_OK); + CHECK(cells == 1); + std::int32_t recording = -1; + REQUIRE(clay_voxel_recording_sculpt_layer(copy, &recording) == CLAY_OK); + CHECK(recording == 0); + REQUIRE(clay_voxel_recording_sculpt_layer(grid, &recording) == CLAY_OK); + CHECK(recording == 1); + + // The level 0 cells, read through the clone's own level switch — the + // source's active level is the document's, and is not changed here. + REQUIRE(clay_voxel_set_active_level(copy, 0) == CLAY_OK); + CHECK(cell_of(copy, 0, 0, 0) == r); + CHECK(cell_of(copy, 1, 0, 0) == g); + CHECK(cell_of(copy, 40, 2, -5) == b); + REQUIRE(clay_voxel_active_level(grid, &active) == CLAY_OK); + CHECK(active == fine); + + // Editing the clone reaches neither the document's grid nor its history. + const size_t depth = undo_depth(doc); + const size_t source_fine = level_count_of(grid, fine); + const size_t source_coarse = level_count_of(grid, 0); + size_t source_pass_cells = 0; + REQUIRE(clay_voxel_sculpt_layer_cell_count(grid, 0, &source_pass_cells) == CLAY_OK); + REQUIRE(clay_voxel_set_active_level(copy, fine) == CLAY_OK); + set_cell(copy, 20, 20, 20, r); + const std::int32_t lo[3] = {-4, -4, -4}, hi[3] = {-1, -1, -1}; + REQUIRE(clay_voxel_fill_box(copy, lo, hi, g) == CLAY_OK); + const std::int32_t erased[3] = {9, 9, 9}; + REQUIRE(clay_voxel_erase(copy, erased) == CLAY_OK); + CHECK(undo_depth(doc) == depth); + CHECK(level_count_of(grid, fine) == source_fine); + CHECK(level_count_of(grid, 0) == source_coarse); + CHECK(cell_of(grid, 20, 20, 20) == 0); + CHECK(cell_of(grid, 9, 9, 9) == g); + size_t pass_cells = 0; + REQUIRE(clay_voxel_sculpt_layer_cell_count(grid, 0, &pass_cells) == CLAY_OK); + CHECK(pass_cells == source_pass_cells); + + // The clone is the caller's; the source stays the document's. + CHECK(clay_voxel_grid_destroy(grid) == CLAY_ERROR_INVALID_ARGUMENT); + CHECK(clay_voxel_grid_destroy(copy) == CLAY_OK); + CHECK(cell_of(grid, 9, 9, 9) == g); + + clay_voxel_grid* untouched = grid; + CHECK(clay_voxel_grid_clone(nullptr, &untouched) == CLAY_ERROR_INVALID_ARGUMENT); + CHECK(clay_voxel_grid_clone(grid, nullptr) == CLAY_ERROR_INVALID_ARGUMENT); + CHECK(untouched == grid); + + clay_document_destroy(doc); +} + +TEST_CASE("c voxel: the bulk read is the box walk, without the box") { + CGrid owned(0.1f); + clay_voxel_grid* grid = owned.grid; + const float rgb[3] = {0.4f, 0.5f, 0.6f}, other[3] = {0.9f, 0.1f, 0.1f}; + std::int32_t a = 0, b = 0; + REQUIRE(clay_voxel_palette_add(grid, rgb, &a) == CLAY_OK); + REQUIRE(clay_voxel_palette_add(grid, other, &b) == CLAY_OK); + + // An empty grid reads nothing, and says so through the count. + size_t count = 99; + REQUIRE(clay_voxel_get_occupied(grid, nullptr, nullptr, 0, &count) == CLAY_OK); + CHECK(count == 0); + + // Sparse: four chunks far apart, so the box is mostly empty. + set_cell(grid, 0, 0, 0, a); + set_cell(grid, 1, 0, 0, b); + set_cell(grid, 70, -20, 5, b); + set_cell(grid, -33, 5, 40, a); + const std::int32_t lo[3] = {2, 2, 2}, hi[3] = {4, 3, 5}; + REQUIRE(clay_voxel_fill_box(grid, lo, hi, a) == CLAY_OK); + check_bulk_read_is_the_box_walk(grid); + + // A short buffer is retryable: the count it needs, and nothing written. + REQUIRE(clay_voxel_get_occupied(grid, nullptr, nullptr, 0, &count) == CLAY_OK); + REQUIRE(count > 1); + std::vector xyz(count * 3, -7), index(count, -7); + size_t got = 0; + CHECK(clay_voxel_get_occupied(grid, xyz.data(), index.data(), count - 1, &got) == + CLAY_ERROR_BUFFER_TOO_SMALL); + CHECK(got == count); + CHECK(xyz[0] == -7); + CHECK(index[0] == -7); + // Either buffer alone is a read of that half. + REQUIRE(clay_voxel_get_occupied(grid, nullptr, index.data(), count, &got) == CLAY_OK); + CHECK(got == count); + CHECK(index[0] != -7); + CHECK(clay_voxel_get_occupied(grid, xyz.data(), index.data(), count, nullptr) == + CLAY_ERROR_INVALID_ARGUMENT); + CHECK(clay_voxel_get_occupied(nullptr, nullptr, nullptr, 0, &got) == + CLAY_ERROR_INVALID_ARGUMENT); + + // A partially refined level: outside its region the cells are INHERITED + // from the parent, and the bulk read has to report them where they are. + const float region_lo[3] = {0.0f, 0.0f, 0.0f}, region_hi[3] = {0.5f, 0.5f, 0.5f}; + size_t fine = 0; + REQUIRE(clay_voxel_add_level_region(grid, region_lo, region_hi, &fine) == CLAY_OK); + REQUIRE(clay_voxel_set_active_level(grid, fine) == CLAY_OK); + std::int32_t whole = 1; + REQUIRE(clay_voxel_level_is_whole(grid, fine, &whole) == CLAY_OK); + REQUIRE(whole == 0); + set_cell(grid, 5, 5, 5, b); + check_bulk_read_is_the_box_walk(grid); +} diff --git a/tests/unit/test_voxel.cpp b/tests/unit/test_voxel.cpp index 089a0c95..758ed705 100644 --- a/tests/unit/test_voxel.cpp +++ b/tests/unit/test_voxel.cpp @@ -1338,3 +1338,80 @@ TEST_CASE("voxel grab: a falloff means the curve it is named after") { // centre. Guards against "fix" by making every falloff rigid. CHECK(c_lin > h_lin); } + +// #658: a clone is a GRID, not the source's session. The implicit copy also +// copies the change sink (the document's undo journal), the open pass capture +// and the recording flag, so an edit to a plain copy lands in the SOURCE's +// history. The clone has to carry the cells and leave every channel behind. +TEST_CASE("a clone carries the cells and none of the source's recording channels") { + VoxelGrid src(0.1f); + const std::uint8_t red = src.palette_add(cf3(1, 0, 0)); + const std::uint8_t blue = src.palette_add(cf3(0, 0, 1)); + src.set({0, 0, 0}, red); + src.set({40, 1, -2}, blue); + REQUIRE(src.add_level() == 1); + REQUIRE(src.set_active_level(1)); + src.set({7, 7, 7}, blue); + src.begin_sculpt_layer("open pass"); + src.set({3, 3, 3}, red); + + // A gesture in flight on the source: the sink and the capture are live. + std::vector sink; + VoxelGrid::SculptLayerOp capture; + src.set_change_sink(&sink); + src.begin_pass_capture(&capture); + + VoxelGrid copy = src.clone(); + CHECK(copy.change_sink() == nullptr); + CHECK_FALSE(copy.recording_sculpt_layer()); + CHECK(copy.level_count() == 2); + CHECK(copy.active_level() == 1); + CHECK(copy.palette_size() == src.palette_size()); + CHECK(copy.sculpt_layer_count() == 1); + CHECK(copy.sculpt_layer_cell_count(0) == src.sculpt_layer_cell_count(0)); + for (std::size_t level = 0; level < 2; ++level) + CHECK(copy.level_occupied_count(level) == src.level_occupied_count(level)); + CHECK(copy.occupied_cells() == src.occupied_cells()); + // Never displayed, so everything it holds is owed to a host; and no write + // has been made to it. + CHECK(copy.dirty_chunk_count(0) == copy.occupied_chunk_keys(0).size()); + CHECK(copy.change_count() == 0); + + // An edit to the clone reaches none of the source's channels. + const std::size_t pass_cells = src.sculpt_layer_cell_count(0); + copy.set({10, 10, 10}, red); + copy.set({3, 3, 3}, 0); + CHECK(sink.empty()); + CHECK(capture.lower_afters.empty()); + CHECK(src.get({10, 10, 10}) == 0); + CHECK(src.get({3, 3, 3}) == red); + CHECK(src.sculpt_layer_cell_count(0) == pass_cells); + + src.set_change_sink(nullptr); + src.end_pass_capture(); +} + +TEST_CASE("occupied_cells reads a level in z, y, x order, inherited cells included") { + VoxelGrid g(0.1f); + const std::uint8_t a = g.palette_add(cf3(1, 0, 0)); + g.set({5, 0, 0}, a); + g.set({-40, 3, 0}, a); + g.set({0, 0, 70}, a); + g.set({0, -1, 70}, a); + const std::vector cells = g.occupied_cells(); + REQUIRE(cells.size() == 4); + // z first, then y: x = -40 comes second because its y is larger. + CHECK(cells[0].cell == VoxelCoord{5, 0, 0}); + CHECK(cells[1].cell == VoxelCoord{-40, 3, 0}); + CHECK(cells[2].cell == VoxelCoord{0, -1, 70}); + CHECK(cells[3].cell == VoxelCoord{0, 0, 70}); + CHECK(cells[0].index == a); + + // A regional level stores only its region; outside it the parent's cells + // are this level's too, eight children each. + const std::size_t fine = g.add_level(math::Aabb{cf3(0, 0, 0), cf3(0.5f, 0.5f, 0.5f)}); + REQUIRE_FALSE(g.level_is_whole(fine)); + CHECK(g.occupied_cells(fine).size() == g.level_occupied_count(fine)); + CHECK(g.occupied_cells(fine).size() == 4 * 8); + CHECK(g.occupied_cells(99).empty()); +} diff --git a/tools/check_c_abi.py b/tools/check_c_abi.py index f105612c..373e0a76 100644 --- a/tools/check_c_abi.py +++ b/tools/check_c_abi.py @@ -864,6 +864,27 @@ def voxel_ownership_exercise(lib, doc) -> list[str]: index = ctypes.c_int32(0) if lib.clay_voxel_get(grid, cells(1, 2, 3), ctypes.byref(index)) != 0 or index.value != 1: errors.append("the refused destroy lost the edit that was already made") + errors += voxel_clone_exercise(lib, grid) + return errors + + +def voxel_clone_exercise(lib, borrowed) -> list[str]: + """A clone of a borrowed grid is the caller's (#658), and the bulk read + hands back its one cell through the size-query pattern.""" + errors = [] + clone = ctypes.c_void_p(0) + if lib.clay_voxel_grid_clone(borrowed, ctypes.byref(clone)) != 0 or not clone.value: + return ["clay_voxel_grid_clone failed on a borrowed grid"] + count = ctypes.c_size_t(0) + if lib.clay_voxel_get_occupied(clone, None, None, 0, ctypes.byref(count)) != 0 \ + or count.value != 1: + errors.append(f"clay_voxel_get_occupied counted {count.value} cells, not 1") + xyz, index = (ctypes.c_int32 * 3)(), (ctypes.c_int32 * 1)() + if lib.clay_voxel_get_occupied(clone, xyz, index, 1, ctypes.byref(count)) != 0 \ + or tuple(xyz) != (1, 2, 3) or index[0] != 1: + errors.append(f"clay_voxel_get_occupied read {tuple(xyz)} = {index[0]}, not (1, 2, 3) = 1") + if lib.clay_voxel_grid_destroy(clone) != 0: + errors.append("clay_voxel_grid_destroy refused a clone, which the caller owns") return errors @@ -1742,6 +1763,9 @@ def ffi_exercise(lib_path: str) -> list[str]: lib.clay_voxel_set_brush.argtypes = [ctypes.c_void_p, cell_p, brush_p, ctypes.c_int32] lib.clay_voxel_sculpt_smooth.argtypes = [ctypes.c_void_p, cell_p, brush_p] lib.clay_voxel_occupied_count.argtypes = [ctypes.c_void_p, ctypes.POINTER(ctypes.c_size_t)] + lib.clay_voxel_grid_clone.argtypes = [ctypes.c_void_p, ctypes.POINTER(ctypes.c_void_p)] + lib.clay_voxel_get_occupied.argtypes = [ctypes.c_void_p, cell_p, cell_p, ctypes.c_size_t, + ctypes.POINTER(ctypes.c_size_t)] lib.clay_voxel_bounds.argtypes = [ctypes.c_void_p, cell_p, cell_p, ctypes.POINTER(ctypes.c_int32)] lib.clay_voxel_flood_select.argtypes = [ctypes.c_void_p, cell_p, ctypes.c_int32, cell_p, From 59a9db77260898de35a377134f4be195d7db98f4 Mon Sep 17 00:00:00 2001 From: Leonardo Araujo dos Santos Date: Sun, 4 Oct 2026 02:43:34 -0400 Subject: [PATCH 2/2] Document the voxel grid clone, bulk read and threading contract (#658) OpenSpec change clone-and-bulk-read-a-voxel-grid with c-abi and voxel-engine deltas and the measurements; docs/05 C ABI section and the README voxel-to-SDF paragraph point hosts at the clone for an off-thread conversion. --- README.md | 5 +- docs/05-claycore-library.md | 2 + .../design.md | 53 +++++++++ .../proposal.md | 110 ++++++++++++++++++ .../specs/c-abi/spec.md | 47 ++++++++ .../specs/voxel-engine/spec.md | 22 ++++ .../clone-and-bulk-read-a-voxel-grid/tasks.md | 47 ++++++++ 7 files changed, 285 insertions(+), 1 deletion(-) create mode 100644 openspec/changes/clone-and-bulk-read-a-voxel-grid/design.md create mode 100644 openspec/changes/clone-and-bulk-read-a-voxel-grid/proposal.md create mode 100644 openspec/changes/clone-and-bulk-read-a-voxel-grid/specs/c-abi/spec.md create mode 100644 openspec/changes/clone-and-bulk-read-a-voxel-grid/specs/voxel-engine/spec.md create mode 100644 openspec/changes/clone-and-bulk-read-a-voxel-grid/tasks.md diff --git a/README.md b/README.md index e0e23431..91f9142a 100644 --- a/README.md +++ b/README.md @@ -392,7 +392,10 @@ volume item that carries the palette per sample, so colour survives). Direct — trilinear interpolation between cell centres and **redistanced**, so the result carries a Lipschitz bound a raymarcher and a blend can trust. Non-destructive: the grid is untouched, and the result is an ordinary volume item in an -ordinary SDF layer. +ordinary SDF layer. To convert off the interface thread, `clay_voxel_grid_clone` +the layer's grid there (a copy of its material chunks, every level and the +palette, in no document) and convert the clone on a worker; `clay.h` states +the threading contract beside each call. **Mesh → voxel** — `VoxelGrid.rasterize_mesh(mesh)` in Python, `clay_voxel_rasterize_mesh` in C. An imported model reached an SDF layer in one diff --git a/docs/05-claycore-library.md b/docs/05-claycore-library.md index 091569a2..a86464cf 100644 --- a/docs/05-claycore-library.md +++ b/docs/05-claycore-library.md @@ -3221,6 +3221,8 @@ windows at every size measured**, and the build follows the history while the cache's SIZE follows the model — 1.43 MiB at 5,000 items and at 20,000 alike — so a budget is sized from one and a build scheduled from the other. +**A grid can leave the interface thread** (#658, ABI 0.123.0): `clay_voxel_grid_clone` deep-copies an owned *or borrowed* grid — every resolution level, the active level, the palette and the sculpt layers — into a handle the caller owns and frees with `clay_voxel_grid_destroy`. Until then a layer's grid was reachable only as a borrow of its document, and a host converting it on a worker rebuilt it from one `clay_voxel_get` per cell of the occupied *box*: empty cells included, the active level only, the sculpt layers lost — 17–46 ms on the interface thread for 5k–100k cells in ClaySpaceDesktop#285. A clone of an 89k-cell grid is 0.07 ms. It is a copy of the **grid, not the session**: the engine's member-wise copy would also carry the document's undo sink, an open pass capture and the recording flag, so `VoxelGrid::clone()` drops all three — edits to a clone reach neither the document nor its history, and a sculpt layer that was recording arrives closed. Its dirty set is every occupied chunk, as a grid read from a file has. `clay_voxel_get_occupied` is the bulk read `clay_voxel_set_many` lacked: every occupied cell of the active level and its index, sorted by z, then y, then x, by the size-query pattern, walking material chunks rather than the box. **Threading** follows `clay_mesh_sculptor_create`'s wording: the clone, the bulk read and `clay_item_volume_from_voxels` are reads; on an owned grid they touch no document and run on any thread, on a borrowed one they may run against a const document and are not safe concurrently with a mutating `clay_document_*` / `clay_voxel_*` call. One more hazard: `clay_voxel_bounds` fills a lazy cache, so two *readers* of a cold grid race — before a worker clones a borrowed grid, warm the cache with one `clay_voxel_bounds` on the owning thread, or keep every other read off that grid while the clone runs. The recommended crossing is clone on the interface thread, `clay_item_volume_from_voxels` on the worker against the clone, and the placement back on the interface thread. + **And the voxel side has one now too** (#86): `clay_voxel_mesh` is the *whole* grid, always, and it costs every occupied chunk on every call — so displaying a sculpt used to cost the model while editing it cost the dab. Two calls close that, in the brick cache's own vocabulary: `clay_voxel_take_dirty_chunks` drains the chunks a mutation could have changed (capacity in, count out, remainder reported — the `clay_brick_cache_take_dirty` shape, not a size query), and `clay_voxel_mesh_chunks` meshes exactly those keys and reports a `clay_voxel_chunk_mesh_range` per key so a host patches GPU sub-ranges instead of rebuilding the buffer. Every mutation feeds the set through the one cell-write choke point, a write that changes nothing dirties nothing, a write on a chunk *face* also dirties the chunk across it — the exposure test reads that cell — and a chunk emptied to nothing is dropped from the grid and *still* reported, because that is the key whose quads a host must remove. Unlike `clay_brick_mesh_range`, these ranges **partition** the mesh with no shared vertex: a voxel face belongs to one cell in one chunk, so clamping the greedy merge to a chunk boundary emits more, smaller quads over the identical surface — never a crack, and no straddler to attribute the way `mesh_bricks` must (#66). The cost is triangle count at chunk seams (+3.7% measured on a realistic sculpt), which is why `clay_voxel_mesh` stays whole-grid for export. Measured on Linux desktop at the device gate's 0.02 cell size — *not* comparable to the device baseline, per `docs/RELEASE.md`; the ratio transfers, the absolute does not — one dab dirties ~2 of 81 occupied chunks and re-meshes in **0.65 ms against 23.3 ms** for the whole grid. ## 12. Dependencies (all permissive, all C/C++) diff --git a/openspec/changes/clone-and-bulk-read-a-voxel-grid/design.md b/openspec/changes/clone-and-bulk-read-a-voxel-grid/design.md new file mode 100644 index 00000000..126e4406 --- /dev/null +++ b/openspec/changes/clone-and-bulk-read-a-voxel-grid/design.md @@ -0,0 +1,53 @@ +## Context + +A voxel layer's grid lives in `Document::voxel_layers` and crosses the ABI as a +borrowed handle that resolves its layer on every call. Converting it to a field +(`to_field`) is the expensive half of grid-to-field and is a const read, so it +can run on a worker — but only against a grid the worker can hold while the +interface thread keeps editing the document. + +## Decisions + +**Clone in the engine, not at the boundary.** `VoxelGrid::clone()` owns the rule +"a copy of the grid is not a copy of its session". Putting it in `clay_c.cpp` +would leave every other caller of the copy constructor — pyclay, the session +code — one member-wise copy away from writing into a history it does not own. +The copy constructor stays (the session layer copies grids by value on purpose, +for snapshots), and the header says when to use which. + +**Close the recording rather than keep it.** A sculpt layer that is recording +in the source arrives closed in the clone, with its record intact. "The next +edit belongs to this pass" is a statement about the document's session; a +worker converting the clone does not edit it, and a host that does edit a clone +has not asked to extend a pass in a grid it never opened one on. + +**The clone is undrawn.** Its dirty set is every occupied chunk, as +`deserialize` leaves a grid, rather than a copy of the source's pending set: +the source's set describes what the source's host has not drawn yet, which says +nothing about a grid nobody has drawn. `change_count` starts at 0 for the same +reason — it counts writes since construction. + +**The bounds cache is left cold.** Copying a warm cache would be correct for +identical data, but the fields are copied while possibly being written by a +concurrent cold reader of the source; starting cold means the clone's answer is +always computed from the clone's own cells. + +**Bulk read: active level, sorted.** Every other cell-addressed call acts on the +active level, so this does too, and another level is one `set_active_level` on a +clone away. The order is z, then y, then x — the order a box walk with x +innermost produces — so a host replacing its walk sees the same sequence. It is +produced by a sort rather than an ordered chunk walk (see the proposal's +measurements). + +**Size query answers from the counters.** Both buffers NULL returns +`occupied_count()`, which for a whole level sums per-chunk counters and walks +no cell. A short capacity is `CLAY_ERROR_BUFFER_TOO_SMALL` with nothing written. + +## Risks + +- A host that clones a BORROWED grid on a worker while the interface thread + reads its bounds races in the lazily mutating bounds cache. The header says so + and gives both remedies (warm it first, or clone on the interface thread); + nothing detects it. +- The bulk read allocates the whole cell list before copying it out: 16 bytes a + cell, 1.4 MB at 89k cells, transient. diff --git a/openspec/changes/clone-and-bulk-read-a-voxel-grid/proposal.md b/openspec/changes/clone-and-bulk-read-a-voxel-grid/proposal.md new file mode 100644 index 00000000..6e664cad --- /dev/null +++ b/openspec/changes/clone-and-bulk-read-a-voxel-grid/proposal.md @@ -0,0 +1,110 @@ +## Why + +**A document's voxel grid could not leave the interface thread except one +cell at a time.** Issue #658, from ClaySpaceDesktop#185 / #285: grid-to-field +froze the application (23k cells, 6.8 s on the interface thread). Moving the +conversion to a worker needed the grid off the document, and the ABI had no +call that copied a grid and no bulk read of its cells. A layer's grid is only +reachable as a BORROW (`clay_document_voxel_layer_by_id`), so the host: + +1. walked the occupied bounding box with one `clay_voxel_get` per cell, empty + cells included, because the box is all it can enumerate; +2. copied the palette entry by entry; +3. rebuilt the grid on the worker with `clay_voxel_set_many`; +4. converted that with `clay_item_volume_from_voxels`. + +That snapshot cost 17–46 ms on the interface thread for 5k–100k cells, which +was the application's whole remaining interface-thread share. It captured the +active level only, dropped the sculpt layers, and cost the box rather than the +cells. + +The threading contract was also unwritten. `clay_mesh_sculptor_create` states +which calls may run on a worker against a const document; +`clay_item_volume_from_voxels` and `clay_voxel_to_layer` said nothing. + +## What the issue's suggested fix got wrong + +The issue says `VoxelGrid` "is copyable by value, so a clone is +`new VoxelGrid(*g)` behind a handle." It is copyable, and that copy is the +defect. The member-wise copy also takes: + +- `change_sink_` — a pointer into the DOCUMENT's undo journal (`History`'s + `open_cells_`), live for the length of a voxel step; +- `pass_capture_` — a pointer to the document's open sculpt-layer pass + record; +- `recording_` — "the next edit belongs to the open sculpt layer". + +An edit to such a copy is appended to the source's history. The engine test +`a clone carries the cells and none of the source's recording channels` installs +a sink and a capture on a grid, clones it, edits the clone, and asserts the sink +and capture are untouched; run against `return VoxelGrid(*this)` it fails five +assertions — the sink is copied, the edit lands in it, and the capture records +the clone's rewrite of a cell of the source's pass. + +Through the C ABI the sink and capture are installed only for the length of +one call (`VoxelStep`), so between calls they are null and a copy made at the +boundary would not carry them today. The recording flag IS live between calls, +and the C test catches it: a naive clone of a grid with an open sculpt layer +reports `clay_voxel_recording_sculpt_layer == 1`. + +## What lands + +```c +clay_result clay_voxel_grid_clone(const clay_voxel_grid* src, clay_voxel_grid** out_owned); +clay_result clay_voxel_get_occupied(const clay_voxel_grid* grid, int32_t* out_xyz, + int32_t* out_index, size_t capacity, size_t* out_count); +``` + +- `VoxelGrid::clone()` — the member-wise copy, then the sink, the capture and + the recording flag cleared, the change counter zeroed, the bounds cache cold, + and every occupied chunk dirty (a grid nothing has drawn, as a grid read from + a file is). +- `VoxelGrid::occupied_cells(level)` — every occupied cell of a level with its + index, walking `material_chunk_keys` (stored and inherited) and sorted by z, + then y, then x. +- THREADING notes in `clay.h` beside the clone, the bulk read, + `clay_item_volume_from_voxels` and `clay_voxel_to_layer`, in the + `clay_mesh_sculptor_create` wording, including the lazily mutating bounds + cache (`grid.h` HAZARD note): two readers of a cold grid race. + +ABI 0.122.0 -> 0.123.0. + +## Measured + +Apple M-series, `-O2`, in-process C++ through `libclay_shared` (so the box walk +below is ~10 ns a call — a host crossing an FFI per cell pays far more, which is +the 17–46 ms the issue reports). A borrowed document layer, a 3-cell-thick +sphere shell, median of 15: + +| cells | box | box walk (`clay_voxel_get`) | `clay_voxel_grid_clone` | `clay_voxel_get_occupied` | +|---:|---:|---:|---:|---:| +| 4,184 | 25³ | 0.16 ms | 0.008 ms | 0.31 ms | +| 10,408 | 37³ | 0.51 ms | 0.008 ms | 0.49 ms | +| 30,880 | 61³ | 2.27 ms | 0.007 ms | 1.12 ms | +| 89,048 | 101³ | 10.96 ms | 0.074 ms | 4.12 ms | + +The clone is the route off the interface thread: a copy of the material +chunks, independent of how many FFI calls a host's language costs. + +## What building it found + +- **The bulk read is mostly its sort.** With the sort disabled the 89k row + reads 1.67 ms, against 4.12 ms with it. An ordered chunk walk (slab by slab, + row groups by `ky`, runs by `kx`) would emit in order without sorting; it was + not built, because it is five nested loops for a call that is not on the + interface-thread path once a host clones, and the simpler design was + preferred. The order is a promise either way. +- **The first walk was slower than the box walk.** It resolved each cell + through `fmod_pos` as `ensure_bounds` does; reading a stored chunk's flat + array in its own z, y, x layout took the 4k row from 0.65 ms to 0.31 ms. The + ancestor read is kept for inherited chunks only. +- **`clay_item_volume_from_voxels` does not touch the bounds cache.** + `to_field` sizes its box from `occupied_chunk_keys`, so the conversion is a + pure read; the hazard belongs to `clay_voxel_bounds`, the raycasts and repair. + The header says that rather than warning about the conversion. + +## Not changed + +- pyclay: nothing in the parity gate requires it (the gate checks that C + reaches what pyclay reaches). A Python grid is a shared value already. +- `clay_voxel_to_layer` stays an interface-thread call: it adds a layer. diff --git a/openspec/changes/clone-and-bulk-read-a-voxel-grid/specs/c-abi/spec.md b/openspec/changes/clone-and-bulk-read-a-voxel-grid/specs/c-abi/spec.md new file mode 100644 index 00000000..6aa86b27 --- /dev/null +++ b/openspec/changes/clone-and-bulk-read-a-voxel-grid/specs/c-abi/spec.md @@ -0,0 +1,47 @@ +## ADDED Requirements + +### Requirement: A voxel grid can be cloned into a grid the caller owns +The ABI SHALL provide `clay_voxel_grid_clone`, which accepts an owned or a +borrowed grid handle and returns a new handle the caller owns and frees with +`clay_voxel_grid_destroy`. The clone SHALL carry every resolution level, the +active level, the palette and the sculpt layers of the source. + +The clone SHALL be in no document. Edits to it SHALL reach neither the source +grid nor the document's undo history, and a sculpt layer that is recording in +the source SHALL arrive closed. A borrowed source SHALL remain the document's: +destroying it SHALL still be refused after it has been cloned. + +#### Scenario: A borrowed layer's grid is cloned whole +- **WHEN** a host clones a borrowed grid holding two levels with the finer one active, a multi-entry palette and an open sculpt layer +- **THEN** the clone reports the same level count, active level, per-level occupied counts, palette colours, cells and sculpt layers, and reports no sculpt layer recording + +#### Scenario: Editing the clone leaves the document alone +- **WHEN** a host edits the clone of a borrowed grid in a document with undo enabled +- **THEN** the document's grid, its sculpt-layer record and its undo depth are unchanged, destroying the clone succeeds, and destroying the borrowed source is still refused + +### Requirement: A voxel grid's occupied cells can be read in one call +The ABI SHALL provide `clay_voxel_get_occupied`, which writes every occupied +cell of the active level and its palette index, ordered by z, then y, then x, +using the size-query pattern: with both buffers NULL it SHALL report the count +without walking any cell, and a capacity below the count SHALL be +`CLAY_ERROR_BUFFER_TOO_SMALL` with the needed count reported and nothing +written. It SHALL walk the chunks that hold material rather than the bounding +box, and SHALL report the cells a partially refined level inherits from its +parent. + +#### Scenario: The bulk read matches a box walk +- **WHEN** a host reads a sparse grid with `clay_voxel_get_occupied`, on a whole level and on a partially refined one +- **THEN** the cells, indices and order equal those a `clay_voxel_get` walk of the bounding box finds, and the count equals `clay_voxel_occupied_count` + +### Requirement: The header states which voxel reads may leave the interface thread +`clay.h` SHALL state, beside `clay_voxel_grid_clone`, `clay_voxel_get_occupied`, +`clay_item_volume_from_voxels` and `clay_voxel_to_layer`, which of them may run +on a worker: on an owned grid the reads touch no document and may run on any +thread; on a borrowed grid they are reads of the document, safe against a const +document and not concurrently with a mutating `clay_document_*` / +`clay_voxel_*` call. It SHALL state that two concurrent readers of a grid whose +bounds cache is cold race, and how a host avoids it. + +#### Scenario: A host finds the threading contract beside the call +- **WHEN** a host integrator reads the declarations of the clone, the bulk read and the two conversion calls +- **THEN** each states its thread footing, and the clone's names the bounds-cache race and its remedy diff --git a/openspec/changes/clone-and-bulk-read-a-voxel-grid/specs/voxel-engine/spec.md b/openspec/changes/clone-and-bulk-read-a-voxel-grid/specs/voxel-engine/spec.md new file mode 100644 index 00000000..51db8e89 --- /dev/null +++ b/openspec/changes/clone-and-bulk-read-a-voxel-grid/specs/voxel-engine/spec.md @@ -0,0 +1,22 @@ +## ADDED Requirements + +### Requirement: A grid clone copies the grid and not its session +`VoxelGrid::clone()` SHALL return a deep copy of every level, the active level, +the palette and the sculpt layers, and SHALL NOT carry the source's change sink, +pass capture or recording state, since those point into the owner's undo +history. The clone SHALL start with every occupied chunk dirty, a change count +of zero and a cold bounds cache. + +#### Scenario: A clone taken mid-gesture records nothing into the source +- **WHEN** a grid with a change sink and a pass capture installed and a sculpt layer recording is cloned, and the clone is edited +- **THEN** the sink and the capture receive nothing, the source's cells and sculpt-layer record are unchanged, and the clone reports no sink and no recording + +### Requirement: A level's occupied cells can be enumerated in a stable order +`VoxelGrid::occupied_cells(level)` SHALL return every occupied cell of the +level with its palette index, ordered by z, then y, then x, including the cells +a partially refined level inherits from its parent, and SHALL return nothing for +a level the grid does not have. + +#### Scenario: Inherited cells are enumerated where they are +- **WHEN** a level is added over a region and the cells outside it are inherited from the parent +- **THEN** the enumeration's size equals the level's occupied count, eight children per parent cell diff --git a/openspec/changes/clone-and-bulk-read-a-voxel-grid/tasks.md b/openspec/changes/clone-and-bulk-read-a-voxel-grid/tasks.md new file mode 100644 index 00000000..60221d67 --- /dev/null +++ b/openspec/changes/clone-and-bulk-read-a-voxel-grid/tasks.md @@ -0,0 +1,47 @@ +## 1. The gap + +- [x] 1.1 `clay.h` had `clay_voxel_grid_create` / `_destroy`, a per-cell + `clay_voxel_get`, and no clone or bulk read; a borrowed grid could only + leave its document through a box walk +- [x] 1.2 The issue's `new VoxelGrid(*g)` copies `change_sink_`, + `pass_capture_` and `recording_`; shown by running the engine test + against `return VoxelGrid(*this)` (five failed assertions) + +## 2. The change + +- [x] 2.1 `VoxelGrid::clone()`: member-wise copy, sink and capture nulled, + recording closed, change count 0, bounds cache cold, every occupied chunk + dirty +- [x] 2.2 `VoxelGrid::occupied_cells(level)` over `material_chunk_keys`, + stored and inherited chunks, sorted z, y, x +- [x] 2.3 `clay_voxel_grid_clone` (owned or borrowed source, owned result) +- [x] 2.4 `clay_voxel_get_occupied` with the size-query pattern and + `CLAY_ERROR_BUFFER_TOO_SMALL` for a short capacity +- [x] 2.5 THREADING notes beside the clone, the bulk read, + `clay_item_volume_from_voxels` and `clay_voxel_to_layer`, naming the + bounds-cache race between two readers +- [x] 2.6 ABI 0.122.0 -> 0.123.0 in `CMakeLists.txt`, `clay.h`, `pyproject.toml` + +## 3. Tests + +- [x] 3.1 `test_c_voxel.cpp` "a clone of a borrowed layer is the whole grid + and none of its document": two levels, non-default active level, a + four-entry palette, an open sculpt layer; per-level counts, palette + colours and cells match; editing the clone leaves the document's grid, + its sculpt-layer record and its undo depth unchanged; destroying the clone + succeeds and destroying the borrow is still refused +- [x] 3.2 `test_c_voxel.cpp` "the bulk read is the box walk, without the box": + sparse grid and a partially refined level, element-for-element equal to a + `clay_voxel_get` box walk and to `clay_voxel_occupied_count`; short buffer + and null arguments +- [x] 3.3 `test_voxel.cpp` engine cases for the clone's channels and the + bulk-read order, inherited cells included +- [x] 3.4 Mutations: naive copy (fails 3.1 on recording and 3.3 on sink, + capture, recording, change count), skipping inherited chunks (fails 3.2 + and 3.3), dropping the sort (fails 3.2 and 3.3) +- [x] 3.5 `tools/check_c_abi.py` FFI-exercises the clone of a borrowed grid and + the bulk read through ctypes + +## 4. Docs + +- [x] 4.1 `docs/05-claycore-library.md` C ABI section, `README.md` Voxel → SDF