Optimize ProceduralSkyMaterial by removing uses of acos and simplifying logic #101973
Conversation
…ng logic The result looks almost the same and runs over twice as fast
|
love to see performance optimizations like this coming down the pipe! they might not be as flashy, but those of us developing for tight performance targets like standalone VR really appreciate it! (I remember profiling some scenes a while back in renderdoc and thinking the procedural sky render was taking oddly long for what I assumed it needed to actually do, so this is great news) |
| @@ -65,7 +67,8 @@ float ProceduralSkyMaterial::get_sky_curve() const { | |||
|
|
|||
| void ProceduralSkyMaterial::set_sky_energy_multiplier(float p_multiplier) { | |||
There was a problem hiding this comment.
This setter must also update "sky_cover_modulate" material param
| @@ -87,7 +95,7 @@ Ref<Texture2D> ProceduralSkyMaterial::get_sky_cover() const { | |||
|
|
|||
| void ProceduralSkyMaterial::set_sky_cover_modulate(const Color &p_sky_cover_modulate) { | |||
| sky_cover_modulate = p_sky_cover_modulate; | |||
| RS::get_singleton()->material_set_param(_get_material(), "sky_cover_modulate", sky_cover_modulate); | |||
| RS::get_singleton()->material_set_param(_get_material(), "sky_cover_modulate", Color(sky_cover_modulate.r, sky_cover_modulate.g, sky_cover_modulate.b, sky_cover_modulate.a * sky_energy_multiplier)); | |||
There was a problem hiding this comment.
sky energy multiplier is multiplying alpha, not the entire thing
There was a problem hiding this comment.
That's no issue, see how vec4 sky_cover_texture was previously calculated in the shader vs. how it is done now. It appears equivalent to me.
There was a problem hiding this comment.
if you say so, just stood out to me at a glance
The changes can be clumped into three categories:
acos(), instead do all calculations using cosines instead of anglesThe result looks almost the same and runs over twice as fast.
Performance
M2 MBP
I used a test scene with 3 4k Viewports that do nothing but render the sky.
Before: 36 FPS (27.78 mspf)
After: 52 FPS (19.23 mspf)
No sky: 75 FPS (13.33 mspf)
Therefore we can say this reduced the cost of rendering the sky (to 409640963 pixels) from 14.45 ms to 5.9 ms! That's over twice as fast. Given that this is our default sky, I think that improvement is well worth making.
Pixel 4
This used a test scene with just a camera, 4 DirectionalLights, and a scaling factor of 2
Before: 44 FPS (22.73 mspf)
After: 52 FPS (19.23 mspf)
No sky: 60 FPS (locked) (16.67 mspf)
With resolution scaling that's rendering 2160 x 4560 pixels per frame with the sky. Saving 3.5 ms on the sky alone is pretty nice. At 1x resolution scale I am locked at 60 FPS in all cases
Looking at the GPU PROFILE when rendering at native res, this PR brings saves about 0.5 ms (of 3.0 ms). Not as much improvement as I expected
Notes:
For posterity, this change was inspired by a blog post from Kostas Anagnostou above the hidden cost of certain shader functions (like acos, atan, etc.). Which led me to a lovely blog by Sebastian Lagarde on approximating acos/asin/atan to drastically reduce instruction count. I searched our codebase and started weeding out places where we use such expensive functions. ProceduralSkyMaterial is the worst offender by far.
After replacing
acos()with an approximation (and seeing a nice performance boost) I realized that the shader could still be vastly improved and ended up removingacos()altogether.TODO
Test on an android device (particularly a low end device)