Allow struts on panels between screen edges if WM is KWin
ClosedPublic

Authored by graesslin on Jul 14 2016, 11:15 AM.

Details

Summary

KWin starting with 5.7 supports struts on panels between screen edges.
Thus we can start setting struts on such panels, it won't exclude a
complete screen. But we don't know how other window managers handle it
and it's in general a rather "dangerous" change.

Thus to not affect other window managers, we check whether KWin is
running and only allow struts on thus panels if KWin is running.
Unfortunately we need to test this every time we go into the code path
as the WM might have changed.

In case the user replaces the window manager at runtime this still can
result in a bad situation.

BUG: 94470
FIXED-IN: 5.8.0

Test Plan

Tested whether it works in general in X11. Further testing
needed by X11, multi-screen users.

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
graesslin updated this revision to Diff 5152.Jul 14 2016, 11:15 AM
graesslin retitled this revision from to Allow struts on panels between screen edges if WM is KWin.
graesslin updated this object.
graesslin edited the test plan for this revision. (Show Details)
graesslin added a reviewer: Plasma.
Restricted Application added a project: Plasma. · View Herald TranscriptJul 14 2016, 11:15 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
graesslin updated this object.Jul 14 2016, 11:19 AM
luebking added inline comments.
shell/panelview.cpp
875

API sanity:

"if (!shouldNotSetStrut())" ...

double negations make people dizzy ;-)

884

qstricmp?
also, how does this react when the WM is replaced?

890

if (numScreens < 2) return false; (ie. true for the switched API - already confused?)

905

the query has a side effect - the strut update should be in the calling branch, yesno?

graesslin added inline comments.Jul 14 2016, 1:26 PM
shell/panelview.cpp
884

also, how does this react when the WM is replaced?

tricky. I think it's a corner case which could be ignored. We don't really have a way to detect it. I would say only "experienced" users know how to do that, they should be prepared for impact.

luebking added inline comments.Jul 14 2016, 1:51 PM
shell/panelview.cpp
884

One would require another signal in KWindowSystem - ultimately, it's only monitoring _NET_SUPPORTING_WM_CHECK on the root window

graesslin updated this revision to Diff 5170.Jul 14 2016, 2:58 PM
  • fixed logic error with platform check
  • changed name to canSetStrut to prevent negation in method name
  • qstricmp
  • move the setExtendedStrut call with no strut into PanelView::updateStruts
mart added a subscriber: mart.Jul 14 2016, 2:59 PM

fine with me.
we are not "officially" supporting other window managers for libplasma anyways

  • fixed logic error with platform check

See? ;-)

Looks good otherwise.

shell/panelview.cpp
953–955

this could go up (spare crashy virtualSize() calls ;-)

sebas added a subscriber: sebas.Jul 20 2016, 2:02 PM

If @luebking is otherwise fine with it, why not merge it?

sebas accepted this revision.Aug 25 2016, 11:23 AM
sebas added a reviewer: sebas.
This revision is now accepted and ready to land.Aug 25 2016, 11:23 AM
This revision was automatically updated to reflect the committed changes.