Modifying an Assets dependencies should mark the Asset as modified

[rectalogic]

Objective

Fixes #20269 and #16159

The issue is when an Asset is modified and it is a dependency of another Asset, that "parent" Asset is not marked as modified. Specifically in the 2 issues above, an Image asset is modified and it is a dependency of StandardMaterial (base_color_texture) or a ColorMaterial (texture) asset. When the Image is modified, neither material is marked modified and so they do not rebuild their cached BindGroup and so the image rendered does not change.

Solution

First, modify VisitAssetDependencies::visit_dependencies to allow early termination via ControlFlow.

Then, for every Assets<A> modification, also queue an UntypedAssetModifiedEvent with the UntypedAssetId. So we have an event queue of modified assets across all asset types. Then for each asset we check if any of its dependency assets are in that set of modified assets, and if so mark the asset itself as modified.

This does mean that any time an asset in Assets<A> is modified, we will iterate all assets in Assets<A> and visit each of their dependencies which is perhaps suboptimal.

Testing

Here is some sample code that demonstrates the issue with both StandardMaterial and ColorMaterial and an Image.

Click to expand sample code

use std::f32::consts::FRAC_PI_4;

use bevy::{
    asset::RenderAssetUsages,
    prelude::*,
    render::render_resource::{Extent3d, TextureDimension, TextureFormat},
};
use rand::Rng;

fn main() {
    let mut app = App::new();
    app.add_plugins(DefaultPlugins)
        .init_resource::<ImageAssetId>()
        .add_systems(Startup, setup)
        .add_systems(Update, update);

    app.run();
}

#[derive(Resource, Default)]
struct ImageAssetId(AssetId<Image>);

fn setup(
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut standard_materials: ResMut<Assets<StandardMaterial>>,
    mut color_materials: ResMut<Assets<ColorMaterial>>,
    mut images: ResMut<Assets<Image>>,
    mut image_id: ResMut<ImageAssetId>,
) {
    commands.spawn((PointLight::default(), Transform::from_xyz(2.0, 2.0, 3.0)));
    commands.spawn((
        Camera {
            order: 0,
            ..default()
        },
        Camera3d::default(),
        Transform::from_xyz(0.0, 0.0, 2.0),
    ));
    commands.spawn((
        Camera {
            order: 1,
            ..default()
        },
        Camera2d,
    ));
    let image_handle = images.add(random_image());
    image_id.0 = image_handle.id();
    commands.spawn((
        Mesh3d(meshes.add(Cuboid::new(1.0, 1.0, 1.0))),
        MeshMaterial3d(standard_materials.add(StandardMaterial {
            base_color_texture: Some(image_handle.clone()),
            ..default()
        })),
        Transform::from_rotation(Quat::from_axis_angle(Vec3::X, FRAC_PI_4)),
    ));

    commands.spawn((
        MeshMaterial2d(color_materials.add(ColorMaterial {
            texture: Some(image_handle),
            ..default()
        })),
        Mesh2d(meshes.add(Rectangle::new(512.0, 512.0))),
        Transform::from_scale(Vec3::splat(0.3)),
    ));
}

fn update(image_id: Res<ImageAssetId>, mut images: ResMut<Assets<Image>>) {
    if let Some(image) = images.get_mut(image_id.0) {
        *image = random_image();
    }
}

fn random_image() -> Image {
    let mut rng = rand::rng();
    let color = [
        rng.random_range(0..=255u8),
        rng.random_range(0..=255u8),
        rng.random_range(0..=255u8),
        255u8,
    ];
    let mut pixels = [0u8; 512 * 512 * 4];
    for chunk in pixels.chunks_exact_mut(4) {
        chunk.copy_from_slice(&color);
    }
    Image::new_fill(
        Extent3d {
            width: 512,
            height: 512,
            ..default()
        },
        TextureDimension::D2,
        &pixels,
        TextureFormat::Rgba8UnormSrgb,
        RenderAssetUsages::default(),
    )
}

[github-actions]

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

[andriyDev]

I haven't given this a thorough review yet, but my initial instinct is that using Modified to indicate that a dependency has changed is wrong. We should have a separate event for that.

For a concrete example, #20430 has to work around an issue today where multiple scene Modified events are causing certain workflows to break. This could expand that to also include anything the scene depends on, which could be textures, meshes, etc. Lots of modified events!

Perhaps I've misinterpreted and a more thorough review will convince me I'm wrong, but I thought I'd give an initial impression.

[tychedelia]

A couple quick comments without going too deep:

1. Thank you for caring about this. This is my all time #1 biggest pet peeve in Bevy and something our users are constantly stubbing their toes on.

2. I'm concerned about performance. I'd been assuming that we'd have to build a reverse dependency tree in order to index from the modified event to its dependents. Scanning all assets every time potentially is expensive. Think of the scenario where we are animating a bunch of materials on the CPU, that's a lot of modified events every frame.

[alice-i-cecile]

We talked about this a bit on Discord, and realized that #11266 + relations to model dependencies would be extremely useful for this.

[andriyDev]

I don't think I'm understanding how this control flow works. Let's say you have an asset with:

