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

fix missed examples in WebGPU update #8553

Merged
merged 3 commits into from
May 16, 2023

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented May 5, 2023

@mockersf mockersf added A-Rendering Drawing game state to the screen C-Examples An addition or correction to our examples labels May 5, 2023
@rparrett
Copy link
Contributor

rparrett commented May 5, 2023

Same issue with anti_aliasing? edit: nevermind, I see that's fixed in taa/mod.rs

(And maybe a separate issue with without_winit?)

) {
// check if the device support the required feature
Copy link
Contributor

@rparrett rparrett May 5, 2023

Choose a reason for hiding this comment

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

If I recall correctly, this was in main partially as a way of demonstrating graceful handling of missing features.

But now with wasm/webgl2 at least, we get a panic before this code gets a chance to run.

Is this code even useful anymore?

sm.js:387 panicked at 'wgpu error: Validation Error

Caused by:
    In Device::create_bind_group_layout
      note: label = `bindless_material_layout`
    Binding 0 entry is invalid
    Features Features(TEXTURE_BINDING_ARRAY) are required but not enabled on the device

', /Users/me/.cargo/registry/src/github.com-1ecc6299db9ec823/wgpu-0.16.0/src/backend/direct.rs:3019:5

Copy link
Contributor

@JMS55 JMS55 May 5, 2023

Choose a reason for hiding this comment

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

That should really be moved to main(), after the default plugin is setup and before the custom material is setup.

Copy link
Member Author

Choose a reason for hiding this comment

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

But now with wasm/webgl2 at least, we get a panic before this code gets a chance to run.

Oh that gives the correct log in wasm/WebGPU, but not in wasm/WebGL2...

That should really be moved to main(), after the default plugin is setup and before the custom material is setup.

That's not possible anymore, the renderer is not available before the winit event loop is started.

Copy link
Contributor

@JMS55 JMS55 May 5, 2023

Choose a reason for hiding this comment

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

Is it only the device that needs to be created in finish()? Can we create an adapter in the main plugin build, and use that to check for feature support?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

A more complete fix would be to introduce a "fake" plugin in the example that just does the check, but nothing else from what's expected from a plugin

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure. What I need is the ability to add a plugin only if certain features are supported. Here's an example: https://github.com/JMS55/bevy/blob/solari/crates/bevy_solari/src/lib.rs#L45-L62.

Copy link
Member Author

@mockersf mockersf May 8, 2023

Choose a reason for hiding this comment

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

That's not going to be possible like that, without putting all the app initialisation as async.

What you can do is add your plugins, but have them do nothing if the features are not available.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't love this solution, but I guess it'll have to do :(

@coreh coreh mentioned this pull request May 11, 2023
20 tasks
@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 May 16, 2023
@mockersf mockersf added this pull request to the merge queue May 16, 2023
Merged via the queue into bevyengine:main with commit e0b1809 May 16, 2023
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-Examples An addition or correction to our examples 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.

post_processing example crashes when trying to run texture_binding_array example no longer runs on main
7 participants