Skip to content

[Merged by Bors] - Fix panic when using globals uniform in wasm builds#6460

Closed
rparrett wants to merge 3 commits intobevyengine:mainfrom
rparrett:fix-wasm-globals
Closed

[Merged by Bors] - Fix panic when using globals uniform in wasm builds#6460
rparrett wants to merge 3 commits intobevyengine:mainfrom
rparrett:fix-wasm-globals

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