[Merged by Bors] - Reduce branching in TrackedRenderPass

[james7132]

Objective

Speed up the render phase for rendering.

Solution

• Follow up [Merged by Bors] - Replace UUID based IDs with a atomic-counted ones #6988 and make the internals of atomic IDs NonZeroU32. This niches the Options of the IDs in draw state, which reduces the size and branching behavior when evaluating for equality.
• Require &RenderDevice to get the device's Limits when initializing a TrackedRenderPass to preallocate the bind groups and vertex buffer state in DrawState, this removes the branch on needing to resize those Vecs.

Performance

This produces a similar speed up akin to that of #6885. This shows an approximate 6% speed up in main_opaque_pass_3d on many_foxes (408.79 us -> 388us). This should be orthogonal to the gains seen there.

---

Changelog

Added: RenderContext::begin_tracked_render_pass.
Changed: TrackedRenderPass now requires a &RenderDevice on construction.
Removed: bevy_render::render_phase::DrawState. It was not usable in any form outside of bevy_render.

Migration Guide

TrackedRenderPass now requires a RenderDevice to construct. To make this easier, use RenderContext.begin_tracked_render_pass instead.

// 0.9
TrackedRenderPass::new(render_context.command_encoder.begin_render_pass(
  &RenderPassDescriptor {
    ...
  },
));

// 0.10
render_context.begin_tracked_render_pass(RenderPassDescriptor {
  ...
});

[kurtkuehnert]

Great change.

I like the NonZeroU32 optimization for the atomic counters.

One issue I have is the creation of the TrackedRenderPass. I think we should unnest this

let render_pass = render_context
            .command_encoder
            .begin_render_pass(&pass_descriptor);
let mut render_pass = TrackedRenderPass::new(&render_context.render_device, render_pass);

into a single statement. This can also be separated into another PR.

Here are some ideas:

1. We implement a method begin_tracked_render_pass on the RenderContext:

let mut render_pass = render_context.begin_tracked_render_pass(&pass_descriptor);

2. We new-type the CommandEncoder and add the begin_tracked_render_pass method to take a RenderDevice as an argument and return a TrackedRenderPass.

let mut render_pass = render_context.command_encoder.begin_tracked_render_pass(&render_context.render_device, &pass_descriptor);

3. We expand on the idea of 2. and make the TrackedRenderPass our default render pass abstraction. We rename it to RenderPass and hide the wgpu one.

let mut render_pass = render_context.command_encoder.begin_render_pass(&render_context.render_device, &pass_descriptor);

4. We create the TrackedRenderPass using its new constructor from the RenderContext.

let mut render_pass = TrackedRenderPass::new(&render_context, &pass_descriptor);

My preferred order would be 1 over 3 over 2 over 4.

What do you say to making the TrackedRenderPass our default RenderPass abstraction? I think this tidies up our API and our users won't have to choose between the two.

If we go with 1 we could also hide the CommandEncoder entirely and elevate the relevant methods up to the RenderContext. Especially when we decide to build an TrackedComputePass abstraction in the future.

[IceSentry]

I may be missing something, but does this means we always allocate a big buffer for every render pass? Is it small enough that it doesn't matter. The previous impl looked like it would just allocate memory when necessary, if you only need 2-3 bind groups this seems wasteful.

Is it just that the speed up is worth the small memory increase?

[james7132]

The latter. The actual size of these Vecs aren't that big (1-2KB at most), and is a static upfront cost compared to the scaling cost of branching on every RenderCommand (not just every entity) that uses it.

The actual limits here are also usually quite small. 16 for both vertex buffers and bind groups, IIRC.

[IceSentry]

Alright, makes sense then, might be worth adding a small note in a comment I guess, but I'll approve anyway

[mockersf]

The actual limits here are also usually quite small. 16 for both vertex buffers and bind groups, IIRC.

Some devices may report u32::MAX for those limits instead of an actual value, #6841 for example for max_storage_buffers_per_shader_stage. I don't know if that's also the case for max_bind_groups or max_vertex_buffers but that's a possibility

[james7132]

After talking about this with cwfitzgerald about this on Discord, it seems like the limits for vertex buffers and bind groups are generally pretty low (max a few hundred). It seems like the limit is hard set in wgpu-hal mostly to avoid reallocations from Vecs.

In the worst case, we should set a reasonable upper limit ourselves (i.e. 1024) which we use min with the Device provided limits to be 100% sure it's not overallocating.

Original discussion: https://discord.com/channels/691052431525675048/743663924229963868/1059900749636702309

[IceSentry]

Yeah, after reading the discussion I had the same conclusion.

[james7132]

Ended up using wgpu-hal's public constants MAX_VERTEX_BUFFERS and MAX_BIND_GROUPS so we're not keeping them out of sync.

[IceSentry]

I really like that it makes using TrackedRenderPass easier on top of the perf gains.

[alice-i-cecile]

bors r+

[bors]

Pull request successfully merged into main.

Build succeeded:

• build-and-install-on-iOS
• build-android
• build (macos-latest)
• build (ubuntu-latest)
• build-wasm
• build (windows-latest)
• build-without-default-features (bevy)
• build-without-default-features (bevy_ecs)
• build-without-default-features (bevy_reflect)
• check-compiles
• check-doc
• check-missing-examples-in-docs
• ci
• markdownlint
• run-examples
• run-examples-on-wasm
• run-examples-on-windows-dx12