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

If and when is VERTEX_WRITABLE_STORAGE needed? #317

Open
sharkdp opened this issue Apr 9, 2024 · 3 comments
Open

If and when is VERTEX_WRITABLE_STORAGE needed? #317

sharkdp opened this issue Apr 9, 2024 · 3 comments
Labels
question Further information is requested

Comments

@sharkdp
Copy link

sharkdp commented Apr 9, 2024

Some bevy_hanabi examples explicitly set VERTEX_WRITABLE_STORAGE to true:

    wgpu_settings
        .features
        .set(WgpuFeatures::VERTEX_WRITABLE_STORAGE, true);

But other examples don't. I couldn't figure out if and when this is really necessary. For example, I can start the gradient example without those three lines, and it seems to work just fine. I can even explicitly set it to false and things look the same. Am I missing something or does this maybe have a performance impact?

I'm mainly asking because explicitly requesting this feature leads to crashes on some older hardware:

… RequestDeviceError { inner: Core(UnsupportedFeature(Features(VERTEX_WRITABLE_STORAGE))) }

If I do not request the feature to be set to true, it starts properly but does not show any particle effects, which I would prefer over crashing on startup. But I'm not sure about the implications on devices where it does actually work.

Thank you for providing this crate. It works great for us!

@djeedai
Copy link
Owner

djeedai commented Apr 9, 2024

Hi @sharkdp, thanks for opening this issue. This one has been on my mind for some time.

Here's some context:

  • I wrote the original code. This dates back I think from very early during prototyping. I can't recall why I added that. I do recall I didn't know WGPU so it's very possible this was a mistake. Or was needed at the time but not anymore.
  • I'm not sure I fully understand the wgpu doc on this feature. And without access to a lot of various hardwares to test it, I've been a bit reluctant to remove it in case it breaks something.

But that status quo is not a good solution. So thanks for reopening that topic.

I couldn't figure out if and when this is really necessary. For example, I can start the gradient example without those three lines, and it seems to work just fine. I can even explicitly set it to false and things look the same. Am I missing something or does this maybe have a performance impact?

I think it's only needed when you have a vertex shader which writes to some buffer. I'm pretty sure 🎆 Hanabi never does that, although there might be some bug where some buffer is bound with write access by mistake even thought it doesn't need it.

I'm mainly asking because explicitly requesting this feature leads to crashes on some older hardware.

This is kind of expected in the sense that we request a feature that the hardware doesn't support. Why a crash you ask? Because I don't think there's a simple mechanism in Bevy to gracefully fail a plugin initialization unfortunately.

If I do not request the feature to be set to true, it starts properly but does not show any particle effects

This is exactly the kind of thing I was worried about removing that feature request. And this incidentally makes me think about the wasm bug which looks similar and is not root caused. By any chance did you run several examples and they all fail? Is there any case where only the first effect shows up and not the other ones (that's exactly the wasm bug which I can't diagnose and blocks warm support)?

I would prefer over crashing on startup.

This is arguable. I get this blocks development, but in general I'm a fervent supporter of crashing hard and fast rather than silently failing, as the former is a lot easier to debug. It depends on context though; you might argue it's just a visual effect and is not breaking your app. Or a more artistically inclined dev might argue this completely break the artistic vision of a game. The best is to give the choice where possible I guess.

Let me know if you can do more testing on various examples in the repo, and which ones break or not. I will try to figure out if there's any vertex write anywhere. But I don't think I can repro with my hardware.

@djeedai djeedai added the question Further information is requested label Apr 9, 2024
@sharkdp
Copy link
Author

sharkdp commented Apr 10, 2024

Thank you for the detailed answer!

If I do not request the feature to be set to true, it starts properly but does not show any particle effects

This is exactly the kind of thing I was worried about removing that feature request.

I think I should have explained this better: If I do not request the feature to be set to true:

  • old hardware: starts properly, no particle effects
  • new hardware: particle effects still work fine

So it's not like there is a regression for cases where it worked before. The fact that it doesn't work on the old hardware setup might be entirely unrelated to VERTEX_WRITABLE_STORAGE.

I would prefer over crashing on startup.

This is arguable. I get this blocks development, but in general I'm a fervent supporter of crashing hard and fast rather than silently failing, as the former is a lot easier to debug. It depends on context though; you might argue it's just a visual effect and is not breaking your app. Or a more artistically inclined dev might argue this completely break the artistic vision of a game. The best is to give the choice where possible I guess.

Absolutely agree with you. Choice would be best. I just wanted to mention that in our specific use case, we would rather still support the old hardware and ignore the absence of particle effects than to not support it at all.

Ideally, I would like to get back some Err(…) at runtime and decide for myself what to do in case particle effects are (for some reason) not supported on the current GPU setup (ignore it, fail hard, notify the user, …)

Let me know if you can do more testing on various examples in the repo, and which ones break or not. I will try to figure out if there's any vertex write anywhere. But I don't think I can repro with my hardware.

So far, I only tested with our game. But I can try to run Hanabi examples on that laptop. Just need to figure out if I can cross-compile them. What exactly would we want to test? Run all of the examples in their current state? And then log if they (1) crash or (2) run, but show no effects at all or (3) run, but only show the first effect?

@djeedai
Copy link
Owner

djeedai commented May 19, 2024

There were a number of bugs fixed recently, which might explain (hopefully) effects not showing up. If you can remove the VERTEX_WRITABLE_STORAGE requirement from the code and test the examples in the repo with your older hardware, that would be a good indication of whether this is needed or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants