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: support depth buffer copies in WebGLBackend.copyFramebufferToTexture #27083

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

aardgoose
Copy link
Contributor

As title.

remove faulty viewport flipping, the test was inverted and the flip is not required for the WebGPUBackend now.
enable screenshots for backdrop node examples.

@aardgoose aardgoose marked this pull request as ready for review October 29, 2023 16:07
@sunag sunag added this to the r159 milestone Oct 30, 2023
@sunag
Copy link
Collaborator

sunag commented Oct 30, 2023

remove faulty viewport flipping, the test was inverted and the flip is not required for the WebGPUBackend now.

Great update in .copyFramebufferToTexture().

Just about the viewport flipping, this code is for WebGLBackend follow the WebGPU specifications, if you run the webgpu_shadertoy example on WebGLBackend it will open the viewport flipped.

image

@aardgoose
Copy link
Contributor Author

aardgoose commented Oct 30, 2023

Hi, The problem I saw is the isFlipY() code isn't working as intended in all cases. isFlipY() returns true for the GLSLNodeBuilder, which inserts code which fails to compile when using the webgpu_backdrop example as a test.

vec2( gl_FragCoord.x, .y - gl_FragCoord.y )

because

const resolution = viewportResolution.build( builder );

just returned an empty string.

Removing the code resulted in the webgpu_backdrop example rendering correctly for both backends, when initially it had rendered with the viewport texture inverted with WebGL (before the recent changes to ViewportNode).

@sunag
Copy link
Collaborator

sunag commented Nov 1, 2023

I understand that this resolves to the webgpu_backdrop example. I am checking the bug that occurred in ViewportNode.
I think we will need to make this inversion in the TextureNode, otherwise we will have an inconsistency problem.

The current code works correctly in webgpu_shadertoy and gives the expected result in both backends,
Let's use this visual reference as test:

material.colorNode = Nodes.viewportTopLeft;
Expected/Current PR
image image

@sunag sunag mentioned this pull request Nov 1, 2023
@aardgoose
Copy link
Contributor Author

rebased on fixed viewport code

@sunag sunag merged commit af6593d into mrdoob:dev Nov 1, 2023
18 checks passed
@sunag
Copy link
Collaborator

sunag commented Nov 1, 2023

Thanks!

@aardgoose aardgoose deleted the depth-copy branch November 2, 2023 11:26
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.

2 participants