bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Do not re-check visibility or re-render shadow maps for point and spot lights for each view

Open coreh opened this issue 1 year ago • 1 comments

Objective

If I understand it correctly, we were checking mesh visibility, as well as re-rendering point and spot light shadow maps for each view. This makes it so that M views and N lights produce M x N complexity. This PR aims to fix that, as well as introduce a stress test for this specific scenario.

Solution

  • Keep track of what lights have already had mesh visibility calculated and do not calculate it again;
  • Reuse shadow depth textures and attachments across all views, and only render shadow maps for the first time a light is encountered on a view;
  • Directional lights remain unaltered, since their shadow map cascades are view-dependent;
  • Add a new many_cameras_lights stress test example to verify the solution

Showcase

110% speed up on the stress test 83% reduction of memory usage in stress test

Before (5.35 FPS on stress test)

Screenshot 2024-09-11 at 12 25 57

After (11.34 FPS on stress test)

Screenshot 2024-09-11 at 12 24 35

Testing

  • Did you test these changes? If so, how?
    • On my game project where I have two cameras, and many shadow casting lights I managed to get pretty much double the FPS.
    • Also included a stress test, see the comparison above
  • Are there any parts that need more testing?
    • Yes, I would like help verifying that this fix is indeed correct, and that we were really re-rendering the shadow maps by mistake and it's indeed okay to not do that
  • How can other people (reviewers) test your changes? Is there anything specific they need to know?
    • Run the many_cameras_lights example
    • On the main branch, cherry pick the commit with the example (git cherry-pick --no-commit 1ed4ace01) and run it
  • If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
    • macOS

coreh avatar Sep 11 '24 04:09 coreh

@coreh, are you able to resolve the merge conflicts here? :)

alice-i-cecile avatar Oct 13 '24 17:10 alice-i-cecile

I'll take a look at this tomorrow and see if the merge conflicts are simple enough.

BenjaminBrienen avatar Oct 29 '24 02:10 BenjaminBrienen

I can't merge this: CI is failing due to disallowed platform-inconsistent float methods. It's an easy fix at least :)

alice-i-cecile avatar Nov 04 '24 14:11 alice-i-cecile