-
-
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: Fix vec2
and vec3
for storageObject
in StorageBufferNode
#27697
Conversation
Hm... Would it be beneficial to re-add the example from #25273? |
The way we were packing the data was wrong as it was actually a lot more difficult than I thought to handle overlapping values when packing everything into
and Also if we were keep going on that unpacking logic via complex I fixed it by packing the datas directly to their proper formats ( const { itemSize } = attribute;
const channel = '.' + vectorComponents.join( '' ).slice( 0, itemSize );
const uvSnippet = `ivec2(${indexSnippet} % ${ propertySizeName }, ${indexSnippet} / ${ propertySizeName })`;
const snippet = this.generateTextureLoad( null, textureName, uvSnippet, null, '0' );
//
this.addLineFlowCode( `${ propertyName } = ${ snippet + channel }` ); Also I believe this will drastically the code in the shader and even slightly improve performances. Since the /cc @sunag |
vec2
and vec3
for storageObject
in StorageBufferNode
What do you think about guide the user to create buffers with padding similar to what happens in std140 layout, and deal with |
Hum, I think it would complexify things for developers as difficult as using interleaved array buffers (no one like these things 😄). Also maybe I'm missing something here? By the way is it antipattern to try to read a buffer through
Or should we add something like Currently all examples are using |
I imagined something simpler for this, similar to the way |
@RenaudRohlinger Is this happening to you too? It seems like a loss of data on the part of the WebGPU (happens after a few seconds ) |
I think an intermediate Node in |
|
||
} else if ( itemSize === 3 ) { | ||
|
||
format = 6407; // patch since legacy doesn't use RGBFormat for rendering but here it's needed for packing optimization |
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.
@Mugen87 Maybe we can introduce the RGBFormat
constant back?
Thanks @sunag! I actually just set back the
Try to replace any compute example ( Regarding the decision to replace vec3 with vec4, I don't quite understand the logic behind it in terms of optimization. Using vec4 will only result in a larger ArrayBuffer and consume more memory unnecessarily if the user needs to utilize a vec3. About the precision issue in the WebGPU side I had it when using non power-of-2 array buffers. Maybe the issue is GPU related and related to how I weirdly fetch indexes in |
I've made some significant updates to how we handle data packing into textures for Pixel Buffer Object fallback of storage buffers.
Previously, we were tackling this with RGBA channels, but it turned out to be broken and doing like so would make things more complex than necessary, especially with overlapping values. For example when extracting vec2 and vec3 from texels, with vec3 data overlapping across texels making the code hard to maintain and scale.
To make things easier we're now packing data directly into their proper formats (Red, RG, RGB, RGBA), which makes the code much cleaner and more straightforward for all cases. Might even bring a small boost performance.
Also updated the example to improve E2E testing:
https://raw.githack.com/renaudrohlinger/three.js/improve-test/examples/webgpu_storage_buffer.html
This contribution is funded by Utsubo