Page 1 of 1

Palette Tonemap improvements

Posted: Wed Sep 07, 2016 16:48
by Rachael
Full branch:
https://github.com/raa-eruanna/gzdoom/t ... te-tonemap

Compare URL:
https://github.com/coelckers/gzdoom/com ... te-tonemap

Not ready to make a pull request just yet, as I am still debating over some parts of this.

As-is - what it does is it simply improves the LUT generation by favoring colors a little bit more like how Doom's original COLORMAP did. In my opinion, the current palette tonemap is a bit quirky and I am not 100% with the colors it chooses - when you go to E2M2 from Doom it becomes painfully obvious as to why.

So what this does is it tries to fix that problem - it makes higher color distances "even more" distant but still close enough that occasionally other ramps can be picked.

Now - my version of BestColor (with some comments and help from dpJudas) is a bit slower than ZDoom's, but not by much. When testing this function, I can get micro-stutters from live colormap generation in ZDoom's software renderer but only if I cripple my CPU. When I run my CPU at full speed the stutters are gone. (And that's only at 2.5GHz, too)

Originally dpJudas wanted everything in that function to be floats instead of doubles, but when I did that the loss of precision was enough to defeat the entire purpose of replacing that function. Otherwise I would've tended to agree - less is better.

So here is what I am wondering, before I make an official pull request:

Should I make this version of BestColor specific only to tonemap gen? If yes - then that's all I need to know.

If not - should I make a separate pull request to replace ZDoom's, or should this version be specific to GZDoom? The function as-is retains the same functionality as ZDoom's, just with a slightly different algorithm, and will be used in GZDoom's software renderer.

Re: Palette Tonemap improvements

Posted: Wed Sep 07, 2016 18:26
by Graf Zahl
If you just want the new BestColor function for the tonemap, make it a separate function. For obvious reasons I am not going to touch the original. It's clearly a speed-over-precision implementation. Ideally this should be done on a case-by-case basis, use the slower one where performance is secondary, use the original where real time color lookup is needed.

Re: Palette Tonemap improvements

Posted: Wed Sep 07, 2016 18:35
by Rachael
Alright, I will do a separate function then. I'll have a PR ready soon.

Re: Palette Tonemap improvements

Posted: Wed Sep 07, 2016 19:03
by Rachael

Re: Palette Tonemap improvements

Posted: Wed Sep 07, 2016 23:54
by Graf Zahl
Merged in.

Just one small note here: The reversion of the Strife file was very dangerous. I already had that stuff merged in and your commit actually reverted it to the broken form. Good thing I noticed it while flattening your commits into one to remove all changes to non-GZDoom files.

Re: Palette Tonemap improvements

Posted: Thu Sep 08, 2016 0:19
by Rachael
Is there a way in the future I can prevent this from happening? Like making Git "forget" the commit history at least as far as that branch goes? Or do I just need to make a new branch with a new pull request?

The Strife change was done manually, as I wasn't sure how to downstream from ZDoom, or I would have just left that in. I actually even had done it long before I even noticed you already did that on ZDoom.

I am still getting used to Git and am a total noob at it. >_<

Re: Palette Tonemap improvements

Posted: Thu Sep 08, 2016 0:26
by Graf Zahl
Don't worry. Git is perfectly capable of detecting an identical commit on two branches, even if it has different origins. That's an essential feature of a DVCS. Subversion would have puked all over the place here, though.

To downstream from ZDoom just do a 'git pull' on the ZDoom repo's master branch.

Re: Palette Tonemap improvements

Posted: Thu Sep 08, 2016 0:30
by Rachael
My experiences with SVN is what got me paranoid (and reverting things). Thank you for the tips though. :)