applets/brightness: Always show displays in order of the D-Bus listing
Note: I picked 6.3 as milestone given that it's a low-impact bug (no reports on bugs.kde.org either) and the change introduces a little bit of extra code. If it works out well and people like the change enough, I wouldn't take issue with cherry-picking this to Plasma/6.2 either.
Commit messages
applets/brightness: Always show displays in order of the D-Bus listing
Previously, the applet's brightness sliders were initialized with the D-Bus listing, but subsequent display additions would always be added at the end. This is mostly fine, but not always.
After this refactoring, the display model is fully in control of determining its row order without help from the main plugin class. Its rows are the intersection of two collections of source data:
- The list of known display names, a direct mirror of the latest list of display names on the screen brightness D-Bus interface.
- A hashmap of display data, for displays that were queried.
Not every known display name necessarily has a corresponding row element, but the order of exposed row elements matches the list of known display names.
Display data is only kept for known display names, unassociated data is removed when names are updated.
This change also makes the plugin more robust against async/coroutine code execution ordering bugs.
applets/brightness: Never miss a display on brightness service registration
Since commit c8f60bbf, the display list gets updated when the org.kde.ScreenBrightness service (provided by PowerDevil) starts or restarts. This exposed a race condition that was already present, but now gets triggered regularly due to tighter timing.
The sequence of events reproducing this bug goes like this:
- The D-Bus service appears,
onServiceRegistered()
is invoked. -
queryAndUpdateDisplays()
starts retrieving display properties. - Soon after the D-Bus service came up, it adds another display.
-
queryAndUpdateDisplays()
finishes. - Only at this point we connect to D-Bus display change signals.
In this sequence, the display from (3) is missed and won't show up in the applet until another display change happens at a later point.
Now that the display model is robust against duplicate insertions,
we can simply move queryAndUpdateDisplays()
from the start of
onServiceRegistered()
to the end of the function, at which point
D-Bus signal connections are already set up. This ensures that
no display additions will be missed, even if they happen immediately
after service startup.