Skip to content

Stream Ptex a tile at a time behind ptex-rs's cache - #137

Open
doubleailes wants to merge 10 commits into
mainfrom
claude/ptex-rs-tile-integration-wsixuc
Open

doubleailes wants to merge 10 commits into
mainfrom
claude/ptex-rs-tile-integration-wsixuc

Conversation

@doubleailes

Copy link
Copy Markdown
Owner

Integrates the tile-streaming API ptex-rs just gained, and tests it — on fixtures, on a new sample scene, and on the Moana island.

CRUST_PTEX_STREAM=1 swaps PtexColor for a PtexStream that pages one tile of one level of one face out of the .ptx under CRUST_PTEX_CACHE_MB. Preloading stays the default, the oracle, and the fallback.

Why this could not be done before

"Known incomplete work" has been specific for a while about what was missing and where it belonged:

That is a limitation of the reader, not of crust: a .ptx is already a per-face mip pyramid and ptex-rs already addresses it randomly through get_data_at_res, so the fix is a PtexCache equivalent in ptex-rs — exactly as the C++ Ptex library ships one — rather than a second cache in crust-assets.

Upstream now ships exactly that: SharedReader is a &self reader over an LRU of decoded blocks under a byte budget, and tile_layout / tile_info / get_tile address one tile without materialising its face. So crust-assets/src/ptex_stream.rs is a sampler over that reader — level selection, tile addressing, the colour decode, a per-thread microcache — and not a cache.

Unlike the .tx path there is no conversion step. maketx exists because a .png is not tiled or mip-mapped; a .ptx is already both. The missing piece was never a format.

The bump itself is a no-op: no use moved and the suite passed unchanged. The package renamed ptex-rsptex-rust in the same range, which only needed the dependency key to become ptex.

Measured on the Moana island

640x360 / 8 spp, against the same build preloading:

preloaded streamed
Ptex resident 5.98 GiB 0.61 GiB 9.8x less
Ptex decode 84.8 s 14.0 s −71 s
Load assets 01:40.7 27.3 s
Traverse prims RSS 47.34 GiB 41.48 GiB −5.86 GiB
peak RSS 51.28 GiB 47.08 GiB −4.20 GiB
Render 13:57.0 14:06.8 +1.2%
total 20:07.4 18:58.7

Three things worth reading off that. The render cost is +1.2%, not the 2.8x the deliberately texture-bound sample scene shows — the island is traversal-bound, and both numbers are honest. Streaming also made the render start faster, by 71 s of avoided decode. And peak fell by less than residency did because peak lands at Commit acceleration structure, the SBVH build transient; the figure this feature moves is the traverse RSS.

The cache held 3.28 MiB of a 2 GiB budget with zero evictions. That is the mechanism, not a disappointment: at this framing the ray cone asks for coarse levels, and a coarse level of a face is a few texels — streaming reads only the resolution the frame resolves, which is structurally what preloading cannot do. The budget is a ceiling rather than an allocation, so it should not be lowered on that number.

CRUST_PTEX_MAX_LOG2 also stops being load-bearing: unset now means uncapped.

The invariant

At a resolution both backends hold, streaming changes where the texels live and nothing else. samples/ptex_quads.usda at 16 spp with both capped alike: 0 of 57 600 pixels differ. Texel for texel, across four fixtures, every face, ~1 400 sample points each, and caps from 0 to authored.

Where they are supposed to differ is measured rather than asserted away: a streamed mip level comes off disk reduced in the file's encoding while a preloaded one is reduced in linear light, so convexity makes the streamed chain the darker — by up to 0.147 on the tiled fixture, and never brighter.

Three problems the island surfaced, all fixed here

Each was invisible on the sample scene and is now pinned by a test.

  1. The budget was per file. SharedReader owns its cache, so N textures each held the full budget. The island binds 3 618 .ptx, so the default 1 GiB would have become tens of GiB — a residency feature unbounded in the texture count. One budget is now divided across the streamed readers.

  2. An even split was still wrong. The island's Ptex is Pareto-distributed: 167 textures hold 97% of the bytes and the median is under a kilobyte. Splitting evenly gives the four textures holding half the bytes 0.3 MiB each — below one face, so nothing caches — and flooring the share instead multiplies to 14.1 GiB. So admission is per texture and priced against the alternative: preload_bytes costs what preloading would from the header alone (no pixel I/O), and anything under CRUST_PTEX_STREAM_MIN_MB (8 MiB) is preloaded. On the island that is 39 streamed, 3 579 preloaded.

  3. --stats was silent about Ptex, so an island run could not say which backend produced it — and the first version of the block then called those 3 579 policy decisions a fallback, which reads as 3 579 errors. Both fixed: the block reports for either backend and names "preloaded under the size threshold" apart from "PRELOADED BECAUSE STREAMING FAILED".

Also corrects a long-standing figure: the island's Ptex at the default cap is 5.98 GiB, not 4.58 GiB — that number is the base without the mip pyramid.

What's in the diff

18 files, +2 422 / −52.

  • crust-assets/src/ptex_stream.rs — the streaming backend
  • crust-assets/tests/ptex_stream.rs — 13 integration tests (the invariant, tile-seam taps, concurrency, the budget, admission)
  • samples/ptex_quads.usda + four .ptx under samples/textures/ — the repository's first scene binding a .ptx, so check_images.sh now covers Ptex by its existing glob. Fixtures are the reference C++ Ptex writer's, copied from ptex-rs (MIT).
  • crust-core/src/stats.rs — the Ptex report block
  • docs/ptex_streaming.md — measurements and reasoning, plus the gaps

cargo fmt, clippy -D warnings and all 29 test targets pass.

Worth knowing before merging

  • Streaming stays opt-in. It is measured on one asset at one framing; the coarse-mip divergence is real if bounded, and preloading remains the oracle.
  • A crust:openpbr material cannot bind Ptex at all, streamed or not — inputs:surfaceMap is read only for UsdPreviewSurface and PxrDisneyBsdf. Unrelated to this change, found while writing the sample scene, documented and left for its own PR.
  • The toolchain needed bumping to 1.98 for openusd 0.7's MSRV; that was pre-existing in the environment, not caused here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf


Generated by Claude Code

Upstream gained a tile-level read path and a thread-safe reader over it:
`tile_layout` / `tile_info` / `get_tile` describe and fetch one tile of one
face at one resolution, the mipmap helpers (`face_num_levels`,
`res_for_level`, `is_res_stored`) answer which resolutions a face holds
without any I/O, and `SharedReader` wraps all of it behind `&self` with an
LRU of decoded pixels under a byte budget.

That last one is the thing "Known incomplete work" said streaming Ptex was
waiting on, and it deliberately lives upstream rather than in crust-assets:
a `.ptx` is already a per-face mip pyramid addressed randomly on disk, so
the cache belongs next to the reader that addresses it, exactly as the C++
Ptex library ships `PtexCache`.

Nothing in crust changes with the bump — no `use` moved and the whole suite
passes — because the rename in the same range (`ptex-rs` -> `ptex-rust`)
touched the package and not the lib. Cargo names a renamed dependency by its
key rather than by its lib target, so the key becomes `ptex`, which is both
the lib name and what every `use` in the workspace already said.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
`CRUST_PTEX_STREAM=1` swaps the fully-decoded `PtexColor` for a `PtexStream`
that pages one tile of one level of one face out of the `.ptx` under a byte
budget (`CRUST_PTEX_CACHE_MB`, default 1024 to match `CRUST_TEX_CACHE_MB` and
so OIIO's). Preloading stays the default and the oracle, and any file that
declines to stream falls back to it — turning residency on can make a render
slower, never break one.

This is the residency half of the Ptex problem, and it is what retires
`CRUST_PTEX_MAX_LOG2` as a *requirement*. That cap was never a tuning knob:
the island's 2 576 238 faces are 4.58 GiB at 32x32 and 494 GiB at full
resolution, so the cap is the thing that makes the island loadable, at the
price of discarding authored detail permanently. A cache scales with itself
instead. The cap still applies when explicitly set — that is what makes the
two backends comparable at a resolution both hold — and streaming is
otherwise uncapped, which is the point of it.

The cache is upstream's, deliberately. `SharedReader` is a `&self` reader over
an LRU of decoded blocks, which is the `PtexCache` equivalent "Known
incomplete work" named as the fix, and it belongs next to the reader that
already addresses the on-disk pyramid rather than beside the `.tx` cache,
whose keys and layout answer a different question. So this module is a
sampler over that reader: level selection, tile addressing, the colour
decode, and a per-thread microcache.

Four microcache slots, not the two `tiled::cache` keeps, and the difference is
measured rather than assumed. A `.tx` grids once over the texture so a tap
straddling a seam is rare; a `.ptx` grids per face, and the faces that get
tiled are the large ones a streamed render lives in. A lookup on a four-tile
corner needs four tiles for its four taps, and with two slots each tap evicts
one the same lookup is about to want: that case hit 0.000 of 1600 fetches
against 0.999 for a tap inside a tile. Four slots take the corner to 0.998
and leave the interior at 0.999.

The invariant, measured end to end on the new sample scene at 16 spp with
both backends capped alike: 0 of 57 600 pixels differ. Unit-level, the same
equality holds texel for texel across four fixtures, every face, and caps
from 0 to authored — including the capped-reduction arm, where a "tile" is
the whole face. The `u8` decode table is pinned bit-for-bit against the
scalar decode so that comparison can be an equality rather than a tolerance.

Where the two are *supposed* to differ is the mip chain, and that is measured
too rather than asserted away: a preloaded pyramid is reduced in linear light,
a streamed one comes off disk reduced in the file's encoding. Convexity makes
the streamed chain the darker one, by up to 0.147 on the tiled fixture and
brighter by exactly 0.

Cost, interleaved min-of-7 on the worst case this scene is built to be (two
textured planes filling frame at depth 3, so nearly every shading call is a
fetch): 0.123s preloaded against 0.342s streamed, with peak RSS 18.36 MiB
against 10.27 MiB at a 4 MiB budget — and that 4 MiB render is bit-identical
to the 1 GiB one, so the budget moves residency and not the image.

`samples/ptex_quads.usda` is the repository's first scene to bind a `.ptx` at
all, so `check_images.sh` now covers Ptex by its `samples/*.usda` glob. The
four fixtures under `samples/textures/` are the reference C++ writer's, kept
in one place so the unit invariant and the rendered one cannot be checked
against different bytes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
`docs/ptex_streaming.md` is the measurements and the reasoning: the invariant
and how it is checked, the mip-chain divergence and its direction, the
four-slot microcache table, the cost, and the gaps. CLAUDE.md gets the
switches, the A/B recipe, and — more to the point — its "Ptex does not
stream" caveat rewritten, since that paragraph asked for exactly the upstream
`PtexCache` equivalent this now consumes.

One finding from building the sample scene is recorded in both, because it is
a trap rather than a limitation of this change: a `crust:openpbr` material
cannot bind Ptex at all. `inputs:surfaceMap` is consulted only for
`UsdPreviewSurface` and `PxrDisneyBsdf`, so a Ptex material authored the
native way renders on its constant `baseColor` — indistinguishable from
`CRUST_PTEX=0`, and nothing warns. Unrelated to residency and left for its
own change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
Two gaps a Moana island run on this branch exposed, neither visible on the
sample scene.

**The budget was per file.** `ptex::SharedReader` owns its cache, which is
right for a library — a `.ptx` is a self-contained pyramid — but it means N
textures opened at `CRUST_PTEX_CACHE_MB` each hold N times it. The `.tx` path
never had this: every streaming texture there shares one `TileCache`, so the
total is the budget by construction. On a stage that binds Ptex per element,
which the island does across its 20, the default 1 GiB would have become tens
of GiB — the feature whose whole purpose is to bound residency, unbounded in
the texture count. `FileAssets` now divides one budget over the streamed
textures as they arrive, floored at 4 MiB a share because a zero budget in
`CacheOptions` disables caching outright and overshooting the total beats
silently turning the cache off. An even split rather than OIIO's demand-driven
pool: that needs a second cache here, which is the design "Known incomplete
work" ruled out, and the property that matters holds either way. The test
asserts the naive total *does* multiply first, so the sharing is testing
something.

**`--stats` was silent about Ptex.** It reported the `.tx` tile cache and
nothing else, so a render could not say which Ptex backend produced it. That
matters precisely on the island, where peak RSS is dominated by geometry and
the SBVH build transient and the Ptex residency is buried inside a much larger
number — the one figure a streamed run and a preloaded one differ by is the
one the report did not print. The new block reports for *both* backends:
`backend` (including `N streamed, M preloaded (fell back)`, which is not a bug
but is when you want to be told), textures and faces, preloaded resident, and
for a streamed run the live resident against the shared budget, the three
fetch tiers and evictions — with the same "raise the budget" hint the `.tx`
block earns when evictions run with the misses.

The render invariant is unchanged: 0 differing pixels at a shared cap.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
A full island render's log settles what the sample scene could not: it binds
**3 618** `.ptx` totalling **5.98 GiB** preloaded, and they are
Pareto-distributed. 167 of them hold 97% of the bytes, the top 25 hold 88%,
the top two (`trunk0001`, referenced twice at 1.29 GiB each) hold 42%, and the
median texture is under a kilobyte.

That breaks the even split this branch shipped one commit ago. Dividing one
budget over 3 618 readers hands the four textures holding half the bytes a
0.3 MiB cache each — smaller than one of their faces, so every read comes back
`oversized` and nothing caches at all — while 3 451 sub-kilobyte files each sit
on a slot they can never fill. And the 4 MiB floor meant to stop that
multiplies out to **14.1 GiB**, worse than the preload it replaces. The fix
was right about where the budget belongs and wrong about how to divide it.

Admission is therefore per texture and priced against the alternative: a
texture smaller than the cache slot it would occupy should just be preloaded.
`PtexStream::preload_bytes` answers what preloading would cost from the parsed
header alone — `face_infos()` carries every face's resolution and no pixel data
— so the test is exact and free rather than a heuristic. Below
`DEFAULT_STREAM_MIN_MB` (8 MiB, `CRUST_PTEX_STREAM_MIN_MB`), preload.

On the island that admits ~39 readers at ~26 MiB each, a real working set, and
preloads 0.54 GiB of small ones: **~1.5 GiB against 5.98 GiB**. A lower
threshold wins on paper (1 MiB gives 1.18 GiB) but starves each reader, and a
total is worthless if nothing caches. `MIN_PTEX_SHARE` drops to 1 MiB and is
now only a backstop — admission is what keeps the count small, and raising the
floor treats the symptom.

One consequence worth knowing, and now documented in the scene itself: the
sample fixtures are kilobytes, so `CRUST_PTEX_STREAM=1` alone preloads them and
the A/B would compare nothing. The recipe takes `CRUST_PTEX_STREAM_MIN_MB=0`,
and `--stats` names the backend either way. With it, the invariant is unchanged
— 0 of 57 600 pixels differ at a shared cap.

The island bucket table in the test is derived from the 3 618 real rows, so it
reproduces the asset's count and total exactly rather than modelling a guess.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
A streamed island render says the backend line was wrong: it read "39
streamed, 3 579 preloaded (fell back)", and those 3 579 did not fall back —
they were declined by the size threshold, working exactly as designed. On a
production stage the mixed report is the *normal* case, so calling it a
fallback reads as 3 579 errors. The two reasons are now counted and named
apart: "preloaded under the size threshold" says the admission rule worked,
"PRELOADED BECAUSE STREAMING FAILED" says a file is broken. Streaming being
off is a third case, distinguished because a user who turned it on and got a
bare "preloaded" needs to be told why.

The run itself is the measurement every claim on this branch was missing —
everything until now was fixtures and a synthetic scene. At 640x360 / 8 spp
against the same build preloading:

  Ptex resident      5.98 GiB -> 0.61 GiB   (9.8x)
  Ptex decode        84.8 s   -> 14.0 s     (Load assets 01:40.7 -> 27.3 s)
  Traverse RSS       47.34    -> 41.48 GiB  (-5.86)
  peak RSS           51.28    -> 47.08 GiB  (-4.20)
  Render             13:57.0  -> 14:06.8    (+1.2%)
  total              20:07.4  -> 18:58.7

Three things worth keeping. The render cost is **+1.2%**, not the 2.8x the
sample scene shows — that scene is built to be the worst case and the island
is traversal-bound; both are honest and the gap is the point. Streaming also
made the render *start* faster, by 71 s of avoided decode, which is why the
whole run finished sooner despite rendering slightly slower. And peak fell by
less than residency did because peak lands at the SBVH build transient, not at
texture load — the figure this feature moves is the traverse RSS.

The cache held **3.28 MiB of a 2 GiB budget with zero evictions**. That is the
mechanism, not a disappointment: the ray cone asks for a coarse level of each
face at this framing, and a coarse level is a few texels, so streaming reads
only the resolution the frame resolves — which is structurally what preloading
cannot do. The budget is a ceiling rather than an allocation, so it is not to
be lowered on that number; `evictions` running with the misses is the signal
that it is too small, and it is zero.

Also corrects the long-standing 4.58 GiB figure for the island's Ptex: that is
the 32x32 base without the mip pyramid, and what a run actually holds is 5.98
GiB.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Stream Ptex tiles through a bounded reader cache

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Adds opt-in, uncapped Ptex tile streaming under a render-wide cache budget.
• Preserves preloading as default, size-based fallback, and bit-identical base-level oracle.
• Adds Ptex statistics, reference fixtures, integration coverage, sample scene, and performance
 guidance.
Diagram

sequenceDiagram
    actor Renderer
    participant Assets as FileAssets
    participant Stream as PtexStream
    participant Reader as SharedReader
    participant Cache as Reader LRU
    participant File as PTX File
    Renderer->>Assets: Load Ptex
    alt Streaming admitted
        Assets->>Stream: Open and rebudget
    else Disabled, small, or failed
        Assets-->>Renderer: Preloaded texture
    end
    Renderer->>Stream: Evaluate face sample
    alt Thread microcache hit
        Stream-->>Renderer: Filtered color
    else Microcache miss
        Stream->>Reader: Request face tile
        Reader->>Cache: Lookup decoded block
        alt Reader cache miss
            Cache->>File: Read compressed tile
            File-->>Cache: Tile bytes
        end
        Cache-->>Stream: Decoded pixels
        Stream-->>Renderer: Filtered color
    end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Upstream cross-texture shared cache
  • ➕ Allocates capacity dynamically to whichever Ptex textures are hot.
  • ➕ Enforces one render-wide budget without equal per-reader partitioning.
  • ➕ Avoids admission thresholds becoming the primary defense against cache fragmentation.
  • ➖ Requires a new ptex-rust API or cache ownership model.
  • ➖ Couples otherwise independent readers through shared cache keys and lifecycle management.
  • ➖ Cannot be implemented locally without duplicating cache responsibilities already owned upstream.
2. Crust-owned Ptex tile cache
  • ➕ Provides complete control over global eviction and demand-driven allocation.
  • ➕ Could normalize streamed mip generation into linear color space.
  • ➖ Duplicates ptex-rust’s decoded-block cache and format knowledge.
  • ➖ Introduces separate cache keys, synchronization, and memory accounting.
  • ➖ Conflicts with the established boundary that Ptex caching belongs beside the reader.

Recommendation: Use the PR’s reader-owned cache with size-based admission and per-reader rebudgeting now. It preserves the correct ownership boundary and bounds production residency without another cache; a future upstream cross-texture cache would be the strongest evolution if equal partitioning becomes limiting.

Files changed (18) +2422 / -52

Enhancement (5) +1072 / -3
lib.rsIntegrate streamed Ptex loading, admission, and budgeting +215/-1

Integrate streamed Ptex loading, admission, and budgeting

• Selects 'PtexStream' when enabled, preloads small or unsupported files, and divides one cache budget across admitted readers. Tracks both backends for residency, fallback, and cache statistics.

crates/crust-assets/src/lib.rs

ptex_stream.rsImplement tile-streamed Ptex sampling +677/-0

Implement tile-streamed Ptex sampling

• Adds mip selection, tile addressing, bilinear and trilinear filtering, color decoding, and a four-slot per-thread microcache over 'ptex::SharedReader'. Supports runtime budgets, optional resolution caps, preload-cost estimation, resilient fallback values, and cache counters.

crates/crust-assets/src/ptex_stream.rs

lib.rsExport Ptex cache statistics +2/-2

Export Ptex cache statistics

• Re-exports 'PtexCacheStats' for use by asset loaders and renderer hosts.

crates/crust-core/src/lib.rs

stats.rsReport Ptex backend and cache activity +177/-0

Report Ptex backend and cache activity

• Adds Ptex residency and lookup counters to 'RenderStats'. Formats preloaded, streamed, and mixed-backend reports with failure reasons, budgets, hit tiers, disk reads, and eviction guidance.

crates/crust-core/src/stats.rs

main.rsSnapshot Ptex statistics after rendering +1/-0

Snapshot Ptex statistics after rendering

• Copies Ptex backend and cache counters from 'FileAssets' into the final render report.

crates/crust-render/src/main.rs

Refactor (1) +54 / -22
ptex_texture.rsShare Ptex mip policy with the streaming backend +54/-22

Share Ptex mip policy with the streaming backend

• Exposes internal mip helpers, parameterizes preloaded opening by resolution cap, and distinguishes an absent cap from the preload default. This lets streamed and preloaded textures be compared at identical resolutions.

crates/crust-assets/src/ptex_texture.rs

Tests (6) +791 / -0
ptex_stream.rsValidate streamed Ptex correctness and residency +632/-0

Validate streamed Ptex correctness and residency

• Adds fixture-backed coverage for exact base-level agreement, reductions, tile seams, microcache isolation, concurrency, budget enforcement, admission policy, fallback behavior, and documented mip divergence.

crates/crust-assets/tests/ptex_stream.rs

ptex_quads.usdaAdd an end-to-end Ptex streaming sample scene +159/-0

Add an end-to-end Ptex streaming sample scene

• Creates Ptex-textured quad panels exercising face mapping, tiled sampling, mip selection, and render-level comparison between preloaded and streamed backends.

samples/ptex_quads.usda

quad_f32.ptxAdd float32 quad Ptex fixture +0/-0

Add float32 quad Ptex fixture

• Adds a reference-writer Ptex file covering three-channel floating-point decoding and multiple quad faces.

samples/textures/quad_f32.ptx

quad_tiled.ptxAdd genuinely tiled quad Ptex fixture +0/-0

Add genuinely tiled quad Ptex fixture

• Adds a non-square, single-channel Ptex file with a large tiled face and stored mip levels for tile-boundary and residency tests.

samples/textures/quad_tiled.ptx

quad_u8.ptxAdd multichannel uint8 quad Ptex fixture +0/-0

Add multichannel uint8 quad Ptex fixture

• Adds a four-channel quad texture covering standard byte decoding, multiple faces, and ignored alpha data.

samples/textures/quad_u8.ptx

tri_u16.ptxAdd uint16 triangle Ptex fixture +0/-0

Add uint16 triangle Ptex fixture

• Adds a triangle-mesh texture covering symmetric reductions, triangle parameterization, and non-table integer decoding.

samples/textures/tri_u16.ptx

Documentation (3) +484 / -18
CLAUDE.mdDocument Ptex streaming operations and production measurements +103/-18

Document Ptex streaming operations and production measurements

• Adds commands, architecture notes, cache-budget semantics, statistics guidance, Moana results, and remaining Ptex limitations. Replaces the prior statement that Ptex streaming was unavailable.

CLAUDE.md

ptex_streaming.mdAdd comprehensive Ptex streaming guide +352/-0

Add comprehensive Ptex streaming guide

• Documents backend selection, correctness guarantees, mip-space divergence, cache architecture, admission policy, statistics, benchmarks, operational commands, and known gaps.

docs/ptex_streaming.md

ptex_fixtures.mdDocument reference Ptex fixtures +29/-0

Document reference Ptex fixtures

• Records fixture provenance, formats, mesh types, channel layouts, and the scenarios each file covers.

samples/textures/ptex_fixtures.md

Other (3) +21 / -9
Cargo.tomlUpgrade ptex-rust for tile and cache APIs +19/-7

Upgrade ptex-rust for tile and cache APIs

• Renames the workspace dependency key to 'ptex', pins the upstream revision exposing tile-level reads and 'SharedReader', and explicitly enables its cache feature.

Cargo.toml

Cargo.tomlAdopt the renamed workspace Ptex dependency +1/-1

Adopt the renamed workspace Ptex dependency

• Switches crust-assets from the former 'ptex-rs' dependency key to 'ptex'.

crates/crust-assets/Cargo.toml

Cargo.tomlAdopt the renamed Ptex development dependency +1/-1

Adopt the renamed Ptex development dependency

• Updates crust-render’s development dependency to the new workspace key.

crates/crust-render/Cargo.toml

@qodo-code-review

qodo-code-review Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

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

Grey Divider


Action required

1. Configured cache limits can be exceeded ✓ Resolved 🐞 Bug ☼ Reliability
Description
rebudget_ptex applies MIN_PTEX_SHARE independently to each reader after dividing the render-wide
budget, so separate reader caches can collectively exceed CRUST_PTEX_CACHE_MB. This occurs
whenever the streamed texture count exceeds the configured budget in MiB—for example, two streams
under a 1 MiB budget—and is directly reachable with the documented zero admission threshold on
larger stages or smaller budgets.
Code

crates/crust-assets/src/lib.rs[R235-237]

+        let share = (ptex_stream::cache_budget_from_env() / streams.len()).max(MIN_PTEX_SHARE);
+        for s in &streams {
+            s.set_budget(share);
Relevance

●●● Strong

Concrete aggregate budget violation from minimum per-reader allocations; directly contradicts
documented render-wide cache limit.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The rebudgeting loop assigns every streamed reader at least MIN_PTEX_SHARE, and each PtexStream
owns a separate upstream cache, so the independently assigned capacities add together and multiply
the aggregate ceiling. The implementation acknowledges this overshoot, while the admission
configuration permits every texture to stream and the documentation states that the requested budget
remains the total, demonstrating that admitted stream counts can violate the promised render-wide
bound.

crates/crust-assets/src/lib.rs[126-137]
crates/crust-assets/src/lib.rs[218-238]
crates/crust-assets/src/ptex_stream.rs[98-118]
docs/ptex_streaming.md[131-153]
crates/crust-assets/src/ptex_stream.rs[344-353]
docs/ptex_streaming.md[145-153]

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

## Issue description
`CRUST_PTEX_CACHE_MB` is presented as a render-wide cache budget, but `rebudget_ptex` assigns every streamed reader at least one MiB. Whenever the admitted stream count exceeds the configured budget in MiB, the aggregate assigned cache capacity exceeds the setting, defeating bounded residency for small budgets or low admission thresholds.

## Fix Focus Areas
- crates/crust-assets/src/lib.rs[126-137]
- crates/crust-assets/src/lib.rs[218-238]
- crates/crust-assets/src/ptex_stream.rs[98-118]

## Recommended Fix
Remove the unconditional per-reader one-MiB floor and distribute the byte budget so the sum of all reader budgets remains at or below the configured total. Allow sub-MiB shares where supported; otherwise constrain the number of admitted streams and decline additional streams or preload them when a useful positive share cannot be provided. Add coverage for multiple admitted streams when the total budget in MiB is lower than the stream count.

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


2. Oversized tiles bypass memory budget ✓ Resolved 🔗 Cross-repo conflict ☼ Reliability
Description
PtexStream::with_tile unconditionally stores each returned owning PixelData handle in four
process-lifetime thread-local microcache slots, whose allocations are neither released by reader
eviction, rebudgeting, or stream removal nor included in SharedReader::bytes_resident. When a tile
exceeds the reader allowance, is evicted, or the budget shrinks, every live render worker can retain
up to four decoded allocations outside CRUST_PTEX_CACHE_MB, including potentially large whole-face
payloads for untiled resolutions.
Code

crates/crust-assets/src/ptex_stream.rs[R479-480]

+            slots.rotate_right(1);
+            slots[0] = Some((id, data));
Relevance

●● Moderate

Untracked thread-local ownership can undermine cache limits, but impact depends on upstream
allocation and worker-lifetime semantics.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The cited code holds four owned PixelData values in thread-local storage, and ptex-rs defines each
value as an owning Arc handle, so retaining a handle extends the underlying allocation's lifetime.
Rebudgeting and residency statistics apply only to SharedReader, while ptex-rs returns oversized
values without inserting them into its accounted LRU; the repository also shows that some untiled
resolutions are returned as a single whole-face tile, demonstrating that this untracked retention
can be substantially larger than ordinary tile metadata.

crates/crust-assets/src/ptex_stream.rs[451-480]
crates/crust-assets/src/ptex_stream.rs[196-205]
crates/crust-assets/src/ptex_stream.rs[344-353]
crates/crust-assets/src/ptex_stream.rs[395-399]
crates/crust-assets/src/ptex_stream.rs[451-481]
crates/crust-assets/tests/ptex_stream.rs[113-120]
External repo: doubleailes/ptex-rs, src/cache.rs [77-106]
External repo: doubleailes/ptex-rs, src/cache.rs [171-190]

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 thread-local microcache retains decoded `ptex::PixelData` allocations after ptex-rs rejects them as oversized or evicts them from its byte-budgeted reader cache. Reader eviction, rebudgeting, and dropping the corresponding stream therefore do not necessarily release these handles, allowing actual Ptex residency to exceed both `CRUST_PTEX_CACHE_MB` and reported reader statistics.

## Fix Focus Areas
- crates/crust-assets/src/ptex_stream.rs[196-205]
- crates/crust-assets/src/ptex_stream.rs[395-399]
- crates/crust-assets/src/ptex_stream.rs[451-481]

## Recommended Fix
Include thread-local tile retention in the residency policy and reporting rather than retaining unaccounted `PixelData` handles. At minimum, do not retain oversized payloads and clear or replace entries when budgets shrink; preferably replace the handle cache with a bounded representation whose allowance is reserved from the render-wide configured total and exposed in statistics, or coordinate with ptex-rs on an API that accounts for live handles until their final clone is dropped. Add coverage using tiles larger than the reader budget and assert that total retained tile storage remains bounded.

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



Remediation recommended

3. Distant streamed textures render darker ✓ Resolved 📘 Rule violation ➹ Performance
Description
PtexStream::eval requests coarse levels directly from SharedReader and applies the gamma decode
after those levels were reduced in the file's encoded colour space, instead of deriving every mip
from a decoded linear base. When minification selects a non-base level, the streamed backend
diverges from the preloaded reference across every material using that texture, with the added
documentation measuring darkening of up to 0.1474.
Code

crates/crust-assets/src/ptex_stream.rs[R471-474]

+        let data = self
+            .reader
+            .get_tile(id.face as usize, res, id.tile as usize)
+            .ok()?;
Relevance

●●● Strong

Streaming coarse levels from encoded files diverges from preloaded linear mip behavior; closely
matches accepted mip correctness feedback.

PR-#135

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2507525 requires Ptex mip levels to be generated in memory from the decoded linear
base without additional file reads. The new stream reads the requested resolution through
get_tile, decodes afterward, and its documentation explicitly states that streamed coarse levels
come from the file's encoding and are darker than preloaded levels.

Rule 2507525: Preload Ptex face data into a single immutable linear buffer with in-memory linear mipmap generation
crates/crust-assets/src/ptex_stream.rs[471-474]
crates/crust-assets/src/ptex_stream.rs[543-551]
docs/ptex_streaming.md[83-101]

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 streamed Ptex path reads previously reduced, display-encoded mip levels from disk and decodes them afterward. This produces darker minified textures and bypasses the required immutable linear mip-pyramid construction.

## Fix Focus Areas
- crates/crust-assets/src/ptex_stream.rs[471-474]
- crates/crust-assets/src/ptex_stream.rs[543-551]
- crates/crust-assets/src/ptex_stream.rs[593-605]

## Recommended Fix
Generate lower Ptex levels from decoded linear base texels, using triangle-aware reduction for triangle faces, and sample those generated levels from immutable storage. Until the streaming cache can preserve this behavior, route mipmapped Ptex textures through the existing compliant preloaded implementation.

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


4. Large cache settings can crash asset loading 🐞 Bug ☼ Reliability
Description
cache_budget_from_env and stream_min_bytes_from_env accept any positive usize and multiply it
by 1,048,576 without checked arithmetic. A sufficiently large accepted environment value overflows
during configuration, panicking in checked builds or wrapping to an unintended cache budget or
admission threshold in release builds.
Code

crates/crust-assets/src/ptex_stream.rs[R61-63]

+        Err(_) => DEFAULT_CACHE_MB,
+    };
+    mb * 1024 * 1024
Relevance

●●● Strong

Unchecked environment-value arithmetic is a deterministic overflow bug requiring straightforward
checked conversion or bounds.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Both added configuration helpers parse arbitrary usize values and immediately multiply by the MiB
conversion factor, with neither a maximum bound nor checked multiplication. Thus values can pass the
stated positive-integer validation while still failing conversion to bytes.

crates/crust-assets/src/ptex_stream.rs[50-64]
crates/crust-assets/src/ptex_stream.rs[104-119]

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 new Ptex cache and stream-threshold environment parsers validate only that the value is a positive integer, then perform unchecked conversion from MiB to bytes. Values that parse as `usize` but exceed the representable byte count can overflow, turning accepted configuration into a panic or an unintended small value.

## Fix Focus Areas
- crates/crust-assets/src/ptex_stream.rs[50-64]
- crates/crust-assets/src/ptex_stream.rs[104-119]

## Recommended Fix
Use `checked_mul(1024 * 1024)` after parsing in both helpers. On overflow, log the same validation warning style used for malformed values and return the corresponding safe default; add tests for oversized numeric input.

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


5. Small textures stream when mips are off 🐞 Bug ≡ Correctness
Description
PtexStream::preload_bytes always sums every halved mip level even when its mip field is false.
With CRUST_PTEX_MIP=0, PtexColor allocates only level zero, so the inflated estimate can admit a
texture to streaming although its actual preload size is below CRUST_PTEX_STREAM_MIN_MB.
Code

crates/crust-assets/src/ptex_stream.rs[R383-390]

+            loop {
+                floats += w * h * 3;
+                if w == 1 && h == 1 {
+                    break;
+                }
+                w = (w / 2).max(1);
+                h = (h / 2).max(1);
+            }
Relevance

●●● Strong

Admission sizing must match mip configuration; accepted mip-generation correctness feedback shows
this team prioritizes such consistency.

PR-#135

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The estimate walks until both dimensions reach one without considering the stream's mip setting. The
preload backend explicitly sets its face level count to one when mipmapping is disabled, and
load_ptex uses this estimate as its streaming admission decision.

crates/crust-assets/src/ptex_stream.rs[373-392]
crates/crust-assets/src/ptex_texture.rs[182-193]
crates/crust-assets/src/lib.rs[452-455]

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

## Issue description
Ptex streaming admission compares `preload_bytes` with the stream threshold, but `preload_bytes` always includes a complete mip pyramid. When `CRUST_PTEX_MIP=0`, the fallback `PtexColor` contains only its base level, so admission is comparing the threshold to a larger representation than the renderer would actually preload.

## Fix Focus Areas
- crates/crust-assets/src/ptex_stream.rs[373-392]
- crates/crust-assets/src/ptex_texture.rs[182-193]
- crates/crust-assets/src/lib.rs[452-455]

## Recommended Fix
Make `preload_bytes` stop after the base level when `self.mip` is false, matching `PtexColor::open_with`. Add an admission test that opens a stream with mipmapping disabled and verifies its predicted size against the no-mip preloaded texture.

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


View medium (2)
6. Streaming bypasses the default cap 📘 Rule violation ≡ Correctness
Description
max_log2_from_env_opt returns None for both an absent and malformed CRUST_PTEX_MAX_LOG2, and
PtexStream::base_res interprets that value as permission to retain each face’s authored resolution
instead of applying the default log2 edge cap of 5. With CRUST_PTEX_STREAM=1, either an unset
value or a typo such as CRUST_PTEX_MAX_LOG2=bad reaches uncapped streamed sampling, while
preloading, fallback handling, and admission pricing use the 32×32 default.
Code

crates/crust-assets/src/ptex_stream.rs[R409-412]

+    fn base_res(&self, res: ptex::Res) -> ptex::Res {
+        let Some(cap) = self.cap else {
+            return res;
+        };
Relevance

●●● Strong

Malformed or absent configuration disabling the documented default cap is a clear behavioral
inconsistency.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2400246 requires a default Ptex log2 cap of 5, but the optional parser returns None for an
absent variable and also after warning about invalid input. PtexStream::open passes that optional
result through, and base_res returns the authored resolution in its None branch, whereas the
preload helper applies unwrap_or(DEFAULT_MAX_LOG2) and therefore retains the 32×32 cap; together,
these citations show that the same missing or malformed setting produces uncapped streaming but
capped preload, fallback, and pricing behavior.

Rule 2400246: CRUST_* debug environment flags must deterministically alter renderer behavior as specified
crates/crust-assets/src/ptex_stream.rs[278-284]
crates/crust-assets/src/ptex_stream.rs[409-418]
crates/crust-assets/src/ptex_texture.rs[469-495]
crates/crust-assets/src/ptex_stream.rs[257-263]
crates/crust-assets/src/lib.rs[443-454]
crates/crust-assets/src/ptex_texture.rs[470-494]
crates/crust-assets/src/lib.rs[452-454]

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 streaming Ptex backend treats both an absent and malformed `CRUST_PTEX_MAX_LOG2` as `None`, which `PtexStream` interprets as no resolution ceiling. Ensure missing or invalid values use the required default log2 edge cap of 5 so streamed sampling, preloading, fallback handling, and admission pricing remain consistent.

## Fix Focus Areas
- crates/crust-assets/src/ptex_texture.rs[469-495]
- crates/crust-assets/src/ptex_stream.rs[35-35]
- crates/crust-assets/src/ptex_stream.rs[278-284]
- crates/crust-assets/src/ptex_stream.rs[409-418]

## Recommended Fix
Pass the validated, defaulted value from `max_log2_from_env()` into `PtexStream::open` so both absent and invalid environment values resolve to `DEFAULT_MAX_LOG2`. Alternatively, change the parsing API or caller to substitute `Some(DEFAULT_MAX_LOG2)` for either case, while retaining the invalid-value warning and the existing per-face clamping logic.

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


7. Test metrics bypass configured logging 📘 Rule violation ◔ Observability
Description
the_microcache_absorbs_most_taps and two other new tests emit measured diagnostics with
eprintln! instead of a tracing macro. Running the integration suite with output enabled sends
cache-rate, mip-divergence, and admission metrics directly through stderr outside the configured
tracing subscriber.
Code

crates/crust-assets/tests/ptex_stream.rs[R249-252]

+    eprintln!(
+        "microcache rate: {:.3} inside a tile, {:.3} on a four-tile corner",
+        stats.micro_rate(),
+        corner_stats.micro_rate()
Relevance

●●● Strong

New diagnostic measurements bypass the repository’s explicit tracing-only logging rule.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2230649 requires diagnostic logging in modified Rust files to use tracing and permits direct
printing only for narrow non-logging output. These three additions print diagnostic measurements
from tests directly to stderr.

Rule 2230649: Use only tracing for application logging in Rust
crates/crust-assets/tests/ptex_stream.rs[249-253]
crates/crust-assets/tests/ptex_stream.rs[394-397]
crates/crust-assets/tests/ptex_stream.rs[624-631]

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

## Issue description
Three new Ptex integration tests use `eprintln!` for diagnostic measurements instead of the required tracing-based logging path.

## Fix Focus Areas
- crates/crust-assets/tests/ptex_stream.rs[249-253]
- crates/crust-assets/tests/ptex_stream.rs[394-397]
- crates/crust-assets/tests/ptex_stream.rs[624-631]

## Recommended Fix
Remove these informational stderr emissions because the assertions already retain failure diagnostics, or replace them with appropriate `tracing` events if the measurements must remain observable.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 129 rules
✅ Cross-repo context — repo relationships
  Explored: repo: doubleailes/ptex-rs (sha: 884007d8)
Review mode: 🧠 Deep: This introduces substantial new streaming/cache-adjacent logic across multiple asset, texture, stats, integration, and test paths, creating many independent opportunities for subtle correctness and resource-management defects.

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +409 to +412
fn base_res(&self, res: ptex::Res) -> ptex::Res {
let Some(cap) = self.cap else {
return res;
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. Streaming bypasses the default cap 📘 Rule violation ≡ Correctness

max_log2_from_env_opt returns None for both an absent and malformed CRUST_PTEX_MAX_LOG2, and
PtexStream::base_res interprets that value as permission to retain each face’s authored resolution
instead of applying the default log2 edge cap of 5. With CRUST_PTEX_STREAM=1, either an unset
value or a typo such as CRUST_PTEX_MAX_LOG2=bad reaches uncapped streamed sampling, while
preloading, fallback handling, and admission pricing use the 32×32 default.
Agent Prompt
## Issue description
The streaming Ptex backend treats both an absent and malformed `CRUST_PTEX_MAX_LOG2` as `None`, which `PtexStream` interprets as no resolution ceiling. Ensure missing or invalid values use the required default log2 edge cap of 5 so streamed sampling, preloading, fallback handling, and admission pricing remain consistent.

## Fix Focus Areas
- crates/crust-assets/src/ptex_texture.rs[469-495]
- crates/crust-assets/src/ptex_stream.rs[35-35]
- crates/crust-assets/src/ptex_stream.rs[278-284]
- crates/crust-assets/src/ptex_stream.rs[409-418]

## Recommended Fix
Pass the validated, defaulted value from `max_log2_from_env()` into `PtexStream::open` so both absent and invalid environment values resolve to `DEFAULT_MAX_LOG2`. Alternatively, change the parsing API or caller to substitute `Some(DEFAULT_MAX_LOG2)` for either case, while retaining the invalid-value warning and the existing per-face clamping logic.

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

Comment on lines +249 to +252
eprintln!(
"microcache rate: {:.3} inside a tile, {:.3} on a four-tile corner",
stats.micro_rate(),
corner_stats.micro_rate()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

4. Test metrics bypass configured logging 📘 Rule violation ◔ Observability

the_microcache_absorbs_most_taps and two other new tests emit measured diagnostics with
eprintln! instead of a tracing macro. Running the integration suite with output enabled sends
cache-rate, mip-divergence, and admission metrics directly through stderr outside the configured
tracing subscriber.
Agent Prompt
## Issue description
Three new Ptex integration tests use `eprintln!` for diagnostic measurements instead of the required tracing-based logging path.

## Fix Focus Areas
- crates/crust-assets/tests/ptex_stream.rs[249-253]
- crates/crust-assets/tests/ptex_stream.rs[394-397]
- crates/crust-assets/tests/ptex_stream.rs[624-631]

## Recommended Fix
Remove these informational stderr emissions because the assertions already retain failure diagnostics, or replace them with appropriate `tracing` events if the measurements must remain observable.

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

Comment thread crates/crust-assets/src/ptex_stream.rs
Comment thread crates/crust-assets/src/lib.rs Outdated
Comment on lines +383 to +390
loop {
floats += w * h * 3;
if w == 1 && h == 1 {
break;
}
w = (w / 2).max(1);
h = (h / 2).max(1);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

6. Small textures stream when mips are off 🐞 Bug ≡ Correctness

PtexStream::preload_bytes always sums every halved mip level even when its mip field is false.
With CRUST_PTEX_MIP=0, PtexColor allocates only level zero, so the inflated estimate can admit a
texture to streaming although its actual preload size is below CRUST_PTEX_STREAM_MIN_MB.
Agent Prompt
## Issue description
Ptex streaming admission compares `preload_bytes` with the stream threshold, but `preload_bytes` always includes a complete mip pyramid. When `CRUST_PTEX_MIP=0`, the fallback `PtexColor` contains only its base level, so admission is comparing the threshold to a larger representation than the renderer would actually preload.

## Fix Focus Areas
- crates/crust-assets/src/ptex_stream.rs[373-392]
- crates/crust-assets/src/ptex_texture.rs[182-193]
- crates/crust-assets/src/lib.rs[452-455]

## Recommended Fix
Make `preload_bytes` stop after the base level when `self.mip` is false, matching `PtexColor::open_with`. Add an admission test that opens a stream with mipmapping disabled and verifies its predicted size against the no-mip preloaded texture.

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

Comment on lines +61 to +63
Err(_) => DEFAULT_CACHE_MB,
};
mb * 1024 * 1024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

7. Large cache settings can crash asset loading 🐞 Bug ☼ Reliability

cache_budget_from_env and stream_min_bytes_from_env accept any positive usize and multiply it
by 1,048,576 without checked arithmetic. A sufficiently large accepted environment value overflows
during configuration, panicking in checked builds or wrapping to an unintended cache budget or
admission threshold in release builds.
Agent Prompt
## Issue description
The new Ptex cache and stream-threshold environment parsers validate only that the value is a positive integer, then perform unchecked conversion from MiB to bytes. Values that parse as `usize` but exceed the representable byte count can overflow, turning accepted configuration into a panic or an unintended small value.

## Fix Focus Areas
- crates/crust-assets/src/ptex_stream.rs[50-64]
- crates/crust-assets/src/ptex_stream.rs[104-119]

## Recommended Fix
Use `checked_mul(1024 * 1024)` after parsing in both helpers. On overflow, log the same validation warning style used for malformed values and return the corresponding safe default; add tests for oversized numeric input.

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

Comment thread crates/crust-assets/src/ptex_stream.rs
claude and others added 4 commits September 21, 2026 10:18
Review catch, and a real one: `rebudget_ptex` gave every streamed reader
`max(budget / n, MIN_PTEX_SHARE)`, so whenever the admitted count exceeded the
budget in MiB the assigned total exceeded the budget. At
`CRUST_PTEX_CACHE_MB=8` with 39 admitted readers that is 39 MiB against 8 —
the setting whose entire job is to bound residency, not bounding it.

The floor looked like prudence: never hand a reader a share too small to be
useful. But it fails in the direction that matters, overshooting *more* the
smaller the budget gets, so it breaks down exactly where the budget is being
taken most seriously. And it could not be fixed by lowering it — any positive
floor multiplies by the reader count.

So the same 1 MiB is now read as a **capacity** rather than a floor. At most
`budget / MIN_PTEX_SHARE` readers may stream, and the budget divides *exactly*
among them. Both properties then hold by construction: every admitted reader
gets at least 1 MiB, and `n * (budget / n) <= budget` because integer division
floors. A texture arriving past the cap is preloaded, counted as a third
policy reason (`budget_full`) and reported by `--stats` with the one hint that
fixes it — distinct from `below_threshold`, because these are textures big
enough to want streaming that the budget could not seat.

That is the honest answer rather than a compromise: there is no cache left to
give such a reader, and a share too small to hold one block caches nothing
anyway — upstream returns that read `oversized`.

Admission is checked before the open, so a texture the budget cannot seat
costs no file I/O at all.

Costs the default path nothing: 1 GiB seats 1 024 readers and the island wants
39, so the measured island numbers are unchanged. It engages only when the
budget is genuinely small.

`the_total_budget_holds_when_readers_outnumber_megabytes` sweeps budgets of
1, 2, 3, 4, 8 and 64 MiB against four fixtures, asserting the assigned total
never exceeds the budget, that no admitted reader is given less than a usable
share, and that each still reads bit-identically to the preloaded oracle at
whatever share it got — a bounded budget must cost detail-per-second, never
correctness. End to end the invariant holds with the cap engaged too: forcing
a mixed backend at a 1 MiB budget still renders 0 of 57 600 pixels different.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
Second review catch on the same invariant, and the sharper one. The
per-thread microcache holds `ptex::PixelData` — decoded tiles the reader's
byte budget does not know about — and retained them unconditionally. The case
that bites is not an ordinary tile (128x128 at four channels is 64 KiB, four
per thread is nothing) but a block upstream has *refused*: a face too big for
the budget comes back `oversized`, deliberately uncached, and this put it
straight back into residency, four slots deep, on every worker thread,
entirely off the books. Reader eviction and re-budgeting could not release it,
and `--stats` could not see it.

Three changes, all needed:

**A slot has a ceiling.** `MICRO_SLOT_MAX` is 256 KiB — above any real Ptex
tile (256x256 at four channels) and below the whole-face reads that are the
oversized case. A tile over it is handed to the caller and dropped, which is
upstream's own rule: the microcache must never re-admit what the reader
declined.

**The allowance comes out of the budget.** `micro_reserve` is
`threads * MICRO_SLOTS * MICRO_SLOT_MAX`, clamped to half the budget, and
`FileAssets` subtracts it before dividing the rest among the readers — so the
two halves of Ptex residency sum to `CRUST_PTEX_CACHE_MB` rather than the
readers alone matching it. Below the clamp the slots shrink instead of the
budget being exceeded, and once a slot falls under a real tile the microcache
retains nothing: every tap goes to the reader, slower and still correct.

**It is reported.** `--stats` prints `thread tiles / reserve` beside the
reader's resident figure, because a number nobody can see is a number nobody
checks against the budget.

Measured at a 1 MiB budget on 4 threads: reserve 512 KiB, slot 32 KiB, and a
whole-face read of the 1024x512 fixture is 512 KiB — retention grows by **0
bytes** over 16 900 taps, every one served by the reader. At the default 1 GiB
the reserve is 4 MiB and a slot 256 KiB, so ordinary tiles are kept exactly as
before and the sample scene still shows a 100% microcache rate against a
1020 MiB reader budget.

`a_tile_larger_than_a_slot_is_never_retained` drives that case and asserts on
the process-wide retained total, then checks the refused tile still reads
bit-identically to the preloaded oracle — a bounded microcache costs re-reads,
never texels. `the_microcache_allowance_comes_out_of_the_budget` pins the
reserve arithmetic across budgets including the clamped ones.

Known and now documented rather than hidden: a tile stays in its slot until
that slot is reused, so dropping a texture does not immediately release its
tiles. That is bounded by the reserve rather than by the texture, which is the
property the budget is about; releasing on drop would need upstream to account
for live handles.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
A `.ptx`'s stored mip levels were reduced before crust ever saw the file,
in the file's own encoding; crust decodes Ptex by 2.2 afterwards. So a
streamed coarse level is `g(mean e)` where the preloaded pyramid gives
`mean g(e)`, and convexity makes the streamed chain the darker of the two
by up to 0.147 on the tiled fixture.

That is the same mismatch `crust:mipspace` guards against for `.tx`, and
there the answer is refusal rather than a footnote -- for the reason the
mismatch is dangerous rather than for tidiness: level 0 stays perfectly
correct and only minification is wrong, so it never presents as a colour
bug but as a filtering one. Describing a defect nobody can see is not a
control. The streaming Ptex path accepted it with a doc note, which was
an inconsistency with the project's own standard.

So it is refused here too. `MipSpace::Linear` is the default and declines
to stream a texture whose lookups could reach such a level, preloading it
instead, reported as its own `backend` reason. The gate asks about the
texture (`PtexStream::chain_is_exact`), not the switch, so the two
configurations with no chain to get wrong still stream: `CRUST_PTEX_MIP=0`
-- exact *and* uncapped, the base level being the bit-identical one -- and
a texture whose every face is one texel under the cap.

`CRUST_PTEX_STREAM_MIPSPACE=file` is the opt-in that takes the file's
chain. It is what the C++ PtexCache does and what every measurement in
docs/ptex_streaming.md was taken with, the island's included -- so the
cost of the default is worth stating plainly: with a pyramid on,
`CRUST_PTEX_STREAM=1` alone now streams nothing on a normal render, and
the island's 5.98 -> 0.61 GiB needs the opt-in. Deliberate, and one
variable to reverse.

Neither remedy the review named is available here. Building the linear
chain from streamed base tiles needs a second pyramid cache -- exactly
the design "Known incomplete work" ruled out -- and would read level 0 to
answer a coarse lookup, defeating streaming where the island uses it. The
honest fix is upstream: a reader that reduces in a declared working space
would let both backends share one chain and retire both gates.

Two new tests pin the gate rather than the wording: every mipmapped
fixture is declined and the same file admitted with the pyramid off
(where streamed and preloaded then agree bit for bit at every footprint),
and a texture capped to one texel per face is admitted. The divergence
test stays, reframed as what the opt-in accepts.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
The Moana section documented the resolution cap as the only answer to
494 GiB of authored Ptex. Streaming replaces it, but on this scene it
takes two variables rather than one, and the second is the easy one to
miss: a mipmapped .ptx is declined by default because its stored levels
were reduced in the file's own encoding, so `CRUST_PTEX_STREAM=1` alone
reproduces the preloaded numbers exactly. Adds the command line, the
measured before/after, and why the default declines.

Also corrects the "Ptex textures are still fully resident" limitation,
which this no longer is -- the remaining restriction is the mip chain,
not residency.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LgzLA1uRAYXcxNWCBzSUCf
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.

2 participants