Monday, January 18, 2010

DirectQ Gamma Creep

Today I finally tracked down and fixed what was probably one of the longest-standing DirectQ bugs: the dreaded "gamma creep" where your screen would gradually get brighter the more you ran DirectQ. This was quite subtle and most people probably wouldn't notice it as the brightness increments were very small, but over time they did build up (especially noticeable if you're developing and running the engine 30+ times in a single logon session).

This was an artefact of the way DirectQ originally handled gamma, plus the way it continued to handle gamma changes. When I originally implemented brightness changing I used D3D gamma, which it turned out did not work as documented (the documentation was not even complete as - while it mentioned the formats used - it said nothing about ranges) and didn't even work at all for windowed modes.

From there (roundabout 1.2 or 1.3 I would guess) I implemented Windows GDI gamma, but instead of tearing down the D3D gamma system I only did half an implementation, so that D3D gamma was still used for reading your gamma, but GDI gamma was used for setting it.

This got messy, so for 1.7 (or was it 1.6?) I switched it to pure GDI all the way, but - not willing to get stuck in and clean out what had evolved into quite an elaborate setup - I rescaled the values to match the ranges used when reading D3D gamma (writing D3D gamma turns out to use a completely different range - sigh) and kept the D3D gamma structs used for storing the values (they were already in the correct layout so they were convenient).

To add to this, for applying gamma changes I had just plugged in the formula used by ID for the old -gamma command-line option without checking that the output was the same as the input for a value of 1. Of course it wasn't (it was slightly higher).

Now, none of this would have mattered most of the time aside from the fact that DirectQ always went through the gamma-change path on startup. Even with values of 1. And it's shutdown path took the values that had been scaled down and scaled them back up. And wasn't even called if the video had failed to initialize.

All in all quite a bit of sloppy work, and a good lesson in the dangers of maintaining legacy code paths that seemed to work OK, even when you know they're ugly and need cleaning out.

This has now been completely ripped apart and simplified, with correct full native GDI gamma all the way (D3D gamma still sucks), using it's own correct data formats, no scaling down and back up, the gamma change formula corrected and triple checked to ensure that output exactly matches input for values of 1, no matter how many passes through it are made, the initial pass through the change routine removed, correct restoration of the initial monitor gamma made if video fails to start up, and the previous 3 or so different codepaths for changing gamma all cleanly consolidated.

Removal of nasty old legacy code does give one a good feeling.

2 comments:

Coranth said...

"Removal of nasty old legacy code does give one a good feeling."

*... snip, snip, snip...* Do I hear the sound of more code-shearing in the future? :)

kempie said...

I noticed this a little when i was playing with mod loading recently. Gamma would creep up every time - which meant that when it crashed to desktop things were pretty extreme. Great to hear that it (as well as the beyond belief thing) is sorted.

Cheers, kempie