unique but stable buffer node ids #32531
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
…hader so use builder.uniforms.index as other uniforms do too
|
|
||
| super.setup( builder ); | ||
|
|
||
| const index = builder.uniforms.index ++; |
There was a problem hiding this comment.
I'm not sure it's safe to increase builder.uniforms.index at this point. This property is solely maintained by the node builder. Modifying it outside of the builder could cause unexpected behavior.
There was a problem hiding this comment.
I originally wanted to use builder.context. See: BufferNode.js#L131
Unfortunately builder.context gets reset somewhere between vertex and fragment shader. Do you have any place where to store a counter that works during the complete shader build process but gets reset for a new build?
There was a problem hiding this comment.
In a way builder.uniforms.index gets misused already in getBufferAttributeFromNode for attributes.
How about abstracting builder.uniforms.index away to a method on NodeBuilder that provides a unique id for uniforms, attributes and also BufferNode?
There was a problem hiding this comment.
I need a closer look until I can see more about this issue. I get the point that blindly using the node id is problematic since structural equal programs become differently, leading to redundant shaders.
2 participants
Related issue: #32524
Fixed #32524 by providing unique and stable buffer node ids.