Skip to content

Archive compact-triangle-storage; sync its specs - #188

Merged
doubleailes merged 2 commits into
mainfrom
archive-compact-triangle-storage
Oct 1, 2026
Merged

doubleailes merged 2 commits into
mainfrom
archive-compact-triangle-storage

Conversation

@doubleailes

Copy link
Copy Markdown
Owner

Closes out the compact-triangle-storage change: the last task (8.4) is measured, the delta specs are corrected to what shipped and synced into openspec/specs/, and the change is archived under openspec/changes/archive/2026-10-01-compact-triangle-storage/. Spec and design-record changes only — no code.

8.4: production-scene acceptance

Earlier marked "not run" because the payloads aren't in the checkout; they live in ~/Workspace/samples/. Measured with --stats at the change's head against 616c53b (the commit before it, rebuilt; it reproduces the recorded 212.95 MiB exactly):

scene target before after
DPEL teapot, level 2 — kernel ≤ 115 MiB 212.95 MiB 110.14 MiB (lanes 82.6 %)
DPEL teapot, level 2 — peak RSS — 1 021.7 MiB 744.4 MiB
ALab frame 1004, level 1 — kernel ≤ 11 GiB 19.64 GiB 10.47 GiB (same 81 836 458 triangles)
ALab, level 1 — lanes filled ≥ 80 % — 83.2 %
ALab, level 1 — peak RSS — 49.01 GiB 36.86 GiB

The UV-table target (≤ 20 B/triangle) is met by construction (24 + 16 + 4 → 12 + 4) but not isolated by measurement: the Traverse prims RSS delta also counts refined positions, normals and indices. It shows the importer holding 25.6 B less per refined triangle, consistent with but not proof of the target.

