[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request 108513: Rewrite of KWin's Screen Edge Handling
From: Martin_Gräßlin <mgraesslin () kde ! org>
Date: 2013-02-07 10:44:40
Message-ID: 20130207104440.26727.62822 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On Feb. 7, 2013, 11:12 a.m., Thomas Lübking wrote:
> > kwin/effects.h, line 143
> > <http://git.reviewboard.kde.org/r/108513/diff/6/?file=111909#file111909line143>
> >
> > F***
> >
> > I ran into that yesterday and wanted to mention it.
> > You forgot to bump the ABI (leading to *very* ominous crash reports here ;-)
8faf62d8b42a782ea0b2efe33ef3915342ac3903
- Martin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108513/#review26832
-----------------------------------------------------------
On Feb. 4, 2013, 9:50 a.m., Martin Gräßlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108513/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2013, 9:50 a.m.)
>
>
> Review request for kwin, Plasma and Arthur Arlt.
>
>
> Description
> -------
>
> Screen Edges may belong to fullscreen windows
>
> Corners are still ours (it's a valid use case to still be able to switch
> window through e.g. Present Windows even when running a fullscreen app).
>
> How is it done? An Edge can be blocked and does no longer trigger if it
> is blocked. For WindowBasedEdges the edge windows get unmapped in the
> blocking case and mapped again when the blocking condition is no longer
> valid.
>
> The blocking is so far connected to:
> * changes of active window
> * changes of fullscreen windows
>
> Whenever one of the events occurs it is checked whether there is:
> 1. an active client
> 2. it is fullscreen
> 3. on the same screen as the edge
>
> If this is the case the edge will be blocked, otherwise unblocked.
>
> New Glow on approaching Screen Edge Effect
>
> Feature stolen from hidden Plasma Panel now available for all edges on
> approach by mouse.
>
> Split out event handling in ScreenEdges in separate methods
>
> Allows to also support xcb_generic_event_t in addition to XEvent.
>
> Notifications when approaching a screen edge
>
> For each edge an additional "approach" area window is created. When the
> mouse enters this approach window, it gets unmapped and a mouse polling
> interval is started. If the mouse leaves the approach area again, the
> window gets mapped again and the mouse polling is stopped.
>
> During the approaching a signal is emitted with a factor in [0.0,1.0] to
> describe how close the mouse is to the edge. 0.0 means far away, 1.0
> means triggering the edge. This signal is passed to the effects to allow
> using this information. E.g. to provide a glow corner effect or to make
> use of it in the cube animation effect to start the animation on desktop
> switch.
>
> Turn ScreenEdges into a Singleton
>
> In fact it already used to be a Singleton as there is just one object
> hold by the Singleton Workspace. So let's make it a proper Singleton
> following our kind of standard approach of having a ::create factory
> method called from Workspace ctor and a ::self to get to the singleton
> instance.
>
> Change the way how screen edges interact with Effects/Scripts
>
> The main difference is that the activation of an edge is no longer
> broadcasted to all effects and scripts, but instead a passed in slot of
> the Effect/Script is invoked.
>
> For this the EffectsHandler API is changed to take the Effect as an
> argument to (un)reserveElectricBorder. As callback slot the existing
> borderActivated is used.
>
> In addition the ScreenEdge monitors the object for beeing destroyed and
> unregisters the the edge automatically. This removes the need from the
> Effect to call unregister in the dtor.
>
> BUG: 309695
> FIXED-IN: 4.11
>
> Rewrite of KWin's Screen Edge Handling
>
> This rewrite is mostly motivated by the need to handle multi screen
> setups correctly. That is have edges per screen and not for the combined
> geometry. Also porting from XLib to XCB has been a motivation for the
> rewrite.
>
> The design of the new ScreenEdge handling is described in the
> documentation of ScreenEdges in screenedge.h.
>
> In addition the following changes have been performed:
> * move configuration from Options to ScreenEdge
> * add screen edge information to Workspace::supportInformation (obviously
> replaces what had been read from Options)
> * have Workspace hold a pointer to ScreenEdges instead of an object
> * forward declaration of ScreenEdges in workspaces.h, this explains the
> seemingly unrelated changes of just another include in some files
>
> BUG: 290887
> FIXED-IN: 4.11
>
> Do not update screen edges when compositing settings changes
>
> The comment says it all: update all settings which can be done through
> the compositing KCM. Years ago screen edges was in the composite KCM, but
> it no longer is. So there is no need to update the edges when the
> compositing settings changes.
>
> Remove interaction of quick tiling with ScreenEdges
>
> Quick tiling/maximizing of Clients is completely independent of the
> screen edges functionality. That is it determines the borders itself.
> Nevertheless there has been some code still around which interacted with
> the screen edges each time a window was moved. This code is completely
> useless.
>
> Remove unused screen edge related methods from kwineffects interface
>
> No effect has ever used these methods and there is no reason why an
> effect should use them. Reserve/unreserve is sufficient as the effect
> will be notified anyway.
>
> Adding some convenient functions to wrap xcb calls
>
> Commonly needed functionality, like
> * move/resize windows
> * restack windows
> * restack to top of stack
>
>
> This addresses bugs 271607, 290887 and 309695.
> http://bugs.kde.org/show_bug.cgi?id=271607
> http://bugs.kde.org/show_bug.cgi?id=290887
> http://bugs.kde.org/show_bug.cgi?id=309695
>
>
> Diffs
> -----
>
> kwin/composite.cpp c27b37f21ec946775497047837c00dc7107c7756
> kwin/effects.h 8d721fceef3e0a041e5a38088aef0de3a897d58d
> kwin/effects.cpp 6845b748e46d0ffec42af58987dd255ff7a03bbb
> kwin/effects/CMakeLists.txt b8d458e6be6397330c10a7d4ab7a4fe8f1e52ffe
> kwin/effects/cube/cube.cpp 3fffbd0577194c8c16bf608a04a2eeedd84b3ba1
> kwin/effects/desktopgrid/desktopgrid.cpp 187d0588bf1e4a59ce442f7606c2da6e181a8f03
> kwin/effects/flipswitch/flipswitch.cpp 445405edb93c39d9e776e6bb7ae40a233a6ab740
> kwin/effects/presentwindows/presentwindows.cpp de325df2451f47bca6effeeedefea960bc3ce089
> kwin/effects/screenedge/CMakeLists.txt PRE-CREATION
> kwin/effects/screenedge/screenedgeeffect.cpp PRE-CREATION
> kwin/effects/screenedge/screenedgeeffect.desktop PRE-CREATION
> kwin/effects/screenedge/screenedgeeffect.h PRE-CREATION
> kwin/events.cpp a39aec07fc9f69438eb38d5569ba4ee0a218db42
> kwin/geometry.cpp 7eb7e94d2804aa512b3da23b81c40ffe4b899811
> kwin/layers.cpp 05b3d6dd0a6349a58eaf7a0b6a275b3ea6efde8b
> kwin/libkwineffects/kwineffects.h 423cfd1140ff4a01fa3e06036383a9ce665b4da7
> kwin/options.h f458637b7f7f8210f6c9de3497ad1d49df8261c0
> kwin/options.cpp 41dda403ce694c7820b8ee04f22f92b99559c1e2
> kwin/screenedge.h 6dd1956332958d11e768c01eb3f91ae411a5cc77
> kwin/screenedge.cpp fb6209c1994345cf36da8dbcf097bb3596890cf3
> kwin/scripting/scriptedeffect.h 830997b50b2f2d1eaf2aa828b94b110612b2080b
> kwin/scripting/scriptedeffect.cpp e9c05091087c8303da53d998d06dc46748b9fdce
> kwin/scripting/scripting.h 077fe667dbf76e4165f10f0f19c8700e54e1350e
> kwin/scripting/scripting.cpp 6c6e258656eb62bf5811984472f0d0776420b4d5
> kwin/scripting/scriptingutils.h 3ed78458131a7fb535e38a08f3c6e7467e9e231b
> kwin/workspace.h b26725d0afdcba339aa79348e824bacaec13f32f
> kwin/workspace.cpp 9183fd66dace05db6d2ad0b55c4c26e1a288ce95
> kwin/xcbutils.h 31bd949b3404d62d8319f0acea521e90a1cfdbeb
>
> Diff: http://git.reviewboard.kde.org/r/108513/diff/
>
>
> Testing
> -------
>
> * effects on various edges
> * action on one edge
> * activated/deactivated actions
> * turned on off screen
> * switch desktop (terrible feature)
> * switch desktop when moving Clients
>
> TODO: unit test for base functionality
>
>
> Thanks,
>
> Martin Gräßlin
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/108513/">http://git.reviewboard.kde.org/r/108513/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On February 7th, 2013, 11:12 a.m. CET, <b>Thomas \
Lübking</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/108513/diff/6/?file=111909#file111909line143" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/effects.h</a> <span style="font-weight: normal;">
(Diff revision 6)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">143</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">virtual</span> <span class="kt">void</span> <span \
class="nf">checkElectricBorder</span><span class="p">(</span><span \
class="k">const</span> <span class="n">QPoint</span> <span \
class="o">&</span><span class="n">pos</span><span class="p">,</span> <span \
class="n">Time</span> <span class="n">time</span><span class="p">);</span></pre></td> \
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">143</font></th> <td bgcolor="#fdfebc" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">virtual</span> <span class="kt">void</span> <span \
class="nf">reserveElectricBorder</span><span class="p">(</span><span \
class="n">ElectricBorder</span> <span class="n">border</span><span class="p">,</span> \
<span class="n">Effect</span> <span class="o">*</span><span \
class="n">effect</span><span class="p">);</span></pre></td> </tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">F***
I ran into that yesterday and wanted to mention it.
You forgot to bump the ABI (leading to *very* ominous crash reports here ;-)</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">8faf62d8b42a782ea0b2efe33ef3915342ac3903</pre> <br />
<p>- Martin</p>
<br />
<p>On February 4th, 2013, 9:50 a.m. CET, Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for kwin, Plasma and Arthur Arlt.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Feb. 4, 2013, 9:50 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Screen Edges may belong to fullscreen windows
Corners are still ours (it's a valid use case to still be able to switch
window through e.g. Present Windows even when running a fullscreen app).
How is it done? An Edge can be blocked and does no longer trigger if it
is blocked. For WindowBasedEdges the edge windows get unmapped in the
blocking case and mapped again when the blocking condition is no longer
valid.
The blocking is so far connected to:
* changes of active window
* changes of fullscreen windows
Whenever one of the events occurs it is checked whether there is:
1. an active client
2. it is fullscreen
3. on the same screen as the edge
If this is the case the edge will be blocked, otherwise unblocked.
New Glow on approaching Screen Edge Effect
Feature stolen from hidden Plasma Panel now available for all edges on
approach by mouse.
Split out event handling in ScreenEdges in separate methods
Allows to also support xcb_generic_event_t in addition to XEvent.
Notifications when approaching a screen edge
For each edge an additional "approach" area window is created. When the
mouse enters this approach window, it gets unmapped and a mouse polling
interval is started. If the mouse leaves the approach area again, the
window gets mapped again and the mouse polling is stopped.
During the approaching a signal is emitted with a factor in [0.0,1.0] to
describe how close the mouse is to the edge. 0.0 means far away, 1.0
means triggering the edge. This signal is passed to the effects to allow
using this information. E.g. to provide a glow corner effect or to make
use of it in the cube animation effect to start the animation on desktop
switch.
Turn ScreenEdges into a Singleton
In fact it already used to be a Singleton as there is just one object
hold by the Singleton Workspace. So let's make it a proper Singleton
following our kind of standard approach of having a ::create factory
method called from Workspace ctor and a ::self to get to the singleton
instance.
Change the way how screen edges interact with Effects/Scripts
The main difference is that the activation of an edge is no longer
broadcasted to all effects and scripts, but instead a passed in slot of
the Effect/Script is invoked.
For this the EffectsHandler API is changed to take the Effect as an
argument to (un)reserveElectricBorder. As callback slot the existing
borderActivated is used.
In addition the ScreenEdge monitors the object for beeing destroyed and
unregisters the the edge automatically. This removes the need from the
Effect to call unregister in the dtor.
BUG: 309695
FIXED-IN: 4.11
Rewrite of KWin's Screen Edge Handling
This rewrite is mostly motivated by the need to handle multi screen
setups correctly. That is have edges per screen and not for the combined
geometry. Also porting from XLib to XCB has been a motivation for the
rewrite.
The design of the new ScreenEdge handling is described in the
documentation of ScreenEdges in screenedge.h.
In addition the following changes have been performed:
* move configuration from Options to ScreenEdge
* add screen edge information to Workspace::supportInformation (obviously
replaces what had been read from Options)
* have Workspace hold a pointer to ScreenEdges instead of an object
* forward declaration of ScreenEdges in workspaces.h, this explains the
seemingly unrelated changes of just another include in some files
BUG: 290887
FIXED-IN: 4.11
Do not update screen edges when compositing settings changes
The comment says it all: update all settings which can be done through
the compositing KCM. Years ago screen edges was in the composite KCM, but
it no longer is. So there is no need to update the edges when the
compositing settings changes.
Remove interaction of quick tiling with ScreenEdges
Quick tiling/maximizing of Clients is completely independent of the
screen edges functionality. That is it determines the borders itself.
Nevertheless there has been some code still around which interacted with
the screen edges each time a window was moved. This code is completely
useless.
Remove unused screen edge related methods from kwineffects interface
No effect has ever used these methods and there is no reason why an
effect should use them. Reserve/unreserve is sufficient as the effect
will be notified anyway.
Adding some convenient functions to wrap xcb calls
Commonly needed functionality, like
* move/resize windows
* restack windows
* restack to top of stack</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">* effects on various edges
* action on one edge
* activated/deactivated actions
* turned on off screen
* switch desktop (terrible feature)
* switch desktop when moving Clients
TODO: unit test for base functionality</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=271607">271607</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=290887">290887</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=309695">309695</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kwin/composite.cpp <span style="color: \
grey">(c27b37f21ec946775497047837c00dc7107c7756)</span></li>
<li>kwin/effects.h <span style="color: \
grey">(8d721fceef3e0a041e5a38088aef0de3a897d58d)</span></li>
<li>kwin/effects.cpp <span style="color: \
grey">(6845b748e46d0ffec42af58987dd255ff7a03bbb)</span></li>
<li>kwin/effects/CMakeLists.txt <span style="color: \
grey">(b8d458e6be6397330c10a7d4ab7a4fe8f1e52ffe)</span></li>
<li>kwin/effects/cube/cube.cpp <span style="color: \
grey">(3fffbd0577194c8c16bf608a04a2eeedd84b3ba1)</span></li>
<li>kwin/effects/desktopgrid/desktopgrid.cpp <span style="color: \
grey">(187d0588bf1e4a59ce442f7606c2da6e181a8f03)</span></li>
<li>kwin/effects/flipswitch/flipswitch.cpp <span style="color: \
grey">(445405edb93c39d9e776e6bb7ae40a233a6ab740)</span></li>
<li>kwin/effects/presentwindows/presentwindows.cpp <span style="color: \
grey">(de325df2451f47bca6effeeedefea960bc3ce089)</span></li>
<li>kwin/effects/screenedge/CMakeLists.txt <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>kwin/effects/screenedge/screenedgeeffect.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>kwin/effects/screenedge/screenedgeeffect.desktop <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>kwin/effects/screenedge/screenedgeeffect.h <span style="color: \
grey">(PRE-CREATION)</span></li>
<li>kwin/events.cpp <span style="color: \
grey">(a39aec07fc9f69438eb38d5569ba4ee0a218db42)</span></li>
<li>kwin/geometry.cpp <span style="color: \
grey">(7eb7e94d2804aa512b3da23b81c40ffe4b899811)</span></li>
<li>kwin/layers.cpp <span style="color: \
grey">(05b3d6dd0a6349a58eaf7a0b6a275b3ea6efde8b)</span></li>
<li>kwin/libkwineffects/kwineffects.h <span style="color: \
grey">(423cfd1140ff4a01fa3e06036383a9ce665b4da7)</span></li>
<li>kwin/options.h <span style="color: \
grey">(f458637b7f7f8210f6c9de3497ad1d49df8261c0)</span></li>
<li>kwin/options.cpp <span style="color: \
grey">(41dda403ce694c7820b8ee04f22f92b99559c1e2)</span></li>
<li>kwin/screenedge.h <span style="color: \
grey">(6dd1956332958d11e768c01eb3f91ae411a5cc77)</span></li>
<li>kwin/screenedge.cpp <span style="color: \
grey">(fb6209c1994345cf36da8dbcf097bb3596890cf3)</span></li>
<li>kwin/scripting/scriptedeffect.h <span style="color: \
grey">(830997b50b2f2d1eaf2aa828b94b110612b2080b)</span></li>
<li>kwin/scripting/scriptedeffect.cpp <span style="color: \
grey">(e9c05091087c8303da53d998d06dc46748b9fdce)</span></li>
<li>kwin/scripting/scripting.h <span style="color: \
grey">(077fe667dbf76e4165f10f0f19c8700e54e1350e)</span></li>
<li>kwin/scripting/scripting.cpp <span style="color: \
grey">(6c6e258656eb62bf5811984472f0d0776420b4d5)</span></li>
<li>kwin/scripting/scriptingutils.h <span style="color: \
grey">(3ed78458131a7fb535e38a08f3c6e7467e9e231b)</span></li>
<li>kwin/workspace.h <span style="color: \
grey">(b26725d0afdcba339aa79348e824bacaec13f32f)</span></li>
<li>kwin/workspace.cpp <span style="color: \
grey">(9183fd66dace05db6d2ad0b55c4c26e1a288ce95)</span></li>
<li>kwin/xcbutils.h <span style="color: \
grey">(31bd949b3404d62d8319f0acea521e90a1cfdbeb)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/108513/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic