Skip to content

WebGLRenderer: Save/restore textureUnits on render state stack#33217

Merged
Mugen87 merged 2 commits intomrdoob:devfrom
SSopin:33207-pmremgenerator-texture
Mar 19, 2026
Merged

WebGLRenderer: Save/restore textureUnits on render state stack#33217
Mugen87 merged 2 commits intomrdoob:devfrom
SSopin:33207-pmremgenerator-texture

Conversation

@SSopin
Copy link
Copy Markdown
Contributor

@SSopin SSopin commented Mar 19, 2026

Fixed #33207

Description

Fixes nested render calls (e.g. PMREMGenerator inside setProgram)
corrupting the shared textureUnits counter, which caused
GL_INVALID_OPERATION errors on shadow map sampler uniforms.

This contribution is funded by Wearitar

  Related issue: mrdoob#33207
  Fixes nested render calls (e.g. PMREMGenerator inside setProgram)
  corrupting the shared textureUnits counter, which caused
  GL_INVALID_OPERATION errors on shadow map sampler uniforms.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 19, 2026

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 359.97
85.53
360.15
85.57
+178 B
+41 B
WebGPU 632.13
175.39
632.13
175.39
+0 B
+0 B
WebGPU Nodes 630.25
175.1
630.25
175.1
+0 B
+0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 492.17
120.11
492.34
120.15
+178 B
+38 B
WebGPU 704.56
190.19
704.56
190.19
+0 B
+0 B
WebGPU Nodes 653.78
177.6
653.78
177.6
+0 B
+0 B

Copy link
Copy Markdown

@travisbreaks travisbreaks left a comment

Choose a reason for hiding this comment

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

Clean fix for a real bug. Nested render calls (PMREMGenerator, render-to-texture, shadow maps) corrupting the shared textureUnits counter is exactly the kind of issue that manifests as intermittent GL_INVALID_OPERATION errors that are hard to track down.

The save/restore pattern on the render state stack is the correct approach.

One observation:

Fallback behavior

textures.setTextureUnits( currentRenderState.state.textureUnits || 0 );

The `|| 0` fallback silently defaults to 0 if `textureUnits` is undefined (i.e., the render state was created before this change, or through a code path that skips the save). This is safe at runtime, but it masks a potential bug: if a render state somehow lacks `textureUnits`, silently resetting to 0 could cause incorrect texture bindings in the restored context.

Consider whether this should be initialized explicitly in the render state constructor/init instead:

currentRenderState.init( camera );
// textureUnits would already be defined as 0 from init

That way the `|| 0` becomes truly unnecessary rather than a silent safety net.

Minor point, not a blocker. The fix itself is correct and well-scoped.

Address review feedback: explicitly initialize textureUnits in
WebGLRenderState and remove the || 0 fallback to avoid silently
masking potential bugs.
@SSopin
Copy link
Copy Markdown
Contributor Author

SSopin commented Mar 19, 2026

Clean fix for a real bug. Nested render calls (PMREMGenerator, render-to-texture, shadow maps) corrupting the shared textureUnits counter is exactly the kind of issue that manifests as intermittent GL_INVALID_OPERATION errors that are hard to track down.

The save/restore pattern on the render state stack is the correct approach.

One observation:

Fallback behavior

textures.setTextureUnits( currentRenderState.state.textureUnits || 0 );

The || 0 fallback silently defaults to 0 if textureUnits is undefined (i.e., the render state was created before this change, or through a code path that skips the save). This is safe at runtime, but it masks a potential bug: if a render state somehow lacks textureUnits, silently resetting to 0 could cause incorrect texture bindings in the restored context.

Consider whether this should be initialized explicitly in the render state constructor/init instead:

currentRenderState.init( camera );
// textureUnits would already be defined as 0 from init

That way the || 0 becomes truly unnecessary rather than a silent safety net.

Minor point, not a blocker. The fix itself is correct and well-scoped.

Fixed.

@Mugen87 Mugen87 added this to the r184 milestone Mar 19, 2026
@Mugen87 Mugen87 merged commit acc8bd1 into mrdoob:dev Mar 19, 2026
10 checks passed
@SSopin SSopin deleted the 33207-pmremgenerator-texture branch March 19, 2026 18:54
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.

PMREMGenerator corrupts texture-unit allocator during first render with shadows

3 participants