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

fake brightness adjust #1845

Merged
merged 32 commits into from
Feb 7, 2024
Merged

Conversation

zxkmm
Copy link
Contributor

@zxkmm zxkmm commented Feb 4, 2024

there are several todo, not sure if we should make it next time or add on this one.

need to test if it damages screen hardware (without edge control) before merge.

@zxkmm zxkmm self-assigned this Feb 4, 2024
@gullradriel
Copy link
Member

gullradriel commented Feb 6, 2024

Hi there. After testing, I like it. A few points:

  • I would also add 75% for those who just wanted less 'brightness' and not a darkened screen
  • It does need to reboot for it to work (no during the tests)
  • The text is too long and truncated in the pop up
  • You may add the on off checkbox in settings as some may choose to hide the brightness icon
    After we can all check to see if we can optimize a thing or two, maybe we will find nothing as it's already pretty decent.

@zxkmm
Copy link
Contributor Author

zxkmm commented Feb 6, 2024

thx!!!! Glad to hear that you didn’t burn your screen while testing! 😂😂

I would also add 75% for those who just wanted less 'brightness' and not a darkened screen

I apologize but 75% seems not quite possible yet, cuz float calculations costs too much resources. I’ll dm you a vid (in that vid, it use float calculations)

It does need to reboot for it to work (no during the tests)

That could implement by pop the navigation view I guess, but it’s a little painful to trigger it from IO class. I think I’ll leave for a while.

You may add the on off checkbox in settings as some may choose to hide the brightness icon

the suatus icon can be turned on and off in the interface settings which @kallanreed wrote.

@gullradriel
Copy link
Member

gullradriel commented Feb 6, 2024

Okay for the 75%, we are leaving aside for now.
For the reboot I see why you are asking it, because to refresh the screen with new settings one have to get inside and outside a menu / an app. Hmm well calling a set_dirty() would call a refresh for everything on screen (I think ?) and avoid the reboot call and even displaying a modal.
The status icon => I'm not talking about hiding it. I'm talking about adding a checkbox in the Settings/FakeBrightness menu which is the reflect of the activation status.
Please also rename it to just 'Brightness'. Even if it's faked it's doing the job ;-)

@zxkmm
Copy link
Contributor Author

zxkmm commented Feb 6, 2024

Hmm well calling a set_dirty() would call a refresh for everything on screen (I think ?) and avoid the reboot call and even displaying a modal.

Thx! I’ll try it. I didn’t read what set_dirty yet. I think it would work.

The status icon => I'm not talking about hiding it. I'm talking about adding a checkbox in the Settings/FakeBrightness menu which is the reflect of the activation status.

Thx! I’ll do it after the car clean.

Please also rename it to just 'Brightness'. Even if it's faked it's doing the job ;-)

lol kk. But can I keep them in the code naming? 😂 cuz we may have real_ brightness in the future😂

@gullradriel
Copy link
Member

lol kk. But can I keep them in the code naming? 😂 cuz we may have real_ brightness in the future😂

No problem ;-)

@zxkmm zxkmm changed the title (WIP) fake brightness adjust fake brightness adjust Feb 6, 2024
@zxkmm zxkmm marked this pull request as ready for review February 6, 2024 16:12
@zxkmm zxkmm marked this pull request as draft February 6, 2024 16:22
@zxkmm zxkmm marked this pull request as ready for review February 6, 2024 16:51
Copy link
Contributor

@NotherNgineer NotherNgineer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but I've suggested some small changes. Thanks.

Copy link
Contributor

@NotherNgineer NotherNgineer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thank you for taking all my suggestions!!

@zxkmm
Copy link
Contributor Author

zxkmm commented Feb 7, 2024

@NotherNgineer Thank you for the review!
Now we wait @jLynx to see if we can have real-brightness.
If he implemented it, and we don’t need greyscale and high contrast (it’s not that useful if we can have real one) then I can close it.

@jLynx
Copy link
Contributor

jLynx commented Feb 7, 2024

If anyone wants to take a crack at real brightness control with me, here is my PR #1859

@NotherNgineer
Copy link
Contributor

Should we change this one back to Draft state so it doesn't get merged until it's determined if real brightness control is possible?

@zxkmm zxkmm merged commit 0370b4e into portapack-mayhem:next Feb 7, 2024
3 checks passed
@zxkmm zxkmm deleted the fake_brightness_adjust branch February 7, 2024 08:07
@zxkmm
Copy link
Contributor Author

zxkmm commented Feb 7, 2024

I merged myself and when anyone finished it, reuse the settings interface and icon and p.mem etc.

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.

4 participants