Summary: | Allow per monitor brightness control in the brightness and color widget | ||
---|---|---|---|
Product: | [Plasma] plasmashell | Reporter: | qlum <qlumreg> |
Component: | Brightness and Color | Assignee: | Plasma Bugs List <plasma-bugs> |
Status: | RESOLVED FIXED | ||
Severity: | wishlist | CC: | 4wy78uwh, albertogomezmarin, ben.schroeder, dougshaw77, gerrit.huebbers, jpetso, kalzwayed, me, natalie_clarius, nate, nicolas.fella, nilskemail+kde, postix, rocketraman, slartibart70 |
Priority: | NOR | ||
Version: | 6.0.0 | ||
Target Milestone: | 1.0 | ||
Platform: | Other | ||
OS: | Linux | ||
Latest Commit: | https://invent.kde.org/plasma/powerdevil/-/commit/88aa87f7e6c291de02e6a585a04ccb0ebfcfd006 | Version Fixed In: | 6.2.0 |
Sentry Crash Report: | |||
Bug Depends on: | 431994 | ||
Bug Blocks: | |||
Attachments: | Screenshot of the brightness window |
Description
qlum
2024-02-28 11:47:39 UTC
There's a mentored project going on right now as part of Season of KDE: https://invent.kde.org/teams/season-of-kde/2024/-/issues/20 There's also work going on in PowerDevil internally to support that project. The overarching task is https://invent.kde.org/plasma/powerdevil/-/issues/19 and I've got a few patches lined up (some in code review, some not yet) to standardize how we deal with brightness-supporting displays. In the end, what we're likely to see is one brightness control for the laptop display (if present) and one for each monitor separately. With KWin being responsible for dimming monitors whose brightness can't be set in hardware. Hopefully some or all of that work will be available to users in Plasma 6.1. Either way, we're working on it! Thanks also for bringing up the fact that with several monitors of the same kind, per-monitor brightness control is not necessarily a good thing. I think we'll want to introduce some linking/grouping of monitors for brightness controls if we want to retain the current behavior. That could get tricky when different kinds of monitors are involved, but preserving the current functionality may be as easy as keeping the current implementation functional and providing a switch between grouped and separate. Great to know it's being worked on. as for keeping the old behavior as a toggle, wouldn't that be worse from a maintainability perspective? Wouldn't adding a simple lock / fold-unfold that simply moves all sliders at the same time be a better solution? More complex grouping / linking to me seems overkill. The current behavior also still fails in cases where not all brightness levels are equal and displays them as 1%. Is this not a duplicate of Bug 470107? (In reply to Nate Graham from comment #4) > Is this not a duplicate of Bug 470107? Comment #1 makes a mention of that at the bottom :) which I can accept, although the fix will likely be the same for both reported bugs. Personally, I quite like the description of this one here and don't mind having a second bug around that's similar, but not entirely the same. If we think we're just as well off with only having one though, I won't stand in the way of that. (In reply to Jakob Petsovits from comment #5) > (In reply to Nate Graham from comment #4) > > Is this not a duplicate of Bug 470107? > > Comment #1 makes a mention of that at the bottom Whoops, that's my own comment. I meant the original bug description. Comment #0, strictly speaking. *** Bug 482560 has been marked as a duplicate of this bug. *** I can't detect a meaningful difference between what's requested here and what's requested in Bug 470107, which is older. But I agree that the description is better here. I think it's fine to mark 470107 and its duplicate as duplicates of this one in the interests of centralizing information about the feature in one onace. *** Bug 479065 has been marked as a duplicate of this bug. *** *** Bug 470107 has been marked as a duplicate of this bug. *** *** Bug 486493 has been marked as a duplicate of this bug. *** Git commit 88aa87f7e6c291de02e6a585a04ccb0ebfcfd006 by Jakob Petsovits. Committed on 23/08/2024 at 19:36. Pushed by jpetso into branch 'master'. applets/brightness: Per-display brightness controls This uses the new per-display D-Bus API from PowerDevil. Following this change, the applet does not use the old BrightnessControl D-Bus interface anymore, only the new ScreenBrightnessControl interface. A new ScreenBrightnessDisplayModel class is added to allow the use of QML Repeater objects in the UI. For scrolling on the icon, the applet does not need to know or specify which displays are changed, this responsibility now lies with the daemon and its AdjustBrightnessRatio/AdjustBrightnessSteps methods. Autotests are adapted accordingly, including changes by Fushan Wen. Note that reported brightness for the mocked backlight display will be lower by 1 in the new API, because minimum brightness of 1 (default for internal displays) is subtracted from the underlying brightness value. Hence we now test for values from 0 to 254. Related: bug 431994, bug 353032, bug 487812 M +15 -7 applets/brightness/package/contents/ui/BrightnessItem.qml M +0 -1 applets/brightness/package/contents/ui/CompactRepresentation.qml M +65 -40 applets/brightness/package/contents/ui/PopupDialog.qml M +44 -18 applets/brightness/package/contents/ui/main.qml M +1 -0 applets/brightness/plugin/CMakeLists.txt M +158 -53 applets/brightness/plugin/screenbrightnesscontrol.cpp M +39 -13 applets/brightness/plugin/screenbrightnesscontrol.h A +103 -0 applets/brightness/plugin/screenbrightnessdisplaymodel.cpp [License: GPL(v2.0+)] A +49 -0 applets/brightness/plugin/screenbrightnessdisplaymodel.h [License: GPL(v2.0+)] M +13 -4 autotests/applets/brightnesstest.py https://invent.kde.org/plasma/powerdevil/-/commit/88aa87f7e6c291de02e6a585a04ccb0ebfcfd006 |