Skip to content

actions/dimdisplay: Don't dim when screen changes are inhibited

Jakob Petsovits requested to merge jpetso/powerdevil:work/jpetso/dimhibit into master

DimDisplay::onIdleTimeout() may not force "Explicit" when triggering the action manually, but it will still record old screen & keyboard brightness values regardless of policy and set m_dimmed to true. We should always do either one or the other.

Hence, track the ChangeScreenSettings policy state within DimDisplay and use that to skip all of onIdleTimeout() if currently inhibiting. As per GitLab issue #29, we're better off without "Explicit" here.

Modeled after the DPMS action which has similar policy tracking code.

Plus an extra commit to improve debug logs for DimDisplay (if the user has enabled them).

The background here is that for some reason my screen turned very dark when switching back to my non-dev user account after spending some time in the dev session. The screen got reduced to "very dark" only after switching back to the running session of my non-dev regular user account. (Somehow I only ever noticed this going one way, not when switching back from regular user to dev account.)

I believe what happens is that DimDisplay records m_oldScreenBrightness as something very low while the regular user session isn't active (not sure how the low value comes to be exactly) and then DimDisplay gets to apply the low brightness once the policy is finally back to allowed, with trigger() letting the brightness setter through after the regular user session gets re-activated.

There seems to be a larger problem in that PowerDevil::Core handles PowerDevil::PolicyAgent::sessionActiveChanged and clears action timers, but on other events such as plugging AC in or out, loadProfile()/loadAction() gets called anyway and idle timers get reinstated. In other words, the code I'm changing here shouldn't even run but it does anyway. Looking at further logs, it seems that subsequent loadProfile() calls can happen on independent events such as AC plugged in/out.

This MR doesn't resolve the larger problem. Judging by my logs, it doesn't even keep DimDisplay::onIdleTimeout() from getting past the inhibition check. It still records old brightness values, tries to set the brightness in the background, and presumably gets blocked by some kind of (systemd-level?) permission check such that the backlight helper itself apparently fails.

So kind of more questions than answers. That said, I feel like this MR is reasonable/coherent in and of itself. It works toward resolving #29 and imho makes the code easier to reason about. If merely onUnavailablePoliciesChanged() can be emitted with an inhibition prior to onIdleTimeout(), that should be a sufficient fix for its code not getting executed in the background. A more comprehensive fix would look at loadProfile() calls directly.

Either way, putting this up for discussion and further experimentation, if so desired. Looks like nothing is ever as easy as I had hoped it to be.

Merge request reports