Skip to content
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

WebGPURenderer: Depth Pixel & Logarithmic Depth Buffer #27243

Merged
merged 2 commits into from
Nov 25, 2023

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Nov 24, 2023

Example
https://raw.githack.com/mrdoob/three.js/dev/examples/webgpu_camera_logarithmicdepthbuffer.html

We introduce an OutputStruct, we added depthPixel which is equivalent to gl_FragDepth, it can be used as assign, e.g: depthPixel.assign( value ) or to include in some WGSL Function, e.g:

import { wgslFn, depthPixel } from 'three/nodes';

const positionTransformFn = wgslFn( `
	fn positionTransformFn( value: f32 ) -> void {

		output.depth = value;

	}
`, [ depthPixel ] );
  • Support to Renderer.logarithmicDepthBuffer
  • NodeMaterial.depthNode = node To set the depth buffer using nodes.
  • WebGLBackend

@sunag sunag added this to the r159 milestone Nov 24, 2023
@sunag sunag marked this pull request as ready for review November 24, 2023 22:38
@sunag sunag merged commit 549a2e3 into mrdoob:dev Nov 25, 2023
11 checks passed
@sunag sunag deleted the dev-depth-pixel branch November 25, 2023 05:26
sunag added a commit that referenced this pull request Nov 25, 2023
export const depth = nodeImmutable( ViewportDepthNode, ViewportDepthNode.DEPTH );
export const depthTexture = nodeProxy( ViewportDepthNode, ViewportDepthNode.DEPTH_TEXTURE );
export const depthPixel = nodeImmutable( ViewportDepthNode, ViewportDepthNode.DEPTH_PIXEL );

depthPixel.assign = ( value ) => depthPixelBase( value );
Copy link
Contributor

Choose a reason for hiding this comment

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

Just depthPixel.assign = depthPixelBase maybe?

return cameraProjectionMatrix.mul( modelViewMatrix ).mul( this.positionNode );
if ( builder.shaderStage === 'fragment' ) {

return varying( builder.context.mvp );
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a bit of a hack to me -- I propose instead to introduce a property like vertexOutput which can be used here (and remove builder.context.mvp then). @sunag What do you think?

AdaRoseCannon pushed a commit to AdaRoseCannon/three.js that referenced this pull request Jan 15, 2024
* WebGPURenderer: Depth Pixel & Logarithmic Depth Buffer

* Update puppeteer.js
AdaRoseCannon pushed a commit to AdaRoseCannon/three.js that referenced this pull request Jan 15, 2024
const renderer = new WebGPURenderer( { antialias: true, logarithmicDepthBuffer: logDepthBuf } );
renderer.setPixelRatio( window.devicePixelRatio );
renderer.setSize( SCREEN_WIDTH / 2, SCREEN_HEIGHT );
renderer.setAnimationLoop( render );
Copy link
Collaborator

@Mugen87 Mugen87 Apr 30, 2024

Choose a reason for hiding this comment

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

While migrating the examples towards setAnimationLoop() I have noticed an issue in this example. The render() function updates both renderers but since setAnimationLoop() is called twice, the frame rate is double as high as expected.

How are apps supposed to use more than one WebGPURenderer with a single global animation loop?

AFAIK, if the example calls setAnimationLoop() only for a one renderer, the internal state of the second one isn't correct (since NodeFrame.update() isn't called and renderer.info isn't updated).

If the example uses an animation loop per renderer, which animation loop does it use to update its "global state"? No matter which one you use, the second animation loop might use frame-late data.

Maybe we need a way to trigger the update logic in the Animation class from outside somehow...

Edit: This is not only issue with multiple renderers. Consider an app that manages its animation loop separately and can't use setAnimationLoop(). How does it update the state of the renderer?

Copy link
Owner

Choose a reason for hiding this comment

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

I'm also battling a bit with this problem as I was planning to introduce renderer.xr.setAnimationLoop() (Quest Browser can have immersive + non immersive at the same time).

Where do the physics run? 🤔

We'll probably have to create an issue for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

..If the example uses an animation loop per renderer, which animation loop does it use to update its "global state"? No matter which one you use, the second animation loop might use frame-late data...

This should not be the expected result, Animation has a frame counter per Renderer, regardless of the user using setAnimationLoop() as would happen in WebGLRenderer, this ensures that the user can still use requestAnimationFrame without harming the events per frame in the Nodes.

While migrating the examples towards setAnimationLoop() I have noticed an issue in this example. The render() function updates both renderers but since setAnimationLoop() is called twice, the frame rate is double as high as expected.

I think this is behavior is correct if two renderers depend on using setAnimationLoop(), both must be isolated, and this conflict of Node updates should not exist as we have an Animation and NodeFrame class per Renderer in WebGPURenderer. It could just use one setAnimationLoop or one requestAnimationFrame for multiples Renderers, if this doesn't work it could be a bug.

Although it is an unusual approach to use more than one device, practically no engine duplicates devices, perhaps the user could solve this with RenderTargets, scissor, etc. if the renderer is working and declared in the same thread, in case of different threads having independent setAnimationLoop should be more useful.

I'm also battling a bit with this problem as I was planning to introduce renderer.xr.setAnimationLoop() (Quest Browser can have immersive + non immersive at the same time).

Shouldn't Renderer.setAnimationLoop() manage these calls internally, and we only have one function for this event?

Copy link
Collaborator

@Mugen87 Mugen87 May 1, 2024

Choose a reason for hiding this comment

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

Okay, I've tried to fix the demo in #28249.

What I'm unsure about:

  • Since Animation isn't used now, how is info.reset() called? Does the app needs to do this?
  • Is the missing call in nodeFrame.update() in Animation an issue? Does the app has to call this method as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since Animation isn't used now, how is info.reset() called? Does the app needs to do this?

I was considreing the user customize the Info if needed, so he could create new classes for this #27889 (comment)

What do you think about share the info between renderers?

const info  = new Info(); // it can be a customized Info class too

renderer1.info = info;
renderer2.info = info;

Is the missing call in nodeFrame.update() in Animation an issue? Does the app has to call this method as well?

It continued to be updated, Animation class of WebGPURenderer have a requestAnimationFrame() internal to sync the frame events, it's canceled if the user dispose the renderer.

const update = ( time, frame ) => {
this.requestId = self.requestAnimationFrame( update );
if ( this.info.autoReset === true ) this.info.reset();
this.nodes.nodeFrame.update();
this.info.frame = this.nodes.nodeFrame.frameId;
if ( this.animationLoop !== null ) this.animationLoop( time, frame );
};

Copy link
Collaborator

@Mugen87 Mugen87 May 2, 2024

Choose a reason for hiding this comment

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

Um, my feeling is the update loop in Animation should only run if the user does use setAnimationLoop().

If the user manages the animation loop on app level and not via the renderer, the user should tell the renderer when to update its internal frame counter, the timer and other frame related components.

A separate Info object which is managed on app level and injected into the renderer (or multiple renderers) could be indeed one solution.

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