Skip to content
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 by Bors] - Fix panic when using globals uniform in wasm builds #6460

Closed
wants to merge 3 commits into from

Conversation

rparrett
Copy link
Contributor

@rparrett rparrett commented Nov 3, 2022

Objective

Fixes #5393

Solution

  • Add padding to GlobalsUniform / Globals to make it 16-byte aligned.

Still not super clear on whether this is a naga thing or an encase thing or what. But now that we're offering globals up to users and #5393 is not just breaking an example, maybe we should do this sort of workaround?

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen O-Web Specific to web (WASM) builds labels Nov 3, 2022
@mockersf
Copy link
Member

mockersf commented Nov 4, 2022

If this is only needed on wasm, could you set a cfg/shaderdef that is only added in wasm?

Copy link
Contributor

@IceSentry IceSentry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah, I forgot about alignment when I made that PR, good catch

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Nov 6, 2022
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Nov 7, 2022
# Objective

Fixes #5393 

## Solution

- Add padding to `GlobalsUniform` / `Globals` to make it 16-byte aligned.

Still not super clear on whether this is a `naga` thing or an `encase` thing or what. But now that we're offering `globals` up to users and #5393 is not just breaking an example, maybe we should do this sort of workaround?
@bors bors bot changed the title Fix panic when using globals uniform in wasm builds [Merged by Bors] - Fix panic when using globals uniform in wasm builds Nov 7, 2022
@bors bors bot closed this Nov 7, 2022
bors bot pushed a commit that referenced this pull request Nov 18, 2022
# Objective

- Fix a panic in wasm when using globals in a shader

## Solution

- Similar to #6460
cart pushed a commit that referenced this pull request Nov 30, 2022
# Objective

- Fix a panic in wasm when using globals in a shader

## Solution

- Similar to #6460
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

Fixes bevyengine#5393 

## Solution

- Add padding to `GlobalsUniform` / `Globals` to make it 16-byte aligned.

Still not super clear on whether this is a `naga` thing or an `encase` thing or what. But now that we're offering `globals` up to users and bevyengine#5393 is not just breaking an example, maybe we should do this sort of workaround?
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Fix a panic in wasm when using globals in a shader

## Solution

- Similar to bevyengine#6460
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior O-Web Specific to web (WASM) builds S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wasm: animate_shader example and other uses of GlobalsUniform panic with validation error
5 participants