-
-
Notifications
You must be signed in to change notification settings - Fork 35.6k
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: StorageBufferNode
Support reading external elements in the WebGL Backend
#27661
Merged
RenaudRohlinger
merged 37 commits into
mrdoob:dev
from
RenaudRohlinger:utsubo/feat/storage_pbo
Feb 6, 2024
Merged
Changes from 4 commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
04c41e8
init pbo
RenaudRohlinger b4b82eb
remove flag
RenaudRohlinger 3a0b66a
regenerate live example
RenaudRohlinger 62238bc
single buffer and alternate for example
RenaudRohlinger 64dee72
cleanup, no idea about circ dep
RenaudRohlinger 9d32e79
test fix circular
RenaudRohlinger e089954
cleanup
RenaudRohlinger 9738dd4
moved pbo management to GLSLNodeBuilder
RenaudRohlinger 9cbafeb
fix screenshots
RenaudRohlinger 8072cc5
Merge remote-tracking branch 'upstream/dev' into utsubo/feat/storage_pbo
RenaudRohlinger 90bf43a
support more ranges
RenaudRohlinger 0f5d614
format dynamically in arrayelementnode
RenaudRohlinger b1c595b
init vertex buffer allocation to prevent issues with some backends
RenaudRohlinger 71a5311
support different type size and update example
RenaudRohlinger a6c5b64
fix files.json
RenaudRohlinger 0b6b1a8
puppeteer need webgpu support
RenaudRohlinger fbb4d63
unbind post pbo
RenaudRohlinger 15033d7
add TODO
RenaudRohlinger affd3bc
Merge branch 'dev' into pr/27661
sunag cbb95e2
cleanup
sunag 80f9766
Move to StorageArrayElementNode
sunag daf8f38
cleanup
sunag 05c53b2
rev
sunag cf55995
add increaseUsage
sunag 17ac24c
Update StorageArrayElementNode.js
sunag 356b712
fixes optmization and revisions
sunag 1c89f4d
revision
sunag 74c6ac4
improved example for E2E tests
RenaudRohlinger 1a22b0b
WIP: Test (1)
sunag de9b203
handle compute with non-PBO -> PBO case
RenaudRohlinger b47480e
revert WIP
sunag c9a21ea
TSL: add `storageObject`
sunag 00f6a5f
revision
sunag 7687fcc
make sure attributeData gets pbo attached in GLSLNodeBuilder
RenaudRohlinger 39691a5
no need transfer anymore
RenaudRohlinger 8936a37
revert compute_texture
RenaudRohlinger 474c0f7
remove copyBufferToSubBuffer
sunag File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,6 @@ | ||
import { texture } from '../Nodes.js'; | ||
import Node, { addNodeClass } from '../core/Node.js'; | ||
import { DataTexture, RGBAFormat, FloatType } from 'three'; | ||
|
||
class ArrayElementNode extends Node { // @TODO: If extending from TempNode it breaks webgpu_compute | ||
|
||
|
@@ -19,13 +21,75 @@ class ArrayElementNode extends Node { // @TODO: If extending from TempNode it br | |
|
||
} | ||
|
||
generate( builder ) { | ||
setup( builder ) { | ||
|
||
if ( this.node.isStorageBufferNode && ! builder.isAvailable( 'storageBuffer' ) ) { | ||
|
||
if ( ! this.node.instanceIndex ) { | ||
|
||
const attribute = this.node.value; | ||
|
||
if ( attribute.pbo === undefined ) { | ||
|
||
const square = Math.sqrt( attribute.array.length / 4 ); | ||
const width = Math.floor( square ); | ||
const height = Math.ceil( square ); | ||
|
||
const pboTexture = new DataTexture( attribute.array, width, height, RGBAFormat, FloatType ); | ||
pboTexture.needsUpdate = true; | ||
pboTexture.isPBOTexture = true; | ||
const pbo = texture( pboTexture ); | ||
pbo.setPrecision( 'high' ); | ||
|
||
attribute.pboNode = pbo; | ||
attribute.pbo = pbo.value; | ||
|
||
|
||
} | ||
|
||
} | ||
|
||
|
||
} | ||
|
||
super.setup( builder ); | ||
|
||
|
||
} | ||
|
||
generate( builder, output ) { | ||
|
||
const nodeSnippet = this.node.build( builder ); | ||
const indexSnippet = this.indexNode.build( builder, 'uint' ); | ||
|
||
if ( this.node.isStorageBufferNode && ! builder.isAvailable( 'storageBuffer' ) ) { | ||
|
||
|
||
const attribute = this.node.value; | ||
|
||
if ( attribute.pboNode ) { | ||
|
||
const nodeUniform = builder.getUniformFromNode( attribute.pboNode, 'texture', builder.shaderStage, builder.context.label ); | ||
|
||
const propertyNameTexture = builder.getPropertyName( nodeUniform ); | ||
|
||
const snippet = /* glsl */` | ||
texelFetch( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can also move to |
||
${propertyNameTexture}, | ||
ivec2( | ||
${indexSnippet} / uint(4) % uint(textureSize(${propertyNameTexture}, 0).x), | ||
${indexSnippet} / (uint(4) * uint(textureSize(${propertyNameTexture}, 0).x)) | ||
), | ||
0 | ||
)[${indexSnippet} % uint(4)]`; | ||
|
||
return output !== 'assign' ? snippet : nodeSnippet; | ||
|
||
} | ||
|
||
|
||
|
||
|
||
return nodeSnippet; | ||
|
||
} | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 circular-reference problem is because of
UniformNode
inArrayElementNode
,UniformNode
will useShaderNode
andShaderNode
will include tryArrayElementNode
again.I think it would be better to move this uniform creation block to
GLSLNodeBuilder
something likebuilder.getPBOUniform()
for example, this could fix the circular reference problem 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.
I think we have to consider a new Node, similar to what was done in
BufferAttributeNode
andbuffer.toAttribute()
as usage. It could bebuffer.toTexture()
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 agree, this feels like something that should better be placed in GLSLNodeBuilder.
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.
Also consider add/move it to
StorageBufferNode.element()
to avoid checks a little, and return and new Node to keep operations more organized, it can even be aStorageArrayElementNode
.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 the feedbacks!
The downside of
buffer.toTexture()
would be that this implies that somehow the developer would expect that something happens in theWebGPU
Backend too.This feature is more like a patch to fix the fact WebGL cannot access other element of array buffers rather than a new feature such as
toAttribute()
.I think I can try to move all this part to
GLSLNodeBuilder
but I'm not sure on how to create a Node here. Maybe once everything works we can think about creating a new Node specific toPixel Buffer Object
, but once again it's only a WebGL thing.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'll be happy to help you with this, let me know when the PR is ready for review so I can analyze it again.
About the example what do you think about renaming it to
webgpu_storage_sorting
?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.
Sounds good to me! Thanks a lot for this 😊