[RyeMutt]

Cuts the cost of the sculpt pinch test that Phase 5 (#560) added. Every sculpt's index buffer is unchanged, word for word.

Phase 5 has a sculpt side skip the triangles with two corners at one point. It finds them from each point's 24-bit code, then confirms every flagged triangle against its positions. That made small sculpts build 8 to 18% slower at LOD 0 and 1.

What changed

• No confirmation for sculpts built from a map. Each point is one of the map's 8-bit samples, and unit_cube_codes gives it its pixel bytes as its code. So two points share a code exactly where they are one point. This is now tested for every byte value, mirrored or not. The placeholders (the sphere and the empty one) still confirm against positions.
• The kernel reports more. grid_coincident_triangles now returns how many triangles are flagged and marks which rows have a flag. It gathers each row's flag words in registers rather than in memory.
• One work block instead of three.
  • At LOD 0 and 1 it fits in the 1 KB lefts array that createSide keeps for doubled flat inner sides. A sculpt side never uses that array, so createSide's stack frame doesn't grow.
  • Past LOD 1 it takes one scratch block.
• Kept triangles are written in whole units.
  • kept_bands writes a band's rows that have no flag as runs of units, found from the marked rows.
  • kept_columns writes each run of identical columns in a grid of up to seven rows (a sculpt's LOD 0) as one unit.
• Prims are untouched. The functions every prim side runs compile to the same instructions as develop's; I checked the disassembly. These are unit_bands, unit_columns, grid_bands, and grid_indices for KeepAll and Kept.

Not done

• Bands of seven quads for unpinched sculpts. That would win back their 2%, but it changes their index order. See Phase 5's band-of-7 note: a band of seven fills a FIFO-16 cache exactly and thrashes after any row with dropped triangles.
• Finding pinches once per sculpt from the map. A sculpt has only one side, so this saves nothing. It would also have to repeat the sampler's /fp:fast arithmetic to know which pixel each vertex read.

What's left is about 5 to 9% at LOD 0 and 1 for sphere and pinched sculpts, and 3 to 4% for unpinched ones. That is mostly computing the codes and running the kernel over every vertex, which any exact test has to do.

Timings

The table gives cycles for a whole sculpt() call:

• Each figure is an rdtsc median, with each run pinned to one core and no build running.
• Each is the mean of four code layouts, three rounds each.
• "No test" is develop with the pinch test switched off.
• "Of test" is the share of the test's cost that this change removes.

sculpt	LOD	develop	no test	this	change	of test
sphere	0	4786	4202	4556	−4.8%	39%
sphere	1	8586	7664	8370	−2.5%	23%
sphere	2	22577	20873	21960	−2.7%	36%
sphere	3	69354	64470	66665	−3.9%	55%
torus	0	4444	4208	4362	−1.8%	35%
torus	1	7537	7077	7378	−2.1%	35%
torus	2	21266	20034	20830	−2.1%	35%
torus	3	67488	63864	65831	−2.5%	46%
plane	0	4376	4139	4325	−1.2%	22%
plane	1	7478	7021	7362	−1.6%	25%
plane	2	21100	20035	20701	−1.9%	37%
plane	3	67307	63743	65476	−2.7%	51%
cylinder	0	4400	4155	4306	−2.1%	38%
cylinder	1	7487	7044	7358	−1.7%	29%
cylinder	2	21171	19966	20723	−2.1%	37%
cylinder	3	67527	63811	65576	−2.9%	53%
pinched plane	0	4994	4334	4761	−4.7%	35%
pinched plane	1	8705	7423	8115	−6.8%	46%
pinched plane	2	25750	22817	24171	−6.1%	54%
pinched plane	3	90941	81255	84686	−6.9%	65%

How much of this is noise:

• Even after averaging, whole builds move 2 to 3% with code placement. A variant of this change that differed only in KeepAll's band loop measured 33 to 95% "of test".
• Timed in place, the pinch path alone (codes, flags and writing indices) takes 24 to 36% less on the sphere sculpt at every LOD.
• llvolume_bench itself, pinned, eight rounds per binary, reads −4.8% to +6.4% for sculpts and −2.2% to +5.2% for prims. Develop relinked at another layout reads −13.8% to +7.5% for prims against itself, so the prim figures are layout, not code.
• On this machine (a 9950X3D), unpinned runs swing 20 to 40% as Windows moves the process between its two CCDs.

Testing

Windows x64, MSVC, RelWithDebInfo:

• llmath_tests passes: 36/36 in ctest, including the SIMD tier binaries.

• New test. It builds each sculpt side's expected indices from a model of the side order plus the exact position test, and compares them word for word. That is 4,560 sides, covering:

  • every stitching type and flag at every LOD;
  • maps whose samples meet along rows, columns or folds, at single points, in no pattern, or not at all;
  • the placeholders;
  • parameters with a hole or a cut.

• The new test catches defects. It passes on develop's llvolume.cpp and on this one. It fails on each injected defect:

  • placeholders skipping the confirmation;
  • column runs taken as one;
  • marked rows ignored.

  The existing test compares triangle sets, and catches only the last.

• Probe against develop. Over 24,552 volumes and 49,192 faces, every face's indices matched develop's. The probe covered:

  • 16 map sizes;
  • few-colour, stepped, pinched, folded, pinwheel, one-colour and noise maps;
  • eight parameter sets;
  • placeholders and unique volumes.

• llvolume_bench gains a pinched plane's rows.

• Not checked in the viewer. Only llmath changes, and only llmath calls the changed kernel.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0c3dd135-b5d1-4bb4-ba24-0866d336d0b7

Commits

Reviewing files that changed from the base of the PR and between 35ed2ff and e4dab6b.

Files selected for processing: 7

• indra/llmath/alsimdkernels.cpp
• indra/llmath/alsimdkernels.h
• indra/llmath/alsimdkernels.inl
• indra/llmath/llvolume.cpp
• indra/llmath/tests/alsimdkernels_test.cpp
• indra/llmath/tests/llvolume_bench.cpp
• indra/llmath/tests/llvolume_test.cpp

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Improved mesh generation for sculpted shapes with collapsed or pinched regions, preserving valid triangle ordering and avoiding unnecessary degenerate triangles.
  • Updated coincident-triangle results to report the number of flagged triangles and provide row flags.

The grid kernel now returns a count of coincident triangles and a bitset of flagged rows. Sculpt index generation uses pinch metadata to select retained triangles and generate grid indices. Tests cover kernel outputs and sculpt index ordering, and benchmarks include a pinched plane map.

Changes: Sculpt Index Generation

Layer / File(s)	Summary
Kernel triangle and row flags indra/llmath/alsimdkernels.h, indra/llmath/alsimdkernels.cpp, indra/llmath/alsimdkernels.inl, indra/llmath/tests/alsimdkernels_test.cpp	The kernel accepts a row-flag output, sets bits for rows with flagged triangles, and returns the number of flagged triangles. Tests verify the count, row-bitset words, sentinel word, and code behavior across SIMD widths.
Retained-triangle grid writers indra/llmath/llvolume.cpp	Grid index generation uses row flags and per-column triangle masks. It groups retained triangles into column units or alternating bands.
Sculpt pinch detection and validation indra/llmath/llvolume.cpp, indra/llmath/tests/llvolume_test.cpp, indra/llmath/tests/llvolume_bench.cpp	Sculpt index generation detects and confirms pinch flags, shares scratch storage, and retains all triangles when the pinch condition does not select a partial set. Tests check index ordering across sculpt configurations. Benchmarks include a pinched plane map.

Priority: Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor

Sequence Diagram(s)

sequenceDiagram
  participant sculpt_indices
  participant grid_coincident_triangles
  participant Unpinched
  participant grid_columns
  sculpt_indices->>grid_coincident_triangles: vertex codes and grid dimensions
  grid_coincident_triangles-->>sculpt_indices: triangle masks and flagged_rows
  sculpt_indices->>Unpinched: confirmed pinch flags
  Unpinched->>grid_columns: retained-triangle patterns
  grid_columns-->>sculpt_indices: generated indices

Loading

Suggested labels: c/cpp, llmath

Suggested reviewers: taylnos

Merge Risk: Minimal · up to e4dab

The sculpt index-generation change is ready to merge after normal checks; no actionable risk remains from this review.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 6 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the main optimization: trusting sculpt sample codes and writing retained triangles by unit.
Description check		The description clearly explains the motivation, implementation, performance results, testing, scope, and known limitations. It omits the formal issue link and checklist fields, but the required techn…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description clearly explains the motivation, implementation, performance results, testing, scope, and known limitations. It omits the formal issue link and checklist fields, but the required technical context is substantially complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 36 functions across 6 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit checks each bit in rows,
Then counts the triangles as it goes.
Through sculpted grids, the indices run,
Past pinched maps beneath the sun.
The rabbit hops where flags now flow.}

_{Comment @coderabbitai help to get the list of available commands.}