godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.15k stars 20.21k forks source link

Change the timing of creating the radiance texture for panoramic sky #19030

Closed BastiaanOlij closed 4 years ago

BastiaanOlij commented 6 years ago

Currently the radiance texture is generated whenever the texture is assigned to the panoramic sky by the logic found here: https://github.com/godotengine/godot/blob/master/drivers/gles3/rasterizer_storage_gles3.cpp#L1353

With viewport textures this is a problem because this happens as the scene is loaded and the viewport texture won't be fully setup at this point in time. It won't have contents until we start rendering. Also it means that if the viewport is updated we never update the radiance map. There is a silver lining there because it stops people from rendering a skymap every frame and grinding their game to a halt but for occasional updates it is a problem.

Both have workarounds, simply re-assigning the texture on the panoramic sky triggers the logic and updates all the buffers required (this is what I do in my sky asset).

Now the first problem I can fix, but the second problem I need some input into hence I haven't applied a PR for this yet.

What I suggest for solving the first problem is by removing the radiance map generation logic from RasterizerStorageGLES3::sky_set_texture and simply setting a flag "is_dirty".

Then just before we render our sky in RasterizerSceneGLES3::_draw_sky we check if this flag is set, and if so update the radiance buffer. There might be a one frame delay in the radiance buffer being used (as we've already rendered our geometry) but I think that is acceptable.

That said, an improvement on this approach that would also solve #2, and which may be doable thanks to the proxy texture approach if we record a little more info, is to trigger rendering the radiance map in response to the viewport being updated. So in our viewport render loop in VisualServerViewport::draw_viewports, once a viewport has been rendered we check all the consumers of the viewport texture for that viewport and do some sort of callback that the consumer registers when it creates the proxy. In the case of a panoramic sky that callback results in the radiance map being updated.

@reduz , @karroffel , any ideas, suggestions, etc? I'll happily build the logic in if we come to a consensus of how we want to approach this.

akien-mga commented 4 years ago

What's the status on this issue?

CC @clayjohn

clayjohn commented 4 years ago

It still seems relevant to me. But it's better to leave bumped to 4.0 as no other users have noticed or cared. Reduz has already done a lot to overhaul sky/environment maps and @BastiaanOlij is doing more work on sky shaders.

Kersoph commented 4 years ago

With 3.2 the time neded for all sorts of calculations when you add a new panorama texture increased due to new features in a difficult region for dynamic skyboxes for my projects and tests. So far I could use dynamic panoramic skyboxes with carefully timing and precalculating as much as I could. To assign a new panoramic sky it took me around 10ms and I added the option to disable sky updates for lower region hardware. With 3.2 it takes 37ms on my hardware with the same settings for my project. If we use the Godot-Sky-Asset-Master as a reference it is 13ms for 3.1.1 and 74ms for 3.2. (GTX 970, i7-2600K) If I change all to the lowest resolution possible (32 radiance & irradiance) it is 9ms for 3.1.1 and 22ms for 3.2 on my pc. 22ms even on the lowest resolution setting gets you always a heavy delay that you stand no chance against as you can not compensate it. From the "Users View" "sky.Panorama = LastTexture;" is an atomic assignment. I think we either parallelize the process (which is prob very difficult as the maps are generatied on the GPU in godot) or provide non atomic assigments.

There may even be some simple solutions: 1- If we provide an additional method like UpdatePanorama(Texture panorama, RadianceMap radiance, IrradianceMap irradiance) we could precalculate the needed maps, interpolate between calculated maps or also distribute the calculation need over several frames and then assign them together. So we still have the current approach but provide also the possibility to assign them directly together. 2- If it is possible to deactivate the auto generation so that we can state specific times when to generate new rad&irrrad. I think its often ok to just update the panoramic sky with a new texture and uptate the rad or irrad just every few frames and distribute the calculation need over several frames. But of course the new uptates must stay in a certain perfornamce region, otherwise we did not really gain anything.

I personally would prefer 1- as it provides me many new options to work with and allows to parallelize many tasks. The dirty flag would improve the timing but to get dynamic skyboxes feasible we need a different approach I think as the calculation time is very high and currently not possible in parallel.

Kersoph commented 4 years ago

For me it looks like https://github.com/godotengine/godot/pull/37179 solves the problem we have with the timing and the dynamic update. Aswell as gives us access to the new Skyshader system which is looking very promising! Infos in the Article https://godotengine.org/article/custom-sky-shaders-godot-4-0 I'm very exited!

clayjohn commented 4 years ago

Closed by #37179

:)