'Re: Review Request 108513: Rewrite of KWin's Screen Edge Handling'

[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-01-22 13:24:06
Message-ID: 20130122132406.29211.49066 () vidsolbach ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108513/
-----------------------------------------------------------

(Updated Jan. 22, 2013, 2:24 p.m.)

Review request for kwin and Arthur Arlt.

Changes
-------

Change interaction of ScreenEdge -> Effect/Script. Now only the Effect/Script which \
has registered a given edge will be invoked.

Description (updated)
-------

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 290887 and 309695.
    http://bugs.kde.org/show_bug.cgi?id=290887
    http://bugs.kde.org/show_bug.cgi?id=309695

Diffs (updated)
-----

  kwin/composite.cpp af3cb37e39fd9a45fb61688bcf8231acc0f0deca 
  kwin/effects.h 8d721fceef3e0a041e5a38088aef0de3a897d58d 
  kwin/effects.cpp 56fddcd96a2211a47c301a94131ab1026f91a98f 
  kwin/effects/cube/cube.cpp 3fffbd0577194c8c16bf608a04a2eeedd84b3ba1 
  kwin/effects/desktopgrid/desktopgrid.cpp 741723191cbcd4bf0457fe3d962ca6bc2c520065 
  kwin/effects/flipswitch/flipswitch.cpp 445405edb93c39d9e776e6bb7ae40a233a6ab740 
  kwin/effects/presentwindows/presentwindows.cpp \
e22c1f328611d9be6a69054376d276a93ffe6c52   kwin/events.cpp \
a39aec07fc9f69438eb38d5569ba4ee0a218db42   kwin/geometry.cpp \
7eb7e94d2804aa512b3da23b81c40ffe4b899811   kwin/layers.cpp \
78f07eb3db800e418b50e7510545ea5b50aff5ad   kwin/libkwineffects/kwineffects.h \
2b8ede9e46d7b49f6dd2804c7979967e976281ad   kwin/options.h \
f458637b7f7f8210f6c9de3497ad1d49df8261c0   kwin/options.cpp \
0f8597cf62eaf4b06e4536666874c403f7a99168   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 \
8b432415a110b628fcd918d22709752cdc5ac1c5   kwin/xcbutils.h \
950fd59aff185821dfcfcfdf29b47f5683661887 

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 />

<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 and Arthur Arlt.</div>
<div>By Martin Gräßlin.</div>

<p style="color: grey;"><i>Updated Jan. 22, 2013, 2:24 p.m.</i></p>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</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;">Change interaction of ScreenEdge -&gt; Effect/Script. Now only the \
Effect/Script which has registered a given edge will be invoked.</pre>  </td>
 </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description  \
(updated)</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;">Change the way how screen edges interact with Effects/Scripts