Delta corrections before sync

  • intersection-kernel: dropped the "one pre-sized arena" promise (task 2.4 merges right subtrees into the left's arrays instead); commit transient 285 vs 524 MiB (the 260/390 pair used a mis-derived baseline); indexed layout 1–8 % slower in cache and 30 % out of it (was "13–30 %"); indexed-packet scenario uses current figures (92 vs 192 B per packet, 78.6 vs 95.5 B/triangle); leaf scenario is six triangles, matching the test.
  • textures: restored the "UV texture on a subdivided mesh" scenario the delta dropped, and added one for the remaining gap (instancer prototypes and motion-blurred instances still use the geometric normal).

Also updated

openspec/specs/usd-scene-import/design.md: after-change rows for the teapot and ALab tables; ALab level 1 now leaves ~24 GiB free on a 61 GiB machine (was ~12).

The user docs (site/) already describe CRUST_TRI_PACKETS and CRUST_BVH_PACKET_SAH, and list no instanced-tangent limitation, so they need no change.

Verification

openspec validate --all: 22 passed, 0 failed.

🤖 Generated with Claude Code

Measure 8.4 on the payloads in ~/Workspace/samples/, against 616c53b:
DPEL teapot level 2 kernel 212.95 -> 110.14 MiB (peak 1021.7 -> 744.4
MiB); ALab frame 1004 level 1 kernel 19.64 -> 10.47 GiB, lanes 83.2 %,
peak 49.01 -> 36.86 GiB. The UV tables' share is not separable from
RSS (importer -25.6 B per refined triangle); <= 20 B by construction.

Correct the deltas to what shipped before syncing: no pre-sized build
arena, commit transient 285 against 524 MiB, indexed 1-8 % slower in
cache and 30 % out of it, six-triangle leaf scenario; keep the
subdivided-UV scenario in textures' known gaps.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Archive compact-triangle-storage and sync shipped specifications

📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Record production-scene memory results and close the final compact-triangle-storage acceptance
 task.
• Correct delta specs to match shipped packet, build, and texture behavior.
• Archive the change and sync its requirements into the canonical OpenSpec specs.
Diagram

graph TD
  M["Scene measurements"] --> A["Archived acceptance"] --> D["Corrected deltas"] --> S["Canonical specs"]
  M --> R["Importer design record"]
Loading
High-Level Assessment

Correcting the change deltas before archiving and syncing them preserves an auditable record of what shipped while keeping canonical requirements current. Editing only the canonical specs would lose that alignment.

Files changed (12) +219 / -25

Documentation (12) +219 / -25
design.mdRecord production-scene acceptance results +10/-4

Record production-scene acceptance results

• Replaces unmeasured teapot and ALab acceptance outcomes with kernel, peak-RSS, and lane-fill results. Distinguishes the UV-table target met by construction from what the combined importer RSS measurement can establish.

openspec/changes/archive/2026-10-01-compact-triangle-storage/design.md

proposal.mdPreserve the change proposal in the archive +0/-0

Preserve the change proposal in the archive

• Archives the compact-triangle-storage proposal without content changes in the supplied diff.

openspec/changes/archive/2026-10-01-compact-triangle-storage/proposal.md

spec.mdPreserve the CLI delta specification +0/-0

Preserve the CLI delta specification

• Archives the requirements for geometry-layout statistics and configuration switches without content changes in the supplied diff.

openspec/changes/archive/2026-10-01-compact-triangle-storage/specs/cli/spec.md

spec.mdCorrect shipped kernel metrics and build behavior +11/-9

Correct shipped kernel metrics and build behavior

• Replaces the pre-sized-arena claim with subtree merging and corrects commit-memory, indexed-packet, and throughput figures. Aligns the overlapping-triangle scenario with the six-triangle test.

openspec/changes/archive/2026-10-01-compact-triangle-storage/specs/intersection-kernel/spec.md

spec.mdDocument remaining normal-map gaps and refined UVs +13/-0

Document remaining normal-map gaps and refined UVs

• Adds scenarios distinguishing unsupported instancer-group and motion-blurred normal mapping from UV textures on subdivided meshes.

openspec/changes/archive/2026-10-01-compact-triangle-storage/specs/textures/spec.md

spec.mdPreserve the importer delta specification +0/-0

Preserve the importer delta specification

• Archives the importer requirements for derived mesh tables and direct-instance tangents without content changes in the supplied diff.

openspec/changes/archive/2026-10-01-compact-triangle-storage/specs/usd-scene-import/spec.md

tasks.mdComplete production-scene acceptance task +2/-2

Complete production-scene acceptance task

• Marks task 8.4 complete with teapot and ALab measurements, including the UV RSS caveat. Updates the validation-task record to reflect successful OpenSpec validation.

openspec/changes/archive/2026-10-01-compact-triangle-storage/tasks.md

spec.mdSync geometry-layout reporting and switch requirements +26/-0

Sync geometry-layout reporting and switch requirements

• Adds canonical requirements for per-component kernel-memory statistics, packet lane fill, and the two geometry-layout environment switches.

openspec/specs/cli/spec.md

spec.mdSync compact triangle and BVH requirements +84/-1

Sync compact triangle and BVH requirements

• Documents shared triangle tables, bit-identical packet layouts, packet-aware leaves, and deterministic, lower-memory builds. Includes corrected measurements and acceptance scenarios from the archived delta.

openspec/specs/intersection-kernel/spec.md

spec.mdNarrow the instanced-tangent limitation +15/-7

Narrow the instanced-tangent limitation

• Specifies normal mapping for directly instanced meshes while retaining the geometric-normal fallback for instancer groups and motion-blurred instances. Confirms refined UV charts remain available on subdivided meshes.

openspec/specs/textures/spec.md

design.mdAdd post-change teapot and ALab memory figures +13/-2

Add post-change teapot and ALab memory figures

• Appends measured kernel, RSS, lane-fill, and importer figures to the existing scene baselines. Updates the estimated free memory for ALab level 1 on a 61 GiB machine.

openspec/specs/usd-scene-import/design.md

spec.mdSync derived mesh-table and tangent requirements +45/-0

Sync derived mesh-table and tangent requirements

• Adds requirements for on-demand tangents, compact corner and Ptex data, and directly instanced normal mapping. Records the remaining instancer and motion-blur limitation.

openspec/specs/usd-scene-import/spec.md

@qodo-code-review

qodo-code-review Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. The Ptex memory contract overcounts cells ✓ Resolved
Description
The new importer requirement describes each stored Ptex dyadic cell as eight bytes, whereas
SubFace contains a single packed u32. Its scenario also calls the cell eight bytes beside the
face ID, slice and density, so readers using the canonical spec to budget refined-face storage get
the wrong per-triangle figure.
Code

openspec/specs/usd-scene-import/spec.md[R310-312]

+index per triangle corner. A subdivided mesh's Ptex sub-faces SHALL be kept as one
+8-byte dyadic cell per triangle (origin, depth and the corner at the origin, in the
+base cage face) from which the triangle's corner coordinates are reconstructed
Relevance

●●● Strong

The stated eight-byte cell size conflicts with the implementation's packed four-byte SubFace
representation.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
SubFace stores its origin, depth and rotation in one u32; FaceMap holds a separate vector of
these cells, so the cell itself does not occupy eight bytes.

crates/crust-core/src/rt_world.rs[104-125]
crates/crust-core/src/rt_world.rs[200-207]
openspec/specs/usd-scene-import/spec.md[316-322]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The canonical and archived specifications call the packed Ptex cell eight bytes, but the stored `SubFace` is four bytes.
## Fix Focus Areas
- openspec/specs/usd-scene-import/spec.md[310-312]
- openspec/specs/usd-scene-import/spec.md[316-322]
- openspec/changes/archive/2026-10-01-compact-triangle-storage/specs/usd-scene-import/spec.md[15-27]
## Recommended Fix
State four bytes per stored cell in both requirements and scenarios, distinguishing it from any separately stored face ID, slice or density.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Six triangles cannot fill two packets ✓ Resolved
Description
The overlapping-triangles scenario says six triangles form two full packets, but each packet has
four lanes and unused tail lanes are inactive. The six-triangle test confirms one leaf and two
packets, with only six of their eight lanes occupied.
Code

openspec/specs/intersection-kernel/spec.md[141]

+- **THEN** they form one leaf of two full packets; with it off they form more than one leaf
Relevance

●●● Strong

Six triangles occupy six lanes, so calling both four-lane packets full is factually incorrect.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The packet definition specifies inactive tail lanes for counts not divisible by four, and the
six-triangle test asserts two packets without asserting that both are full.

crates/crust-rt/src/triangle.rs[278-287]
crates/crust-rt/src/bvh/tests.rs[304-309]
crates/crust-rt/src/bvh/tests.rs[335-341]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The six-triangle scenario incorrectly calls both four-lane packets full.
## Fix Focus Areas
- openspec/specs/intersection-kernel/spec.md[137-141]
- openspec/changes/archive/2026-10-01-compact-triangle-storage/specs/intersection-kernel/spec.md[67-72]
## Recommended Fix
Say the triangles form one leaf of two packets, with two inactive lanes in the second packet; keep the archived and canonical scenarios consistent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Refined Ptex maps lack stored corner UVs ✗ Dismissed
Description
The new per-corner table requirement specifies one dyadic cell per refined Ptex triangle instead of
three explicitly stored cage-mapped corner UVs. For subdivided Ptex meshes, remap_subdivided_faces
compresses temporary corner coordinates into SubFace, and FaceMap::resolve reconstructs them at
a hit rather than reading stored corners.
Code

openspec/specs/usd-scene-import/spec.md[R310-312]

+index per triangle corner. A subdivided mesh's Ptex sub-faces SHALL be kept as one
+8-byte dyadic cell per triangle (origin, depth and the corner at the origin, in the
+base cage face) from which the triangle's corner coordinates are reconstructed
Relevance

●●● Strong

The specification conflicts with the repository rule requiring explicit refined Ptex corner UVs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2626599 requires explicit per-corner UVs in the retained refined Ptex face mapping. The added
specification instead mandates a dyadic cell; the importer converts refined corners to that cell,
and hit resolution reconstructs the corners from it.

Rule 2626599: Refined Ptex triangles must store explicit cage-mapped corner UVs and face count checks must use authored cage faces
openspec/specs/usd-scene-import/spec.md[310-312]
crates/crust-core/src/scene/usd_import/mesh.rs[1019-1048]
crates/crust-core/src/rt_world.rs[228-247]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The synced specification requires compact dyadic cells, while the compliance contract requires three explicit cage-mapped corner UVs stored for each refined Ptex triangle.

## Fix Focus Areas
- openspec/specs/usd-scene-import/spec.md[310-312]
- crates/crust-core/src/scene/usd_import/mesh.rs[1019-1048]
- crates/crust-core/src/rt_world.rs[205-258]

## Recommended Fix
Retain the three refined cage-mapped UV corners in the final face map, use them during hit resolution, and update the specification to describe that representation.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (1)
4. UV charts can exceed the memory limit ✓ Resolved
Description
The refined-mesh scenario caps a chart's added resident memory at 20 bytes per triangle, but UvMap
retains UV values in addition to 12 bytes of corner indices and four bytes of density per triangle.
A level-2 chart with more than half a retained UV value per triangle exceeds that cap, so the
scenario does not hold for all UV-textured meshes it covers.
Code

openspec/specs/usd-scene-import/spec.md[R327-329]

+- **THEN** the image is bit-identical to the stored-table render, and the traverse
+  phase's resident memory per refined triangle is at most 20 bytes above the same mesh
+  without a chart
Relevance

●● Moderate

The memory cap is plausible by construction, but the provided evidence does not establish it for
every chart.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The stored chart includes a variable-length vector of eight-byte UV values as well as fixed-size
per-triangle corner and density vectors. Subdivision retains refined face-varying values and their
indices, without imposing a value-to-triangle ratio that would establish the stated bound.

crates/crust-core/src/rt_world.rs[305-326]
crates/crust-core/src/scene/subdiv.rs[333-353]
crates/crust-core/src/scene/usd_import/mesh.rs[1179-1191]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The unconditional 20-byte resident-memory limit excludes the chart's separately stored UV values.
## Fix Focus Areas
- openspec/specs/usd-scene-import/spec.md[324-329]
- openspec/changes/archive/2026-10-01-compact-triangle-storage/specs/usd-scene-import/spec.md[29-34]
## Recommended Fix
Limit the 20-byte claim to per-triangle tables excluding shared UV values, or state a bound that accounts for the chart's value count. Update the archived scenario consistently.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 127 rules
✅ Cross-repo context — repo relationships
Review mode: ⚖️ Balanced: Although code is unchanged, the PR materially revises synchronized behavioral specifications and measured acceptance claims across multiple concerns, requiring careful consistency and contract review.

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread openspec/specs/usd-scene-import/spec.md Outdated
Comment thread openspec/specs/intersection-kernel/spec.md Outdated
Comment thread openspec/specs/usd-scene-import/spec.md Outdated
Comment thread openspec/specs/usd-scene-import/spec.md Outdated
…bound

Address review: the shipped SubFace packs the cell into one u32 beside
the face id `faces` already holds (the 8 B was the design's plan); the
overlapping-triangles leaf is two packets, not two full ones; the chart
scenario states its 16 B per-triangle tables beside the shared values
instead of an RSS bound the values can exceed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@doubleailes
doubleailes merged commit 4bd8fad into main Oct 1, 2026
10 checks passed
@doubleailes
doubleailes deleted the archive-compact-triangle-storage branch October 1, 2026 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant