Downsample fallout bloom + light extract for fillrate-bound GPUs#4157
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughBloom and light extraction passes now render into reduced sub-tile framebuffers instead of full map resolution. Each pass computes scaled dimensions from tile-scale constants, updates texture/FBO allocation, and adds tile-scale uniforms to the extraction shaders while keeping composite stages full-resolution. ChangesFallout Effects Sub-tile Resolution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
🧹 Nitpick comments (3)
src/client/render/gl/passes/FalloutBloomPass.ts (1)
17-17: ⚡ Quick winMove
BLOOM_TILE_SCALEtorender-settings.json.This tuning constant is hardcoded at module level. Per coding guidelines, all per-pass tuning constants should live in
gl/render-settings.jsonso they can be tweaked without code changes.Consider adding it under the
falloutBloomsection and reading it at construct time like other settings.Suggested approach
In
render-settings.json:"falloutBloom": { + "tileScale": 8, "broilSpeedCold": 0.0018,In this file:
-const BLOOM_TILE_SCALE = 8; - // ...inside constructor: +const tileScale = settings.falloutBloom.tileScale; -this.bloomW = Math.max(1, Math.floor(mapW / BLOOM_TILE_SCALE)); -this.bloomH = Math.max(1, Math.floor(mapH / BLOOM_TILE_SCALE)); +this.bloomW = Math.max(1, Math.floor(mapW / tileScale)); +this.bloomH = Math.max(1, Math.floor(mapH / tileScale));Store
tileScaleas a field if needed indraw().As per coding guidelines: "All per-pass tuning constants (alpha, radii, colors, etc.) should be defined in
gl/render-settings.json".🤖 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 `@src/client/render/gl/passes/FalloutBloomPass.ts` at line 17, BLOOM_TILE_SCALE is a hardcoded per-pass tuning constant; move it into gl/render-settings.json under a falloutBloom.tileScale field and read it in the FalloutBloomPass constructor (the class FalloutBloomPass and its draw() method currently rely on BLOOM_TILE_SCALE). Update the constructor to accept or load settings (e.g., from the existing settings loader used by other passes) and store tileScale on the instance for use in draw(), replacing references to the module-level BLOOM_TILE_SCALE with this instance property.src/client/render/gl/passes/FalloutLightPass.ts (1)
14-14: ⚡ Quick winMove
LIGHT_TILE_SCALEtorender-settings.json.Same issue as in
FalloutBloomPass— this tuning constant should live in the settings file. Consider adding it under thelightingsection.Suggested approach
In
render-settings.json:"lighting": { + "falloutTileScale": 8, "ambient": 1,In this file, read it from
settings.lighting.falloutTileScaleat construct time.As per coding guidelines: "All per-pass tuning constants (alpha, radii, colors, etc.) should be defined in
gl/render-settings.json".🤖 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 `@src/client/render/gl/passes/FalloutLightPass.ts` at line 14, Replace the hardcoded constant LIGHT_TILE_SCALE in FalloutLightPass with a value read from the render settings: remove or stop using the const LIGHT_TILE_SCALE and, in the FalloutLightPass constructor, read settings.lighting.falloutTileScale (from gl/render-settings.json) and store it on the instance (e.g., this.falloutTileScale) for use where LIGHT_TILE_SCALE was referenced; also add a new fallback/default in render-settings.json under "lighting" named "falloutTileScale" so existing behavior is preserved if the setting is missing.vite.config.ts (1)
225-225: ⚡ Quick winMake network binding configurable for security.
Setting
host: trueexposes the dev server to the local network. While this can be useful for testing on mobile devices or remote development, it should be opt-in rather than always-on. Consider making this configurable via environment variable.🔧 Suggested approach
server: { port: 9000, - host: true, + host: process.env.VITE_HOST === "true", // Automatically open the browser when the server starts open: process.env.SKIP_BROWSER_OPEN !== "true",Add a comment documenting when to enable:
// Set VITE_HOST=true to expose dev server on the network (for mobile testing, etc.) host: process.env.VITE_HOST === "true",🤖 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 `@vite.config.ts` at line 225, The dev server currently hardcodes host: true which always exposes the server to the network; change the Vite config's host property to be opt-in via an environment variable (e.g., use process.env.VITE_HOST === "true") and add a brief comment explaining when to enable it, updating the host assignment in the exported config object (where host: true is set) so network binding is only enabled when the env var is present.
🤖 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.
Nitpick comments:
In `@src/client/render/gl/passes/FalloutBloomPass.ts`:
- Line 17: BLOOM_TILE_SCALE is a hardcoded per-pass tuning constant; move it
into gl/render-settings.json under a falloutBloom.tileScale field and read it in
the FalloutBloomPass constructor (the class FalloutBloomPass and its draw()
method currently rely on BLOOM_TILE_SCALE). Update the constructor to accept or
load settings (e.g., from the existing settings loader used by other passes) and
store tileScale on the instance for use in draw(), replacing references to the
module-level BLOOM_TILE_SCALE with this instance property.
In `@src/client/render/gl/passes/FalloutLightPass.ts`:
- Line 14: Replace the hardcoded constant LIGHT_TILE_SCALE in FalloutLightPass
with a value read from the render settings: remove or stop using the const
LIGHT_TILE_SCALE and, in the FalloutLightPass constructor, read
settings.lighting.falloutTileScale (from gl/render-settings.json) and store it
on the instance (e.g., this.falloutTileScale) for use where LIGHT_TILE_SCALE was
referenced; also add a new fallback/default in render-settings.json under
"lighting" named "falloutTileScale" so existing behavior is preserved if the
setting is missing.
In `@vite.config.ts`:
- Line 225: The dev server currently hardcodes host: true which always exposes
the server to the network; change the Vite config's host property to be opt-in
via an environment variable (e.g., use process.env.VITE_HOST === "true") and add
a brief comment explaining when to enable it, updating the host assignment in
the exported config object (where host: true is set) so network binding is only
enabled when the env var is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5f003daa-d279-4171-8ab8-2c0e8216a6b8
⛔ Files ignored due to path filters (3)
src/client/render/gl/shaders/day-night/fallout-light.frag.glslis excluded by!**/*.glslsrc/client/render/gl/shaders/fallout-bloom/extract.frag.glslis excluded by!**/*.glslsrc/client/render/gl/shaders/shared/blur.frag.glslis excluded by!**/*.glsl
📒 Files selected for processing (4)
src/client/render/gl/passes/FalloutBloomPass.tssrc/client/render/gl/passes/FalloutLightPass.tssrc/client/render/gl/render-settings.jsonvite.config.ts
evanpelle
changed the title
github-project-automation
Bot
moved this from Triage
to Complete
in OpenFront Release Management
1 participant
Description:
On low-end machines, the fillrate was too high causing framerate to drop. The graphical difference is pretty negligible since fallout & light are meant to be blurred anyways.
Reduces fillrate cost of the fallout bloom and fallout-light passes on low-end GPUs:
mapW/8 × mapH/8(64× fewer fragments). Output is heavily blurred + LINEAR-magnified, so the visual difference is minimal.Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
evan