Bug 493111

Summary: Power management feature "dim display automatically" resets external display brightness to match the internal display brightness
Product: [Plasma] Powerdevil Reporter: Dmitrii Chermnykh <chermnykh2001>
Component: generalAssignee: Plasma Bugs List <plasma-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: chermnykh2001, jpetso, natalie_clarius, nate
Priority: NOR    
Version: 6.1.90   
Target Milestone: ---   
Platform: NixOS   
OS: Linux   
Latest Commit: Version Fixed In: 6.2.1
Sentry Crash Report:

Description Dmitrii Chermnykh 2024-09-14 11:04:04 UTC
STEPS TO REPRODUCE
1. Have powerdevil built without ddcutil support (so that the external screen uses plasma's color management as its brightness source)
2. Have multiple displays (in my case it's internal laptop screen an an external monitor)
3. Configure power management to dim the displays automatically after some time
4. Set the external screen brightness to 100%; set the internal screen brightness to any value that is not 100% or 0% (e.x. 30%)
5. Wait the configured dimming timeout
6. Move mouse cursor to cancel screen dimming

OBSERVED RESULT
The brightness for all displays is set to 30% causing the perceived brightness of the external display to be low

EXPECTED RESULT
The internal display brightness should at 30%; the external display brightness should stay at 100% as configured 

SOFTWARE/OS VERSIONS
Operating System: NixOS 24.11
KDE Plasma Version: 6.1.90
KDE Frameworks Version: 6.5.0
Qt Version: 6.7.2
Kernel Version: 6.10.9-zen1 (64-bit)
Graphics Platform: Wayland
Processors: 16 × AMD Ryzen 7 5800H with Radeon Graphics
Memory: 23.3 GiB of RAM
Graphics Processor: NVIDIA GeForce RTX 3050 Ti Laptop GPU/PCIe/SSE2
Comment 1 Jakob Petsovits 2024-09-28 03:07:44 UTC
Plans to fix dimming are generally detailed in the dimming rework issue at https://invent.kde.org/plasma/powerdevil/-/issues/38.

But let me see if perhaps it's possible to put a less comprehensive fix into Plasma 6.2 that takes care of this problem at least for basic use.
Comment 2 Natalie Clarius 2024-09-28 18:48:52 UTC
Am I understanding it correctly that the issue you're reporting is "Automatic display dimming is also applied to external displays" which you think it shouldn't because it doesn't affect laptop battery usage?
Comment 3 Dmitrii Chermnykh 2024-09-28 20:20:41 UTC
(In reply to Natalie Clarius from comment #2)
> Am I understanding it correctly that the issue you're reporting is
> "Automatic display dimming is also applied to external displays" which you
> think it shouldn't because it doesn't affect laptop battery usage?
No, I think the dimming should also be applied for the external displays because it provides indication for the user that the system has not received input for the configured time

The issue is that after dimming effect exits the brightness of the external display is being reset to the value of the brightness of the internal display, while I expect that it should be reset to its previous value (before the dimming)
Comment 4 Jakob Petsovits 2024-09-28 22:37:56 UTC
To put it another way:

EXPECTED:
[internal] 50% -> dimmed 15% -> 50%
[external] 100% -> dimmed 30% -> 100%

OBSERVED:
[internal] 50% -> dimmed 15% -> 50%
[external] 100% -> dimmed 15% -> 50% (both of these are wrong)

I've got a patch, need to test a few edge cases before submitting.
Comment 5 Bug Janitor Service 2024-10-01 21:31:04 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/433
Comment 6 Jakob Petsovits 2024-10-07 15:09:24 UTC
Git commit d9f58068be5acafb065fbab2aae6f90b68776557 by Jakob Petsovits.
Committed on 07/10/2024 at 15:05.
Pushed by jpetso into branch 'master'.

daemon/actions/dimdisplay: Set brightness separately per display

With 6.2's per-display brightness changes, setBrightness()
under Wayland will, in many setups, affect multiple displays
with different perceived screen brightness for a given
brightness percentage.

Before this patch, the DimDisplay action would only remember a
single brightness value (of the first display) and then set this
value to potentially multiple displays when dimming stops again.
This would lead to brightness transitions like this:

* internal display: 50% -> dimmed 15% -> 50%
* external display: 100% -> dimmed 15% -> 50%

Instead, dimming adjustments should apply to each display
individually, like this:

* internal display: 50% -> dimmed 15% -> 50%
* external display: 100% -> dimmed 30% -> 100%

This commit makes it work that way. Non-dimmed brightness values
are stored in a map, one value for each display that was connected
at the time the dimming started. When restoring, these individual
brightness values will be restored to all displays that are still
connected.

This brings the original DimDisplay code in line with the new
per-display brightness API. It still won't save you from bugs
about dimming changes while one of your displays is disconnected.
PowerDevil issue #38 provides a more comprehensive plan for
making multi-monitor dimming with disconnections actually robust.

M  +68   -22   daemon/actions/bundled/dimdisplay.cpp
M  +5    -2    daemon/actions/bundled/dimdisplay.h

https://invent.kde.org/plasma/powerdevil/-/commit/d9f58068be5acafb065fbab2aae6f90b68776557
Comment 7 Jakob Petsovits 2024-10-07 15:14:16 UTC
Git commit 95ee5f6b045579ade741a0f0f81f1446102262a4 by Jakob Petsovits.
Committed on 07/10/2024 at 15:10.
Pushed by jpetso into branch 'Plasma/6.2'.

daemon/actions/dimdisplay: Set brightness separately per display

With 6.2's per-display brightness changes, setBrightness()
under Wayland will, in many setups, affect multiple displays
with different perceived screen brightness for a given
brightness percentage.

Before this patch, the DimDisplay action would only remember a
single brightness value (of the first display) and then set this
value to potentially multiple displays when dimming stops again.
This would lead to brightness transitions like this:

* internal display: 50% -> dimmed 15% -> 50%
* external display: 100% -> dimmed 15% -> 50%

Instead, dimming adjustments should apply to each display
individually, like this:

* internal display: 50% -> dimmed 15% -> 50%
* external display: 100% -> dimmed 30% -> 100%

This commit makes it work that way. Non-dimmed brightness values
are stored in a map, one value for each display that was connected
at the time the dimming started. When restoring, these individual
brightness values will be restored to all displays that are still
connected.

This brings the original DimDisplay code in line with the new
per-display brightness API. It still won't save you from bugs
about dimming changes while one of your displays is disconnected.
PowerDevil issue #38 provides a more comprehensive plan for
making multi-monitor dimming with disconnections actually robust.


(cherry picked from commit d9f58068be5acafb065fbab2aae6f90b68776557)

Co-authored-by: Jakob Petsovits <jpetso@petsovits.com>

M  +68   -22   daemon/actions/bundled/dimdisplay.cpp
M  +5    -2    daemon/actions/bundled/dimdisplay.h

https://invent.kde.org/plasma/powerdevil/-/commit/95ee5f6b045579ade741a0f0f81f1446102262a4