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.

Sign up for GitHub

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

Jump to bottom

RibbonModifier render effects are not applied on Macs #322

Closed
tompuric opened this issue May 13, 2024 · 6 comments · Fixed by #324
Closed

RibbonModifier render effects are not applied on Macs #322

tompuric opened this issue May 13, 2024 · 6 comments · Fixed by #324
Labels
A - internal Internal change on a core system C - bug Something isn't working

Comments

@tompuric
Copy link

Crate versions
bevy version: 0.13.2
bevy_hanabi version: 0.11.0-dev

Testing environments

  • Windows: INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "Windows 10 Pro", kernel: "19045", cpu: "Intel(R) Core(TM) i7-6700 CPU @ 3.40GHz", core_count: "4", memory: "31.9 GiB" }
  • Mac: INFO bevy_diagnostic::system_information_diagnostics_plugin::internal: SystemInfo { os: "MacOS 14.4.1 ", kernel: "23.4.0", cpu: "Apple M1 Pro", core_count: "10", memory: "32.0 GiB" }

Describe the bug

I can't confirm if it's just me, but when running examples that utilise the RibbonModifier on my Mac, the effect is not applied.
All other effects work correctly. No issues on my windows machine.

Expected behavior

I expect to see the same behaviour as shown in the example screenshots and my windows.

To Reproduce

Utilise main branch on Mac.

Screenshots

Running the examples in windows

hanabi-firework
hanabi-ribbon
hanabi-worms

Running the examples in mac

Screenshot 2024-05-14 at 12 44 04 AM Screenshot 2024-05-14 at 12 44 23 AM Screenshot 2024-05-14 at 12 45 04 AM
@djeedai
Copy link
Owner

djeedai commented May 13, 2024

#202 also showed some suspicious behavior on macOS. And there's still some similar issue blocking wasm support. Overall my gut feeling is that I suspect Vulkan and DX are more robust to some subtle bug which makes things go horribly wrong on Metal and WASM. But not sure.

I'll try to have a look on a mac if I can.

@senpaaii
Copy link

Can confirm I repeated the same steps on my M1 Mac and had the exact same results as @tompuric

@djeedai
Copy link
Owner

djeedai commented May 14, 2024

I have some similar repro, I will try to figure out if I can debug the GPU on macOS.

@djeedai
Copy link
Owner

djeedai commented May 14, 2024

Ok I know what's going on, this is a storage buffer alignment issue. Most desktop platforms require a 32-byte alignment, but Metal requires a 256-byte one, and some code ignores that and aligns to 32 bytes always.

@djeedai djeedai added C - bug Something isn't working A - internal Internal change on a core system labels May 15, 2024
djeedai added a commit that referenced this issue May 16, 2024
@djeedai
Handle padding for storage buffers
ab14ad6 
Handle the padding of the `GpuRenderGroupIndirect` structure so that it
complies with the GPU device requirements. This previously worked on
Windows because Vulkan or GPUs there require a 32-byte alignment which
was manually provided (hard-coded in Rust code). However this breaks on
Appled Mx GPUs with Metal, where the min alignment is 256 bytes.

Fixes #322
@djeedai
Copy link
Owner

djeedai commented May 16, 2024 •
edited

Any chance someone can try the u/pad256 branch? @tompuric or @senpaaii? I've fixed several bugs, although the instancing.rs example still exhibits a bug where some particles spawn but don't move (add with SPACE, delete with DELETE). But all the padding ones and some out of bound access in the clone modifier are fixed, and that fixes all other examples for me on macOS.

@tompuric
Copy link
Author

Amazing, it's working ❤️

I've attached some low quality gifs just to showcase it working.

hanabi_ribbon
hanabi_worms
hanabi_firework

@djeedai djeedai mentioned this issue May 18, 2024
Merged
djeedai added a commit that referenced this issue May 18, 2024
@djeedai
Fix storage padding and re-enable buffer clean-up (#324)
eb669f5 
Fix the padding of all storage buffers to comply with the GPU device requirements, which on most Desktop platforms is 32 bytes, but can be different (for example on macOS M1 and later, it's generally 256 bytes). This fixes a number of issues where effects are not rendering at all, or display strong artifacts. The change is likely over-doing it by padding all structures used in arrays, however this can be optimized as a later pass.

Fix a bug in the `CloneModifier` where the shader code was allocating cloned particles without checking the buffer capacity, leading to underflow error and stomping over other parts of the buffer.

Re-enable buffer cleaning up on effect deletion, which was commented out during the ribbon change but not fixed, and left disabled. This caused the buffers to fill, leaving no space for new effects even after some effects were deleting, which resulted in newly spawned effects not updating (code was dispatching 0 workground in update pass).

Fixes #322
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A - internal Internal change on a core system C - bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix storage padding and re-enable buffer clean-up djeedai/bevy_hanabi
3 participants
@tompuric @djeedai @senpaaii