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] - use error scope to handle errors on shader module creation #3675

Closed

Conversation

jakobhellermann
Copy link
Contributor

@jakobhellermann jakobhellermann commented Jan 14, 2022

related: #3289

In addition to validating shaders early when debug assertions are enabled, use the new error scopes API when creating a shader module.

I chose to keep the early validation (and thereby parsing twice) when debug assertions are enabled in, because it lets as handle errors ourselves and display them with pretty colors, while the error scopes API just gives us a string we can display.

This change pulls in futures-util as a new dependency for future.now_or_never(). I can inline that part of futures-lite into bevy_render to keep the compilation time lower if that's preferred.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Jan 14, 2022
use futures_util::future::FutureExt;
let error = render_device.wgpu_device().pop_error_scope();
if let Some(Some(wgpu::Error::Validation { description, .. })) =
error.now_or_never()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

now_or_never will return Some if the future is ready and None otherwise.
On native wgpu will yield the error immediatly while on wasm it may take some time.

I used it here to not make the pipeline cache more complicated by storing and polling futures, which is IMO not worth it for this change.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen C-Usability A simple quality-of-life change that makes Bevy easier to use and removed S-Needs-Triage This issue needs to be labelled labels Jan 14, 2022
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

Please update this on top of main and we can try to get it merged, unless @cart wants changes due to the additional dependency or otherwise. :)

@jakobhellermann
Copy link
Contributor Author

Rebased it on main and inlined the futures-util dependency since it's just ~20 lines

8860540#diff-cc220cd7b30bdab53f07cc69085af49c438247ef41ebd0d0921d87013d74271cR544-R576

@jakobhellermann
Copy link
Contributor Author

Rebased on main again and added a comment explaining the usage of now_or_never

https://github.com/bevyengine/bevy/pull/3675/files#diff-cc220cd7b30bdab53f07cc69085af49c438247ef41ebd0d0921d87013d74271cR139-R142

Copy link
Member

@cart cart left a comment

Choose a reason for hiding this comment

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

This looks good to me. I appreciate that you removed the extra dependency!

crates/bevy_render/src/render_resource/pipeline_cache.rs Outdated Show resolved Hide resolved
@cart
Copy link
Member

cart commented Mar 29, 2022

bors r+

bors bot pushed a commit that referenced this pull request Mar 29, 2022
related: #3289

In addition to validating shaders early when debug assertions are enabled, use the new [error scopes](https://gpuweb.github.io/gpuweb/#error-scopes) API when creating a shader module.

I chose to keep the early validation (and thereby parsing twice) when debug assertions are enabled in, because it lets as handle errors ourselves and display them with pretty colors, while the error scopes API just gives us a string we can display.



This change pulls in `futures-util` as a new dependency for `future.now_or_never()`. I can inline that part of futures-lite into `bevy_render` to keep the compilation time lower if that's preferred.
@bors bors bot changed the title use error scope to handle errors on shader module creation [Merged by Bors] - use error scope to handle errors on shader module creation Mar 29, 2022
@bors bors bot closed this Mar 29, 2022
aevyrie pushed a commit to aevyrie/bevy that referenced this pull request Jun 7, 2022
…e#3675)

related: bevyengine#3289

In addition to validating shaders early when debug assertions are enabled, use the new [error scopes](https://gpuweb.github.io/gpuweb/#error-scopes) API when creating a shader module.

I chose to keep the early validation (and thereby parsing twice) when debug assertions are enabled in, because it lets as handle errors ourselves and display them with pretty colors, while the error scopes API just gives us a string we can display.



This change pulls in `futures-util` as a new dependency for `future.now_or_never()`. I can inline that part of futures-lite into `bevy_render` to keep the compilation time lower if that's preferred.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
…e#3675)

related: bevyengine#3289

In addition to validating shaders early when debug assertions are enabled, use the new [error scopes](https://gpuweb.github.io/gpuweb/#error-scopes) API when creating a shader module.

I chose to keep the early validation (and thereby parsing twice) when debug assertions are enabled in, because it lets as handle errors ourselves and display them with pretty colors, while the error scopes API just gives us a string we can display.



This change pulls in `futures-util` as a new dependency for `future.now_or_never()`. I can inline that part of futures-lite into `bevy_render` to keep the compilation time lower if that's preferred.
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-Usability A simple quality-of-life change that makes Bevy easier to use
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants