-
-
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
Examples: Added webgpu_multiple_rendertargets_readback
#28211
Conversation
I removed the .html at the end of "webgpu_mrt_readback.html", now: "webgpu_mrt_readback",
I'll improve the example with time. Although it works exactly as it should, an example should also be as efficient as possible in order to convey an efficient programming style. I would also like to add a size selection so that you can see different readback resolutions. |
Oh, has the extension of the readRenderTargetPixelsAsync already been taken into account where you are testing it? This doesn't work without #28197 |
better code. Previously, new instances were constantly being created, which was unnecessary and inefficient.
examples/webgpu_mrt_readback.html
Outdated
if(selector == 2){ | ||
const pixelBuffer0 = await renderer.readRenderTargetPixelsAsync(readbackTarget, 0, 0, width, height, 0); //zero is optional | ||
|
||
const pixelBufferTexture0 = new THREE.DataTexture(pixelBuffer0, 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.
Seems that DataTexture
and Material
are created in each frame too
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.
Ah, something else slipped through my fingers. Yes, of course that shouldn't be the case
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.
Now it looking good. Have you meanwhile seen the readback_Normal with taking #28197 into account?
I took the opportunity to make the example a little better. I then deactivated the #28197 extension and then the readback_Normal didn't work for me either. So the same result as yours. That will definitely be the cause for you too. |
Now no new instances are created during the update, only data is updated as it should be
Now no new instances are created during the update, only data is updated as it should be
Now no new instances are created during the update, only data is updated as it should be
The normal readback does not work for me as well. Please make sure to rebase the branch so it is based on latest dev. You should not expect from reviewers to copy over changes from other branches. This branch must be fully functional so it can be properly reviewed. |
examples/webgpu_mrt_readback.html
Outdated
const width = readbackTarget.width; | ||
const height = readbackTarget.height; | ||
|
||
if(selector == 2){ |
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.
The formatting of the code is still not correct. Please make sure ESLint properly lints the code according to the project's code style.
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.
My apologies, I thought I did that with the rebase. This is still new for me and I admit there is still a lot I don't know about github.
By formatting do you mean
if( selector == 2 ) {
instead of
if(selector == 2){
?
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.
Yes. And ideally use ===
for strict comparison.
Don't worry if you need time to get things going.
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.
Thanks for your understanding. I can easily solve the formatting issue. About the rebase, is there anyone I can ask how I do it correctly?
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.
You can use the merge or rebase command. Normally you start by fetching the latest commit from the upstream repository:
git fetch upstream
You can then merge the latest changes on dev
into your branch:
git merge upstream/dev
Or perform a rebase:
git rebase upstream/dev
The are many resources online which explain the difference between merge and rebase. Both have their pros and cons. However, rebase is ideal for situations when you want to move the point where you have branched to a new starting point. This is exactly what you are looking for in this instance.
corrected formatting and strict comparison
corrected formatting and strict comparison
corrected formatting and strict comparison
corrected formatting and strict comparison
I won't get back to it until tonight as I'm at work. |
I made some revisions, I think it's ok now. Thank you for the example and fixes :) |
webgpu_multiple_rendertargets_readback
Oh, merged? I haven't done the rebase yet. |
This is a new example that demonstrates the use of multiple render targets in conjunction with the readRenderTargetPixelsAsync. The readRenderTargetPixelsAsync function was extended by an index with PR #28197 in order to be able to read any texture from a multiple renderTarget
Related issue: #28197