Summary: | Brightness slider for connected TV is duplicated | ||
---|---|---|---|
Product: | [Plasma] Powerdevil | Reporter: | Bart Ribbers <bribbers> |
Component: | general | Assignee: | Plasma Bugs List <plasma-bugs> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | jpetso, natalie_clarius, nate, slartibart70, username85312 |
Priority: | NOR | ||
Version: | 6.1.90 | ||
Target Milestone: | --- | ||
Platform: | Other | ||
OS: | Linux | ||
Latest Commit: | https://invent.kde.org/plasma/powerdevil/-/commit/ea91fa1ba728ba387c81c5cce2fde1ca34c0c83f | Version Fixed In: | 6.2.1 |
Sentry Crash Report: | |||
Attachments: | brightness and color |
Description
Bart Ribbers
2024-09-29 18:28:00 UTC
I'm assuming you don't actually have more than one screen connected, right? If you do, the presence of one slider per screen is intentional, and the bug is that one of then is broken. I think you misunderstood. There are 2 sliders _for the same screen_. I do have 2 other screens plugged in and they have their own perfectly functioning sliders. But the 3rd screen gets 2 sliders which has the broken behaviour I mentioned so the applet ends up with 4 sliders in total for 3 screens. Interestingly enough that's what happened yesterday, right now I just see the 3 sliders as expected. I'm not sure what the difference here is. I see. How strange! Could you paste the output of `kscreen-doctor -o`? I can reproduce something similar, except for "Built-in Screen". Reproduction steps (tested on a Wayland session): 1. Laptop with one monitor plugged in, both screens enabled. 2. In Power Management settings, set lid action to "Sleep" and check "Even when an external monitor is connected". 3. Suspend the laptop by closing the lid. 4. Disconnect monitor cable. 5. Wake up by raising the lid. OBSERVED: Built-in Screen exists twice now. (Also, the external monitor brightness slider is gone, but that's expected.) Like Bart says, moving the bottom slider also moves the top one with it, but moving the top slider does not move the bottom slider. Both will affect brightness. Lists by kscreen-doctor and org.kde.ScreenBrightness display names are correct. EXPECTED: Only one slider, for Built-in Screen, remains. Let's do some debugging to see if this is a bug in the applet or in sending D-Bus DisplayAdded/DisplayRemoved signals. same here: After initial login, i have to sliders for internal screen (laptop) and external (usb-c) Let the machine sleep, and afterwards we have 2 sliders for usb-c connected screen. And, the display brightness is way off: On the monitor-controls, the setting is: brightness: 30 Plasma shows initially: built-in: 20%, HP/usb-c: 50% Overall brightness is as i want it. After sleep, the external monitor is way too dark, i even need to put it to over 70% to get a similar brightness as before. This is just awful... how can i stop plasma from handling the brightness? (any off-switch somewhere?) === kscreen-doctor -o Output: 1 eDP-1 enabled connected priority 1 Panel Modes: 1:2880x1800@90*! 2:2880x1800@60 3:1920x1200@90 4:1920x1080@90 5:1600x1200@90 6:1680x1050@90 7:1280x1024@90 8:1440x900@90 9:1280x800@90 10:1280x720@90 11:1024x768@90 12:800x600@90 13:640x480@90 14:1600x1200@60 15:1280x1024@60 16:1024x768@60 17:2560x1600@60 18:1920x1200@60 19:1280x800@60 20:2880x1620@60 21:2560x1440@60 22:1920x1080@60 23:1600x900@60 24:1368x768@60 25:1280x720@60 Geometry: 0,0 2304x1440 Scale: 1.25 Rotation: 1 Overscan: 0 Vrr: incapable RgbRange: unknown HDR: disabled Wide Color Gamut: disabled ICC profile: none Color profile source: sRGB Brightness control: supported, set to 20% Output: 2 DP-5 enabled connected priority 2 DisplayPort Modes: 26:3840x2160@60*! 27:3840x2160@30 28:1920x1200@60 29:1920x1080@60 30:1920x1080@60 31:1920x1080@60 32:1920x1080@50 33:1600x1200@60 34:1680x1050@60 35:1600x900@60 36:1280x1024@60 37:1440x900@60 38:1280x800@60 39:1280x720@60 40:1280x720@60 41:1280x720@60 42:1280x720@50 43:1024x768@60 44:800x600@60 45:720x576@50 46:720x576@50 47:720x480@60 48:720x480@60 49:720x480@60 50:720x480@60 51:640x480@60 52:640x480@60 53:640x480@60 54:720x400@70 55:1600x1200@60 56:1280x1024@60 57:1024x768@60 58:2560x1600@60 59:3200x1800@60 60:2880x1620@60 61:2560x1440@60 62:1920x1080@60 63:1600x900@60 64:1368x768@60 65:1280x720@60 Geometry: 2304,0 3840x2160 Scale: 1 Rotation: 1 Overscan: 0 Vrr: incapable RgbRange: unknown HDR: incapable Wide Color Gamut: incapable ICC profile: none Color profile source: sRGB Brightness control: supported, set to 78% Operating System: Fedora Linux 40 KDE Plasma Version: 6.2.0 KDE Frameworks Version: 6.7.0 Qt Version: 6.7.2 Kernel Version: 6.10.13-200.fc40.x86_64 (64-bit) Graphics Platform: Wayland Processors: 16 × AMD Ryzen 7 PRO 7840U w/ Radeon 780M Graphics Memory: 58,5 GiB of RAM Graphics Processor: AMD Radeon 780M Manufacturer: LENOVO Product Name: 21K5000JGE System Version: ThinkPad P14s Gen 4 Created attachment 174478 [details]
brightness and color
even with the increased brightness (here: 78% or more), the external screen does not return to its initial brightness. Only logoff/login helps (this is annoying to say the least!!!) I have already disabled power-management > dim automatically : never Let's see if this helps A 'restart' of the brightness of plasma could be very helpful. Brightnessctl and ddcutil can't oversteer what plasma is doing at the moment... so better to have the plasma 'feature' switchable in the future until testing is thoroughly done and this thing works properly??? (In reply to slartibart70 from comment #7) > even with the increased brightness (here: 78% or more), the external screen > does not return to its initial brightness. That's a different bug. Un-dimming back to a each monitor's original brightness was Bug 493111 which is fixed for 6.2.1, at least as good (or bad) as Plasma ever was at reverting brightness after wake-up. Still needs improvements as mentioned there. There's also Bug 482713 for more robust handling of DDC/CI errors in PowerDevil. There's also https://invent.kde.org/plasma/kwin/-/issues/238 to avoid confusing KWin software brightness fallbacks if PowerDevil does end up losing access to DDC/CI anyway. Thank you for posting details, and please try to stay on topic within this bug. This one is for duplicated controls only, whereas unexpected brightness differences between applet and monitor hardware should be discussed in a different bug report. yes, thank you for the reminder... things gotten off hand, i was just angry about having the necessary logoff/login to restore initial brightness. Anyway, the power-options 'dim automatically : never' do help! I'm hoping for the best with 6.2.1 :-) (In reply to Jakob Petsovits from comment #4) > I can reproduce something similar, except for "Built-in Screen". > > (...) > > Let's do some debugging to see if this is a bug in the applet or in sending > D-Bus DisplayAdded/DisplayRemoved signals. QDBusViewer confirms that PowerDevil emits the correct DisplayRemoved signals, meaning no issues in the daemon internally related to this. So the next question was whether the applet will correctly receive these signals and update the display model. It turns out that the async nature of checkDisplayNames()/updateDisplayNames() in ScreenBrightnessControl is what causes this issue. In a nutshell, the change signals come in fairly quickly, and the use of QCoro means that another call to queryAndInsertDisplay() may be fired before the previous one had finished. I've got a fix for this, which bluntly rips out the use of async functions where display model changes are involved. Which is most of them, really. I'm sure there's a more efficient fix, but I'll stick to just making it correct for now. A possibly relevant merge request was started @ https://invent.kde.org/plasma/powerdevil/-/merge_requests/435 (In reply to Jakob Petsovits from comment #10) > I've got a fix for this, which bluntly rips out the use of async functions > where display model changes are involved. Which is most of them, really. I'm > sure there's a more efficient fix, but I'll stick to just making it correct > for now. I decided to go for the slightly more efficient fix and submitted the MR with QCoro/async left in place, but still making sure the update isn't executed twice at the same time. Git commit 7e69b58422bfb6fd99dbc422a00cbe87579b1116 by Jakob Petsovits. Committed on 07/10/2024 at 22:51. Pushed by jpetso into branch 'master'. applets/brightness: Avoid producing duplicate display sliders Sometimes a display configuration change results in more than one change signal. These can happen in quick succession of each other. The async code in ScreenBrightnessControl had a problem with this, because it would sometimes interleave parts of its model update function with another call of the same function. This would cause duplicate brightness sliders to appear for the same display. The solution is to make sure we don't try to update the model while another update is already running. One approach would be to simply take QCoro and async calls out in favor of synchronous calls which are easier to reason about. However, this would mean we block plasmashell for a longer time. Instead, this commit introduces a call guard to the update function which prevents another asynchronous call from entering its main body of code while an existing update is already in the works. After the update has finished, we will repeat the update process in case another update request had been issued in the meantime. M +32 -19 applets/brightness/plugin/screenbrightnesscontrol.cpp M +3 -2 applets/brightness/plugin/screenbrightnesscontrol.h https://invent.kde.org/plasma/powerdevil/-/commit/7e69b58422bfb6fd99dbc422a00cbe87579b1116 Git commit ea91fa1ba728ba387c81c5cce2fde1ca34c0c83f by Jakob Petsovits. Committed on 08/10/2024 at 04:31. Pushed by jpetso into branch 'Plasma/6.2'. applets/brightness: Avoid producing duplicate display sliders Sometimes a display configuration change results in more than one change signal. These can happen in quick succession of each other. The async code in ScreenBrightnessControl had a problem with this, because it would sometimes interleave parts of its model update function with another call of the same function. This would cause duplicate brightness sliders to appear for the same display. The solution is to make sure we don't try to update the model while another update is already running. One approach would be to simply take QCoro and async calls out in favor of synchronous calls which are easier to reason about. However, this would mean we block plasmashell for a longer time. Instead, this commit introduces a call guard to the update function which prevents another asynchronous call from entering its main body of code while an existing update is already in the works. After the update has finished, we will repeat the update process in case another update request had been issued in the meantime. (cherry picked from commit 7e69b58422bfb6fd99dbc422a00cbe87579b1116) Co-authored-by: Jakob Petsovits <jpetso@petsovits.com> M +32 -19 applets/brightness/plugin/screenbrightnesscontrol.cpp M +3 -2 applets/brightness/plugin/screenbrightnesscontrol.h https://invent.kde.org/plasma/powerdevil/-/commit/ea91fa1ba728ba387c81c5cce2fde1ca34c0c83f *** Bug 494552 has been marked as a duplicate of this bug. *** |