https://bugs.kde.org/show_bug.cgi?id=498913
--- Comment #8 from Jakob Petsovits <jpe...@petsovits.com> --- (In reply to Saul Fautley from comment #7) > Thanks very much for the feedback on this Jakob. However I'm not convinced > this is the cause of the issue at hand, or at least not the full picture. Thanks, yeah, I seem to have judged too quickly, especially in light of you having already tracked down the commit that caused this for you. > First off, brightness updates using the slider were much quicker right > before the update you pushed for > https://bugs.kde.org/show_bug.cgi?id=481793. Was it your patch that also > added the 1 second delay? No, the timeout was added by this commit, initially for Plasma 6.1: https://invent.kde.org/plasma/powerdevil/-/commit/fd277d84bc0f5ddf55075a23a479f2944eaabee7 The commit that fixed Bug 481793 was merged to the master branch a few weeks after the timeout was introduced, but it was backported to 6.0.4 whereas the timeout was not. So users would have had access to the fix before the timeout appeared. Not sure how you tracked down the exact culprit, just note that both happened within a month from each other. > Secondly, the delay isn't only seen when using the slider. It's also present > when changing brightness via dbus, e.g. `qdbus6 org.kde.ScreenBrightness > /org/kde/ScreenBrightness/display0 SetBrightness 50 0`. Nate added the "with > slider" to the title of this bug, which I've removed now because I don't > think it's accurate. Yep, the slider is only one way to interface with the PowerDevil's brightness service and does not add any extra delay. Thanks for the correction. > I understand not wanting to spam monitors with ddc messages when moving the > slider. But surely the timeout should only apply to the brightness slider > widget then and not to any brightness change made with powerdevil? I was pondering that as well at the time and you do have a point. Part of why we picked the current approach was certainly because it was easier to implement, and still possible to improve on later if necessary. Part of it comes down to, is the applet the only spot we worry about? What about dimming (mainly handled by KWin), or multiple brightness keys in quick succession (handled by the daemon), or another app entirely? Are we expecting all of them to think about these issues from the start? Back at the time, I was prototyping/suggesting an extra parameter for the setBrightness() operation that would specify the type of brightness change requested - one-shot like with power state profile changes, or continuous like with the slider. It got a little abstract (among other things, laptop backlight displays need different behavior than external monitors) and I didn't move forward with it in the end as I felt there were more important things to tackle. Also, Qt's slider component doesn't have an API to let me know when the mouse button / touchpad finger is pressed down and raised back up again, and also there's scroll wheel events which obviously don't have an explicit beginning or end. So in the end it's murky anyway, and questionable whether I'd even get the API right to make it actually good. But I'd be happy to review patches from someone else who wants to take a new shot at limiting the delay only to slider and slider-like movement. -- You are receiving this mail because: You are watching all bug changes.
