Unify RenderLayers and TargetCamera (Part 1: RenderGroups) [ADOPT ME]

[UkoeHB]

Objective

• Address various ambiguities and issues with the separation between RenderLayers and TargetCamera.
• Closes Unify RenderLayers and TargetCamera and provide a single way to configure which camera entities are rendered to #12468.

Solution

See changelog. This is Part 1 of a 2-part rework. Part 2 will update UI to remove TargetCamera and use affiliated cameras instead.

Part 2 planned changelog:

• Remove TargetCamera.
• Use PropagateRenderGroups::Camera by default on UI cameras. This will set the camera affiliation in InheritedRenderGroups equal to the UI cameras (unless an entity has a RenderGroups component with a manually-specified camera affiliation, which will take precedence).

Part 2 planned migration:

• Replace TargetCamera components that you added manually to cameras with PropagateRenderGroups::Camera components. If those cameras need to see all 'default render layer' entities, then also add a CameraView::default() component.

Changelog

• Introduced a unified RenderGroups component that merges the functionality of RenderLayers and TargetCamera.
  • Includes RenderLayers (reworked) and an optional 'affiliated camera'.
• Reworked RenderLayers to store a growable bitmask, instead of being fixed to 32 bits. It can now store up to 64 render layers on the stack (including the default render layer), and then will allocate for more bits.
• Replaced Layer with RenderLayer as a user-facing abstraction for interacting with RenderLayers. A RenderLayer is a bit index, equivalent to the existing Layer type.
  • Added DEFAULT_RENDER_LAYER = RenderLayer(0). By default all entities and cameras without RenderGroups or CameraView components will be in the default layer.
• Added CameraView as a camera-specific component for controlling which RenderLayers are visible to the camera. Cameras automatically see any entity with a RenderGroups component that has a camera affiliation equal to the given camera.
• Added a PropagateRenderGroups component for controlling how RenderGroups are propagated down hierarchies.
  • Added an InheritedRenderGroups component that merges RenderGroups components with propagated RenderGroups. This component is managed automatically in PropagateRenderGroupsSet. It is used internally for rendering, and can be queried by users to e.g. debug visibility issues.
• Added PropagateRenderGroupsSet, which runs in the VisibilityPropagate set.

Migration Guide

• Replace Layer with RenderLayer.
• Replace RenderLayers components with RenderGroups components on non-camera entities.
• Replace RenderLayers components with CameraView components on camera entities.

TODOs

• Complete integration.
  • This PBR shader uses render layers internally (GpuDirectionalLight and ViewUniform and DirectionalLight and View). Now that it is a growable bitset, this doesn't work unless we set an upper cap on bits and pass in an array of ints for the flags.
  • prepare_lights in bevy_pbr is cloning ExtractedDirectionalLight, which contains a RenderGroups. This makes it potentially-allocating due to the growable bitset. There is also a clone when extracting from the world (also for ViewUniform).
  • Check pixel_grid_snap, render_to_texture examples.
• Task list: Add tests, examples and benchmarks for RenderLayers and TargetCamera rework #12588

Follow-up

• Part 2: Remove TargetCamera and use affiliated cameras in for UI instead.
• Refactor gizmos/GizmoConfig to support displaying different gizmos in different cameras. This may require a deeper rethink of how gizmos are designed, since they are currently special-cased.

[viridia]

I want to point out that there's a couple other options here - not that they are necessarily better.

One option is to combine the 'inherited' status as a boolean property within the RenderGroups component, rather than making it a separate marker component. I don't know whether this is better or not, it really depends on how much overhead there is for storing separate marker components.

You could take that further and have the property be an enum:

enum RenderGroupInheritance {
  /// This render group was calculated automatically
  Implicit = 0,
  /// This render group was explicitly set on the entity, but should not cascade to descendants
  Explicit,
  /// This render group has been explicitly set on the entity, and should cascade to descendants
  ExplicitPropagating
}

This means reducing the total number of components involved on each entity. Whether that is an optimization or de-optimization, I don't know.

It also raises the question: if an explicit propagating entity has an explicit non-propagating child, what should the render groups for its grandchild be?

[UkoeHB]

Working on the implementation for propagating render groups now. I think I can manage the following rules without needing to change the time complexity of existing code:

• Any entity with PropagateRenderGroups will propagate a RenderGroup to its children (derived from the PropagateRenderGroups variant).
• If an entity with PropagateRenderGroups is in-tree, it will block up-tree propagation and 'take over'. Entities can only inherit one propagated RenderGroups, unless they have PropagateRenderGroups in which case they can't inherit.

Allowing PropagateRenderGroups to merge together as you traverse the tree would require more time-complex hierarchy traversal due to a 'dependency diamond' issue.

It is also possible, although I think probably better for a follow-up PR, to allow a component RenderGroupsInheritancePolicy with filtering rules:

• Auto: Doesn't do anything extra.
• Skip: Don't merge inherited groups, but pass them to children.
• Block: Don't merge inherited groups, and prevent propagation to children.

EDIT: Given the complexity of the propagation algorithm for this PR, I doubt RenderGroupsInheritancePolicy can be easily integrated.

[viridia]

Something that would be helpful, I think, is a high-level description of the propagation algorithm. For example, the stylesheet propagator that I wrote a few months ago could be described by the following summary: "Starting with a query of all UI entities that can be styled, start by identifying the roots and then recursively visit all descendants accessible via the query. Each visited node calculates the composition of the inherited style with the style defined on that node, if any. Mutations to the state of each node are applied differentially to minimize change detection signals."

[UkoeHB]

Something that would be helpful, I think, is a high-level description of the propagation algorithm.

I wrote a description and notes at the top of the propagate_render_groups.rs file.

[viridia]

Something that would be helpful, I think, is a high-level description of the propagation algorithm.

I wrote a description and notes at the top of the propagate_render_groups.rs file.

That looks really good.

Part of the motivation for wanting this description is to be able to make an argument as to why this approach is better than alternatives (such as an exclusive system with world access, or a giant query with a bunch of optional components which are then accessed via entity id instead of sequential scanning), both of which are obvious patterns for hierarchical traversals.

[tychedelia]

This looks great! Thanks for tackling this, it's going to be a huge help.

Regarding your open questions:

How to put reasonable limits on RenderLayer?

I think, at best, we should warn. My use case is a little niche because I'm allowing users to spawn cameras at will, but I'd rather have them work around poor performance than be arbitrarily limited. In my case, the allocations from going about 64 layers are unlikely to matter, especially compared to whatever other lurking inefficiencies there are in the renderer with having so many cameras.

This PBR shader uses render layers internally (GpuDirectionalLight and ViewUniform and DirectionalLight and View).

I commented on the issue here #12468 (comment), but I think the preference is to remove RenderLayers from the shader code entirely and compute light visibility on the cpu as property of the view. We could include layers as a dynamic array on the view itself, but iirc there were problems including the layers on the light struct as they're already nested inside another array.

[tychedelia]

Nit: I can't decide if "view" is too overloaded here. I totally understand the description of the component as "things I can see", but might get confused in it's relation to something like ExtractedView in render world.

[UkoeHB]

Yeah it's pretty overloaded, but it feels really comfortable for this use-case.

[UkoeHB]

Changing this may invalidate directional_depth_texture_array_index, someone who understands this needs to check.

[UkoeHB]

I commented on the issue here #12468 (comment), but I think the preference is to remove RenderLayers from the shader code entirely and compute light visibility on the cpu as property of the view. We could include layers as a dynamic array on the view itself, but iirc there were problems including the layers on the light struct as they're already nested inside another array.

Ok the check has been moved to the CPU code. I added a skip field to the GpuDirectionalLight struct, which allows the shader to skip directional lights not visible to a particular view. It's kind of a hack, but hard to see a better option that doesn't involve a major refactor of this dense code.

[UkoeHB]

I think, at best, we should warn.

Added a warning at 1024 layers, and a panic failsafe if you exceed 1mill, which would be 24MB per RenderLayers struct.

[viridia]

@alice-i-cecile @UkoeHB I tried to make an example, but I couldn't get it to work - possibly I am doing something wrong. Here's what I did:

• Start with the example update_gltf_scene which spawns two copies of the flight helmet model.
• Add RenderGroups::default().add(1).clone() to the first model spawn. (I'm not sure this is the best way to create a RenderGroups with a given layer).
• Add RenderGroups::default().add(2).clone() to the second model spawn.
• Finally, add RenderGroups::default().add(1).clone() to the camera spawn.

At this point I would have expected the second model not to be visible. However, both models were visible.

[UkoeHB]

At this point I would have expected the second model not to be visible. However, both models were visible.

A default RenderGroups starts with the default render layer. Use RenderGroups::from() instead. Also, the camera uses CameraView, not RenderGroups.

[viridia]

If I try RenderGroups::from(RenderLayers(2)) I get the following error:

error[E0391]: cycle detected when getting the resolver for lowering
  |
  = note: ...which requires normalizing `bevy_render::view::visibility::render_groups::RenderLayers::iter_layers::{opaque#0}`...
  = note: ...which requires looking up limits...
  = note: ...which requires getting the crate HIR...
  = note: ...which again requires getting the resolver for lowering, completing the cycle
  = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

[viridia]

I changed the camera to CameraView, but I still see both models.

[UkoeHB]

I changed the camera to CameraView, but I still see both models.

Can you share your code? Maybe in a gist. If there is a bug then I will fix it ASAP.

[viridia]

Start with the existing update_gltf_scene example (which is relatively short), and change the setup function to the following:

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn((
        DirectionalLightBundle {
            transform: Transform::from_xyz(4.0, 25.0, 8.0).looking_at(Vec3::ZERO, Vec3::Y),
            directional_light: DirectionalLight {
                shadows_enabled: true,
                ..default()
            },
            ..default()
        },
        RenderGroups::default().add(1).clone(),
    ));
    commands.spawn((
        Camera3dBundle {
            transform: Transform::from_xyz(-0.5, 0.9, 1.5)
                .looking_at(Vec3::new(-0.5, 0.3, 0.0), Vec3::Y),
            ..default()
        },
        EnvironmentMapLight {
            diffuse_map: asset_server.load("environment_maps/pisa_diffuse_rgb9e5_zstd.ktx2"),
            specular_map: asset_server.load("environment_maps/pisa_specular_rgb9e5_zstd.ktx2"),
            intensity: 1500.0,
        },
        CameraView::default().add(1).clone(),
    ));

    // Spawn the scene as a child of this entity at the given transform
    commands.spawn((
        SceneBundle {
            transform: Transform::from_xyz(-1.0, 0.0, 0.0),
            scene: asset_server.load("models/FlightHelmet/FlightHelmet.gltf#Scene0"),
            ..default()
        },
        RenderGroups::default().add(1).clone(),
    ));

    // Spawn a second scene, and add a tag component to be able to target it later
    commands.spawn((
        SceneBundle {
            scene: asset_server.load("models/FlightHelmet/FlightHelmet.gltf#Scene0"),
            ..default()
        },
        MovedScene,
        RenderGroups::default().add(2).clone(),
    ));
}

[github-actions]

The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.

[github-actions]

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

[UkoeHB]

Changing this to ADOPT ME status since I don't have time or motivation to continue working on it.

[tychedelia]

Opened a PR to port removing the layer limit (which seemed less controversial) #13317