Add support for negative gamma-correction levels

[JNechaevsky]

A carbon copy of implementation from Woof, names came from there as well.

Few thoughts:

• Decide something about existing configs, users will have darkest gamma-correction level. On newly created configs gamma will be set to OFF level by default.
• Add "Gamma-correction level №" message for Heretic, otherwise it is unclear which exactly level is using.

[fabiangreffrath]

Given that with this implementation all gamma values will get a new meaning, we could just as well introduce a new config variable, e.g. gamma2, and ignore the former one. Then everybody will start again with the default value and we don't mess up config file compatibility more than it is now.

[JNechaevsky]

Indeed, that's was I thinking. I've learned this trick while working on CRL, which is also using extended gamma levels and screen size, while I was trying to preserve compatibility with DOS version of default.cfg file.

Let it be crispy_gamma then, and placed nicely along with other Crispy variables.

[fabiangreffrath]

Fine with me.

[JNechaevsky]

All done now and ready to be reviewed.

[fabiangreffrath]

I'd prefer if you could make this static in I_SetGammaTable() and move that check there, so that it's safe to call this function whenever deemed necessary, but the tables are only calculated once in any case.

[JNechaevsky]

Sure, fixed in 3873123. I got a bit confused of calling I_SetGammaTable() between palleted and true color render, but this approach solves it properly, thanks for fresh look!

[fabiangreffrath]

Sorry for being pedantic, but somehow I don't like the fact that I_SetPalette() has to check each time if the gamma values have already been calculated. They should be calculated once as early as possible and then be taken as granted. They replace static values after all.

[JNechaevsky]

This was primary reason of confusion. My initial idea was to put I_SetPalette() to I_InitGraphics() below this line and then do not make any farther calls to I_SetGammaTable() at all. And it's working fine for palleted render, for true color, however, I'm starting to get black screen.

I don't understand why. Such call to I_SetGammaTable() is not limited to #ifndef CRISPY_TRUECOLOR, and gamma values are must be set. What else missing then?

[JNechaevsky]

@fabiangreffrath, allow me please to clarify about last commit c51afc8:

• For palleted render it seems to be correct: we defining gamma tables right before applying palette. It's "calculate first, draw second" logics.
• For true color... It's just disgusting. It does not feels to be the right place for calling I_SetGammaTable(), but this is the only way I was able to make it work w/o extra checks. Even if I_SetGammaTable() is moved outside of #ifndef CRISPY_TRUECOLOR macroses in I_InitGraphics(), it simply doesn't work, matters no where exactly it is placed. I'm completelly out of ideas why - gamma2table does not seems to be affected by CRISPY_TRUECOLOR in any ways and it's not palette related thing. So what could it be then? Kaboom! 🤯

[fabiangreffrath]

Alright, your fix is all fine. Let me try to explain why. 😉

In the non-truecolor case, I_SetGammaTable() is called from I_InitGraphics() right before PLAYPAL is loaded and applied to screenbuffer->format->palette. This is the first time the gamma table is actually used in this case.

As you can see in src/doom/d_main.c, I_InitGraphics() is called rather late. Very late, actually, in the never-returning D_DoomLoop(), right before entering the D_RunFrame() loop. That is, everything else in this game is already initialized before the graphics are set up, which in turn is the earliest situation in which the gamma tables is used. But only in the non-truecolor case!

Let's stay in src/doom/d_main.c and have a look at the single large D_DoomMain() function. You'll see that it calls a lot of initialization stuff, among these R_Init, before it calls D_DoomLoop() in the last line of that function. This means that R_Init() and thus R_InitColormaps() is always called before I_InitGraphics() and so the gamma table is never set up at that point.

Now, why does it make a difference for truecolor and non-truecolor rendering? Because in the case of the palette renderer, we just load the colormaps from the COLORMAP lump and be done with it. However, for the truecolor renderer we need to calculate the colormaps entries and have to take the current gamma level into account. That's why this function will calculate an all-black colormap if the gamma table hasn't been calculated before. And so, it's perfectly right to call I_SetGammaTable() right before R_InitColormaps() in the truecolor case and much later during I_InitGraphics() in the non-truecolor case.

[JNechaevsky]

Thank you very much for this not explanation, but whole guide, a road map. I was walking this path strictly forward, while small close look backward was needed. True color's if (!gamma2table) was a small beacon which led to the right path, and this whole complicated structure of calls was the real reason behind confusion.

So yeah, I_InitGraphics() is called after R_Init(), it is clear now. This is also making next perfectly clear: we need to get rid #ifndef CRISPY_TRUECOLOR macros and make both renders happy. How? Simple! We need exactly one call for I_SetGammaTable() right before R_InitColormaps() but without any marcoses. True color render wants it right there, and palleted render will be ready for it when it will be calling I_InitGraphics().

The only personal nitpick is having I_ upper level function in the in R_ lower level function, but it have to be I_ as it placed in upper level and it's shared for all four games. Not really nice, but makes sense for understanding where I_SetGammaTable() is located.

We can simplify even farther by having exactly one call for I_SetGammaTable(), but then it should be placed in int main() function which is a-b-s-o-l-u-t-e-l-y no go, it simply doesn't belong there matters no how simple it is!

[JNechaevsky]

Have to make small clarification here. I_SetGammaTable() is called slightly earlier than in other games, as D_IntroTick() calls right below seems to be using gfx drawing functions. Such earlier call should be safe, but forgive me, I don't want to dig into Strife code deeper, it's simply not my game...

[fabiangreffrath]

Looks good, thank you!

[JNechaevsky]

So be it then. 🤗 I'm absolutely sure everyone will like this, even if it wasn't requested often.

Next on the list:

• All games: palette change check every game tic, not every frame.
• Heretic: visplanes drawing wants for sector->special check as well. This case safe for render and issue itself barely can be found in original Heretic, but I found it a bit interesting in retrospect. I'll explain in PR, the fix itself is just a question of two lines of code.
• Heretic: remove MAXBUTTONS limit.
• Automap: I have some worries about brown line color in Heretic, and now I see same issue in Woof's ZDoom automap colors. Technically, get-from-colormaps is working fine, but one that color is yucky, it could be better. Probably worth to discuss primary in Woof, but without rush.
• True color for Hexen and Strife: 😶

[fabiangreffrath]

Thanks for this, I'm looking forward to your plans!