Contrast and Brightness slider become disabled under certain circumstances
If you drag the contrast slider with the mouse cursor and pull it up to the gamma slider, this will cause both the contrast and brightness slider to be locked, and the gamma to become unresponsive.
This can only be undone by resetting to defaults.
This occurs as of r9393, and back to at least r6775, haven't tested earlier versions yet.
Edit: Confirmed to also occur in NBlood.
Edit 2: The first few contents of gammaTable
, normally values in the range of {0, 256, 512, 768, 1024, ...}, are all set to 65535 if this occurs. I believe this may be an overflow where the increment is set way too high when the mouse cursor grabs onto one slider, and then moves onto another.
Edit 3: Surprisingly, this can occur even without using the mouse cursor. Simply decrease the gamma by 0.05, and the brightness once by 0.05 with the keyboard, and the issue will already occur.
Edit 4: This issue is a bit more complex than a simple error. First, watch the following video:
https://cdn.discordapp.com/attachments/309183622143803392/858665750482518076/2021-06-27_12-55-29.mp4
Interestingly, the contrast and brightness sliders are locked as soon as the Contrast is set to 1.05, and the gamma is 0.95. Since this isn't an unreasonable setting at all, something is clearly wrong here.
Bisection reveals that this latter issue occurs since r7078 (5297a5f3). However, this commit merely swaps out the previous min/max macro for standard library versions. The problem is only apparent if we look at what videoSetGamma()
computes:
int32_t videoSetGamma(void)
{
if (novideo)
return 0;
int32_t i;
uint16_t gammaTable[768];
float gamma = max(0.1f, min(4.f, g_videoGamma));
float contrast = max(0.1f, min(3.f, g_videoContrast));
float bright = max(-0.8f, min(0.8f, g_videoBrightness));
float invgamma = 1.f / gamma;
float norm = powf(255.f, invgamma - 1.f);
if (lastvidgcb[0] == gamma && lastvidgcb[1] == contrast && lastvidgcb[2] == bright)
return 0;
// This formula is taken from Doomsday
for (i = 0; i < 256; i++)
{
float val = i * contrast - (contrast - 1.f) * 127.f;
if (gamma != 1.f)
val = powf(val, invgamma) / norm;
val += bright * 128.f;
gammaTable[i] = gammaTable[i + 256] = gammaTable[i + 512] = (uint16_t)max(0.f, min(65535.f, val * 256.f));
}
...
Assume contrast
is 1.05
and gamma
is 0.95
as we have seen in the video (invgamma ~= 1.053
). Then, in the first iteration of the loop over i
, we first compute val = - 6.35
. Since gamma != 1.0
is true, we then compute val = (val ^ invgamma) / norm
. I.e. we compute -6.35 ^ 1.053, which has no real-valued solution, and as such the standard library function outputs NaN
.
This NaN is then passed into std::min. Here is why r7077 and r7078 differ. In r7077, min
and max
were defined as:
#define max(a, b) (((a) > (b)) ? (a) : (b))
#define min(a, b) (((a) < (b)) ? (a) : (b))
Note that the <
and >
operators always return false if either of the arguments is NaN
. Casting NaN
to (uint16_t) returns 0
. As such, in the case of r7077, the code computes gammaTable[0] = 0
.
However, if we replace these operators with the std::min
and std::max
implementations, the min
actually returns 65535.f, and therefore gammaTable[0] = 65535.f
. This causes the errors we see in the video.
As such, the formula from Doomsday is wrong.