Bug 495544

Summary:	Disable PowerDevil/KWin brightness integration until regressions are fixed
Product:	[Plasma] kwin	Reporter:	Jakob Petsovits <jpetso>
Component:	output configuration	Assignee:	KWin default assignee <kwin-bugs-null>
Status:	RESOLVED INTENTIONAL
Severity:	task	CC:	m.kurz, nate, postix, xaver.hugl
Priority:	NOR
Version First Reported In:	git master
Target Milestone:	---
Platform:	Other
OS:	Linux
See Also:	https://bugs.kde.org/show_bug.cgi?id=495490
Latest Commit:		Version Fixed In:
Sentry Crash Report:

Description Jakob Petsovits 2024-10-29 19:56:29 UTC

SUMMARY
Brightness controls through KWin hurt more than they help right now, and there isn't a path forward for fixing all of the issues in the 6.2 time frame due to feature and string freeze requirements:

1. Bug 494408 is partially fixed by a patch like https://invent.kde.org/plasma/kwin/-/merge_requests/6667 (Initially adopt current brightness of external brightness device). However, this requires KWin and PowerDevil to depend on an updated plasma-wayland-protocols, once https://invent.kde.org/libraries/plasma-wayland-protocols/-/merge_requests/86 is merged. Although this has not seen any discussion so far, either.

2. As Xaver points out in kwin!6667 linked above,

> when the user [after having 100% default brightness assigned] changes the brightness of the screen
> with the OSD, that change gets overwritten after hotplug events, giving the appearance that the
> brightness gets "reset" - as it's not obvious they have to set it in Plasma instead now

https://discuss.kde.org/t/powerdevil-is-re-setting-my-display-brightness-after-the-display-wakes-up/23079/9 notes that we can only apply heuristics to determine whether KWin should adopt or ignore external brightness changes. In 6.2 we do not have the necessary information to even apply these heuristics.  (One of the mentioned factors for the heuristic depends on a proper dimming API, i.e. https://invent.kde.org/plasma/powerdevil/-/issues/38, which again requires breaking feature freeze.)

Even if we did have all these heuristics implemented, they would not always lead to the correct decision.

3. Because KWin now owns and uses brightness controls for all screens, external brightness services and applets are now useless if they don't go through the new PowerDevil D-Bus brightness API. This affects software such as https://github.com/digitaltrails/vdu_controls and https://github.com/denilsonsa/midi-pipewire-volume, in addition to common terminal-based use of ddcutil either directly or via scripts.

4. There is no off switch, so right now we're asking users to set POWERDEVIL_NO_DDCUTIL=1 if stuff doesn't work out. The longer we don't fix user's workflows, the more users will keep this environment variable in place indefinitely.

https://discuss.kde.org/t/powerdevil-is-re-setting-my-display-brightness-after-the-display-wakes-up/23079/11 mentions that unexpected brightness changes are a blow to professional calibrated setups. It's a little ironic that KWin, which now has so many great color management features, can also be responsible for breaking these setups by insisting to handle monitor brightness.


OBSERVED RESULT
We broke important use cases without providing timely fixes or a "Disable brightness controls for this display" checkbox to users.

EXPECTED RESULT
If we can't fix widespread issues in a timely manner, we should disable KWin integration by default for the time being.

SOFTWARE/OS VERSIONS
Linux/KDE Plasma:
KDE Plasma Version: 6.2.80 (around 6.2.2)
KDE Frameworks Version: 6.8
Qt Version: 6.8
Graphics platform: Wayland

Let's revert PowerDevil/KWin to only affect HDR screens until we can get these issues figured out (presumably in 6.3).

Let's bring it back only once we have a toggle in the KScreen KCM that allows disabling brightness controls per display.

Comment 1 Zamundaaa 2024-10-29 22:49:59 UTC

> https://discuss.kde.org/t/powerdevil-is-re-setting-my-display-brightness-after-the-display-wakes-up/23079/11 mentions that unexpected brightness changes are a blow to professional calibrated setups. It's a little ironic that KWin, which now has so many great color management features, can also be responsible for breaking these setups by insisting to handle monitor brightness.
That comment complains about software brightness... but wrongly so. Software brightness is very much a serious method and does not invalidate a profiled setup, and it's actually the only way to correctly change the brightness with an ICC profile, as they only contain data about one fixed backlight level.

I'm fine with making powerdevil no longer integrate with KWin for 6.2, but I just wanted to emphasize that for color management we actually really benefit from having control over it because of that whole issue with ICC profiles only supporting one fixed setup. In the long term it would be best if we could control all the screen settings, record them when the screen is profiled, and ensure they're set the very same way whenever that profile is used.

About
> a "Disable brightness controls for this display" checkbox
I don't think it's worth having that for external tools (no need to repeat my opinion about that here), but I've seen a few people complain their monitors do weird things like change some contrast option instead of the backlight, so I don't think we can get around it.

Comment 2 Bug Janitor Service 2024-11-03 22:27:24 UTC

A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/455

Comment 3 Jakob Petsovits 2024-11-03 23:14:45 UTC

(In reply to Zamundaaa from comment #1)
> I just wanted to emphasize that for color management we actually really
> benefit from having control over it because of that whole issue with ICC
> profiles only supporting one fixed setup. In the long term it would be best
> if we could control all the screen settings, record them when the screen is
> profiled, and ensure they're set the very same way whenever that profile is
> used.

This sounds like a really good feature.

For the record, I agree that KWin should own all types of brightness values going forward. We just have to be more careful about not regressing existing use cases, or providing viable alternatives if breakage is indeed unavoidable.

Comment 4 Jakob Petsovits 2025-01-05 18:17:26 UTC

Closing as per https://invent.kde.org/plasma/powerdevil/-/merge_requests/455#note_1094984, and because the last 6.2 release (6.2.5) is out without the reversion having been merged. Sorry everyone who was hoping for this to go in. We did get some of the issues fixed since 6.2.0, both in the 6.2 branch and a few extra fixes for the upcoming Plasma 6.3, and we'll continue to make the experience as smooth as it should be.

There doesn't appear to be a good resolution status for "decided not to move forward with this", so I'll pick RESOLVED INTENTIONAL as perhaps the least bad option.