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

fix missed examples in WebGPU update #8553

Merged
merged 3 commits into from
May 16, 2023
Merged

fix missed examples in WebGPU update #8553

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
509bf90
fix missed examples in WebGPU update
mockersf May 5, 2023
b300daa
format
mockersf May 5, 2023
24d7666
plugin pattern to check for feature support
mockersf May 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion crates/bevy_core_pipeline/src/taa/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ impl Plugin for TemporalAntiAliasPlugin {
let Ok(render_app) = app.get_sub_app_mut(RenderApp) else { return };

render_app
.init_resource::<TAAPipeline>()
.init_resource::<SpecializedRenderPipelines<TAAPipeline>>()
.add_systems(ExtractSchedule, extract_taa_settings)
.add_systems(
Expand All @@ -84,6 +83,12 @@ impl Plugin for TemporalAntiAliasPlugin {
],
);
}

fn finish(&self, app: &mut App) {
let Ok(render_app) = app.get_sub_app_mut(RenderApp) else { return };

render_app.init_resource::<TAAPipeline>();
}
}

/// Bundle to apply temporal anti-aliasing.
Expand Down
10 changes: 8 additions & 2 deletions examples/2d/mesh2d_manual.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,21 @@ impl Plugin for ColoredMesh2dPlugin {
Shader::from_wgsl(COLORED_MESH2D_SHADER),
);

// Register our custom draw function and pipeline, and add our render systems
// Register our custom draw function, and add our render systems
app.get_sub_app_mut(RenderApp)
.unwrap()
.add_render_command::<Transparent2d, DrawColoredMesh2d>()
.init_resource::<ColoredMesh2dPipeline>()
.init_resource::<SpecializedRenderPipelines<ColoredMesh2dPipeline>>()
.add_systems(ExtractSchedule, extract_colored_mesh2d)
.add_systems(Render, queue_colored_mesh2d.in_set(RenderSet::Queue));
}

fn finish(&self, app: &mut App) {
// Register our custom pipeline
app.get_sub_app_mut(RenderApp)
.unwrap()
.init_resource::<ColoredMesh2dPipeline>();
}
}

/// Extract the [`ColoredMesh2d`] marker component into the render app
Expand Down
13 changes: 11 additions & 2 deletions examples/shader/post_processing.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,6 @@ impl Plugin for PostProcessPlugin {
};

render_app
// Initialize the pipeline
.init_resource::<PostProcessPipeline>()
// Bevy's renderer uses a render graph which is a collection of nodes in a directed acyclic graph.
// It currently runs on each view/camera and executes each node in the specified order.
// It will make sure that any node that needs a dependency from another node
Expand Down Expand Up @@ -97,6 +95,17 @@ impl Plugin for PostProcessPlugin {
],
);
}

fn finish(&self, app: &mut App) {
// We need to get the render app from the main app
let Ok(render_app) = app.get_sub_app_mut(RenderApp) else {
return;
};

render_app
// Initialize the pipeline
.init_resource::<PostProcessPipeline>();
}
}

/// The post process node used for the render graph
Expand Down
29 changes: 14 additions & 15 deletions examples/shader/texture_binding_array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,6 @@ fn main() {
let mut app = App::new();
app.add_plugins(DefaultPlugins.set(ImagePlugin::default_nearest()));

let render_device = app.world.resource::<RenderDevice>();

// check if the device support the required feature
if !render_device
.features()
.contains(WgpuFeatures::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING)
{
error!(
"Render device doesn't support feature \
SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING, \
which is required for texture binding arrays"
);
return;
}

app.add_plugin(MaterialPlugin::<BindlessMaterial>::default())
.add_systems(Startup, setup)
.run();
Expand All @@ -47,7 +32,21 @@ fn setup(
mut meshes: ResMut<Assets<Mesh>>,
mut materials: ResMut<Assets<BindlessMaterial>>,
asset_server: Res<AssetServer>,
render_device: Res<RenderDevice>,
) {
// check if the device support the required feature
Copy link
Contributor

@rparrett rparrett May 5, 2023 •
edited
Loading

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 •
edited
Loading

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 •
edited
Loading

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.

Getting the adapter is async: https://docs.rs/wgpu/latest/wgpu/struct.Instance.html#method.request_adapter
Getting the device is async: https://docs.rs/wgpu/latest/wgpu/struct.Adapter.html#method.request_device

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.

@JMS55 would https://github.com/bevyengine/bevy/pull/8553/files#diff-86a9f1eba7a2e2bcb940a56d71acb7675c9c24ab6981ed7f221a10f9117514f0R32 work for you?

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 •
edited
Loading

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 :(

if !render_device
.features()
.contains(WgpuFeatures::SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING)
{
error!(
"Render device doesn't support feature \
SAMPLED_TEXTURE_AND_STORAGE_BUFFER_ARRAY_NON_UNIFORM_INDEXING, \
which is required for texture binding arrays"
);
return;
}

commands.spawn(Camera3dBundle {
transform: Transform::from_xyz(2.0, 2.0, 2.0).looking_at(Vec3::new(0.0, 0.0, 0.0), Vec3::Y),
..Default::default()
Expand Down