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

Screenpad brightness adjustment values #3067

Closed
2 tasks done
fscottcopeland opened this issue Sep 4, 2024 · 15 comments
Closed
2 tasks done

Screenpad brightness adjustment values #3067

fscottcopeland opened this issue Sep 4, 2024 · 15 comments
Labels
question Further information is requested

Comments

@fscottcopeland
Copy link

Rules

  • I made myself familiar with the Readme, FAQ and Troubleshooting.
  • I understand that, if insufficient information will be provided, my issue will be closed without an answer.

Is your feature request related to a problem? Please describe

No. It's not related to any problems, it's just a preference / QoL feature.

Describe the solution you'd like

The current implementation of screenpad adjustment is in 10% increments, however it looks like it has been implemented as a 0-255 range which is equally split (e.g. 10% is 25.5, 20% is 51, 100% is 255). However this isn't exactly how the eye perceives things.

E.g. going from the lowest value of 1, to 2, is a much bigger perceived difference, although technically it's only a <1% change in brightness value, in reality it's much higher.

Perhaps it's subjective, however I set some values in AHK and to me, they seem much more accurate.

I'd be interested to know your thoughts. Perhaps you could make it possible to set the brightness percentage points as 0-255 values as an advanced config setting, so that users can change it themselves?

This is what set for myself, where each value corresponds to a "10%" increase, where "4" is 0%, and "255" is 100%

brightnessValues := ["4","6","9","14","21","32","48","73","111","169","255"]
Also, it's possible to hide/disable the screen using the dedicated screenpad button. It seems a little superfluous to have this feature duplicated from within the brightness controls. Could it also be possible to remove this control as an advanced parameter?

Describe alternatives you've considered

No response

Device and Model

Zephyrus Duo 2023 GX650PY

Additional information.

No response

@seerge
Copy link
Owner

seerge commented Sep 4, 2024

@fscottcopeland i can replace 10% increments (which are indeed in a range from 0 to 255 as the size on 1 byte) with something like factors of 2.

So it will be 0, 1, 2, 4, 8, 16, 32, 64, 128, 255 (not 256 yeah). Would that be an option ?

But honestly that value (from 0 to 255) is not necessarily a strength of a signal or some physical value, it's still interpreted internally by firmware. So Asus could easily "adjust" it for proper eye perception already.

Are you sure that current 10% steps don't look equal ? And also ... do you even need 10 steps ? May be something like 5 will be enough ?

@seerge seerge added the question Further information is requested label Sep 5, 2024
@fscottcopeland
Copy link
Author

Yeah, to be fair 10 steps is probably OTT. I set it up a while ago and think I did it that way to match up with the number of steps that windows uses. Tbh, all I probably need is 5 steps as you've mentioned.

The brightness values aren't linear, I don't think. I remember testing it, and setting the values as you've mentioned, but it just didn't 'seem' right.

As an example going from 90% to 100% in the current implementation yields a very small (if any) perceptible brightness increase.

I believe it actually requires the values to be set logarithmically rather than linearly. Found a few articles suggesting this such as https://www.linkedin.com/pulse/when-nits-enough-quantifying-brightness-wide-gamut-displays-john-ho

In my implementation, I used Brightness = X^1.231 where X is the previous brightness value (starting at 4), plus a small bit of rounding.

It's an interesting conundrum though. It might be that each screen is calibrated differently and there's no right answer.

@seerge
Copy link
Owner

seerge commented Sep 6, 2024

@fscottcopeland ok, I have copied your array of values instead of just calculating brightness as value / 100 * 255 :)

Try this build
GHelper.zip

Also, it's possible to hide/disable the screen using the dedicated screenpad button. It seems a little superfluous to have this feature duplicated from within the brightness controls. Could it also be possible to remove this control as an advanced parameter?

As for me - it's convenient when 0 brightness means that screenpad is off. This is the feature that i really miss in windows default brightness controls for example.

I'm a mac user mainly, and there 0 brightness completely turns off display :) (i.e. it turns backlight completely off)

@fscottcopeland
Copy link
Author

Yeah. That's fair I suppose. Tbh, I think the existing implementation is fine enough - especially as I make use of the custom fan build too, and I don't want you to have to maintain multiple builds - you're doing so much as is, it's not fair for you!

@fscottcopeland
Copy link
Author

I still think it'd be useful to maybe have an option to enable/disable the brightness controls from turning off the display though. As I say, the Zeph Duo has a dedicated button already to do just that. I've found it frustrating when turning the brightness down, that I hold the button for too long, and it turns the screen off instead!

@fscottcopeland fscottcopeland reopened this Sep 7, 2024
@seerge
Copy link
Owner

seerge commented Sep 7, 2024

@fscottcopeland what is the downside in turning screenpad off when brightness is 0?

@fscottcopeland
Copy link
Author

For me, two things mainly:
I generally have hwinfo graphs on by bottom screen. If I accidentally turn off the screen, it often messes up the layout of the graphs, as well as any other windows I have open on the screen.
Also, it causes the windows animation when screens are (dis)connected, which often kicks me out of full-screen games etc.

@seerge
Copy link
Owner

seerge commented Sep 7, 2024

@fscottcopeland ok, i was not aware that when disabled it also disappears from displays in windows

Try this build
GHelper.zip

seerge added a commit that referenced this issue Sep 7, 2024
@fscottcopeland
Copy link
Author

That's removed the "Screenpad off" setting, but still has the "hidden" setting. I think this is probably superfluous too? As that can also be controlled by the dedicated screenpad button. What do you think?

@seerge
Copy link
Owner

seerge commented Sep 7, 2024

@fscottcopeland hidden is just a wording for brightness = 0

@fscottcopeland
Copy link
Author

Also another strange thing - all brightness settings work with instant effect, except brightness 100% which has a delayed effect, by a second or so.

@seerge
Copy link
Owner

seerge commented Sep 7, 2024

@fscottcopeland this build should say 0% instead of "hidden" in a toast message, and also shouldn't do a delay when you set 0% or 100%.

Delay was added for screenpad toggle button (so you can quickly press button few times to get desired state) before actual command happens #2296

GHelper.zip

seerge added a commit that referenced this issue Sep 7, 2024
@fscottcopeland
Copy link
Author

Nice! That's fantastic dude, thank you.
Is there any chance this stuff will be able to be merged into the main build / #2272 build? Maybe as an optional thing, so it doesn't mess up other people's brightness, if they're happy with how it is currently?
I'm still fine-tuning the log scale ramp - if there was an advanced param to set the exact brightness points, something like that would be amazing.

@seerge
Copy link
Owner

seerge commented Sep 7, 2024

@fscottcopeland all changes will be in next release, they are already merged (as notification above shows).
There is no advanced param to set exact brightness points, i have just hardcoded them.

Thanks

@seerge seerge closed this as completed Sep 7, 2024
@fscottcopeland
Copy link
Author

Awesome, thanks. Gotcha.

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

No branches or pull requests

2 participants