Repository navigation
feat(codegen): route compressed (ASTC) atlases through the catalog adapter - #348
Conversation
…apter
The async streaming asset catalog (engine#450) loads atlases via the
assembler-generated ImageBackendAdapter, whose decode/upload call
BackendGfx.decodeImage/uploadTexture directly — bypassing gfx's
synchronous compressed seam (loadTextureFromMemory). So ASTC blobs
always hit the CPU decoder and fail.
The generated adapter now:
- decode: branches on BackendGfx.isCompressed(data); for compressed
blobs it dupes the raw bytes (the catalog frees them after upload),
reads dims from the header via BackendGfx.compressedDims, sets
.compressed = true, and skips the CPU decoder.
- upload: routes .compressed blobs through BackendGfx.uploadCompressed
(verbatim), RGBA8 through uploadTexture as before.
Both branches are @hasDecl-guarded (isCompressed/compressedDims and
uploadCompressed), so backends without ASTC support — or a future
headless backend — still generate valid code and always CPU-decode.
Adds a header-only compressedDims(data) ?struct{width,height} helper +
gfx.zig re-export to all four backends (sokol/bgfx/raylib/wgpu), reusing
each backend's existing validateAstc to read dims without decoding, so
the adapter can set correct DecodedImage dims before upload.
Part of the ASTC cold-start epic (labelle-gfx#269). Proven on the real
flying-platform-labelle game (sokol/Android, Tab A7): the menu atlas
returned error.LoadFailed until this fix; with it, cold start dropped
from ~24s to ~1s.
PR SummaryMedium Risk Overview When a backend exposes the full compressed API (
Reviewed by Cursor Bugbot for commit cbb951b. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Code Review
This pull request introduces a header-only dimensions reading mechanism (compressedDims) for compressed ASTC textures across multiple backends (bgfx, raylib, sokol, and wgpu), allowing the async asset-catalog adapter to retrieve image dimensions without CPU decoding. Additionally, the code generator in asset_wiring.zig is updated to route compressed textures directly to uploadCompressed when supported. The review feedback highlights a potential issue where a backend implementing isCompressed and compressedDims but lacking uploadCompressed could cause a runtime crash. It is recommended to strengthen the @hasDecl guard in the generated code to also verify the presence of uploadCompressed and update the corresponding test assertions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // worker-thread decode from main-thread upload, so the divert happens here. | ||
| // Guarded by `@hasDecl` so backends without compressed support (or a future | ||
| // headless backend) still generate valid code and just always CPU-decode. | ||
| try w.print("{s} if (@hasDecl(BackendGfx, \"isCompressed\") and @hasDecl(BackendGfx, \"compressedDims\")) {{\n", .{indent}); |
There was a problem hiding this comment.
If a backend implements isCompressed and compressedDims but does not implement uploadCompressed (or if it is temporarily disabled/removed), the decode function will still divert and return raw compressed bytes in decoded.pixels. Then, in upload, since uploadCompressed is missing, it will fall back to calling BackendGfx.uploadTexture with the raw compressed bytes, leading to runtime crashes or rendering corruption.
To prevent this, the decode guard should also check for the presence of uploadCompressed to ensure the backend fully supports the entire compressed texture lifecycle before diverting.
try w.print("{s} if (@hasDecl(BackendGfx, \"isCompressed\") and @hasDecl(BackendGfx, \"compressedDims\") and @hasDecl(BackendGfx, \"uploadCompressed\")) {{\n", .{indent});
| try std.testing.expect(std.mem.indexOf(u8, main_zig, "BackendGfx.unloadTexture(tex)") != null); | ||
| // Compressed (ASTC) blobs route through the @hasDecl-guarded path: | ||
| // decode diverts before the CPU decoder, upload uses uploadCompressed. | ||
| try std.testing.expect(std.mem.indexOf(u8, main_zig, "@hasDecl(BackendGfx, \"isCompressed\") and @hasDecl(BackendGfx, \"compressedDims\")") != null); |
There was a problem hiding this comment.
…iew) The generated ImageBackendAdapter previously gated the compressed-upload path on isCompressed+compressedDims only, while the upload step referenced BackendGfx.uploadCompressed via a runtime `if`. A backend exposing the probe decls but NOT uploadCompressed would generate an adapter that fails to compile (dangling reference to the missing decl). Introduce a single `supports_compressed` comptime flag requiring the ENTIRE lifecycle (isCompressed + compressedDims + uploadCompressed), and wrap both the decode-divert and upload arms in `if (comptime supports_compressed)` so the inner blocks are not semantically analyzed when the flag is comptime-false — backends lacking compressed support generate code that still compiles and always CPU-decodes. Update backend_wiring_tests to assert the strengthened 3-decl guard and the comptime-gated upload arm. Verified: zig build test green; sokol/bgfx/raylib/wgpu backends EXIT 0; the null backend (no compressed decls) also compiles, exercising the comptime-false gate.
|
Addressed both Gemini HIGH findings in Fix: The generated const supports_compressed = @hasDecl(BackendGfx, "isCompressed") and @hasDecl(BackendGfx, "compressedDims") and @hasDecl(BackendGfx, "uploadCompressed");requiring the entire compressed lifecycle (probe + dims + upload). Both the
Verification:
|
…t (CI) The generated catalog adapter referenced engine.DecodedImage.compressed and BackendGfx.uploadCompressed whenever the backend exposed the full compressed seam (true for raylib). But the released/main engine's DecodedImage lacks the compressed field (only present in engine#632), so the Examples integration test failed building the raylib example against the released engine: error: no field named 'compressed' in struct DecodedImage Add @Hasfield(engine.DecodedImage, "compressed") to the supports_compressed conjunction so the whole compressed decode/upload path is comptime-pruned when building against an engine without the field (falls back to CPU decode). With engine#632 + a compressed backend it stays true and the ASTC fast path is active. The assembler PR no longer requires the engine release to land first. Update backend_wiring_tests assertion to match the new expression.
|
The Examples integration test failure was a forward-reference to an unreleased engine field: the generated catalog adapter referenced Fixed by adding Verified locally (Zig 0.16.0):
|
Reverts a78b339. Silent CPU-decode fallback against an old engine hides misconfigurations; the maintainer prefers the assembler PR to depend on the engine version that ships DecodedImage.compressed (engine#632). Restores supports_compressed to gate on exactly the three backend decls (isCompressed + compressedDims + uploadCompressed). All other PR work (comptime-gated decode/upload divert from 2360004, compressedDims on all 4 backends) is intact. Content of both touched files now equals 2360004.
|
Reverted the `@hasField(engine.DecodedImage, "compressed")` back-compat gate (commit a78b339) per maintainer preference. A silent CPU-decode fallback against an old engine hides misconfigurations, so the codegen now gates `supports_compressed` on exactly the three backend decls (`isCompressed` + `compressedDims` + `uploadCompressed`) again, and this PR simply depends on the engine version that ships `DecodedImage.compressed`. The Examples integration check will stay RED until engine #632 (which adds `DecodedImage.compressed`) is released and this PR's example engine pin is bumped. That is an intentional stacked-PR forward dependency, not a defect — all other PR work (the comptime-gated decode/upload divert, `compressedDims` on all 4 backends) is intact, `zig build test` is green, and all 4 backend packages build standalone (EXIT 0). |
Problem
The async streaming asset catalog (labelle-engine#450) loads atlases via the assembler-generated
ImageBackendAdapter, whosedecode/uploadcallBackendGfx.decodeImage/uploadTexturedirectly — bypassing gfx's synchronous compressed seam (loadTextureFromMemory). So ASTC blobs always hit the CPU decoder and fail.Proven on the real flying-platform-labelle game (sokol / Android, Galaxy Tab A7): the menu atlas returned
error.LoadFaileduntil this fix. With it, cold start dropped from ~24s to ~1s.Fix
The generated adapter now branches on
isCompressed:decode: ifBackendGfx.isCompressed(data), dupe the raw bytes verbatim (the catalog frees them after upload), read dims from the ASTC header viaBackendGfx.compressedDims, set.compressed = true, and skip the CPU decoder. Otherwise CPU-decode as before.upload:.compressedblobs route throughBackendGfx.uploadCompressed(uploaded as-is); RGBA8 goes throughuploadTextureunchanged.Both branches are
@hasDecl-guarded (isCompressed/compressedDimsfor decode,uploadCompressedfor upload), so any backend without ASTC support — or a future headless backend — still generates valid code and always CPU-decodes.compressedDimsadded to all 4 backendsAdded a header-only
pub fn compressedDims(data) ?struct { width: u32, height: u32 }helper +gfx.zigre-export to sokol, bgfx, raylib, and wgpu, each reusing its existingvalidateAstcto read width/height from the ASTC header without decoding. This lets the adapter set correctDecodedImagedims before the (later, main-thread) upload.Tests
zig build testpasses (assembler codegen +backend_wiring_tests, updated to assert the new compressed branch +@hasDeclguards in the generated text).zig build --build-file backends/{sokol,bgfx,raylib,wgpu}/build.zig test.Required paired PRs (cross-repo)
The generated code references
engine.DecodedImage.compressedand the backends' compressed methods. For consumers to compile, these must be released / pinned:DecodedImage.compressedfieldEpic
Part of the ASTC cold-start epic: labelle-gfx#269.