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

Fix for GLES3 radiance cubemap update #97069

Merged
merged 1 commit into from
Sep 17, 2024

Conversation

patwork
Copy link
Contributor

@patwork patwork commented Sep 16, 2024

Info

With this fix, changing the WorldEnvironment's sky colours in Compatibility mode updates the radiance cubemap automatically, just as it happens in Vulcan. The same applies to the Preview Environment button in the editor.

The logical condition is now the same as in the Vulcan version. Could I ask for a review by the authors of the original pieces of code? @clayjohn @BastiaanOlij

// Update radiance cubemap
if (sky->reflection_dirty && (sky->processing_layer > max_processing_layer || update_single_frame)) {

// Update radiance cubemap
if (sky->reflection.dirty && (sky->processing_layer >= max_processing_layer || update_single_frame)) {

Bug

Previously, making changes to the sky colours in Compatibility mode only produced an effect in the background of the scene. 3D objects were still illuminated by the previous environment, as the cubemap was not regenerated.

Lighting of objects was updated only after reloading the scene or creating a new WorldEnvironment or setting the process_mode in Sky to SKY_MODE_REALTIME.

  1. scene
    scr 2024-09-16 09-39-40

  2. after changing sky colors in preview
    scr 2024-09-16 09-40-38

  3. after saving changes into worldenvironment
    scr 2024-09-16 09-40-49

Notes

I don't know to what extent this is normal behaviour, but for both GLES and Vulcan, in the default process_mode (SKY_MODE_AUTOMATIC), the radiance cubemap regenerates after a some delay (in Vulcan even after a few seconds). When SKY_MODE_REALTIME mode is enabled, the map regenerates immediately (maybe this mode should be selected by default when working in the editor?).

@patwork patwork requested a review from a team as a code owner September 16, 2024 10:33
@AThousandShips AThousandShips added this to the 4.4 milestone Sep 16, 2024
@clayjohn clayjohn added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Sep 16, 2024
@clayjohn
Copy link
Member

I don't know to what extent this is normal behaviour, but for both GLES and Vulcan, in the default process_mode (SKY_MODE_AUTOMATIC), the radiance cubemap regenerates after a some delay (in Vulcan even after a few seconds). When SKY_MODE_REALTIME mode is enabled, the map regenerates immediately (maybe this mode should be selected by default when working in the editor?).

I think this is because it is updating over a few frames and the editor, by default only redraws when something changes. Can you double check by wiggling the mouse in the viewport to see if that makes it update right away?

Copy link
Contributor

@BastiaanOlij BastiaanOlij left a comment

Choose a reason for hiding this comment

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

Interesting typo. Seems to indeed make sense, if I recall correctly we're processing +1 once all the layers are filled and we need to create the final result. So in GLES we just ended up not doing that final step.

@BastiaanOlij
Copy link
Contributor

I don't know to what extent this is normal behaviour, but for both GLES and Vulcan, in the default process_mode (SKY_MODE_AUTOMATIC), the radiance cubemap regenerates after a some delay (in Vulcan even after a few seconds). When SKY_MODE_REALTIME mode is enabled, the map regenerates immediately (maybe this mode should be selected by default when working in the editor?).

This is an interesting remark, because we're spreading updating the radiance map over a few frames, and the editor will only render a frame when there is a reason to do so, this can indeed be delayed any given time.
In runtime this is not a problem as we will render every frame so the radiance map is updated with a few milliseconds.

Not sure if setting it to update always is a good fix. I think a better fix here is to enhance the logic that skips rendering the frame in the editor when nothing has changed to also check if we're done updating radiance maps, or shadow maps, or any other things we spread out over multiple frames. I think that falls outside of this issue though.

@akien-mga akien-mga merged commit 9c4e3fb into godotengine:master Sep 17, 2024
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 4.3.1.

@akien-mga akien-mga removed the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Sep 17, 2024
@patwork
Copy link
Contributor Author

patwork commented Sep 17, 2024

Thank you for reviews and merge.

Indeed, wiggling the mouse in the editor causes the cubemap to update faster.

@Calinou
Copy link
Member

Calinou commented Sep 17, 2024

I think a better fix here is to enhance the logic that skips rendering the frame in the editor when nothing has changed to also check if we're done updating radiance maps, or shadow maps, or any other things we spread out over multiple frames.

Indeed, we have similar issues with TAA, volumetric fog or SDFGI.

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.

6 participants