D8796: Support dynamic output enabling/disabling from KScreen
This revision was automatically updated to reflect the committed changes. Closed by commit R108:01c1870e9dde: Support dynamic output enabling/disabling from KScreen (authored by davidedmundson). REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8796?vs=22629=22692 REVISION DETAIL https://phabricator.kde.org/D8796 AFFECTED FILES plugins/platforms/drm/drm_backend.cpp plugins/platforms/drm/drm_backend.h plugins/platforms/drm/drm_output.cpp plugins/platforms/drm/drm_output.h plugins/platforms/drm/screens_drm.cpp To: davidedmundson, #plasma, graesslin Cc: ngraham, luebking, broulik, graesslin, plasma-devel, kwin, #kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
graesslin accepted this revision. This revision is now accepted and ready to land. REPOSITORY R108 KWin BRANCH master REVISION DETAIL https://phabricator.kde.org/D8796 To: davidedmundson, #plasma, graesslin Cc: ngraham, luebking, broulik, graesslin, plasma-devel, kwin, #kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
davidedmundson updated this revision to Diff 22629. davidedmundson added a comment. Restricted Application edited projects, added KWin; removed Plasma. skip monitors in changeset that don't exist without aborting REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8796?vs=22508=22629 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8796 AFFECTED FILES plugins/platforms/drm/drm_backend.cpp plugins/platforms/drm/drm_backend.h plugins/platforms/drm/drm_output.cpp plugins/platforms/drm/drm_output.h plugins/platforms/drm/screens_drm.cpp To: davidedmundson, #plasma Cc: ngraham, luebking, broulik, graesslin, plasma-devel, kwin, #kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
davidedmundson added inline comments. INLINE COMMENTS > graesslin wrote in drm_backend.cpp:553 > why return? Shouldn't it be a continue? Can do. Though I'd want to change the existing loop too to keep it consistent. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D8796 To: davidedmundson, #plasma Cc: luebking, broulik, graesslin, plasma-devel, kwin, #kwin, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
graesslin added inline comments. INLINE COMMENTS > drm_backend.cpp:553 > +qCWarning(KWIN_DRM) << "Could NOT find DrmOutput matching " > << it.key()->uuid(); > +return; > +} why return? Shouldn't it be a continue? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D8796 To: davidedmundson, #plasma Cc: luebking, broulik, graesslin, plasma-devel, kwin, #kwin, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
davidedmundson updated this revision to Diff 22508. davidedmundson added a comment. Restricted Application edited projects, added Plasma; removed KWin. Make sure we always enable monitors before disabling them Never remove the final screen REPOSITORY R108 KWin CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D8796?vs=22282=22508 BRANCH master REVISION DETAIL https://phabricator.kde.org/D8796 AFFECTED FILES plugins/platforms/drm/drm_backend.cpp plugins/platforms/drm/drm_backend.h plugins/platforms/drm/drm_output.cpp plugins/platforms/drm/drm_output.h plugins/platforms/drm/screens_drm.cpp To: davidedmundson, #plasma Cc: luebking, broulik, graesslin, plasma-devel, kwin, #kwin, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
davidedmundson added a comment. > Does KScreen allow to disable all screens? The user gets a prompt "are you sure you want to disable all outputs?" If they press the "I'm an idiot" button, then it will. > and if yes: how does KWin behave in this case? Very badly. > How will the Qt QPA behave: Very badly. > Ignore the request as "probably temporary" In theory, we should get atomic batched the change requests from kscreen. There's no reason for it to do that. I'll make it never remove the last screen. If I make it process any changes for currently disabled screens first, that should handle atomicly switching monitors nicely. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D8796 To: davidedmundson, #plasma Cc: luebking, broulik, graesslin, plasma-devel, kwin, #kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
luebking added a comment. Ignore the request as "probably temporary" and just freeze rendering? REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D8796 To: davidedmundson, #plasma Cc: luebking, broulik, graesslin, plasma-devel, kwin, #kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
broulik added a comment. Given how well Qt handled the no screen case in the past I'd advise against being able to disable all of them. REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D8796 To: davidedmundson, #plasma Cc: broulik, graesslin, plasma-devel, kwin, #kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
graesslin added a comment. Does KScreen allow to disable all screens? And if yes: how does KWin behave in this case? I had been thinking about this for some time and are not sure how KWin should handle this situation. I can see reasons to say "user asked for it, so all screens should go off" and I can see reason to say "that's an invalid configuration request: ignore". REPOSITORY R108 KWin REVISION DETAIL https://phabricator.kde.org/D8796 To: davidedmundson, #plasma Cc: graesslin, plasma-devel, kwin, #kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
D8796: Support dynamic output enabling/disabling from KScreen
davidedmundson created this revision. davidedmundson added a reviewer: Plasma. Restricted Application added a project: KWin. Restricted Application added subscribers: KWin, kwin, plasma-devel. REVISION SUMMARY We need to keep the DrmOutput object to still have the WaylandOutputDevice. Screens currently start off enabled as before. In order to keep KWin to have a correct index based list of screens we need to store a second vector of currently enabled outputs for the screens interface. TEST PLAN Had dual screens. Disabled/Enabled each one through the kscreen KCM REPOSITORY R108 KWin BRANCH master REVISION DETAIL https://phabricator.kde.org/D8796 AFFECTED FILES plugins/platforms/drm/drm_backend.cpp plugins/platforms/drm/drm_backend.h plugins/platforms/drm/drm_output.cpp plugins/platforms/drm/drm_output.h plugins/platforms/drm/screens_drm.cpp To: davidedmundson, #plasma Cc: plasma-devel, kwin, #kwin, bwowk, ZrenBot, progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart
