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] - Shader Imports. Decouple Mesh logic from PBR #3137

Closed
wants to merge 3 commits into from

Conversation

cart
Copy link
Member

@cart cart commented Nov 15, 2021

Shader Imports

This adds "whole file" shader imports. These come in two flavors:

Asset Path Imports

// /assets/shaders/custom.wgsl

#import "shaders/custom_material.wgsl"

[[stage(fragment)]]
fn fragment() -> [[location(0)]] vec4<f32> {
    return get_color();
}
// /assets/shaders/custom_material.wgsl

[[block]]
struct CustomMaterial {
    color: vec4<f32>;
};
[[group(1), binding(0)]]
var<uniform> material: CustomMaterial;

Custom Path Imports

Enables defining custom import paths. These are intended to be used by crates to export shader functionality:

// bevy_pbr2/src/render/pbr.wgsl

#import bevy_pbr::mesh_view_bind_group
#import bevy_pbr::mesh_bind_group

[[block]]
struct StandardMaterial {
    base_color: vec4<f32>;
    emissive: vec4<f32>;
    perceptual_roughness: f32;
    metallic: f32;
    reflectance: f32;
    flags: u32;
};

/* rest of PBR fragment shader here */
impl Plugin for MeshRenderPlugin {
    fn build(&self, app: &mut bevy_app::App) {
        let mut shaders = app.world.get_resource_mut::<Assets<Shader>>().unwrap();
        shaders.set_untracked(
            MESH_BIND_GROUP_HANDLE,
            Shader::from_wgsl(include_str!("mesh_bind_group.wgsl"))
                .with_import_path("bevy_pbr::mesh_bind_group"),
        );
        shaders.set_untracked(
            MESH_VIEW_BIND_GROUP_HANDLE,
            Shader::from_wgsl(include_str!("mesh_view_bind_group.wgsl"))
                .with_import_path("bevy_pbr::mesh_view_bind_group"),
        );

By convention these should use rust-style module paths that start with the crate name. Ultimately we might enforce this convention.

Note that this feature implements run time import resolution. Ultimately we should move the import logic into an asset preprocessor once Bevy gets support for that.

Decouple Mesh Logic from PBR Logic via MeshRenderPlugin

This breaks out mesh rendering code from PBR material code, which improves the legibility of the code, decouples mesh logic from PBR logic, and opens the door for a future MaterialPlugin<T: Material> that handles all of the pipeline setup for arbitrary shader materials.

Removed RenderAsset<Shader> in favor of extracting shaders into RenderPipelineCache

This simplifies the shader import implementation and removes the need to pass around RenderAssets<Shader>.

RenderCommands are now fallible

This allows us to cleanly handle pipelines+shaders not being ready yet. We can abort a render command early in these cases, preventing bevy from trying to bind group / do draw calls for pipelines that couldn't be bound. This could also be used in the future for things like "components not existing on entities yet".

Next Steps

  • Investigate using Naga for "partial typed imports" (ex: #import bevy_pbr::material::StandardMaterial, which would import only the StandardMaterial struct)
  • Implement MaterialPlugin<T: Material> for low-boilerplate custom material shaders
  • Move shader import logic into the asset preprocessor once bevy gets support for that.

Fixes #3132

@cart cart added the A-Rendering Drawing game state to the screen label Nov 15, 2021
@cart cart added this to the Bevy 0.6 milestone Nov 15, 2021
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Review labels Nov 15, 2021
@alice-i-cecile
Copy link
Member

RenderCommands are now fallible
This allows us to cleanly handle pipelines+shaders not being ready yet. We can abort a render command early in these cases, preventing bevy from trying to bind group / do draw calls for pipelines that couldn't be bound. This could also be used in the future for things like "components not existing on entities yet".

Closely related to #2241.

@Davier
Copy link
Contributor

Davier commented Nov 16, 2021

How do all this interact with shader hot-reloading? Is there a technical reason why the PBR shaders are using include_str!() instead of asset_server.load()?

@inodentry
Copy link
Contributor

@Davier i'm pretty sure that the reason is that the AssetServer is meant for loading things from the application's assets dir.

Shaders that come included with bevy (or community plugins) aren't distributed in that way, so they gotta get loaded differently.

@cart
Copy link
Member Author

cart commented Nov 16, 2021

Yup its a matter of bundling shaders inside crates. We might ultimately sort out some way to not bundle these types of assets inside the binary to cut down on deployment sizes, but there are a ton of UX issues to solve there:

  • When / how will we copy crate assets into the user's asset folder? (ex: a cargo feature that dumps them when the app runs?). We can't punt this job to users.
  • How do we pick which assets to copy in? Every asset in the crate tree?
  • How do we protect against users deleting these assets?

An editor might make it easier to implement these features, but I'm not even convinced that its worth it generally. At the very least, its not something I want to solve now.

@cart
Copy link
Member Author

cart commented Nov 16, 2021

(resolved conflicts)

@parasyte
Copy link
Contributor

parasyte commented Nov 17, 2021

Fixes #3132

Confirmed the fix with a local patch:

diff --git a/pipelined/bevy_render2/src/render_phase/draw.rs b/pipelined/bevy_render2/src/render_phase/draw.rs
index 2f26c540..6a5e9759 100644
--- a/pipelined/bevy_render2/src/render_phase/draw.rs
+++ b/pipelined/bevy_render2/src/render_phase/draw.rs
@@ -249,7 +249,9 @@ where
         item: &P,
     ) {
         let param = self.state.get(world);
-        C::render(view, item, param, pass);
+        if let RenderCommandResult::Failure = C::render(view, item, param, pass) {
+            bevy_utils::tracing::warn!("RenderCommandResult::Failure");
+        }
     }
 }
 

And seeing some log lines infrequently on startup, but no panics:

$ RUST_BACKTRACE=1 cargo run --example shader_defs_pipelined

Nov 16 17:26:18.553  INFO bevy_render2::renderer: AdapterInfo { name: "NVIDIA GeForce RTX 3090", vendor: 4318, device: 8708, device_type: DiscreteGpu, backend: Vulkan }
Nov 16 17:26:18.666  WARN bevy_render2::render_phase::draw: RenderCommandResult::Failure
Nov 16 17:26:18.666  WARN bevy_render2::render_phase::draw: RenderCommandResult::Failure
Nov 16 17:26:19.639  INFO bevy diagnostic: frame_count                     :  175.000000  (avg 175.000000)
Nov 16 17:26:19.640  INFO bevy diagnostic: frame_time                      :    0.005569s (avg 0.005556s)
Nov 16 17:26:19.640  INFO bevy diagnostic: fps                             :  179.970701  (avg 180.037299)
Nov 16 17:26:20.639  INFO bevy diagnostic: frame_count                     :  355.000000  (avg 355.000000)
Nov 16 17:26:20.639  INFO bevy diagnostic: frame_time                      :    0.005540s (avg 0.005558s)
Nov 16 17:26:20.640  INFO bevy diagnostic: fps                             :  179.993052  (avg 179.973838)

@cart
Copy link
Member Author

cart commented Nov 17, 2021

@parasyte

Fixes #3132

Awesome. Thanks for checking!

@cart
Copy link
Member Author

cart commented Nov 18, 2021

bors r+

bors bot pushed a commit that referenced this pull request Nov 18, 2021
## Shader Imports

This adds "whole file" shader imports. These come in two flavors:

### Asset Path Imports

```rust
// /assets/shaders/custom.wgsl

#import "shaders/custom_material.wgsl"

[[stage(fragment)]]
fn fragment() -> [[location(0)]] vec4<f32> {
    return get_color();
}
```

```rust
// /assets/shaders/custom_material.wgsl

[[block]]
struct CustomMaterial {
    color: vec4<f32>;
};
[[group(1), binding(0)]]
var<uniform> material: CustomMaterial;
```

### Custom Path Imports

Enables defining custom import paths. These are intended to be used by crates to export shader functionality:

```rust
// bevy_pbr2/src/render/pbr.wgsl

#import bevy_pbr::mesh_view_bind_group
#import bevy_pbr::mesh_bind_group

[[block]]
struct StandardMaterial {
    base_color: vec4<f32>;
    emissive: vec4<f32>;
    perceptual_roughness: f32;
    metallic: f32;
    reflectance: f32;
    flags: u32;
};

/* rest of PBR fragment shader here */
```

```rust
impl Plugin for MeshRenderPlugin {
    fn build(&self, app: &mut bevy_app::App) {
        let mut shaders = app.world.get_resource_mut::<Assets<Shader>>().unwrap();
        shaders.set_untracked(
            MESH_BIND_GROUP_HANDLE,
            Shader::from_wgsl(include_str!("mesh_bind_group.wgsl"))
                .with_import_path("bevy_pbr::mesh_bind_group"),
        );
        shaders.set_untracked(
            MESH_VIEW_BIND_GROUP_HANDLE,
            Shader::from_wgsl(include_str!("mesh_view_bind_group.wgsl"))
                .with_import_path("bevy_pbr::mesh_view_bind_group"),
        );
```

By convention these should use rust-style module paths that start with the crate name. Ultimately we might enforce this convention.

Note that this feature implements _run time_ import resolution. Ultimately we should move the import logic into an asset preprocessor once Bevy gets support for that.

## Decouple Mesh Logic from PBR Logic via MeshRenderPlugin

This breaks out mesh rendering code from PBR material code, which improves the legibility of the code, decouples mesh logic from PBR logic, and opens the door for a future `MaterialPlugin<T: Material>` that handles all of the pipeline setup for arbitrary shader materials.

## Removed `RenderAsset<Shader>` in favor of extracting shaders into RenderPipelineCache

This simplifies the shader import implementation and removes the need to pass around `RenderAssets<Shader>`.

##  RenderCommands are now fallible

This allows us to cleanly handle pipelines+shaders not being ready yet. We can abort a render command early in these cases, preventing bevy from trying to bind group / do draw calls for pipelines that couldn't be bound. This could also be used in the future for things like "components not existing on entities yet". 

# Next Steps

* Investigate using Naga for "partial typed imports" (ex: `#import bevy_pbr::material::StandardMaterial`, which would import only the StandardMaterial struct)
* Implement `MaterialPlugin<T: Material>` for low-boilerplate custom material shaders
* Move shader import logic into the asset preprocessor once bevy gets support for that.

Fixes #3132
@bors
Copy link
Contributor

bors bot commented Nov 18, 2021

@bors bors bot changed the title Shader Imports. Decouple Mesh logic from PBR [Merged by Bors] - Shader Imports. Decouple Mesh logic from PBR Nov 18, 2021
@bors bors bot closed this Nov 18, 2021
@james7132 james7132 mentioned this pull request Mar 7, 2022
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 targeted quality-of-life change that makes Bevy easier to use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants