fix(linux): make Bloom actually run on Linux — device limits, screenshots, window close#146
Conversation
bloom_init_window on Linux had drifted behind the Windows crate and bloom_shared::attach, which both negotiate these already: - max_storage_buffers_per_shader_stage: the PT kernel binds 9 (accum + moments + reservoir ping-pongs on top of instance/geo data) against wgpu's default of 8. This was a hard crash on startup, not a fallback — every game reaching PT init aborted with "Too many bindings of type StorageBuffers in Stage ShaderStages(COMPUTE), limit is 8, count was 9". - max_sampled_textures/samplers_per_shader_stage: raised to the adapter's limits. Latent until now — the refractive/translucent profile binds 19 sampled textures against a default cap of 16. The identical shortfall is what stopped the shooter opening on macOS (see attach.rs). - PT-2 TEXTURE_BINDING_ARRAY + non-uniform indexing, and the binding-array element budget: never requested here, so textured path-trace hit shading silently compiled the card-only stub. Also add COPY_SRC to the swapchain usage. bloom_take_screenshot reads the swapchain back via copy_texture_to_buffer; without the usage bit that is a validation error that aborts the process (verified by removing it again: non-unwinding panic + core dump on the first takeScreenshot call). Verified on AMD Phoenix1 / RADV / XWayland: 138/138 shared tests pass including the 9 golden-image renders, and the Bloom Shooter runs at 59 FPS with `ssgi trace backend = hw-ray-query`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sumer
src/models/index.ts declared BUCKET_* twice with conflicting values and
unloadModel twice. Perry emits one LLVM function per exported const, so
clang rejected the module outright ("invalid redefinition of function
...__BUCKET_OPAQUE"). Perry then linked the whole module as empty
_perry_init_* stubs, so every game importing bloom/models failed to link
with dozens of undefined references (createMesh, drawModelTransform,
setAmbientLight, compileMaterialInstancedBucket, ...). The Bloom Shooter
could not be built at all.
Kept the BUCKET_* block that matches the live wire mapping decoded by
Renderer::compile_material_instanced_bucket (0=Opaque, 1=Cutout,
2=Additive, 3=Transparent) — the numbering the shooter's BUCKET_ADDITIVE
and BUCKET_CUTOUT call sites depend on. The removed block carried Phase 4b
numbering (TRANSPARENT=1, REFRACTIVE=2, ADDITIVE=3, CUTOUT=4), which
matched neither the FFI nor the Rust Bucket enum's discriminants. Refractive
has no wire value in that FFI; compileRefractiveMaterial covers it, and
nothing referenced BUCKET_REFRACTIVE.
Both unloadModel definitions were byte-identical; kept the documented one.
A scan of src/**/*.ts finds no other duplicate top-level exports.
Not Linux-specific — this broke macOS and Windows identically.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…cleanly
The X11 backend never put WM_DELETE_WINDOW in the window's WM_PROTOCOLS, so
the titlebar ✕ / Alt-F4 / session logout gave the WM no polite way to ask the
game to quit. It dropped the X connection instead, and Xlib's default IO-error
handler called exit(1) from under the running frame:
XIO: fatal IO error 62 (Timer expired) on X server ":0"
after 23355 requests (23354 known processed) with 0 events remaining.
`windowShouldClose()` never went true, so `closeWindow()` and everything the
game does after its loop — audio teardown, save-on-exit — were skipped. Only
DestroyNotify was handled, which arrives too late to be useful here.
Intern the atoms once at window creation, XSetWMProtocols before mapping, and
translate the resulting ClientMessage into `should_close`.
Verified with the Bloom Shooter: sending a real WM_DELETE_WINDOW ClientMessage
now exits 0 with no XIO error in the log (previously exit 1 + XIO fatal).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughLinux X11 shutdown handling and wgpu initialization now support close events, path-tracing resource requirements, and framebuffer readback. Model exports remove duplicate declarations and align instanced-material bucket IDs. ChangesLinux window runtime
Model export cleanup
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant WindowManager
participant x11_impl
participant Engine
x11_impl->>WindowManager: Register WM_DELETE_WINDOW
WindowManager->>x11_impl: Send matching ClientMessage
x11_impl->>Engine: Set should_close
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@native/linux/src/lib.rs`:
- Around line 680-684: Update the Linux swapchain configuration to use
RENDER_ATTACHMENT combined with only the COPY_SRC bit supported by
surface_caps.usages. In the Linux screenshot path, detect when COPY_SRC is
unavailable and return the existing unsupported-screenshot error instead of
attempting copy_texture_to_buffer.
- Around line 631-658: Update the adapter-limit handling before request_device
to validate Bloom’s fixed minimum budgets: at least 19 sampled textures and 9
storage buffers per shader stage. Reject initialization with a clear error or
select the existing fallback renderer when adapter_limits cannot satisfy either
minimum; do not lower required_limits to an unsupported adapter value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba7264fb-c126-477f-960c-1ac73410d8ae
⛔ Files ignored due to path filters (1)
native/linux/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
native/linux/src/lib.rssrc/models/index.ts
| // The refractive/translucent material profile binds up to 19 | ||
| // sampled textures in the fragment stage (5 material maps + env/ | ||
| // BRDF/3 shadow cascades/env-diffuse + planar reflection + 3 texture | ||
| // arrays + the group-4 scene_color/scene_depth/impulse/motion inputs). | ||
| // wgpu's default is 16. Raise to whatever the adapter actually | ||
| // supports — every real Vulkan/GL GPU exposes ≥128 — so opaque/ | ||
| // transparent materials are unaffected and refractive ones link. | ||
| let adapter_limits = adapter.limits(); | ||
| required_limits.max_sampled_textures_per_shader_stage = required_limits | ||
| .max_sampled_textures_per_shader_stage | ||
| .max(adapter_limits.max_sampled_textures_per_shader_stage); | ||
| required_limits.max_samplers_per_shader_stage = required_limits | ||
| .max_samplers_per_shader_stage | ||
| .max(adapter_limits.max_samplers_per_shader_stage); | ||
| // PT-2: binding arrays have their own element budget, default 0. | ||
| // Take whatever the adapter offers; the renderer checks the | ||
| // granted value against its fixed array size before compiling | ||
| // the textured kernel variant. | ||
| if required_features.contains(pt_tex_mask) { | ||
| required_limits.max_binding_array_elements_per_shader_stage = | ||
| adapter_limits.max_binding_array_elements_per_shader_stage; | ||
| } | ||
| // PT-4: the path-trace kernel binds 9 storage buffers (accum + | ||
| // moments + reservoir ping-pongs on top of instance/geo data); | ||
| // the wgpu default limit is 8. | ||
| required_limits.max_storage_buffers_per_shader_stage = required_limits | ||
| .max_storage_buffers_per_shader_stage | ||
| .max(adapter_limits.max_storage_buffers_per_shader_stage.min(16)); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching lib.rs:"
fd 'lib\.rs$' . || true
echo
echo "Relevant section in native/linux/src/lib.rs:"
sed -n '600,675p' native/linux/src/lib.rs
echo
echo "Search for required_limits and DeviceDescriptor/Adapter configuration:"
rg -n "required_limits|max_sampled_textures|max_samplers|max_storage_buffers|DeviceDescriptor|request_adapter|request_device" native/linux/src/lib.rs native -g '*.rs' | head -200Repository: Bloom-Engine/engine
Length of output: 22087
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Search for Bloom renderer resource budget constants/usages:"
rg -n "19 sampled|128|PT-2|PT-4|path-trace kernel|9 storage|max_binding_array_elements_per_shader_stage|req.*binding|fixed array size|max_binding_array_elements" native/shared native/linux native/windows native/web | head -200
echo
echo "Check native/windows and shared attach mirroring:"
sed -n '750,802p' native/windows/src/lib.rs
sed -n '155,182p' native/shared/src/attach.rsRepository: Bloom-Engine/engine
Length of output: 15576
🌐 Web query:
Bloom Engine native linux renderer 19 sampled textures 9 storage buffers path-trace kernel
💡 Result:
The Bloom Engine is a cross-platform graphics engine written in TypeScript that compiles to native binaries (macOS, Windows, Linux, iOS, tvOS) and WASM for the web [1][2]. Its renderer is built on the wgpu API [1][2], which abstracts graphics backends such as Vulkan, Metal, DirectX 12, and WebGPU [1]. Regarding your query about a "19 sampled textures 9 storage buffers path-trace kernel" configuration, there is no public documentation or technical evidence indicating that the Bloom Engine utilizes a specific hardcoded configuration of 19 sampled textures and 9 storage buffers for a path-tracing kernel. The engine's current development focus involves GPU-driven rendering techniques, including indirect multi-draw and GPU culling [3], as well as advanced lighting features like global illumination probes and variance-driven refinement [4][5]. While the engine explores compute-heavy tasks like global illumination feedback loops (WSRC) [4], it explicitly notes that some "exotic" features such as native hardware ray tracing are currently off the table due to the constraints of the wgpu abstraction layer [2]. Technical discussions within the Bloom Engine repository often reference compute shader dispatch and binding patterns [4][5][3], but do not align with the specific numbers mentioned in your query. It is possible that these numbers refer to a custom implementation or a specific project configuration not part of the core Bloom Engine codebase.
Citations:
- 1: https://github.com/Bloom-Engine/engine
- 2: https://bloomengine.dev/blog/where-bloom-stands
- 3: perf ticket 009: GPU-driven rendering — indirect multi-draw + GPU cull #28
- 4: Lumen follow-up: multi-bounce GI via WSRC feedback #25
- 5: Lumen follow-up: variance-driven refinement probe layer #20
Reject adapters below Bloom’s fixed resource budgets.
The Linux/Windows path-tracing setup documents hard requirements of 19 sampled textures and 9 storage buffers, but the device request currently downgrades to the adapter’s lower limits. Query the requested limits against these minima before request_device; if an adapter cannot support them, fail initialization clearly or select a fallback renderer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@native/linux/src/lib.rs` around lines 631 - 658, Update the adapter-limit
handling before request_device to validate Bloom’s fixed minimum budgets: at
least 19 sampled textures and 9 storage buffers per shader stage. Reject
initialization with a clear error or select the existing fallback renderer when
adapter_limits cannot satisfy either minimum; do not lower required_limits to an
unsupported adapter value.
| // COPY_SRC: bloom_take_screenshot reads the swapchain back; | ||
| // without it the readback copy is a validation error that | ||
| // aborts the process the first time a game calls it. | ||
| usage: wgpu::TextureUsages::RENDER_ATTACHMENT | ||
| | wgpu::TextureUsages::COPY_SRC, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,120p'
echo "== target file size/outline =="
wc -l native/linux/src/lib.rs || true
ast-grep outline native/linux/src/lib.rs --view compact 2>/dev/null | sed -n '1,220p' || true
echo "== relevant lines =="
sed -n '620,730p' native/linux/src/lib.rs | cat -nRepository: Bloom-Engine/engine
Length of output: 10276
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== screenshot and screenshot-related locations =="
rg -n "bloom_take_screenshot|bloom_register_geisterhand_screenshot|COPY_SRC|swapchain|swap_chain|SurfaceCapabilities|surface_caps|configure" native/linux/src/lib.rs native/linux -g '*.rs' || true
echo "== renderer surface usage/configuration context =="
rg -n "SurfaceConfiguration|RENDER_ATTACHMENT|COPY_SRC|bloom_take_screenshot|screen reader|readback|present" --glob '*.rs' .
echo "== likely screenshots section =="
sed -n '1140,1284p' native/linux/src/lib.rs | cat -nRepository: Bloom-Engine/engine
Length of output: 37222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== windows surface config around COPY_SRC =="
sed -n '790,835p' native/windows/src/lib.rs | cat -n
echo "== web copy-src handling =="
sed -n '160,195p' native/web/src/lib.rs | cat -n
echo "== shared attach =="
sed -n '220,255p' native/shared/src/attach.rs | cat -n
echo "== shared renderer surface config =="
sed -n '1989,2032p' native/shared/src/renderer/mod.rs | cat -n
echo "== screenshot implementation in shared renderer =="
sed -n '11160,11245p' native/shared/src/renderer/mod.rs | cat -n
echo "== wasm screenshot =="
sed -n '334,350p' native/web/src/parity_ffi.rs | cat -nRepository: Bloom-Engine/engine
Length of output: 13276
🌐 Web query:
wgpu SurfaceCapabilities usages TextureUsages configure surface copy-src documentation
💡 Result:
To use a surface texture as a source for copy operations (e.g., using COPY_SRC), you must ensure that your SurfaceConfiguration includes the appropriate TextureUsages when calling surface.configure. 1. Query Surface Capabilities: Before configuring the surface, retrieve the supported texture usages for your specific adapter by calling surface.get_capabilities(adapter) [1][2]. This returns a SurfaceCapabilities struct, which contains a usages field specifying which TextureUsages are supported by the surface [3][2]. Note that TextureUsages::RENDER_ATTACHMENT is the only usage guaranteed to be supported across all surfaces [3][4][5]. 2. Configure the Surface: When you call surface.configure, you must provide a SurfaceConfiguration [1][4]. The usage field in this struct defines the allowed operations for textures retrieved from the surface via get_current_texture [4][5]. To perform a copy-src operation (such as copy_texture_to_buffer), the SurfaceConfiguration's usage must include TextureUsages::COPY_SRC [6][7]. 3. Important Considerations: - Compatibility: Always check the usages returned by get_capabilities before configuring [2]. If you request a usage not supported by the underlying surface/adapter, the configuration may fail or cause runtime issues [1][8]. - Performance and Driver Behavior: Different backends (Vulkan, DX12, Metal) have varying levels of support for surface usages beyond RENDER_ATTACHMENT [8]. Always ensure the requested flags are subset of the capabilities returned by the surface [2]. - COPY_SRC Usage: TextureUsages::COPY_SRC specifically allows a texture to be used as the source in command encoder operations like copy_texture_to_buffer or copy_texture_to_texture [6][7]. Example flow: let caps = surface.get_capabilities(&adapter); let config = wgpu::SurfaceConfiguration { usage: caps.usages & wgpu::TextureUsages::COPY_SRC | wgpu::TextureUsages::RENDER_ATTACHMENT, format: caps.formats, //... other fields }; surface.configure(&device, &config);
Citations:
- 1: https://docs.rs/wgpu/latest/wgpu/struct.Surface.html
- 2: https://docs.rs/wgpu/latest/wgpu/struct.SurfaceCapabilities.html
- 3: https://wgpu.rs/doc/wgpu/struct.SurfaceCapabilities.html
- 4: https://wgpu.rs/doc/wgpu/api/surface/type.SurfaceConfiguration.html
- 5: https://docs.rs/wgpu/latest/wgpu/type.SurfaceConfiguration.html
- 6: https://docs.rs/wgpu/latest/wgpu/struct.TextureUsages.html
- 7: https://wgpu.rs/doc/wgpu_types/texture/struct.TextureUsages.html
- 8: DX12 backend crashes silently when making the surface COPY_DST gfx-rs/wgpu#2234
Check COPY_SRC support before configuring the Linux swapchain.
surface.configure(...) requires the requested usages to be present in surface_caps.usages; RENDER_ATTACHMENT is not the surface’s only guaranteed usage flag. Configuring with unguarded COPY_SRC can fail window initialization on surfaces that only support render attachment, even though the Linux screenshot path relies on copy_texture_to_buffer of the current swapchain texture. Use RENDER_ATTACHMENT | (surface_caps.usages & TextureUsages::COPY_SRC) as the configured usage, and fail the screenshot path with an unsupported-screenshot error when COPY_SRC is unavailable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@native/linux/src/lib.rs` around lines 680 - 684, Update the Linux swapchain
configuration to use RENDER_ATTACHMENT combined with only the COPY_SRC bit
supported by surface_caps.usages. In the Linux screenshot path, detect when
COPY_SRC is unavailable and return the existing unsupported-screenshot error
instead of attempting copy_texture_to_buffer.
1 participant
Brings the Linux target up to working order. Validated on AMD Phoenix1 / RADV / XWayland with the golden-image suite, Bloom Jump, and the Bloom Shooter.
What was broken
1. Device limits/features had drifted behind Windows and
bloom_shared::attach(5b156cd)bloom_init_windowon Linux requestedwgpu::Limits::default()plus onlymax_bind_groups. Everything else the renderer leans on was missing:max_storage_buffers_per_shader_stage— the PT kernel binds 9 against a default of 8. This was a hard crash on startup, not a graceful fallback. Any game reaching PT init aborted:max_sampled_textures_per_shader_stage/max_samplers_per_shader_stage— the refractive/translucent profile binds 19 sampled textures against a default cap of 16. Latent on Linux; the identical shortfall is what stopped the shooter opening on macOS (see the comment inattach.rs).TEXTURE_BINDING_ARRAY+ non-uniform indexing, and the binding-array element budget — never requested, so textured path-trace hit shading silently compiled the card-only stub.Also adds
COPY_SRCto the swapchain usage.bloom_take_screenshotreads the swapchain back viacopy_texture_to_buffer; without the usage bit that is a validation error that aborts the process — confirmed by removing it again and getting a non-unwinding panic + core dump on the firsttakeScreenshot()call.2. Duplicate exports broke every
bloom/modelsconsumer on every platform (8d6132d)src/models/index.tsdeclaredBUCKET_*twice with conflicting values, andunloadModeltwice. Perry emits one LLVM function per exported const, so clang rejected the module (invalid redefinition of function ...__BUCKET_OPAQUE), Perry linked it as empty_perry_init_*stubs, and games failed with dozens of undefined references (createMesh,drawModelTransform,setAmbientLight,compileMaterialInstancedBucket, …). The Bloom Shooter could not be built at all.Kept the block matching the live wire mapping in
Renderer::compile_material_instanced_bucket(0=Opaque, 1=Cutout, 2=Additive, 3=Transparent) — the numbering the shooter'sBUCKET_ADDITIVE/BUCKET_CUTOUTcall sites depend on. The removed block carried Phase 4b numbering, which matched neither the FFI nor the RustBucketdiscriminants. Nothing referencedBUCKET_REFRACTIVE;compileRefractiveMaterialcovers that bucket. A scan ofsrc/**/*.tsfinds no other duplicate top-level exports.Not Linux-specific — this broke macOS and Windows identically.
3. Closing the window killed the game instead of shutting it down (
fb79f52)WM_DELETE_WINDOWwas never added to the window'sWM_PROTOCOLS, so the titlebar ✕ gave the WM no polite way to ask us to quit. It dropped the X connection and Xlib's default IO-error handler calledexit(1)mid-frame:windowShouldClose()never went true, socloseWindow()and every shutdown path after it (audio teardown, save-on-exit) were skipped.Verification
cargo test --releaseinnative/shared: 138/138 pass, including the 9 golden-image renders on real hardwaretools/validate-ffi.js:ok linux: 473 exports cover 472 manifest functions(the 2 remaining failures are pre-existing watchos/webbloom_is_key_repeatedgaps onmain, untouched here)ssgi trace backend = hw-ray-query(HW ray query granted via Vulkan)takeScreenshotis pixel-exact: requested(220,80,60)→ written(220,80,60)WM_DELETE_WINDOWto the shooter now exits 0 with no XIO error (previously exit 1 + XIO fatal)Known gaps found but deliberately not changed here
examples/pongno longer compiles on any platform — it usesColor.White/Color.Black, but GH Duplicate identifier Color in core/index.ts #53 deliberately stopped re-exporting that palette (Coloris now the RGBA type only). Fix isColors.*/ColorConstants.*. Separate concern from this PR.display_scale()correctly returns 1.0. That is the documented EN-019b gap, not a code bug.🤖 Generated with Claude Code
Summary by CodeRabbit