-
-
Couldn't load subscription status.
- Fork 36.1k
WebGPU: Logarithmic Depth Buffer Rename/Revision + GTAONode Fixes #29870
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WebGPU: Logarithmic Depth Buffer Rename/Revision + GTAONode Fixes #29870
Conversation
…ZToLogarithmicDepth' and modified it to expect a negative viewZ value in order maintain consistency with 'viewZToOrthographicDepth' and 'viewZToPerspectiveDepth' 2. Added function 'logarithmicDepthToViewZ' 3. Fixed AO not working when 'logarithmicDepthBuffer' is true
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
Clean up.
Clean up.
|
@PoseidonEnergy Tested the change locally and it works as expected! Awesome PR 👍 ! |
Add `setGroup()`.
|
@PoseidonEnergy Your fiddle for testing the TSL functions is super interesting! Somewhat related: The project is actually looking for a way to unit test TSL functions (see #28708). It would be great to establish a basic procedure in our current unit tests (see https://github.com/mrdoob/three.js/tree/dev/test/unit) for testing TSL modules in isolated routines. |
| // TODO: How do we get the cameraNear/cameraFar nodes to use the shadow camera so we don't have to declare local ones here? | ||
| const cameraNearLocal = uniform( 'float' ).onRenderUpdate( () => shadow.camera.near ); | ||
| const cameraFarLocal = uniform( 'float' ).onRenderUpdate( () => shadow.camera.far ); | ||
| const cameraNearLocal = reference( 'near', 'float', shadow.camera ).setGroup( renderGroup ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one question, did the previous approach using uniform present any problems? Was there any specific reason for changing to reference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did only switch to reference() for consistency reasons since this API is used in the rest of the file.
Fixed #29797.
perspectiveDepthToLogarithmicDepthtoviewZToLogarithmicDepthand modified it to expect a negativeviewZvalue in order maintain consistency withviewZToOrthographicDepthandviewZToPerspectiveDepthSee new curves here on Desmos: https://www.desmos.com/calculator/uyqk0vex1u
logarithmicDepthToViewZfor use in GTAONode.jslogarithmicDepthBufferistrueRelated issue: #29797
Description
The name of the function
perspectiveDepthToLogarithmicDepthis misleading because it actually expects aviewZvalue (i.e.positionView.z), and NOT a depth value (i.e. a number between 0 and 1). In addition, theviewZvalue that it previously expected had to be a positive number, which differed from its sister functionsviewZToOrthographicDepthandviewZToPerspectiveDepth, which both expect a negativeviewZ. In this pull request, the function in question is now namedviewZToLogarithmicDepth, and expects a negativeviewZlike the rest in its family.AO was not working properly in the webgpu_postprocessing_ao example when
logarithmicDepthBufferwastrue. The functionlogarithmicDepthToViewZwas added to ViewportDepthNode.js, so it can now be used in GTAONode.js to obtain the proper depth values required for AO.Here is a JSFiddle I made to demonstrate how
viewZToLogarithmicDepthrelates to the rest of the depth functions:https://jsfiddle.net/go6f2y4u
And here is an animation of the JSFiddle: