-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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: Allow specifying texture index for MRT in readRenderTargetPixelsAsync()
.
#28197
Conversation
The readRenderTargetAsync function accepts a renderTarget, but so far has no option to choose between the different textures of a renderTarget if it has more than just one texture. That's why I added an index = 0 so that everyone who has used the function so far has no disadvantages because the first texture is always selected by default.
Clean up.
readRenderTargetPixelsAsync()
.
|
||
return this.backend.copyTextureToBuffer( renderTarget.texture, x, y, width, height ); | ||
return this.backend.copyTextureToBuffer( renderTarget.textures[ index ], x, y, width, height ); |
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.
Does this code...actually work? At least the WebGL backend won't work until gl.readBuffer()
is used in WebGLTextureUtils.copyTextureToBuffer()
.
I'm not sure about required workflow in WebGPU. Do you mind demonstrating the updated method with an example? E.g. you could use the code from https://threejs.org/examples/webgpu_multiple_rendertargets but instead of rendering to the screen, you extract (color and normal) values via readRenderTargetPixelsAsync()
.
I made an example. Since this is my first one I must have missed something in the PR. It works for me. #28198
The textures read back from the GPU were created with a smaller render target. Users have to be careful with readRenderPixelsAsync because the CPU is not intended to receive large amounts of data from the GPU. Maybe a size selection button in the GUI would be nice for different readback sizes? |
Okay, then it seems to work for WebGPU but not WebGL. However, that can be fixed at a later point as well. @aardgoose Does this change look good to you? |
It looks fine, actually it works with WebGL. The texture copied is always mapped to color attachment 0 so no need for readBuffer() changes. |
But that means it's not possible to read subsequent color attachments, just 0, right? So in the demo it should not be possible to extract normal values. |
It works for both, it maps the chosen texture to a temporary framebuffer as attachment 0, I've tested the path and demo to check. |
@Mugen87 In the third screenshot you can see the normal texture read back. In this case, index 0 is then set to 1 (line 218 reads back the normal ones). I now see that it would be better if the respective readback were in the respective selector, then both would not always be read back but only the selected one. |
Not sure what you mean by that. Do you mind rephrasing? |
readRenderTargetPixelsAsync()
. readRenderTargetPixelsAsync()
.
Let's talk about this issue in #28198. |
Related: #22403
The readRenderTargetAsync function accepts a renderTarget, but so far has no option to choose between the different textures of a renderTarget if it has more than just one texture. That's why I added an index = 0 so that everyone who has used the function so far has no disadvantages because the first texture is always selected by default.