Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exit light calculation early when pixel outside of light bounding rectangle #90920

Merged
merged 1 commit into from Apr 26, 2024

Conversation

clayjohn
Copy link
Member

@clayjohn clayjohn commented Apr 19, 2024

Potential fix for #81152
Fixes: #90905

Right now when rendering a CanvasItem for each pixel in the fragment shader we loop over all lights that intersect that CanvasItem and calculate their contribution to the pixel. If the pixel is outside of the light's texture, we set the alpha value of the final result to 0 which results in that light not contributing to the pixel.

Unfortunately, this means that we read from the light's texture still and run the light shader for that light, even though it won't have any effect.

Normally, using a continue op in a loop is not a good idea is it can stop the compiler from doing certain beneficial optimizations. But in this case, the compiler can't optimize this loop for many other reasons, so this ends up being a net win.

Performance testing

iGPU RD iGPU compatibility Pixel 4 RD Pixel 4 compatibility
Before 240 FPS 140 FPS 42 FPS 50 FPS
After 370 FPS 275 FPS 60 FPS (locked) 60 FPS (locked)
  • Note the Pixel 4 does not support not using VSync, so it is unclear what the actual performance gain is as the device caps at 60 FPS.

Further testing

This PR would benefit from being tested on even lower end mobile, on web export, and on a desktop device with a dedicated GPU (done)

Light2DTestUpdated.zip

…tangle

This hugely improves the performance of rendering PointLight2Ds
Copy link
Member

@lawnjelly lawnjelly left a comment

Choose a reason for hiding this comment

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

Shaders look fine to me, although as said it would be nice to get some more testing on low end mobile and dedicated GPU.

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally, it works as expected.

llvmpipe RD (1080p) llvmpipe compatibility1 4090 RD (4K) 4090 compatibility (4K)
Before 22 FPS N/A 2667 FPS 2584 FPS
After 27 FPS N/A 3520 FPS 3646 FPS

Footnotes

  1. I couldn't figure out how to benchmark this one reliably (I can use Xephyr but this breaks any performance expectations due to its own overhead). LIBGL_ALWAYS_SOFTWARE=1 does not work on NVIDIA; it only has an effect on Mesa drivers.

@tdaven
Copy link
Contributor

tdaven commented Apr 23, 2024

1. I couldn't figure out how to benchmark this one reliably (I can use Xephyr but this breaks any performance expectations due to its own overhead). `LIBGL_ALWAYS_SOFTWARE=1` does not work on NVIDIA; it only has an effect on Mesa drivers. [↩](#user-content-fnref-1-33b3f095a6e19db5ec7ccee2585c9c85)

If your system is using glvnd and EGL you might be able to use this:
https://gitlab.freedesktop.org/glvnd/libglvnd/-/blob/master/src/EGL/icd_enumeration.md?ref_type=heads

@akien-mga akien-mga merged commit 22c8a27 into godotengine:master Apr 26, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug with "Cached Lights" in 2D, destroying the illumination system
5 participants