D25807: Allow to set the available screen rect/region from outside through dbus

mvourlakos added a comment. In D25807#584697 , @trmdi wrote: > Thank you so much @davidedmundson! > > @mvourlakos: I think you can improve Latte now. :) of course! REPOSITORY R120 Plasma Workspace REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi added a comment. Thank you so much @davidedmundson! @mvourlakos: I think you can improve Latte now. :) REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D25807 To: trmdi, #plasma, mvourlakos, davidedmundson, mart, apol Cc: davidre, plasma-devel,

D25807: Allow to set the available screen rect/region from outside through dbus

This revision was automatically updated to reflect the committed changes. Closed by commit R120:e8e360862fb7: Allow to set the available screen rect/region from outside through dbus (authored by trmdi). REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 72387. trmdi added a comment. - Drop the test method REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=72384=72387 BRANCH add-otherShellHelper (branched from master) REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

davidedmundson accepted this revision. davidedmundson added a comment. This revision is now accepted and ready to land. maybe drop the ::test method when merging REPOSITORY R120 Plasma Workspace BRANCH add-otherShellHelper (branched from master) REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 72384. trmdi added a comment. - Use const reference for some parameters REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71980=72384 BRANCH add-otherShellHelper (branched from master) REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi added inline comments. INLINE COMMENTS > davidedmundson wrote in strutmanager.h:25 > what's this about? This is just for easily testing using the qdbus command. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D25807 To: trmdi, #plasma, mvourlakos,

D25807: Allow to set the available screen rect/region from outside through dbus

davidedmundson added inline comments. INLINE COMMENTS > strutmanager.h:22 > +public Q_SLOTS: > +void setAvailableScreenRect(QString service, QString screenName, > QRect rect); > +void setAvailableScreenRegion(QString service, QString screenName, > QList rects); (const

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi added a comment. Ping! REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D25807 To: trmdi, #plasma, mvourlakos, davidedmundson, mart, apol Cc: davidre, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot,

D25807: Allow to set the available screen rect/region from outside through dbus

mvourlakos added inline comments. INLINE COMMENTS > shellcorona.h:94 > +// plasmashellCorona's value > +QRegion _availableScreenRegion(int id) const; > +QRect _availableScreenRect(int id) const; maybe concerning naming, plasmaAvailableScreenRegion would be better > shellcorona.h:95

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71980. trmdi added a comment. - Merge branch 'master' of https://anongit.kde.org/plasma-workspace into add-otherShellHelper - Just rename the screenId parameter of test() REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71979. trmdi added a comment. - Use screenName instead of id in setAvailableScreenRect/Region() REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71802=71979 BRANCH add-otherShellHelper (branched from

D25807: Allow to set the available screen rect/region from outside through dbus

mvourlakos added a comment. In D25807#581178 , @trmdi wrote: > In D25807#581161 , @mvourlakos wrote: > > > In D25807#581126 , @trmdi wrote: > > > > >

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi added a comment. In D25807#581161 , @mvourlakos wrote: > In D25807#581126 , @trmdi wrote: > > > @mvourlakos > > Latte can guard that the order of screens within Latte is synced with

D25807: Allow to set the available screen rect/region from outside through dbus

mvourlakos added a comment. In D25807#581126 , @trmdi wrote: > @mvourlakos > Latte can guard that the order of screens within Latte is synced with plasmashell right? Because I see this:

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi added a comment. @mvourlakos Latte can guard that the order of screens are the same with plasmashell right? Because I see this: https://github.com/KDE/latte-dock/blob/master/app/screenpool.cpp REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D25807

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi added a comment. In D25807#578640 , @davidedmundson wrote: > The problem is that on wayland (and to some extent x11 is racey) the order of screens within QApplication::screens might not be in sync across processes. Is it possible

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71802. trmdi added a comment. - Remove unneeded code REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71650=71802 BRANCH add-otherShellHelper (branched from master) REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

davidre added a comment. In D25807#579144 , @trmdi wrote: > In D25807#579125 , @davidre wrote: > > > I have a question about naming. If I had an application like panel or latte I would call

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi added a comment. In D25807#579125 , @davidre wrote: > I have a question about naming. If I had an application like panel or latte I would call setAvailableScreenRect? That seems kinda backwards to me? That part of the screen is not

D25807: Allow to set the available screen rect/region from outside through dbus

davidre added a comment. I have a question about naming. If I had an application like panel or latte I would call setAvailableScreenRect? That seems kinda backwards to me? That part of the screen is not available isn't it but rather not available anymore? Maybe I'm understanding the

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71650. trmdi added a comment. - Use QString type for the dbus screenId parameters REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71649=71650 BRANCH add-otherShellHelper (branched from master)

D25807: Allow to set the available screen rect/region from outside through dbus

davidedmundson added a comment. Much nicer! Re: string Vs int It can be ints within plasmashell, it just needs to be strings over the dbus protocol. The problem is that on wayland (and to some extent x11 is racey) the order of screens within QApplication::screens might not be

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71649. trmdi added a comment. - Add another missed line REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71648=71649 BRANCH add-otherShellHelper (branched from master) REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi added a comment. @davidedmundson Concerning the type of screenId should be QString, I think it need another patch, because Plasma::Corona's availableScreenRect also requires that type: https://api.kde.org/frameworks/plasma-framework/html/corona_8cpp_source.html#l00331 REPOSITORY

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71648. trmdi marked 2 inline comments as done. trmdi added a comment. - Add a missed line REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71647=71648 BRANCH add-otherShellHelper (branched from master)

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71647. trmdi added a comment. - Follow David's guide REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71594=71647 BRANCH add-otherShellHelper (branched from master) REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. Concept is fine. INLINE COMMENTS > othershellhelper.cpp:7 > + > +OtherShellHelper::OtherShellHelper(ShellCorona *plasmashellCorona) : >

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71594. trmdi added a comment. - No need to setConnected manually REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71488=71594 BRANCH add-otherShellHelper (branched from master) REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71488. trmdi added a comment. - Improve comment REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71408=71488 BRANCH add-otherShellHelper (branched from master) REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71408. trmdi added a comment. - emit signals when setConnected REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71391=71408 BRANCH add-otherShellHelper (branched from master) REVISION DETAIL

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71391. trmdi added a comment. - Redirect the signals - Rename the functions - Merge branch 'master' of https://anongit.kde.org/plasma-workspace into add-otherShellHelper REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi updated this revision to Diff 71079. trmdi added a comment. - Fix typo REPOSITORY R120 Plasma Workspace CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D25807?vs=71077=71079 BRANCH add-otherShellHelper REVISION DETAIL https://phabricator.kde.org/D25807 AFFECTED FILES

D25807: Allow to set the available screen rect/region from outside through dbus

mvourlakos added a comment. @trmdi this is a big change so it needs first to be approved theoretically from main plasma devs INLINE COMMENTS > shellcorona.cpp:1046 > > QRegion ShellCorona::availableScreenRegion(int id) const > +if >

D25807: Allow to set the available screen rect/region from outside through dbus

trmdi created this revision. trmdi added reviewers: Plasma, mvourlakos. Herald added a project: Plasma. Herald added a subscriber: plasma-devel. trmdi requested review of this revision. REVISION SUMMARY Allow to set the available screen rect/region from outside through dbus. REPOSITORY R120