Review Request 115910: Screenedge show support for Clients

Thomas Lübking thomas.luebking at gmail.com
Thu Feb 20 19:40:09 UTC 2014



> On Feb. 20, 2014, 4:59 p.m., Thomas Lübking wrote:
> > kwin/screenedge.cpp, line 1032
> > <https://git.reviewboard.kde.org/r/115910/diff/1/?file=245042#file245042line1032>
> >
> >     i think this is "wrong".
> >     
> >     you're trying to detect the orientation of the panel, but the client likely *knows* the orientation of the panel (and where to activate it) - eg. old kicker (bottom panel) could collapse horizontally.
> >     
> >     imo the client should tell kwin where to (preferably) add the border (through the client message)
> >     
> >     internal heuristics could only serve as failsafe for lazy clients.
> 
> Martin Gräßlin wrote:
>     ah yes the client message, that could work. Client message got added after that code part got written, so it didn't occur to me ;-)
> 
> Thomas Lübking wrote:
>     I wonder whether the client message should rather be replaced by a property?
>     It would cover the "kwin restarted" situation as well as be more robust against races (plasma starts before kwin) and also export the condition to other clients.
> 
> Martin Gräßlin wrote:
>     I first tried with a property, but the problem is that there is no way for the client to propagate that it should get hidden. So in the end I had property and the client message in addition resulting in the property being ignored all the time. Thus I removed it again.
>     
>     The KWin restarted situation is in my opinion quite well handled by having the client being shown again. And it's a corner case, our users shouldn't restart KWin ;-)

The client could set the property when it wants to be hidden (and also hint on which edge to add the trigger)
When showing it, kwin would withdraw the property and withdrawing the property could be used by the client to indicate "show me again" (because the user turned it into a permanent panel or whatever)


- Thomas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115910/#review50385
-----------------------------------------------------------


On Feb. 20, 2014, 1:51 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115910/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2014, 1:51 p.m.)
> 
> 
> Review request for kwin and Plasma.
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> Screenedge show support for Clients
> 
> This provides a new protocol intended to be used by auto-hiding panels
> to make use of the centralized screen edges. To use it a Client can
> send a client message of type _KDE_NET_WM_SCREEN_EDGE_SHOW to KWin.
> KWin will hide the Client (hide because unmap or minimize would break
> it) and create an Edge. If that Edge gets triggered the Client is shown
> again. If the Client doesn't border a screen edge the Client gets shown
> immediately so that we never end in a situation that we cannot unhide
> the auto-hidden panel again. The exact process is described in the
> documentation of ScreenEdges.
> 
> If KWin gets restarted the Client gets shown again.
> 
> As this is a KWin specific extension we need to discuss what it means
> for Clients using this feature with other WMs: it does nothing. As
> the Client gets hidden by KWin and not by the Client, it just doesn't
> get hidden if the WM doesn't provide the feature. In case of an
> auto-hiding panel this seems like a good solution given that we don't
> want to hide it if we cannot unhide it. Of course there's the option
> for the Client to provide that feature itself and if that's wanted we
> would need to announce the feature in the _NET_SUPPORTED atom. At the
> moment that doesn't sound like being needed as Plasma doesn't want to
> provide an own implementation.
> 
> The implementation comes with a small test application showing how
> the feature is intended to be used.
> 
> 
> Diffs
> -----
> 
>   kwin/atoms.h 1690067c5d1da59f38f9e77ef64eacfbc1faa0cf 
>   kwin/atoms.cpp 904f5efe4a32e3673dae9e6da92bf4336def660d 
>   kwin/client.cpp 36431bfc33418a207de12fa8cc95a35539256366 
>   kwin/events.cpp 1fa6e425d4dac7d661612e5d090c3c9c8f4b1a18 
>   kwin/screenedge.h 60f5fd669ccc5eb627feffa460552558d1765b31 
>   kwin/screenedge.cpp 04cf0d6d5262ab84d88559b6dc85e099efec77bf 
>   kwin/tests/CMakeLists.txt 3fa16f21c617a8f4b39b2bbd39b534b6a11e8d14 
>   kwin/tests/screenedgeshowtest.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/115910/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140220/79ca8a29/attachment.html>


More information about the Plasma-devel mailing list