Turn on GPU culling automatically if the target platform supports it. by pcwalton · Pull Request #16670 · bevyengine/bevy

pcwalton · 2024-12-06T00:11:13Z

This commit removes the undocumented GpuCulling component in favor of automatically turning GPU culling on when the platform supports it. The main reason to make GPU culling automatic is that GPU culling enables indirect mode. Indirect mode is needed for multidraw (#16427), because non-indirect multidraw doesn't exist in wgpu. Since multidraw is such a win for performance, when that feature is supported the small performance tax that indirect mode incurs is virtually always worth paying.

CPU culling is always used in addition to GPU culling unless the NoCpuCulling component is placed on the camera. This results in some amount of redundant computation on the GPU, but the overhead is negligible. I figured that the GPU time savings gained from skipping this computation when not needed didn't outweigh the costs of the added complexity that would be necessarily to implement that optimization, especially with GPU two-phase occlusion culling on the horizon.

Migration Guide

The GpuCulling component has been removed. GPU culling is now automatically enabled for all cameras if the hardware and platform support it.

This commit removes the undocumented `GpuCulling` component in favor of automatically turning GPU culling on when the platform supports it. The main reason to make GPU culling automatic is that GPU culling enables indirect mode. Indirect mode is needed for multidraw (bevyengine#16427), because non-indirect multidraw doesn't exist in `wgpu`. Since multidraw is such a win for performance, when that feature is supported the small performance tax that indirect mode incurs is virtually always worth paying. CPU culling is always used in addition to GPU culling unless the `NoCpuCulling` component is placed on the camera. This results in some amount of redundant computation on the GPU, but the overhead is negligible. I figured that the GPU time savings gained from skipping this computation when not needed didn't outweigh the costs of the added complexity that would be necessarily to implement that optimization, especially with GPU two-phase occlusion culling on the horizon.

alice-i-cecile

Good, I think this is a better design.

This patch makes shadows use multidraw when the camera they'll be drawn to has the `GpuCulling` component. This results in a significant reduction in drawcalls; Bistro Exterior drops to 3 drawcalls for each shadow cascade. Note that PR bevyengine#16670 will remove the `GpuCulling` component, making shadows automatically use multidraw. Beware of that when testing this patch; before bevyengine#16670 lands, you'll need to manually add `GpuCulling` to your camera in order to see any performance benefits.

JMS55

Approved once docs are fixed (see CI).

bushrat011899

Tested many_cubes in Windows 10 on a i5 1240P Framework 13 and everything appears to be working as expected. Code definitely looks cleaner too!

alice-i-cecile · 2024-12-10T03:10:03Z

Testing via example run.

pcwalton · 2024-12-10T05:19:45Z

I believe the failure is #15981.

rparrett · 2024-12-10T13:03:13Z

These seem to be the finished runs:

mac:
https://pixel-eagle.com/project/b25a040a-a980-4602-b90c-d480ab84076d/run/5862/compare/5815
linux:
https://pixel-eagle.com/project/b25a040a-a980-4602-b90c-d480ab84076d/run/5863/compare/5816

Looks like ssr, custom_shader_instancing, deferred_rendering are probably real issues, and the rest are known flakiness.

This patch makes shadows use multidraw when the camera they'll be drawn to has the `GpuCulling` component. This results in a significant reduction in drawcalls; Bistro Exterior drops to 3 drawcalls for each shadow cascade. Note that PR #16670 will remove the `GpuCulling` component, making shadows automatically use multidraw. Beware of that when testing this patch; before #16670 lands, you'll need to manually add `GpuCulling` to your camera in order to see any performance benefits.

…tomatic-gpu-culling

This patch makes shadows use multidraw when the camera they'll be drawn to has the `GpuCulling` component. This results in a significant reduction in drawcalls; Bistro Exterior drops to 3 drawcalls for each shadow cascade. Note that PR bevyengine#16670 will remove the `GpuCulling` component, making shadows automatically use multidraw. Beware of that when testing this patch; before bevyengine#16670 lands, you'll need to manually add `GpuCulling` to your camera in order to see any performance benefits.

multidraw. This commit resolves most of the failures seen in bevyengine#16670. It contains two major fixes: 1. The prepass shaders weren't updated for bindless mode, so they were accessing `material` as a single element instead of as an array. I added the needed `BINDLESS` check. 2. If the mesh didn't support batch set keys (i.e. `get_batch_set_key()` returns `None`), and multidraw was enabled, the batching logic would try to multidraw all the meshes in a bin together instead of disabling multidraw. This is because we checked whether the `Option<BatchSetKey>` for the previous batch was equal to the `Option<BatchSetKey>` for the next batch to determine whether objects could be multidrawn together, which would return true if batch set keys were absent, causing an entire bin to be multidrawn together. This patch fixes the logic so that multidraw is only enabled if the batch set keys match *and are `Some`*. Additionally, this commit adds batch key support for bins that use `Opaque3dNoLightmapBinKey`, which in practice means prepasses. Consequently, this patch enables multidraw for the prepass when GPU culling is enabled. When testing this patch, try adding `GpuCulling` to the camera in the `deferred_rendering` and `ssr` examples. You can see that these examples break without this patch and work properly with it.

default. This patch replaces the undocumented `NoGpuCulling` component with a new component, `NoIndirectDrawing`, effectively turning indirect drawing on by default. Indirect mode is needed for the recently-landed multidraw feature (bevyengine#16427). Since multidraw is such a win for performance, when that feature is supported the small performance tax that indirect mode incurs is virtually always worth paying. To ensure that custom drawing code such as that in the `custom_shader_instancing` example continues to function, this commit additionally makes GPU culling take the `NoFrustumCulling` component into account. This PR is an alternative to bevyengine#16670 that doesn't break the `custom_shader_instancing` example. **PR bevyengine#16755 should land first in order to avoid breaking deferred rendering, as multidraw currently breaks it**.

pcwalton · 2024-12-10T23:19:40Z

Closed in favor of #16755 and #16757.

…d multidraw. (#16755) This commit resolves most of the failures seen in #16670. It contains two major fixes: 1. The prepass shaders weren't updated for bindless mode, so they were accessing `material` as a single element instead of as an array. I added the needed `BINDLESS` check. 2. If the mesh didn't support batch set keys (i.e. `get_batch_set_key()` returns `None`), and multidraw was enabled, the batching logic would try to multidraw all the meshes in a bin together instead of disabling multidraw. This is because we checked whether the `Option<BatchSetKey>` for the previous batch was equal to the `Option<BatchSetKey>` for the next batch to determine whether objects could be multidrawn together, which would return true if batch set keys were absent, causing an entire bin to be multidrawn together. This patch fixes the logic so that multidraw is only enabled if the batch set keys match *and are `Some`*. Additionally, this commit adds batch key support for bins that use `Opaque3dNoLightmapBinKey`, which in practice means prepasses. Consequently, this patch enables multidraw for the prepass when GPU culling is enabled. When testing this patch, try adding `GpuCulling` to the camera in the `deferred_rendering` and `ssr` examples. You can see that these examples break without this patch and work properly with it. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

…y default. (#16757) This patch replaces the undocumented `NoGpuCulling` component with a new component, `NoIndirectDrawing`, effectively turning indirect drawing on by default. Indirect mode is needed for the recently-landed multidraw feature (#16427). Since multidraw is such a win for performance, when that feature is supported the small performance tax that indirect mode incurs is virtually always worth paying. To ensure that custom drawing code such as that in the `custom_shader_instancing` example continues to function, this commit additionally makes GPU culling take the `NoFrustumCulling` component into account. This PR is an alternative to #16670 that doesn't break the `custom_shader_instancing` example. **PR #16755 should land first in order to avoid breaking deferred rendering, as multidraw currently breaks it**. ## Migration Guide * Indirect drawing (GPU culling) is now enabled by default, so the `GpuCulling` component is no longer available. To disable indirect mode, which may be useful with custom render nodes, add the new `NoIndirectDrawing` component to your camera.

…d multidraw. (bevyengine#16755) This commit resolves most of the failures seen in bevyengine#16670. It contains two major fixes: 1. The prepass shaders weren't updated for bindless mode, so they were accessing `material` as a single element instead of as an array. I added the needed `BINDLESS` check. 2. If the mesh didn't support batch set keys (i.e. `get_batch_set_key()` returns `None`), and multidraw was enabled, the batching logic would try to multidraw all the meshes in a bin together instead of disabling multidraw. This is because we checked whether the `Option<BatchSetKey>` for the previous batch was equal to the `Option<BatchSetKey>` for the next batch to determine whether objects could be multidrawn together, which would return true if batch set keys were absent, causing an entire bin to be multidrawn together. This patch fixes the logic so that multidraw is only enabled if the batch set keys match *and are `Some`*. Additionally, this commit adds batch key support for bins that use `Opaque3dNoLightmapBinKey`, which in practice means prepasses. Consequently, this patch enables multidraw for the prepass when GPU culling is enabled. When testing this patch, try adding `GpuCulling` to the camera in the `deferred_rendering` and `ssr` examples. You can see that these examples break without this patch and work properly with it. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

This patch makes shadows use multidraw when the camera they'll be drawn to has the `GpuCulling` component. This results in a significant reduction in drawcalls; Bistro Exterior drops to 3 drawcalls for each shadow cascade. Note that PR bevyengine#16670 will remove the `GpuCulling` component, making shadows automatically use multidraw. Beware of that when testing this patch; before bevyengine#16670 lands, you'll need to manually add `GpuCulling` to your camera in order to see any performance benefits.

…d multidraw. (bevyengine#16755) This commit resolves most of the failures seen in bevyengine#16670. It contains two major fixes: 1. The prepass shaders weren't updated for bindless mode, so they were accessing `material` as a single element instead of as an array. I added the needed `BINDLESS` check. 2. If the mesh didn't support batch set keys (i.e. `get_batch_set_key()` returns `None`), and multidraw was enabled, the batching logic would try to multidraw all the meshes in a bin together instead of disabling multidraw. This is because we checked whether the `Option<BatchSetKey>` for the previous batch was equal to the `Option<BatchSetKey>` for the next batch to determine whether objects could be multidrawn together, which would return true if batch set keys were absent, causing an entire bin to be multidrawn together. This patch fixes the logic so that multidraw is only enabled if the batch set keys match *and are `Some`*. Additionally, this commit adds batch key support for bins that use `Opaque3dNoLightmapBinKey`, which in practice means prepasses. Consequently, this patch enables multidraw for the prepass when GPU culling is enabled. When testing this patch, try adding `GpuCulling` to the camera in the `deferred_rendering` and `ssr` examples. You can see that these examples break without this patch and work properly with it. --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>

…y default. (bevyengine#16757) This patch replaces the undocumented `NoGpuCulling` component with a new component, `NoIndirectDrawing`, effectively turning indirect drawing on by default. Indirect mode is needed for the recently-landed multidraw feature (bevyengine#16427). Since multidraw is such a win for performance, when that feature is supported the small performance tax that indirect mode incurs is virtually always worth paying. To ensure that custom drawing code such as that in the `custom_shader_instancing` example continues to function, this commit additionally makes GPU culling take the `NoFrustumCulling` component into account. This PR is an alternative to bevyengine#16670 that doesn't break the `custom_shader_instancing` example. **PR bevyengine#16755 should land first in order to avoid breaking deferred rendering, as multidraw currently breaks it**. ## Migration Guide * Indirect drawing (GPU culling) is now enabled by default, so the `GpuCulling` component is no longer available. To disable indirect mode, which may be useful with custom render nodes, add the new `NoIndirectDrawing` component to your camera.

pcwalton added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 6, 2024

pcwalton requested review from Elabajaba, JMS55 and bushrat011899 December 6, 2024 00:11

alice-i-cecile added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Dec 6, 2024

alice-i-cecile approved these changes Dec 6, 2024

View reviewed changes

pcwalton requested a review from IceSentry December 6, 2024 06:48

Merge branch 'main' into automatic-gpu-culling

fc0cb48

pcwalton mentioned this pull request Dec 6, 2024

Use multidraw for shadows when GPU culling is in use. #16692

Merged

JMS55 approved these changes Dec 6, 2024

View reviewed changes

Fix docs

c8faa32

pcwalton added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Dec 6, 2024

bushrat011899 approved these changes Dec 8, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into automatic-gpu-culling

5e40825

pcwalton added 2 commits December 9, 2024 21:13

Unbreak WebGPU

b6f28a5

Merge remote-tracking branch 'origin/main' into automatic-gpu-culling

968d759

Merge branch 'main' into automatic-gpu-culling

89c1313

alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Dec 10, 2024

pcwalton added 2 commits December 10, 2024 10:45

Merge remote-tracking branch 'origin/main' into automatic-gpu-culling

efdb1d6

Merge fallout

afbef1a

Merge remote-tracking branch 'pcwalton/automatic-gpu-culling' into au…

c33cb46

…tomatic-gpu-culling

github-actions bot mentioned this pull request Dec 10, 2024

16670 bevyengine/bevy-example-runner#63

Closed

pcwalton mentioned this pull request Dec 10, 2024

Update the prepass shaders and fix the batching logic for bindless and multidraw. #16755

Merged

pcwalton mentioned this pull request Dec 10, 2024

Make indirect drawing opt-out instead of opt-in, enabling multidraw by default. #16757

Merged

pcwalton closed this Dec 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Turn on GPU culling automatically if the target platform supports it.#16670

Turn on GPU culling automatically if the target platform supports it.#16670
pcwalton wants to merge 10 commits intobevyengine:mainfrom
pcwalton:automatic-gpu-culling

pcwalton commented Dec 6, 2024

Uh oh!

alice-i-cecile left a comment

Uh oh!

JMS55 left a comment

Uh oh!

bushrat011899 left a comment

Uh oh!

alice-i-cecile commented Dec 10, 2024

Uh oh!

pcwalton commented Dec 10, 2024

Uh oh!

rparrett commented Dec 10, 2024

Uh oh!

pcwalton commented Dec 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

pcwalton commented Dec 6, 2024

Migration Guide

Uh oh!

alice-i-cecile left a comment

Choose a reason for hiding this comment

Uh oh!

JMS55 left a comment

Choose a reason for hiding this comment

Uh oh!

bushrat011899 left a comment

Choose a reason for hiding this comment

Uh oh!

alice-i-cecile commented Dec 10, 2024

Uh oh!

pcwalton commented Dec 10, 2024

Uh oh!

rparrett commented Dec 10, 2024

Uh oh!

pcwalton commented Dec 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants