Skip to content

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

Merged
alice-i-cecile merged 2 commits intobevyengine:mainfrom
pcwalton:multidraw-shadows
Dec 10, 2024
Merged

Use multidraw for shadows when GPU culling is in use.#16692
alice-i-cecile merged 2 commits intobevyengine:mainfrom
pcwalton:multidraw-shadows

Conversation

@pcwalton
Copy link
Copy Markdown
Contributor

@pcwalton pcwalton commented Dec 6, 2024

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.

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.
@pcwalton 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 pcwalton added this to the 0.16 milestone Dec 6, 2024
@pcwalton pcwalton requested a review from kristoff3r December 6, 2024 23:32
Copy link
Copy Markdown
Contributor

@kristoff3r kristoff3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, tested a few lighting/shadow examples with/without GpuCulling.

@JMS55 JMS55 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 7, 2024
@alice-i-cecile
Copy link
Copy Markdown
Member

Testing the example runs before merging <3

@pcwalton
Copy link
Copy Markdown
Contributor Author

@alice-i-cecile Success

@alice-i-cecile alice-i-cecile added this pull request to the merge queue Dec 10, 2024
Merged via the queue into bevyengine:main with commit 7236070 Dec 10, 2024
BD103 pushed a commit to BD103/bevy that referenced this pull request Dec 10, 2024
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.
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants