sebas created this revision. sebas added a reviewer: graesslin. sebas added a subscriber: Plasma. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel.
REVISION SUMMARY Use a timer to avoid catching configChanged signals after we set changes. The long version: TL;DR: We have a race condition when the kscreen daemon starts. It looks up a known config, then applies it and subsequently resaves the config. The same happens on config changes, it writes, then re-reads and then re-writes the config change. I've managed to prevent this from happening by adding a timer that does avoids saving the config as a direct reaction to our own config changes. So what happens on kded5 startup after loading the kscreen2 module: - the kscreen config is requested and received - the kscreen daemon (KD) looks into its config directory for a suitable config file (a config file is identified by a combined hash of all screen attached, so unique per connected set of outputs) - KD usually finds a config - KD ignores configChanged events before it starts ... - a KScreen::SetConfigOperation to apply the "known config" - SetConfigOperation returns after a while (say 100ms later) - we re-enable the change monitor - we receive a configChanged signal - we save the new config (usually to the existing config file) I don't think this behavior is desirable. I don't see a reason why the daemon should save its config right after applying it. I think this causes more problems than we want, since the startup may overwrite the user's config. This behavior seems to be desired by the code in KD, it's already blocking configChanged signals during the SetOperation (which, to be honest may result in nightmarish behavior in any way, so it might be a kludge which aims too short). From libkscreen perspective, SetConfigOperation::finished cannot guarantee that all configChanged signals are already fired and that it's safe to watch for new, independent changes now. At least on X11, we simply don't know, and what we can do is wait a bit and cross fingers that we're not catching our own noise. The changed signal *may* come from a re-request of the edid information, but this is a bit hard to track down, and not too useful, anyway, since changed Edid may affect a large number of a screen's properties. In the Wayland backend, that's a different story and we can prevent this behavior at an earlier stage, so this timer is "probably not needed" (I haven't tested that). This effectively prevents KD from catching reactions to its own changes and does not trigger saving the config file on every login. It still reacts to changes from libkscreen, but will avoid re-saving the config a few times. The timer may not be the neatest of solutions for this, but it does help narrowing down the problem and may be a last resort action. Most importantly, it avoids the re-writing of the config on startup and plugging/unplugging a monitor effectively. The timer value of 100ms is also used in kwin, which should make the behavior (which is no problem in kwin) more solid. CCBUG:346961 CCBUG:358011 REPOSITORY rKSCREEN KScreen BRANCH sebas/setop-race REVISION DETAIL https://phabricator.kde.org/D1730 AFFECTED FILES kded/daemon.cpp kded/daemon.h EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: sebas, graesslin Cc: plasma-devel, Plasma, sebas
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel