Skip to content

WGSLNodeBuilder: Improve isCustomStruct() checking #30525

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

Merged
merged 3 commits into from
Apr 22, 2025

Conversation

Spiri0
Copy link
Contributor

@Spiri0 Spiri0 commented Feb 14, 2025

#30394 #30487

I recreated the PR because I had big problems with creating the screenshot and the branch was completely destroyed as a result. I was only able to create the screenshot with changes in puppeteer.js and the screenshot still doesn't fit.
The code itself works perfectly. @sunag you have a new node in mind for the structArray anyway. That's why I'm holding off on experimenting with screenshots for the moment

Copy link

github-actions bot commented Feb 14, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 336.33
78.33
336.33
78.33
+0 B
+0 B
WebGPU 547.22
151.68
547.37
151.73
+146 B
+49 B
WebGPU Nodes 546.57
151.52
546.72
151.57
+146 B
+49 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 465.78
112.28
465.78
112.28
+0 B
+0 B
WebGPU 621.61
168.13
621.75
168.18
+146 B
+52 B
WebGPU Nodes 576.48
157.44
576.63
157.49
+146 B
+51 B

@Spiri0
Copy link
Contributor Author

Spiri0 commented Feb 18, 2025

@sunag Is there a way to update parts of a storageBufferAttribute from the CPU side?
The advantage of the CPU is that it can pre-calculate geometries with 64bit precision
That's my naive thought, but that doesn't work.

positionBuffer.array.set( instancePositions, instanceSlot * instancePositions.length );
positionBuffer.needsUpdate = true;

I also had an example with interleaved buffers in mind but that would be a bit more extensive.

@Spiri0 Spiri0 force-pushed the struct_array_ectension2 branch from d48652d to 3376eb8 Compare March 28, 2025 16:24
@Spiri0
Copy link
Contributor Author

Spiri0 commented Mar 28, 2025

@sunag I've removed the example because I can't get the screenshot to work. I have no idea why it keeps going black. It works perfectly when I run the example itself. I also get a black screen with other examples.
This PR works perfectly, as I've been including it in my threejs version for the past three releases to keep my apps running.

I'll probably need this also for my voxelizer in the future. I created it to simulate real buoyancy. With the voxelizer I can calculate the volume of misshapen bodies very well.

https://github.com/Spiri0/Threejs-WebGPU-Voxelizer

I also switched the IFFT ocean to buffers. It works much better that way.

@sunag
Copy link
Collaborator

sunag commented Apr 1, 2025

@Spiri0 I'll check this PR this weekend. Thanks!

@Spiri0
Copy link
Contributor Author

Spiri0 commented Apr 12, 2025

@sunag The extension in src/nodes/core/StructNode.js doesn't even have anything to do with the structArray. The extension in src/renderers/webgpu/nodes/WGSLNodeBuilder.js is sufficient for structArrays. I just find it useful to read the number of members of a struct elsewhere. I use it. This way I avoid unnecessary hard coding.

@sunag sunag added this to the r176 milestone Apr 22, 2025
@sunag
Copy link
Collaborator

sunag commented Apr 22, 2025

@Spiri0 Can you confirm if with this change works for you?

@Spiri0
Copy link
Contributor Author

Spiri0 commented Apr 22, 2025

@Spiri0 Can you confirm if with this change works for you?

Yes, that looks good, it's working. I see you solved it elsewhere in the code, interesting.

@sunag
Copy link
Collaborator

sunag commented Apr 22, 2025

Yes, it should be useful for other functions like builder.getPropertyName(). Thanks for testing and sorry for the delay.

@sunag sunag changed the title reworked struct array example WGSLNodeBuilder: Improve isCustomStruct() checking Apr 22, 2025
@sunag sunag merged commit 0eddad5 into mrdoob:dev Apr 22, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants