Skip to content

Commit

Permalink
Introduce SystemLabel's for RenderAssetPlugin, and change Image
Browse files Browse the repository at this point in the history
… preparation system to run before others (#3917)

# Objective

Fixes `StandardMaterial` texture update (see sample code below).

Most probably fixes #3674 (did not test)

## Solution

Material updates, such as PBR update, reference the underlying `GpuImage`. Like here: https://github.com/bevyengine/bevy/blob/9a7852db0f22eb41f259a1afbb4926eb73863a10/crates/bevy_pbr/src/pbr_material.rs#L177

However, currently the `GpuImage` update may actually happen *after* the material update fetches the gpu image. Resulting in the material actually not being updated for the correct gpu image.

In this pull req, I introduce new systemlabels for the renderassetplugin. Also assigned the RenderAssetPlugin::<Image> to the `PreAssetExtract` stage, so that it is executed before any material updates.

Code to test.

Expected behavior:
* should update to red texture

Unexpected behavior (before this merge):
* texture stays randomly as green one (depending on the execution order of systems)

```rust
use bevy::{
    prelude::*,
    render::render_resource::{Extent3d, TextureDimension, TextureFormat},
};

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(setup)
        .add_system(changes)
        .run();
}

struct Iteration(usize);

#[derive(Component)]
struct MyComponent;

fn setup(
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<StandardMaterial>>,
    mut images: ResMut<Assets<Image>>,
) {
    commands.spawn_bundle(PointLightBundle {
        point_light: PointLight {
            ..Default::default()
        },
        transform: Transform::from_xyz(4.0, 8.0, 4.0),
        ..Default::default()
    });

    commands.spawn_bundle(PerspectiveCameraBundle {
        transform: Transform::from_xyz(-2.0, 0.0, 5.0)
            .looking_at(Vec3::new(0.0, 0.0, 0.0), Vec3::Y),
        ..Default::default()
    });

    commands.insert_resource(Iteration(0));

    commands
        .spawn_bundle(PbrBundle {
            mesh: meshes.add(Mesh::from(shape::Quad::new(Vec2::new(3., 2.)))),
            material: materials.add(StandardMaterial {
                base_color_texture: Some(images.add(Image::new(
                    Extent3d {
                        width: 600,
                        height: 400,
                        depth_or_array_layers: 1,
                    },
                    TextureDimension::D2,
                    [0, 255, 0, 128].repeat(600 * 400), // GREEN
                    TextureFormat::Rgba8Unorm,
                ))),
                ..Default::default()
            }),
            ..Default::default()
        })
        .insert(MyComponent);
}

fn changes(
    mut materials: ResMut<Assets<StandardMaterial>>,
    mut images: ResMut<Assets<Image>>,
    mut iteration: ResMut<Iteration>,
    webview_query: Query<&Handle<StandardMaterial>, With<MyComponent>>,
) {
    if iteration.0 == 2 {
        let material = materials.get_mut(webview_query.single()).unwrap();

        let image = images
            .get_mut(material.base_color_texture.as_ref().unwrap())
            .unwrap();

        image
            .data
            .copy_from_slice(&[255, 0, 0, 255].repeat(600 * 400));
    }

    iteration.0 += 1;
}
```

Co-authored-by: Carter Anderson <mcanders1@gmail.com>
  • Loading branch information
blaind and cart committed Mar 29, 2022
1 parent 31bd4ec commit 6844a6f
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 6 deletions.
46 changes: 43 additions & 3 deletions crates/bevy_render/src/render_asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,28 +39,68 @@ pub trait RenderAsset: Asset {
) -> Result<Self::PreparedAsset, PrepareAssetError<Self::ExtractedAsset>>;
}

#[derive(Clone, Hash, Debug, PartialEq, Eq, SystemLabel)]
pub enum PrepareAssetLabel {
PreAssetPrepare,
AssetPrepare,
PostAssetPrepare,
}

impl Default for PrepareAssetLabel {
fn default() -> Self {
Self::AssetPrepare
}
}

/// This plugin extracts the changed assets from the "app world" into the "render world"
/// and prepares them for the GPU. They can then be accessed from the [`RenderAssets`] resource.
///
/// Therefore it sets up the [`RenderStage::Extract`](crate::RenderStage::Extract) and
/// [`RenderStage::Prepare`](crate::RenderStage::Prepare) steps for the specified [`RenderAsset`].
pub struct RenderAssetPlugin<A: RenderAsset>(PhantomData<fn() -> A>);
pub struct RenderAssetPlugin<A: RenderAsset> {
prepare_asset_label: PrepareAssetLabel,
phantom: PhantomData<fn() -> A>,
}

impl<A: RenderAsset> RenderAssetPlugin<A> {
pub fn with_prepare_asset_label(prepare_asset_label: PrepareAssetLabel) -> Self {
Self {
prepare_asset_label,
phantom: PhantomData,
}
}
}

impl<A: RenderAsset> Default for RenderAssetPlugin<A> {
fn default() -> Self {
Self(PhantomData)
Self {
prepare_asset_label: Default::default(),
phantom: PhantomData,
}
}
}

impl<A: RenderAsset> Plugin for RenderAssetPlugin<A> {
fn build(&self, app: &mut App) {
if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
let prepare_asset_system = prepare_assets::<A>.label(self.prepare_asset_label.clone());

let prepare_asset_system = match self.prepare_asset_label {
PrepareAssetLabel::PreAssetPrepare => prepare_asset_system,
PrepareAssetLabel::AssetPrepare => {
prepare_asset_system.after(PrepareAssetLabel::PreAssetPrepare)
}
PrepareAssetLabel::PostAssetPrepare => {
prepare_asset_system.after(PrepareAssetLabel::AssetPrepare)
}
};

render_app
.init_resource::<ExtractedAssets<A>>()
.init_resource::<RenderAssets<A>>()
.init_resource::<PrepareNextFrameAssets<A>>()
.add_system_to_stage(RenderStage::Extract, extract_render_asset::<A>)
.add_system_to_stage(RenderStage::Prepare, prepare_assets::<A>);
.add_system_to_stage(RenderStage::Prepare, prepare_asset_system);
}
}
}
Expand Down
11 changes: 8 additions & 3 deletions crates/bevy_render/src/texture/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ pub use hdr_texture_loader::*;
pub use image_texture_loader::*;
pub use texture_cache::*;

use crate::{render_asset::RenderAssetPlugin, RenderApp, RenderStage};
use crate::{
render_asset::{PrepareAssetLabel, RenderAssetPlugin},
RenderApp, RenderStage,
};
use bevy_app::{App, Plugin};
use bevy_asset::{AddAsset, Assets};

Expand Down Expand Up @@ -52,8 +55,10 @@ impl Plugin for ImagePlugin {
app.init_asset_loader::<HdrTextureLoader>();
}

app.add_plugin(RenderAssetPlugin::<Image>::default())
.add_asset::<Image>();
app.add_plugin(RenderAssetPlugin::<Image>::with_prepare_asset_label(
PrepareAssetLabel::PreAssetPrepare,
))
.add_asset::<Image>();
app.world
.resource_mut::<Assets<Image>>()
.set_untracked(DEFAULT_IMAGE_HANDLE, Image::default());
Expand Down

0 comments on commit 6844a6f

Please sign in to comment.