Meshes Changed after mesh_resource_provider_system don't get initialized

[4-rodrigo-salazar]

With the latest performance fixes on master, there is an issue with creating meshes in stages that happen after mesh_resource_provider_system. I've debugged it and reduced it to this sample case (from my actual code which hits this).

This code will panic on bevy master:

With: thread 'main' panicked at 'Attribute Vertex_Position is required by shader, but not supplied by mesh. Either remove the attribute from the shader or supply the attribute (Vertex_Position) to the mesh. ', crates\bevy_render\src\pipeline\pipeline_compiler.rs:223:17

  use bevy::prelude::*;

  fn main() {
    App::build()
      .add_plugins(DefaultPlugins)
       // add_stage ends up adding it last in the stage order?
      .add_stage("panic_due_to_stage_order")
      .add_system_to_stage("panic_due_to_stage_order", panic_due_to_stage_order)
      .run();
  }

  fn panic_due_to_stage_order(commands: &mut Commands, mut materials: ResMut<Assets<ColorMaterial>>) {
    commands.spawn(SpriteBundle {
        material: materials.add(ColorMaterial::color(Color::RED)),
        ..Default::default()
    });
  }

The issue is that in the new optimized mesh_resource_provider_system implementation we are using Changed<> to detect when we need to initialize the render_pipeline for the mesh. Unfortunately, the mesh has not yet been flagged for Changed at the time the mesh_resource_provider_system runs and Changed gets reset every frame.

Ideally it shouldn't matter at what stage you spawn your entity/components.

Some half-baked ideas on how to fix this in bevy (obviously without reverting the perf improvement):

• Just run mesh_resource_provider_system again at the end of the frame, it could capture the entities that Changed and weren't processed in the first pass. This seems a bit hacky. Also there's no way currently to pin a system at the end anyway.
• Introduce a ChangedLastFrame<> query filter. This would allow us to build a system that can handle changes to entities and the order of the system doesn't matter. In this mesh system case we would check both this ChangedLastFrame and Changed<>

I'd like to contribute a fix to resolve this, if we can figure out what's a good way to fix this... Perhaps there's a trivial way I am not seeing!

[cart]

Ideally it shouldn't matter at what stage you spawn your entity/components.

I don't think I agree. The whole point of stages is to be able to make different assertions about what has been done and what hasn't been done. I personally think it is 100% ok to say "meshes should be spawned before POST_UPDATE". The UPDATE stage is where app logic is supposed to go (and spawning entities is "app logic"). You can always create new stages directly after the UPDATE stage.

[4-rodrigo-salazar]

Ah in that case if we take that stance (which seems reasonable), it seems like we need some way to make this failure feel less like undefined behavior (an assertion that happens a lot sooner)? In this case, the failure ends up happening pretty far down the line and takes a lot of digging to correlate to stage order.

[cart]

I think the "give someone a helpful error if they insert logic into the wrong stage" problem is a bit of a rabbit hole:

1. There are so many classes of error that fall into this category. We would be adding conditionals to literally everything
2. It would be expensive to do these checks everywhere to handle every case (both computationally and from a dev time perspective)
3. Even if the computational cost of the checks was negligible and we had infinite dev time to implement them, it would still have a cost on maintainability + overall code size.

Id rather just have a rule of thumb that "app logic goes in UPDATE". everything else is undefined behavior.

[4-rodrigo-salazar]

Gotcha, you've convinced me