struct MyAsset(#[dependency] Vec<Handle<Mesh>>);

If for whatever reason one of those meshes returns ControlFlow::Break (because it was modified this frame), the entire MyAsset stops iterating, right?

[rectalogic]

Right, it's short-circuiting searching dependencies, as soon as it finds a match, that's enough and we can stop looking.

[andriyDev]

Ohh I understand now. This walk of the assets is allowed to early return because we give it a visit function that can return Break. The existing uses of VisitDependencies don't early return because its visit function never returns Break.

Thanks for this explanation!!

[andriyDev]

Oh this literally iterates through every single asset and checking if it (directly or indirectly) depends on one of the modified assets. I think this is a dealbreaker for me - iterating through every asset seems like it'll be very slow.

[rectalogic]

Yeah, this was my biggest concern too that I called out in the initial PR comment. But I figured I would get see what others thought.

[rectalogic]

We talked about this a bit on Discord, and realized that #11266 + relations to model dependencies would be extremely useful for this.

Interesting. My motivation for this was for playing video - every bevy video implementation I found requires the user to modify the material they are using every frame so the material displays it's updated Asset<Image> video frame dependency (or else they use Sprite which does not have this issue).

https://github.com/rectalogic/bevy_av1/blob/9f92f3679aa942b9fc66c1a2d9ae9a17d281a56a/examples/demo3d.rs#L70-L71

https://github.com/PortalCloudInc/bevy_video/blob/eca2ba442d5f0c718e286cce44a5a5f0c0f69c9d/examples/video_streaming/src/main.rs#L78-L80

https://github.com/Jeff425/bevy_h264/blob/ed0e8344ef1312af7af9f865b2c7d75c14668f44/example/src/main.rs#L55

https://github.com/sethblocks/bevy_video/blob/893fbf73bf69061987e10fd444556fb7804a51d8/examples/video_streaming/src/main.rs#L67-L69

I was trying to find a way to automate/simplify that for the user, but perhaps this could be done in the video crate using an AsAssetId component for the Handle<Image> and a relationship to the target material that needs to be updated on AssetChanged or something.

[rectalogic]

We talked about this a bit on Discord, and realized that #11266 + relations to model dependencies would be extremely useful for this.

I prototyped something similar to this - AssetDependent<A> and AssetDependencyOf<A> relationship components each wrapping an AssetId<A> (so e.g. you can have an AssetDependent<ColorMaterial> with an AssetDependencyOf<Image> dependency - if the Image asset is modified, the ColorMaterial will be marked modified).

So we can monitor AssetDependencyOf<A> for changes Or<(Changed<AssetDependencyOf<A>>, AssetChanged<AssetDependencyOf<A>>)> and add a marker AssetDependencyChanged component to the dependent entity when any dependency changes.

Then for any dependents with AssetDependencyChanged, we can mark their contained asset as modified.

I changed the cpu_draw example to demonstrate using these. I'm not sure how ergonomic it is to use though.

You can see the changes here, I didn't do a PR since I'm not sure about this approach:

main...rectalogic:bevy:assetrelations

[rectalogic]

I prototyped something similar to this - AssetDependent<A> and AssetDependencyOf<A> relationship components each wrapping an AssetId<A>

I totally botched this, I modeled after the child/parent relationship stuff but we need a "child with multiple parents" (e.g. an Image dependency with multiple materials using it) not a "parent with multiple children". I'll see if I can rework it.

[andriyDev]

Bevy ECS does not currently have many-to-many relations and we need many-to-many relations to describe this relationship. An image may be used by multiple materials, and a material may use multiple images. Neither direction supports the one-to-many we have today.

We will have this one day, and with #11266, we'd just be able to delete a boatload of code.

[JMS55]

I wonder if this would fix #15081

[rectalogic]

Bevy ECS does not currently have many-to-many relations and we need many-to-many relations to describe this relationship.

Out of interest I continued beating this dead horse :)

I think you can model many-to-many by duplicating the AssetId wrapper components.

I have a Dependencies RelationshipTarget component, and a DependencyOf Relationship component.
Then I have AssetDependent<A: Asset> and AssetDependency<A: Asset> components each wrapping AssetId<A> and implementing AsAssetId.

So if Asset A has dependencies B and C, and Asset D also depends on B and C, we can model it with these entities:

e1 AssetDependent(A) Dependencies
e2 AssetDependency(B) DependencyOf(e1)
e3 AssetDependency(C) DependencyOf(e1)

e4 AssetDependent(D) Dependencies
e5 AssetDependency(B) DependencyOf(e4)
e6 AssetDependency(C) DependencyOf(e4)

Then we can monitor AssetDependency for changes and mark its relationship target as changed.

So it's modeling the asset dependencies with a parallel network of ECS entities with (duplicated) asset components and relationships.

This seems to work in a limited case (I modified the cpu_draw example to use it) but I don't think there's an easy way to automatically keep this in sync with the actual asset hierarchy (e.g. if you later add a new dependency to an existing asset, you would need to also manually add/modify the asset component wrappers and relationships). I can see how making Asset itself be a Component and many-2-many relationship support would help here.

Here's my implementation main...rectalogic:bevy:assetrelations

[cart]

Just doing a quick drive by review, but I agree with the other comments. This removes granularity from the system in a way that is not general purpose. Materials happen to have bind groups that care about dependencies, but there are plenty of other systems that don't want to rebuild just because a dependency changes (ex: scenes). In the general case, each asset is its own "thing" and the handles it contain are "pointers", not that "thing" itself.

We can consider "dependency changed" events, but I don't think we can enable them by default for everything, as they would likely be too expensive. We could consider an opt-in subscriber system though.