Stowage/godot
2
0
Fork 0
mirror of https://github.com/godotengine/godot.git synced 2025-10-27 19:54:27 +00:00
Code Issues Projects Releases Packages Wiki Activity

Fix uninitialized value in Tonemap

Browse source 
And anything that uses luminance.

The class Luminance in luminance.cpp is in charge of averaging the
luminance of all pixels.
It performs multiple passes until it reaches a 1x1 texture containing
the total average. This is standard luminance averaging on GPU.

Then the "result" of this frame and the "prev_frame_result" are averaged
together at a certain speed to mimic eye adaptation.

Then this avarege becomes the "source" for the next frame. This is done
here:

```cpp
SWAP(p_luminance_buffers->current,
p_luminance_buffers->reduce.write[p_luminance_buffers->reduce.size() -
1]);
```

So far pretty normal stuff.

**The problem is: prev_frame_result IS UNINITIALIZED**. Therefore it's
possible for prev_frame_result to contain garbage values like -5+e15
which causes the screen to stay black for a minute until eye adaptation
catches up.

Windows will always force allocations to be reset to 0, but Linux does
not do that.
However Windows just delays the bug; because it's possible for VMA to
reuse a block.

You can repro this bug by downloading Bistro, creating a camera,
selecting a default scene; and then launching Bistro.

Everything will work fine.
Until you decide to resize the window. It takes a few tries on Godot,
but eventually the screen becomes black.

If you wait around a minute, the screen will "unblack" itself back to
normal.

Even if it's not stuck in black after resize, you may notice that every
resize is inconsistent in how the eye adaptation catches up (i.e.
sometimes it flashes to white, sometimes it does not).

If you can't repro the bug, you need to try harder by doing arbitrary
resizes until it triggers.
Also, I advise to try this on Linux; since Windows' sanitization of
memory gets in the way.

There's probably multiple tickets already filled around issues that were
rooted in luminance calculations starting from uninitialized memory.

This PR sets a default value of 0, which causes the screen to always
flash white after resize. Setting a different value like 0.1 makes the
flash effect weaker. Setting it to a high value like 5.0 makes the
screen flash from dark instead.

This bug can be backported to 4.3. I don't know if it can be backported
to earlier; as the render graph makes sure the texture_clear() calls
gets issued in the right place; whereas in <= 4.2 it might be
problematic depending on when Luminance::LuminanceBuffers::configure is
being called.
This commit is contained in:
Matias N. Goldberg 2025-02-20 16:51:41 -03:00
parent 8ed125b429
commit 8888f9e649
1 changed files with 5 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

8
servers/rendering/renderer_rd/effects/luminance.cpp
View file

@ -102,9 +102,10 @@ void Luminance::LuminanceBuffers::configure(RenderSceneBuffersRD *p_render_buffe
tf.usage_bits = RD::TEXTURE_USAGE_COLOR_ATTACHMENT_BIT | RD::TEXTURE_USAGE_SAMPLING_BIT;
} else {
tf.usage_bits = RD::TEXTURE_USAGE_STORAGE_BIT;
if (final) {
tf.usage_bits |= RD::TEXTURE_USAGE_SAMPLING_BIT;
}
}
if (final) {
tf.usage_bits |= RD::TEXTURE_USAGE_SAMPLING_BIT | RD::TEXTURE_USAGE_CAN_COPY_TO_BIT;
}
RID texture = RD::get_singleton()->texture_create(tf, RD::TextureView());
@ -112,6 +113,7 @@ void Luminance::LuminanceBuffers::configure(RenderSceneBuffersRD *p_render_buffe
if (final) {
current = RD::get_singleton()->texture_create(tf, RD::TextureView());
RD::get_singleton()->texture_clear(current, Color(0.0, 0.0, 0.0), 0u, 1u, 0u, 1u);
break;
}
}