Page MenuHomeWildfire Games

Remove gamma ramp setting.
ClosedPublic

Authored by wraitii on Jun 15 2019, 4:42 PM.

Details

Reviewers
vladislavbelov
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rP23722: Do not change the gamma of the display on startup.
Summary

This was added in rP258. I believe the idea was to allow users to increase contrast and such (see here).

However, this seems to trigger warnings on some windows computers, and furthermore it sets the gamma of the whole computer, conflicting with flux and related dimming tools.

On modern computers, increasing contrast/brightness should be done using post-proc shaders, not by changing the gamma of the whole computer.

Test Plan

Check that nothing was missed and that it still compiles.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
temp
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 7940
Build 12924: Vulcan BuildJenkins
Build 12923: arc lint + arc unit

Event Timeline

wraitii created this revision.Jun 15 2019, 4:42 PM

I think the patch should be tested on multi-monitor environment and with a custom color space.

Stan added a subscriber: Stan.Jun 15 2019, 4:47 PM

I think the patch should be tested on multi-monitor environment and with a custom color space.

Multi monitor sounds fair, custom color space might be weird, since the following code would be messing it up :P

In D1976#82463, @Stan wrote:

custom color space might be weird, since the following code would be messing it up :P

It depends, because color correction may present on each side: os, gpu driver, monitor.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/ps/GameSetup/Config.h
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/ps/GameSetup/Config.cpp
|   1| /*·Copyright·(C)·2017·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2017"

source/ps/VideoMode.cpp
|   1| /*·Copyright·(C)·2018·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2018"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/1727/display/redirect

@Stan if you want to test something :p

vladislavbelov accepted this revision.EditedMay 30 2020, 10:11 PM

I think we need to have control of gamma, but it shouldn't affect the whole display. As refs says:

Use this function to set the gamma ramp for the display that owns a given window.

So I agree to remove SDL_SetWindowGammaRamp.

Don't forget to check years.

This revision is now accepted and ready to land.May 30 2020, 10:11 PM
Stan added a comment.May 31 2020, 9:16 AM

Don't forget to edit the readme file: binaries/system/readme.txt when committing

(Also, told you I'd find someone) :D

Thanks for the look both of you, this was one of these things that had been annoying me for a while :)

This revision was automatically updated to reflect the committed changes.