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
42 changes: 36 additions & 6 deletions bindings/c/clay.h
Original file line number Diff line number Diff line change
Expand Up @@ -3459,7 +3459,16 @@ typedef struct clay_consolidation_cost {
* The document is not changed, and that includes an INSTANCE layer's sharing:
* clay_layer_consolidate severs a shared edit list before it bakes, and this
* call does not, because it does not bake. Asking what a bake would cost must
* never be the thing that unlinks a subtool. */
* never be the thing that unlinks a subtool.
*
* Refused: CLAY_ERROR_NOT_FOUND for a layer that is not there;
* CLAY_ERROR_UNSUPPORTED for a voxel or a mesh layer, before anything is
* sampled, with clay_last_error naming which (#659) — consolidation bakes an
* SDF edit list and those layers carry none; CLAY_ERROR_INVALID_ARGUMENT for a
* null argument, a malformed params or cost, and for an SDF layer with nothing
* to bake ("nothing to consolidate": empty, unbounded, or a region that holds
* no surface). A host can therefore tell "wrong kind of layer" from "nothing
* there yet" by the code alone. */
clay_result clay_layer_consolidation_cost(const clay_document* doc, clay_layer_id layer,
const clay_consolidation_params* params,
const float region_min[3], const float region_max[3],
Expand Down Expand Up @@ -3575,7 +3584,10 @@ clay_result clay_layer_consolidation_cost(const clay_document* doc, clay_layer_i
* nothing and zeroes, following clay_layer_warp_cost_get's rule that a host
* walking a stack of mixed kinds should not have to special-case them. On a
* protected layer clay_layer_consolidate refuses anyway, so advising a bake it
* would reject is bad advice rather than an error condition.
* would reject is bad advice rather than an error condition. This is the one
* consolidation call that answers a voxel or mesh layer: the ones that bake or
* price a bake refuse it with CLAY_ERROR_UNSUPPORTED (#659), and "should I?" is
* a question with an answer where "do it" is not.
*
* out_cost may be NULL. Added in ABI 0.86.0. */
clay_result clay_layer_consolidation_advice(const clay_document* doc, clay_layer_id layer,
Expand Down Expand Up @@ -3612,6 +3624,14 @@ clay_result clay_layer_consolidation_advice(const clay_document* doc, clay_layer
* Refused on a protected layer, and refused BEFORE the bake, so a locked layer
* does not cost a full resampling to say no.
*
* Refused with CLAY_ERROR_UNSUPPORTED on a voxel or a mesh layer, before the
* protection check and before the params are read, with clay_last_error
* naming the representation (#659). Through 0.123.0 those layers fell
* through to the bake and answered CLAY_ERROR_INVALID_ARGUMENT "nothing to
* consolidate", the answer meant for an EMPTY SDF layer, which a host passing
* the detail on showed an artist as "already optimised". An empty SDF layer
* still answers CLAY_ERROR_INVALID_ARGUMENT, so the two stay distinct.
*
* What survives: the surface, at `cell_size`. What does not: every parameter
* of every item absorbed, and every colour but the first one's. Hidden items
* are left alone — they contribute nothing to the field, so absorbing them
Expand Down Expand Up @@ -3701,16 +3721,24 @@ typedef struct clay_region_merge {
* and band; changed sampling settings can require a larger actual out_merge.
*
* `params.region` is ignored; the closure replaces it. `out_merge` may be NULL.
* Refused, with the document unchanged, on a missing, non-SDF, protected or
* empty layer, and when the region reaches nothing at all. */
* Refused, with the document unchanged: CLAY_ERROR_NOT_FOUND on a missing
* layer; CLAY_ERROR_UNSUPPORTED on a voxel or a mesh layer, naming which in
* clay_last_error (#659); CLAY_ERROR_INVALID_ARGUMENT on a protected or empty
* SDF layer, and when the region reaches nothing at all. */
clay_result clay_layer_consolidate_region(clay_document* doc, clay_layer_id layer,
const float region_min[3], const float region_max[3],
const clay_consolidation_params* params,
clay_consolidation_cost* out_cost,
clay_region_merge* out_merge);

/* What that call WOULD absorb, without baking anything — so a host can show the
* region whose parameters are about to be lost before the artist commits. */
* region whose parameters are about to be lost before the artist commits.
*
* Refused as that call refuses a layer: CLAY_ERROR_NOT_FOUND when it is not
* there and CLAY_ERROR_UNSUPPORTED on a voxel or a mesh layer (#659), rather
* than a plan that absorbs nothing — a preview must not show a merge the
* commit would reject for a different reason. An empty SDF layer plans zero
* absorbed roots and succeeds. */
clay_result clay_layer_plan_region_merge(const clay_document* doc, clay_layer_id layer,
const float region_min[3], const float region_max[3],
clay_region_merge* out_merge);
Expand All @@ -3724,7 +3752,9 @@ clay_result clay_layer_plan_region_merge(const clay_document* doc, clay_layer_id
* is sugar over this one with a null token, so there is one implementation
* rather than two that could drift.
*
* `token` may be NULL, which is exactly the older call. A cancelled consolidate
* `token` may be NULL, which is exactly the older call. It refuses what that
* call refuses, in the same order — CLAY_ERROR_UNSUPPORTED on a voxel or a
* mesh layer among them (#659). A cancelled consolidate
* returns CLAY_ERROR_CANCELLED and leaves the document BYTE-IDENTICAL: the bake
* builds a volume and installs it at the end, so a cancel is a discard rather
* than a partial commit, and a host never has to undo one. */
Expand Down
67 changes: 46 additions & 21 deletions bindings/c/clay_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9621,6 +9621,31 @@ void write_cost(const scene::ConsolidationCost& src, clay_consolidation_cost* ou
write_desc(out, out->struct_size, filled);
}

// The layer a consolidating call acts on, refused in this order: NOT_FOUND
// when it is not there; UNSUPPORTED when it is a voxel or a mesh layer; and,
// for a call that `edits`, INVALID_ARGUMENT when it is protected.
//
// Consolidation bakes an SDF edit list and those two layers carry none. Left
// to the bake, they answered "nothing to consolidate", the answer meant for an
// EMPTY SDF layer (#659), and a host could not tell "wrong kind of layer" from
// "nothing there yet". The representation goes before protection because a
// locked grid is still a grid: "locked" would send the artist to unlock a
// layer that can never be consolidated.
clay_result find_consolidatable(const clay_document* doc, clay_layer_id layer_id, bool edits,
const scene::Layer** out_layer) {
const scene::Layer* layer = doc->doc.document.find_layer(layer_id);
if (!layer) return fail(CLAY_ERROR_NOT_FOUND, "layer not found");
if (layer->kind != scene::LayerKind::Sdf)
return fail(CLAY_ERROR_UNSUPPORTED,
std::string("consolidation applies to SDF layers: layer ") +
std::to_string(layer_id) + " is a " +
(layer->kind == scene::LayerKind::Voxel ? "voxel" : "mesh") + " layer");
if (edits && layer->protected_from_edits())
return fail(CLAY_ERROR_INVALID_ARGUMENT, "layer is protected (ghosted or locked)");
*out_layer = layer;
return CLAY_OK;
}

clay_result read_consolidation(const clay_consolidation_params* params, const float region_min[3],
const float region_max[3], scene::ConsolidationParams* out) {
clay_consolidation_params p;
Expand Down Expand Up @@ -9680,10 +9705,11 @@ clay_result clay_layer_consolidation_cost(const clay_document* doc, clay_layer_i
clay_consolidation_cost* out_cost) {
if (!doc || !params || !out_cost)
return fail(CLAY_ERROR_INVALID_ARGUMENT, "null document, params or cost");
const scene::Layer* layer = doc->doc.document.find_layer(layer_id);
if (!layer) return fail(CLAY_ERROR_NOT_FOUND, "layer not found");
const scene::Layer* layer = nullptr;
clay_result r = find_consolidatable(doc, layer_id, /*edits=*/false, &layer);
if (r != CLAY_OK) return r;
scene::ConsolidationParams p;
clay_result r = read_consolidation(params, region_min, region_max, &p);
r = read_consolidation(params, region_min, region_max, &p);
if (r != CLAY_OK) return r;
r = begin_out_cost(out_cost);
if (r != CLAY_OK) return r;
Expand Down Expand Up @@ -9763,12 +9789,11 @@ clay_result clay_layer_consolidate_cancellable(clay_document* doc, clay_layer_id
clay_consolidation_cost* out_cost,
clay_cancel_token* token) {
if (!doc || !params) return fail(CLAY_ERROR_INVALID_ARGUMENT, "null document or params");
const scene::Layer* layer = doc->doc.document.find_layer(layer_id);
if (!layer) return fail(CLAY_ERROR_NOT_FOUND, "layer not found");
if (layer->protected_from_edits())
return fail(CLAY_ERROR_INVALID_ARGUMENT, "layer is protected (ghosted or locked)");
const scene::Layer* layer = nullptr;
clay_result r = find_consolidatable(doc, layer_id, /*edits=*/true, &layer);
if (r != CLAY_OK) return r;
scene::ConsolidationParams p;
clay_result r = read_consolidation(params, region_min, region_max, &p);
r = read_consolidation(params, region_min, region_max, &p);
if (r != CLAY_OK) return r;
if (out_cost) {
r = begin_out_cost(out_cost);
Expand Down Expand Up @@ -9817,6 +9842,13 @@ clay_result read_region(const float region_min[3], const float region_max[3],
return CLAY_OK;
}

// An out_merge the caller may omit, validated before anything is sampled.
clay_result read_optional_merge(const clay_region_merge* out_merge) {
if (!out_merge) return CLAY_OK;
clay_region_merge probe;
return read_desc(out_merge, kRegionMergeOriginal, &probe);
}

void write_merge(const scene::RegionMerge& plan, clay_region_merge* out) {
const std::uint32_t declared = out->struct_size;
clay_region_merge filled{};
Expand Down Expand Up @@ -9844,8 +9876,8 @@ clay_result clay_layer_plan_region_merge(const clay_document* doc, clay_layer_id
if (r != CLAY_OK) return r;
math::Aabb region;
if ((r = read_region(region_min, region_max, &region)) != CLAY_OK) return r;
const scene::Layer* layer = doc->doc.document.find_layer(layer_id);
if (!layer) return fail(CLAY_ERROR_NOT_FOUND, "layer not found");
const scene::Layer* layer = nullptr;
if ((r = find_consolidatable(doc, layer_id, /*edits=*/false, &layer)) != CLAY_OK) return r;
write_merge(scene::plan_region_merge(*layer, region), out_merge);
return CLAY_OK;
}
Expand All @@ -9859,23 +9891,16 @@ clay_result clay_layer_consolidate_region(clay_document* doc, clay_layer_id laye
math::Aabb region;
clay_result r = read_region(region_min, region_max, &region);
if (r != CLAY_OK) return r;
if (out_merge) {
clay_region_merge probe;
if ((r = read_desc(out_merge, kRegionMergeOriginal, &probe)) != CLAY_OK) return r;
}
const scene::Layer* layer = doc->doc.document.find_layer(layer_id);
if (!layer) return fail(CLAY_ERROR_NOT_FOUND, "layer not found");
if (layer->protected_from_edits())
return fail(CLAY_ERROR_INVALID_ARGUMENT, "layer is protected (ghosted or locked)");
if ((r = read_optional_merge(out_merge)) != CLAY_OK) return r;
const scene::Layer* layer = nullptr;
if ((r = find_consolidatable(doc, layer_id, /*edits=*/true, &layer)) != CLAY_OK) return r;

scene::ConsolidationParams p;
// The region goes to the merge, not through the params: the closure
// replaces params.region, so passing it twice would invite a caller to set
// one and mean the other.
if ((r = read_consolidation(params, nullptr, nullptr, &p)) != CLAY_OK) return r;
if (out_cost) {
if ((r = begin_out_cost(out_cost)) != CLAY_OK) return r;
}
if (out_cost && (r = begin_out_cost(out_cost)) != CLAY_OK) return r;

scene::ConsolidationCost cost;
scene::RegionMerge plan;
Expand Down
13 changes: 13 additions & 0 deletions docs/05-claycore-library.md
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,19 @@ is **not** an error — it succeeds, advises nothing and zeroes, following
`clay_layer_warp_cost_get`'s rule that a host walking a stack of mixed kinds
should not have to special-case them.

The calls that bake or price a bake do **not** follow that rule. Since #659
`clay_layer_consolidation_cost`, `clay_layer_consolidate` (and `_cancellable`),
`clay_layer_plan_region_merge` and `clay_layer_consolidate_region` refuse a
voxel or a mesh layer with `CLAY_ERROR_UNSUPPORTED`, before the protection check
and before anything is sampled, and `clay_last_error` names the representation
("consolidation applies to SDF layers: layer 3 is a voxel layer"). Before, those
layers fell through to the bake and answered `CLAY_ERROR_INVALID_ARGUMENT`
"nothing to consolidate" — the answer for an *empty SDF* layer, which a host
passing the detail through showed an artist as "already optimised". An empty
SDF layer still answers `CLAY_ERROR_INVALID_ARGUMENT`, so the code alone tells
"wrong kind of layer" from "nothing there yet". `clay_layer_consolidation_state`
stays a plain query and answers 0.

**The engine still never bakes on its own**, and that is the settled answer to
the standing question of automatic background consolidation. Consolidation is
destructive; an engine firing it on a background thread would be mutating a
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# refuse-consolidating-a-voxel-or-mesh-layer

Refuse consolidating a voxel or mesh layer as unsupported, by name, instead of answering "nothing to consolidate" (#659)
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## Why
Consolidating a voxel or a mesh layer answered `CLAY_ERROR_INVALID_ARGUMENT` with the detail meant for an EMPTY SDF layer (#659). Probed at v0.120.1 on a freshly added voxel layer:

```
clay_layer_consolidate -> CLAY_ERROR_INVALID_ARGUMENT
"nothing to consolidate: the layer is empty, unbounded, or the region contains no surface"
```

ClaySpaceDesktop showed that detail on `layer/optimize` over a grid layer, and artists read it as "already optimised". The real answer is "this action only applies to SDF layers". A host could not tell the two apart without asking `clay_document_layer_info` first, so every host had to repeat the representation check to give the right reason.

The entry points only checked for a missing or protected layer. `clay_layer_consolidation_cost` and `clay_layer_consolidate_cancellable` (and `clay_layer_consolidate` through it) then baked, and the bake returns false for a layer with no SDF content (`src/scene/consolidate.cpp`). `clay_layer_consolidate_region` failed the same way with "nothing to merge". `clay_layer_plan_region_merge` succeeded with a plan that absorbs nothing, for a merge the commit would then refuse.

## What Changes
- `clay_layer_consolidation_cost`, `clay_layer_consolidate`, `clay_layer_consolidate_cancellable`, `clay_layer_plan_region_merge` and `clay_layer_consolidate_region` refuse a voxel or mesh layer with `CLAY_ERROR_UNSUPPORTED`. The check runs right after the not-found check, before the protection check and before the params are read or anything is sampled. `clay_last_error` names the representation: "consolidation applies to SDF layers: layer N is a voxel layer" (or "a mesh layer"). The wording follows `clay_document_instance_layer`'s refusal.
- An empty SDF layer still answers `CLAY_ERROR_INVALID_ARGUMENT` "nothing to consolidate" (or "nothing to merge"). `clay_layer_plan_region_merge` still plans zero absorbed roots for it and succeeds.
- `clay_layer_consolidation_advice` is unchanged. It still succeeds, advises nothing and zeroes on a voxel or mesh layer. Its header has documented that behaviour since 0.86.0, following `clay_layer_warp_cost_get`'s rule for a stack of mixed kinds, and a test asserts it. "Should I bake this?" has an answer for a grid; "bake this" does not. `clay_layer_consolidation_state` also stays a plain query that answers 0.
- The `clay.h` comments on all five refusing calls, the advice note, and `docs/05` state the new error contract.

## What building it found
- The triage plan also put `clay_layer_consolidation_advice` behind the refusal. That would have broken a documented contract and an existing test ("the advice refuses before it samples, and a mixed stack is an answer"). So it was left alone, and the new test pins its answer for a voxel and a mesh layer.
- Representation goes before protection on purpose. A locked grid would otherwise answer "layer is protected", which sends the artist to unlock a layer that could never be consolidated.

## Impact
This is a behaviour change on an error path only. A call that used to fail with `CLAY_ERROR_INVALID_ARGUMENT` on a voxel or mesh layer now fails with `CLAY_ERROR_UNSUPPORTED`. `clay_layer_plan_region_merge` on such a layer used to succeed with an empty plan and now fails. No signature, struct or enumerator changes, and the ABI version line does not move. No format change.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
## MODIFIED Requirements

### Requirement: Consolidation across the ABI
The C API SHALL expose consolidating a layer and reporting its cost before it is paid, reusing what a volume already reports — bytes, brick count, sample count and sample Lipschitz. The addition SHALL be purely additive.

The cost SHALL also carry what the sample Lipschitz IMPLIES — the declared
Lipschitz and the safe step scale — because those are the numbers a host budgets
a frame against, and deriving them from `sqrt(3) x max(l, 1)` in every binding
would be re-implementing a kernel combinator outside the kernel.

Consolidation applies to SDF layers. The calls that bake, price a bake or plan a region merge — `clay_layer_consolidation_cost`, `clay_layer_consolidate`, `clay_layer_consolidate_cancellable`, `clay_layer_plan_region_merge` and `clay_layer_consolidate_region` — SHALL refuse a voxel or a mesh layer with `CLAY_ERROR_UNSUPPORTED`, and `clay_last_error` SHALL name the representation it found. The refusal SHALL come before the protection check and before anything is sampled, and SHALL leave the document unchanged. An SDF layer with nothing to bake SHALL keep answering `CLAY_ERROR_INVALID_ARGUMENT`, so a host can tell "wrong kind of layer" from "nothing there yet" by the code alone. `clay_layer_consolidation_advice` and `clay_layer_consolidation_state` are queries and SHALL keep answering a voxel or mesh layer: not advised, and not consolidated.

#### Scenario: The cost is knowable before consolidating
- **WHEN** a host asks what consolidating a layer would cost
- **THEN** it gets the memory and resolution it would spend, without the document changing

#### Scenario: The quote is the bill
- **WHEN** a host quotes a consolidation and then performs it with the same parameters
- **THEN** the brick count and byte count it was quoted are the ones it pays

#### Scenario: A voxel or mesh layer is refused as unsupported
- **WHEN** a host asks to consolidate a voxel or a mesh layer, to price that consolidation, or to plan or perform a region merge on it
- **THEN** the call returns `CLAY_ERROR_UNSUPPORTED`, `clay_last_error` names the layer as a voxel or a mesh layer, and the document and its undo history are unchanged

#### Scenario: An empty SDF layer still has nothing to consolidate
- **WHEN** a host asks to consolidate an SDF layer that holds no items
- **THEN** the call returns `CLAY_ERROR_INVALID_ARGUMENT` with "nothing to consolidate", not `CLAY_ERROR_UNSUPPORTED`
Loading
Loading