hardening(ui): drop unbounded GLSL sink + stop WebGL-context rebuild on uniform tweaks (#11088)#11102
Conversation
…on uniform tweaks (#11088) Fix 1: useBackgroundApplyChannel no longer accepts raw GLSL text via the payload 'source' field — presets are the only source of shader code, so a crafted bounded-for GPU bomb (which passes the static gate + GL compile) has no path to the compiler. No shipped broadcaster ever sent 'source' (plugin-app-control sends preset ids only), so the branch was dead but dangerous. Fix 2: ProgrammableShaderBackground splits its single effect. The heavy renderer-build/compile effect is keyed on 'source' only; a separate light effect mutates live uniformDefs.u_*.value (and u_color) in place on uniform/color changes, repainting the static frame under reduced motion. Uniform tweaks no longer tear down the WebGL context (browsers cap ~16 live contexts) or recompile the shader; rendering output is identical. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
|
❌ PR title does not match the required pattern. Please use one of these formats:
|
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
GLSL background hardening (#11088, follow-up to #11083)
Two latent hardening fixes from the post-merge review of the programmable shader background. No rendered-output change → visual audit N/A (Fix 1 removes an unreachable path; Fix 2 renders byte-identically, just without a context teardown).
Fix 1 — drop the unbounded raw-GLSL sink (defense-in-depth)
useBackgroundApplyChannelaccepted a rawsourcefield from anybackground:applybroadcaster (explicitSource). The static gate banswhile/doand caps size but does not boundfor-loop counts, so a craftedfor(int i=0;i<200000;i++)slips past the gate + GL compile and stalls the GPU for one pathological frame (the frame-time watchdog needs 5 consecutive slow frames — the first freezes first).Fix: drop the
explicitSourcebranch entirely — presets are the only source of shader code, so raw text has no path to the compiler. Confirmed no shipped caller broadcasts a rawsourcefield (grep). This is the cleaner root fix (remove the surface) vs. a for-loop heuristic.Fix 2 — stop rebuilding the WebGL context on uniform-only tweaks (perf)
The single build effect was keyed
[source, uniforms, color], so a uniform-only tweak tore down + rebuilt theTHREE.WebGLRenderer+ WebGL context and recompiled the shader (browsers cap ~16 live contexts → churn + GC pressure).Fix: split into a heavy build/compile effect keyed on
sourceonly (compiling againstuniformsRef/colorRefcurrent values) + a lightweight effect keyed on[uniforms, color]that mutatesuniformDefs.u_*.valuein place (with auniformsEqualshort-circuit and a reduced-motion static repaint). The running rAF loop picks up the new values next frame.Evidence (fail-without-fix)
ProgrammableShaderBackground.rebuild.test.tsx(THREE mocked; counts renderer constructions): a uniform/color-only change keepsrendererCount === 1and mutates the live uniform in place; a source change rebuilds. Reverting the dep-split → 2 tests fail (rendererCountclimbs on tweak). Independently re-verified.useBackgroundApplyChannel.test.tsx: the raw-sourcepath is no longer accepted.uniformsEqualexport +u_colorVector3type verified.Refs #11088 #11083 #10694
🤖 Generated with Claude Code