Skip to content

TSL: Ensure memory alignment for struct() #31151

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 8 commits into from
May 23, 2025
Merged

TSL: Ensure memory alignment for struct() #31151

merged 8 commits into from
May 23, 2025

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented May 23, 2025

Related issue: #31146, #30983 (comment)

Description

Ensure memory alignment for struct(). adds more robust verification.

/cc @holtsetio

@sunag sunag changed the title TSL: Ensure memory alignment for 'struct()' TSL: Ensure memory alignment for struct() May 23, 2025
Copy link

github-actions bot commented May 23, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 337.26
78.64
337.26
78.64
+0 B
+0 B
WebGPU 550.64
152.68
551.16
152.84
+522 B
+162 B
WebGPU Nodes 549.99
152.53
550.51
152.69
+522 B
+163 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 468.12
113.14
468.12
113.14
+0 B
+0 B
WebGPU 625.95
169.45
625.8
169.42
-151 B
-33 B
WebGPU Nodes 580.8
158.77
580.65
158.74
-151 B
-34 B

@sunag sunag marked this pull request as ready for review May 23, 2025 03:33
@mrdoob mrdoob added this to the r177 milestone May 23, 2025
@holtsetio
Copy link
Contributor

I can still see one edge case with wrong alignment:

struct({ 
  a: 'mat3',
  b: 'float'
})

Because each mat3 occupies 12 byte in wgpu memory, this struct would need 16 bytes, but with the code from this PR only 12 bytes would be allocated.

Also to be pedantic, line 116 of StructTypeNode.js:
offset += ( chunkOffset % boundary );
should be
offset += ( boundary - chunkOffset );
I think, right?
However since the only case I can think of where this line get executed is for chunkOffset = 4 and boundary = 8, in the case of vec2s, so the result is the same.

@holtsetio
Copy link
Contributor

New approach looks good to me! :)

@sunag sunag merged commit c9bb800 into mrdoob:dev May 23, 2025
12 checks passed
@sunag sunag deleted the dev-align branch May 23, 2025 18:53
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.

3 participants