Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement gamma correction in main config menu #1805

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

rainmakerv3
Copy link
Contributor

This PR adds a setting for Gamma correction in the settings dialog Graphics tab. While there is a gamma correction tool in the advanced debug menu, this one is intended is more accessible for regular users and the setting can also be saved (the devtools gamma setting is not currently saved).

Useful in the short term while the PR for gamma is being sorted out, in the long term also may be useful for games lacking a native Brightness setting

image

@friyin
Copy link

friyin commented Dec 16, 2024

It would be a good idea having a numeric field along the slider and clarify what side makes it brighter and darker.

@rainmakerv3
Copy link
Contributor Author

image

Sounds good, this is what I have now

@Foul-Tarnished
Copy link

The default value should be at the center, and display 1.00

@rainmakerv3 rainmakerv3 marked this pull request as draft December 16, 2024 19:34
@rainmakerv3
Copy link
Contributor Author

Forgot to rename widgets from default names, will set as draft to clean up, but please feel free to comment on things other than widget naming

@rainmakerv3
Copy link
Contributor Author

rainmakerv3 commented Dec 16, 2024

  • used decimal form for gamma label (ex. 1.234 instead of 1234), but I didn't label the default, I feel it's cluttered and unsure if needed
  • properly named widgets and layouts
  • fixed not saving on close as I did for the other config variables in a previous PR

@rainmakerv3 rainmakerv3 marked this pull request as ready for review December 16, 2024 21:50
@GHU7924
Copy link

GHU7924 commented Dec 16, 2024

33

I'll add a few options, just in case you decide to finalize it.

EDIT: I meant the scale 0-100%, the middle = 50%, I did not notice that I wrote 0% instead of 50%.

@rainmakerv3
Copy link
Contributor Author

Whats the intent for the percentage boxes? What effect should they have?

@GHU7924
Copy link

GHU7924 commented Dec 17, 2024

Whats the intent for the percentage boxes? What effect should they have?

The current value, only as a percentage. When I saw the value 1533, I personally did not perceive it in any way, I immediately have a question - "What is it?" Perhaps interest is also an unsuccessful offer, I agree. Then it is worth making the values 1.0 or 1.00, etc.

Percentage boxes... This way, the value stands out more than just the numbers on the background.

@rainmakerv3
Copy link
Contributor Author

Whats the intent for the percentage boxes? What effect should they have?

The current value, only as a percentage. When I saw the value 1533, I personally did not perceive it in any way, I immediately have a question - "What is it?" Perhaps interest is also an unsuccessful offer, I agree. Then it is worth making the values 1.0 or 1.00, etc.

Percentage boxes... This way, the value stands out more than just the numbers on the background.

I believe gamma is usually represented in decimal values though, and I did change this in the previous commit, as such. This also makes it consistent with the values on the devtool widget.

Will leave it like this for now pending review

image

@Foul-Tarnished
Copy link

Foul-Tarnished commented Dec 17, 2024 via email

@MajorP93
Copy link
Contributor

MajorP93 commented Dec 17, 2024

@Foul-Tarnished
In my opinion in this case -100% to 100% would make more sense.
Default value should be 0% in that case.
I think that would be more intuitive.
If a user wants to increase brightness by 20% they select gamma 20%.
If they want to make the image 20% darker they select -20% and so on.

@GHU7924
Copy link

GHU7924 commented Dec 17, 2024

@Foul-Tarnished @MajorP93 In fact, this is not so important, in any case, one side is darker and the other is lighter.

@rainmakerv3 I just opened the NVIDIA Control Panel.

Values from and to: 0,30 - 2,80 and 1,00 as default.
NCP-Gamma

You can do the same.

EDIT: For example
Gamma

@DemoJameson
Copy link
Contributor

I think it's more customary to have the left side darker and the right side brighter

@rainmakerv3
Copy link
Contributor Author

I think it's more customary to have the left side darker and the right side brighter

This is true, but if I read the code right, it's the system call itself that treats it the opposite way (having larger values be darker). My thought is that I want to stay as consistent as I can with current code (i.e. devtools widget, which treats it this way) and still be intuitive for users. And since lighter/darker is labeled anyway. I foresee no usability issues with this whatsoever in its current state.

Guys, I think everyone will have very different opinions on UI, that's always the case, but I think the remaining comments are getting very subjective in terms of whats better/worse, so I'm inclined to leave this as is, and maybe the reviewer can just have the final say.

@rainmakerv3
Copy link
Contributor Author

Added the following description text on hovering the mouse, but only for English

image

@Foul-Tarnished
Copy link

This is for end-user, add logic to invert it or normalize it.
Being more true to how sRGB or the PS4 handles it is not the goal.

Right should be brighter
middle (default) should be 100% or 1.00 (or 0-10 like in most games like BB, with 5 being the middle then)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants