-
-
Notifications
You must be signed in to change notification settings - Fork 36.1k
WebGPUTextureUtils: Fix readback buffer size. #31765
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
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
| const readBuffer = device.createBuffer( | ||
| { | ||
| size: width * height * bytesPerTexel, | ||
| size: ( height - 1 ) * bytesPerRow + width * bytesPerTexel, // see https://github.com/mrdoob/three.js/issues/31658#issuecomment-3229442010 |
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.
Is this more readable?
( width * bytesPerTexel ) + ( ( height - 1 ) * bytesPerRow )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.
IMO, the "( height - 1 ) * bytesPerRow" bit should come first since "( width * bytesPerTexel )" represents the last row of data.
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.
Then this?
( height - 1 ) * bytesPerRow + ( width * bytesPerTexel )Or even this?
( ( height - 1 ) * bytesPerRow ) + ( width * bytesPerTexel )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 vote for the last one!
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.
Updated: 6c7d807
Definitely improves code clarity 👍 .
|
Thanks @greggman! |
Fixed #31658.
Description
Turns out we have a bug in our readback buffer size computation in the WebGPU backend. We must honor the row padding in the buffer computation except for the last row. Credits to @greggman who explained the fix at the Chromium bug tracker and here: #31658 (comment) 🙌 !