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

[RFC] backlight: convert linear to gamma float brightness #6

Open
wants to merge 1 commit into
base: aosp-12.0
Choose a base branch
from

Conversation

massonju
Copy link

@massonju massonju commented Feb 7, 2023

The issue we have with our device is that the brightness of the panel is very low most of the time when we control it
through the Android UI.
I'm not totally convinced that this patch is the best solution, that's why I added RFC tag in the title in order to open a discussion and maybe have your opinion on that.

The brightness controller in Android frameworks convert initial value from the slider to a new value not linear [1].
The value is modified by convertGammaToLinearFloat.

The current implementation in light HAL doesn't take account the none linearity of this incoming value, these values are logarithmic [2].

In order to get back a linear value, one solution is to re-convert the incoming value to a linear form: convertLinearToGammaFloat.

By doing this the brightness follow in a "normal way" the Android UI slider.

The implementation convertLinearToGammaFloat has been highly inspired by the one in BrightnessUtils [3].

[1] https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-12.0.0_r34/packages/SystemUI/src/com/android/systemui/settings/brightness/BrightnessController.java#354

[2] https://tunjid.medium.com/reverse-engineering-android-pies-logarithmic-brightness-curve-ecd41739d7a2

[3] https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-12.0.0_r34/packages/SettingsLib/src/com/android/settingslib/display/BrightnessUtils.java#129

Thanks

The brightness controller in Android frameworks convert initial value
from the slider to a new value not linear [1].
The value is modified by convertGammaToLinearFloat.

The current implementation in light HAL doesn't take account the none
linearity of this incoming value, these values are logarithmic [2].
Thus the brightness of the panel is very low most of the time.

In order to get back a linear value, one solution is to re-convert the
incoming value to a linear form: convertLinearToGammaFloat.

The implementation convertLinearToGammaFloat has been highly inspired
by the one in BrightnessUtils [3].

[1] https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-12.0.0_r34/packages/SystemUI/src/com/android/systemui/settings/brightness/BrightnessController.java#354

[2] https://tunjid.medium.com/reverse-engineering-android-pies-logarithmic-brightness-curve-ecd41739d7a2

[3] https://android.googlesource.com/platform/frameworks/base/+/refs/tags/android-12.0.0_r34/packages/SettingsLib/src/com/android/settingslib/display/BrightnessUtils.java#129

Signed-off-by: Julien Masson <[email protected]>
@spencercw
Copy link

For what it's worth, my device also benefits from this change.

@makohoek
Copy link

makohoek commented Jun 3, 2024

@MarijnS95 any feedback on this?

If you need help with maintaining android-lights-hal, let me know !

@MarijnS95 MarijnS95 closed this Oct 3, 2024
@MarijnS95
Copy link
Member

Reasons for closing:

  • Author doesn't seem to be convinced in the first place? (putting burden on maintainers and other contributors to do their work for them);

  • Content is contradictory: title and diff say that the input value is linear and must be converted to gamma (space?), yet the description (commit body) says:

    light HAL doesn't take account the none linearity of this incoming value

    And a similar inversion appears in:

    The brightness controller in Android frameworks convert initial value from the slider to a new value not linear

    (Linking code that converts from gamma to linear space, following the documentation of convertGammaToLinearFloat());

  • Zero source claiming that Linux' /brightness sysfs property works in the same "gamma space" that Android operates in ( the one you're converting to here);

  • Broken link formatting;

  • Poor grammar.

@MarijnS95
Copy link
Member

Reopening upon request from @makohoek on IRC, assuming that counts as a commitment towards addressing the above points (at the very least clarifying them to me) and making the PR actionable.

@MarijnS95 MarijnS95 reopened this Oct 3, 2024
@MarijnS95
Copy link
Member

Also note that I think we should move backlight brightness to the HWC API where it was "newly added" in Android 11:

https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/-/issues/44
https://gitlab.freedesktop.org/drm-hwcomposer/drm-hwcomposer/-/merge_requests/105

That doesn't go through an RGBA8 color value.

@makohoek
Copy link

makohoek commented Oct 3, 2024

Yes, I was not aware that HWC was capable of controlling the brightness. To me, it makes much more sense to do the implementation there. Thank you for pointing that out.

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