D27458: Set a better position for Krunner in wayland

[Tranter Madi]

trmdi closed this revision.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D27458

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Tranter Madi]

trmdi updated this revision to Diff 76629.
trmdi added a comment.


  Rename m_realVisible to m_requestedVisible

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27458?vs=76597=76629

BRANCH
  arcpatch-D27458 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27458

AFFECTED FILES
  krunner/view.cpp
  krunner/view.h
  shell/strutmanager.cpp
  shell/strutmanager.h

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[David Edmundson]

davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  Lets go for it.

INLINE COMMENTS

> view.cpp:411
> +{
> +m_realVisible = visible;
> +

this is a very confusing name.

It refers to the visible state krunner has set, but most importantly NOT the 
state of the QWindow (which is arguably the real state)

REPOSITORY
  R120 Plasma Workspace

BRANCH
  arcpatch-D27458 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27458

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Tranter Madi]

trmdi updated this revision to Diff 76597.
trmdi added a comment.


  Small fix.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27458?vs=76596=76597

BRANCH
  arcpatch-D27458 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27458

AFFECTED FILES
  krunner/view.cpp
  krunner/view.h
  shell/strutmanager.cpp
  shell/strutmanager.h

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Tranter Madi]

trmdi updated this revision to Diff 76596.
trmdi added a comment.


  Never show before position is set

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27458?vs=75883=76596

BRANCH
  arcpatch-D27458 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D27458

AFFECTED FILES
  krunner/view.cpp
  krunner/view.h
  shell/strutmanager.cpp
  shell/strutmanager.h

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Tranter Madi]

trmdi added a comment.


  In D27458#618437 , @davidedmundson 
wrote:
  
  >   Idea is nice.
  >   
  >
  > Does this not jitter on first load? Starting off in 0,0 then moving across?
  >  We now have position async() but we're doing that after we've shown the 
window.
  
  
  I don't really understand the problem you're talking about, but it shows up 
in a normal way without any change.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D27458

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[David Edmundson]

davidedmundson added a comment.


  
  
  >> Does this not jitter on first load? Starting off in 0,0 then moving across?
  >>  We now have position async() but we're doing that after we've shown the 
window.
  > 
  > I don't think so. The `show()` is in the callback.
  
  /A/ show() is there. It seems to be absolutely useless.
  
  We call show in View::display()
  
  Then we position on showEvent or resizeEvent

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D27458

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Méven Car]

meven added a comment.


  In D27458#618437 , @davidedmundson 
wrote:
  
  >   Idea is nice.
  >   
  >
  > Does this not jitter on first load? Starting off in 0,0 then moving across?
  >  We now have position async() but we're doing that after we've shown the 
window.
  
  
  I don't think so. The `show()` is in the callback.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D27458

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[David Edmundson]

davidedmundson added a comment.


  Idea is nice.
  
  Does this not jitter on first load? Starting off in 0,0 then moving across?
  We now have position async() but we're doing that after we've shown the 
window.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D27458

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Méven Car]

meven added a comment.


  ping @davidedmundson

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D27458

To: trmdi, #plasma, davidedmundson
Cc: meven, apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Tranter Madi]

trmdi updated this revision to Diff 75883.
trmdi added a comment.


  Improve code style

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27458?vs=75879=75883

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27458

AFFECTED FILES
  krunner/view.cpp
  shell/strutmanager.cpp
  shell/strutmanager.h

To: trmdi, #plasma, davidedmundson
Cc: apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Tranter Madi]

trmdi updated this revision to Diff 75879.
trmdi added a comment.


  Use asyncCall

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27458?vs=75828=75879

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27458

AFFECTED FILES
  krunner/view.cpp
  shell/strutmanager.cpp
  shell/strutmanager.h

To: trmdi, #plasma, davidedmundson
Cc: apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Aleix Pol Gonzalez]

apol added a comment.


  +1 on fixing this

INLINE COMMENTS

> view.cpp:240
> +QDBusInterface strutManager("org.kde.plasmashell", "/StrutManager", 
> "org.kde.PlasmaShell.StrutManager");
> +QDBusReply pr = strutManager.call("availableScreenRect", 
> shownOnScreen->name());
> +if (pr.isValid()) {

This is a blocking call, it would be ideal to find a way around this...

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D27458

To: trmdi, #plasma, davidedmundson
Cc: apol, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D27458: Set a better position for Krunner in wayland

[Tranter Madi]

trmdi created this revision.
trmdi added reviewers: Plasma, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
trmdi requested review of this revision.

REVISION SUMMARY
  In wayland, QScreen::availableGeometry() returns QScreen::geometry(), we 
could get a better value from plasmashell
  
  CCBUG: 386114

TEST PLAN
  Krunner isn't overlapped by a top panel in all cases

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D27458

AFFECTED FILES
  krunner/view.cpp
  shell/strutmanager.cpp
  shell/strutmanager.h

To: trmdi, #plasma, davidedmundson
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart