Skip to content

Conversation

@mrdoob
Copy link
Owner

@mrdoob mrdoob commented Oct 28, 2025

Related issue: #32114

Description

Port the GGX VNDF (Visible Normal Distribution Function) importance sampling implementation from WebGLRenderer's PMREM to WebGPURenderer's TSL-based PMREM generator.

This implementation provides more accurate environment map prefiltering by using Monte Carlo integration with VNDF importance sampling to represent the GGX BRDF for physically-based rendering.

Changes to PMREMUtils.js:

  • Added GGX VNDF sampling helper functions:
    • radicalInverse_VdC: Van der Corput radical inverse
    • hammersley: Hammersley sequence for quasi-Monte Carlo sampling
    • importanceSampleGGX_VNDF: GGX VNDF importance sampling (Heitz 2018)
    • ggxConvolution: Main convolution function using VNDF sampling

Changes to PMREMGenerator.js:

  • Added GGX_SAMPLES constant (2048 samples)
  • Removed _axisDirections (no longer needed with GGX filtering)
  • Added _ggxMaterial property
  • Replaced blur-based _applyPMREM with GGX filtering
  • Added _applyGGXFilter method for incremental roughness filtering
  • Added _getGGXShader function to create GGX material
  • Updated documentation to reflect GGX VNDF usage

Technical notes:

  • Uses texture() instead of texture(null) for EmptyTexture default
  • Implements incremental roughness filtering to avoid over-blurring
  • Applies blur strength mapping (0.05 + roughness * 0.95) for quality
  • Performs two-pass rendering: pingPong target then back to cubeUV

@mrdoob mrdoob added this to the r181 milestone Oct 28, 2025
@mrdoob mrdoob force-pushed the claude/session-011CUZfd82vQUmMrBBmDS1CQ branch from 8002744 to 38007c4 Compare October 28, 2025 14:26
@github-actions
Copy link

github-actions bot commented Oct 28, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 354.98
86.28
354.98
86.28
+0 B
+0 B
WebGPU 606.33
170
608.96
170.87
+2.63 kB
+876 B
WebGPU Nodes 604.94
169.73
607.57
170.6
+2.63 kB
+873 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 486.63
121.03
486.63
121.03
+0 B
+0 B
WebGPU 675.44
185.38
678.04
186.21
+2.6 kB
+824 B
WebGPU Nodes 617.43
168.61
620.03
169.49
+2.6 kB
+879 B

@mrdoob mrdoob force-pushed the claude/session-011CUZfd82vQUmMrBBmDS1CQ branch from 38007c4 to 4511210 Compare October 28, 2025 14:31
@mrdoob mrdoob marked this pull request as ready for review October 28, 2025 14:44
@mrdoob
Copy link
Owner Author

mrdoob commented Oct 28, 2025

@sunag What do you think?

@mrdoob mrdoob force-pushed the claude/session-011CUZfd82vQUmMrBBmDS1CQ branch from 912b121 to 0e004ba Compare October 28, 2025 15:03
Port the GGX VNDF (Visible Normal Distribution Function) importance
sampling implementation from WebGLRenderer's PMREM to WebGPURenderer's
TSL-based PMREM generator.

This implementation provides more accurate environment map prefiltering
by using Monte Carlo integration with VNDF importance sampling to
represent the GGX BRDF for physically-based rendering.

Changes to PMREMUtils.js:
- Added GGX VNDF sampling helper functions:
  - radicalInverse_VdC: Van der Corput radical inverse
  - hammersley: Hammersley sequence for quasi-Monte Carlo sampling
  - importanceSampleGGX_VNDF: GGX VNDF importance sampling (Heitz 2018)
  - ggxConvolution: Main convolution function using VNDF sampling
- Optimized shader code by removing unnecessary .toVar() calls

Changes to PMREMGenerator.js:
- Added GGX_SAMPLES constant (1024 samples, optimized for performance)
- Removed _axisDirections (no longer needed with GGX filtering)
- Added _ggxMaterial property
- Replaced blur-based _applyPMREM with GGX filtering
- Added _applyGGXFilter method for incremental roughness filtering
- Added _getGGXShader function to create GGX material
- Updated documentation to reflect GGX VNDF usage

Technical notes:
- Uses texture() instead of texture(null) for EmptyTexture default
- Helper functions don't use setLayout (TSL pattern for nested Fn)
- Implements incremental roughness filtering to avoid over-blurring
- Applies blur strength mapping (0.05 + roughness * 0.95) for quality
- Performs two-pass rendering: pingPong target then back to cubeUV
- Reduced sample count to 1024 (vs WebGL's 2048) for better performance
@mrdoob mrdoob force-pushed the claude/session-011CUZfd82vQUmMrBBmDS1CQ branch from 0e004ba to 8541cd1 Compare October 28, 2025 15:04
@mrdoob
Copy link
Owner Author

mrdoob commented Oct 28, 2025

Oh, it may be unstable now. Trying to make it perform faster so the e2e doesn't timeout.

@sunag
Copy link
Collaborator

sunag commented Oct 28, 2025

@sunag What do you think?

The implementation seems perfect. 👏
I would just reduce the number of uses of .toVar() if possible to make the code cleaner.

@sunag
Copy link
Collaborator

sunag commented Oct 28, 2025

Oh, it may be unstable now. Trying to make it perform faster so the e2e doesn't timeout.

It seems that the material is being compiled multiple times even with the ggx material cache 🤔

@mrdoob
Copy link
Owner Author

mrdoob commented Oct 28, 2025

Feel free to continue the PR. It's sleep time for me 🤓

mrdoob and others added 4 commits October 29, 2025 10:47
Performance optimization: removed .toVar() from the s variable in
importanceSampleGGX_VNDF as it is never reassigned and only used
for reading on the next line.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@sunag
Copy link
Collaborator

sunag commented Oct 29, 2025

I'm still investigating, but the performance issue isn't related to the code but to the caching system associated with LightsNode. It will certainly improve performance in the other examples as well.

@mrdoob
Copy link
Owner Author

mrdoob commented Oct 29, 2025

Seems like 2048 is too much for WebGPURenderer.

I'll set it back to 1024 and next month I'll try to profile the renderer and see if I can find any performance improvements.

@mrdoob mrdoob merged commit 4342d10 into dev Oct 29, 2025
10 checks passed
@mrdoob mrdoob deleted the claude/session-011CUZfd82vQUmMrBBmDS1CQ branch October 29, 2025 07:19
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.

4 participants