Post-processing vid_contrast/vid_brightness fix

Moderator: Graf Zahl

Locked
User avatar
Rachael
Developer
Developer
Posts: 3606
Joined: Sat May 13, 2006 10:30

Post-processing vid_contrast/vid_brightness fix

Post by Rachael » Wed Sep 21, 2016 8:10

https://github.com/coelckers/gzdoom/pull/108

This may be a bit OCD on my part (sorry) but I never liked how the shaders handled the vid_contrast and vid_brightness.

This does a considerable change - it removes the clamping from the contrast (which looked really ugly when combined with vid_brightness) and moves the gamma calculation to be last. (It probably could be first, instead?)

The result, I think, is a much more beautiful change when using these options, and they function a little more as they are intended (fixing renders on otherwise very dark monitors).
Spoiler: Zen Sarcasm

dpJudas
Developer
Developer
Posts: 798
Joined: Sat Jul 23, 2016 7:53

Re: Post-processing vid_contrast/vid_brightness fix

Post by dpJudas » Wed Sep 21, 2016 16:27

Note that this code needs to be in sync with the same math in OpenGLFrameBuffer::DoSetGamma (https://github.com/coelckers/gzdoom/blo ... r.cpp#L235). Otherwise the settings will produce different results depending on whether you're running windowed (shader gamma) or full screen (hardware gamma).

As for changing the math behind the gamma controls, I haven't studied the old math closely and just ported it directly over.

My gamma settings are pretty much the same as they are for all computer games the last 15 years: turn up the gamma a bit (never understood the need for such dark games) and then never touch those controls again. As a result, I have little opinion if your adjusted version is better or worse than the original. :)

dpJudas
Developer
Developer
Posts: 798
Joined: Sat Jul 23, 2016 7:53

Re: Post-processing vid_contrast/vid_brightness fix

Post by dpJudas » Wed Sep 21, 2016 16:45

By the way, the 'max' in the shader is needed to make sure that pow isn't using a negative value. The result of a negative value passed into pow is undefined, and due to our usage of rgba16f as render buffers we can, at least theoretically, have negative values as input to this function.

User avatar
Rachael
Developer
Developer
Posts: 3606
Joined: Sat May 13, 2006 10:30

Re: Post-processing vid_contrast/vid_brightness fix

Post by Rachael » Wed Sep 21, 2016 17:38

The negative clamping inadvertently killed it, in my opinion. Needless to say, if I go as far as fixing GZDoom's hardware gamma, I'll likely need to fix ZDoom's, too, which isn't a huge issue but it seems like this code could potentially be in 4 places (ZDoom might have a shader version, as well... but D3D shaders are almost the same language anyway, so shouldn't be an issue). I'm definitely going to have to play around with it. At any rate, I might just do the gamma pow first.

EDIT: Actually - ZDoom doesn't use contrast/brightness, only Gamma, so I can leave that one alone.
Spoiler: Zen Sarcasm

User avatar
Rachael
Developer
Developer
Posts: 3606
Joined: Sat May 13, 2006 10:30

Re: Post-processing vid_contrast/vid_brightness fix

Post by Rachael » Wed Sep 21, 2016 18:11

Fixed.
Spoiler: Zen Sarcasm

dpJudas
Developer
Developer
Posts: 798
Joined: Sat Jul 23, 2016 7:53

Re: Post-processing vid_contrast/vid_brightness fix

Post by dpJudas » Wed Sep 21, 2016 18:23

Looks good now when looking at the diff.

User avatar
Rachael
Developer
Developer
Posts: 3606
Joined: Sat May 13, 2006 10:30

Re: Post-processing vid_contrast/vid_brightness fix

Post by Rachael » Wed Sep 21, 2016 18:40

Thank you. :) I think next time I do a pull request for GZDoom, though, I am going to have to wipe and recreate my source tree. Having all that commit history, even if it's only pulls, kinda bugs me. XD
Spoiler: Zen Sarcasm

User avatar
Graf Zahl
GZDoom Developer
GZDoom Developer
Posts: 7148
Joined: Wed Jul 20, 2005 9:48
Location: Germany
Contact:

Re: Post-processing vid_contrast/vid_brightness fix

Post by Graf Zahl » Wed Sep 21, 2016 19:50

This bugs me, too. Please clean this up. In its current state it will create one huge mess because all those merges will be pulled into the permanent commit history.

User avatar
Rachael
Developer
Developer
Posts: 3606
Joined: Sat May 13, 2006 10:30

Re: Post-processing vid_contrast/vid_brightness fix

Post by Rachael » Wed Sep 21, 2016 20:11

Spoiler: Zen Sarcasm

dpJudas
Developer
Developer
Posts: 798
Joined: Sat Jul 23, 2016 7:53

Re: Post-processing vid_contrast/vid_brightness fix

Post by dpJudas » Wed Sep 21, 2016 20:31

Out of curiosity, how did you get rid of those commits?

User avatar
Rachael
Developer
Developer
Posts: 3606
Joined: Sat May 13, 2006 10:30

Re: Post-processing vid_contrast/vid_brightness fix

Post by Rachael » Wed Sep 21, 2016 20:34

I zipped my changed files, then deleted the branch, deleted my entire GZDoom repo, and cloned it again. Then unzipped the files back. Obviously I unzipped an old version of the files, but I fixed that in my next commit.

There's probably a much better way to do this. I know git allows you to change commit history - but I've never figured out how to actually do that. It's probably better the way I did it, anyway, as it ensures no unnecessary changes are made outside of the files I changed.

I had problems with Windows caching a ghost GZDoom folder but that seemed to be fixed by doing a chkdsk.
Spoiler: Zen Sarcasm

dpJudas
Developer
Developer
Posts: 798
Joined: Sat Jul 23, 2016 7:53

Re: Post-processing vid_contrast/vid_brightness fix

Post by dpJudas » Wed Sep 21, 2016 20:41

Okay, the method I might have used myself. Was hoping you had the golden nuggets on how to edit the history without me having to google it (just cba!). Interesting to know that github let you keep the pull request number even when you nuked the entire branch. :)

User avatar
Rachael
Developer
Developer
Posts: 3606
Joined: Sat May 13, 2006 10:30

Re: Post-processing vid_contrast/vid_brightness fix

Post by Rachael » Wed Sep 21, 2016 20:44

It didn't. That's my third pull request for the same feature, today. The original, 106, is too embarassing to even discuss. We don't talk about 106 here. ;)

What you're probably noticing is a lack of edit messages on my posts. phpBB doesn't tag moderator edits unless the moderator attaches a message to it, but they still show in the MCP except when you edit your own post. I don't really know why phpBB did that this way.
Spoiler: Zen Sarcasm

Locked

Return to “Closed Feature Suggestions”

Who is online

Users browsing this forum: No registered users and 1 guest