From kwin  Fri Jan 24 13:17:26 2014
From: =?utf-8?q?Martin_Gr=C3=A4=C3=9Flin?= <mgraesslin () kde ! org>
Date: Fri, 24 Jan 2014 13:17:26 +0000
To: kwin
Subject: Re: Review Request 115073: Turn built-in effects into a library kwin links against
Message-Id: <20140124131726.10270.41830 () probe ! kde ! org>
X-MARC-Message: https://marc.info/?l=kwin&m=139056946527964
MIME-Version: 1
Content-Type: multipart/mixed; boundary="--===============0080333791330130879=="

--===============0080333791330130879==
Content-Type: multipart/alternative;
 boundary="===============7170051323754292578=="

--===============7170051323754292578==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit


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

(Updated Jan. 24, 2014, 1:17 p.m.)


Status
------

This change has been marked as submitted.


Review request for kwin.


Repository: kde-workspace


Description
-------

As all effects have always been compiled into the same .so file it's
questionable whether resolving the effects through a library is useful
at all. By linking against the built-in effects we gain the following
advantages:
* don't have to load/unload the KLibrary
* don't have to resolve the create, supported and enabled functions
* no version check required
* no dependency resolving (effects don't use it)
* remove the KWIN_EFFECT macros from the effects

All the effects are now registered in an effects_builtins file which
maps the name to a factory method and supported or enabled by default
methods.

During loading the effects we first check whether there is a built-in
effect by the given name and make a shortcut to create it through that.
If that's not possible the normal plugin loading is used.

Completely unscientific testing [1] showed an improvement of almost 10
msec during loading all the effects I use.

[1] QElapsedTimer around the loading code, start kwin five times, take
average.


Diffs
-----

  kwin/effects/trackmouse/trackmouse.cpp 21ec1db 
  kwin/effects/windowgeometry/windowgeometry.cpp 3c20bb2 
  kwin/effects/wobblywindows/wobblywindows.cpp 271f7ba 
  kwin/effects/zoom/zoom.cpp 97848ae 
  kwin/effects/mouseclick/mouseclick.cpp 22b28e7 
  kwin/effects/mousemark/mousemark.cpp 88cee2f 
  kwin/effects/presentwindows/presentwindows.cpp a0120b5 
  kwin/effects/resize/resize.cpp 86b106f 
  kwin/effects/screenedge/screenedgeeffect.cpp 7661ede 
  kwin/effects/screenshot/screenshot.cpp be3c9f0 
  kwin/effects/sheet/sheet.cpp ee5eddf 
  kwin/effects/showfps/showfps.cpp 1324165 
  kwin/effects/showpaint/showpaint.cpp 5a7a044 
  kwin/effects/slide/slide.cpp 7ed5408 
  kwin/effects/slideback/slideback.cpp f280e03 
  kwin/effects/slidingpopups/slidingpopups.cpp 823023e 
  kwin/effects/snaphelper/snaphelper.cpp a8cd1a9 
  kwin/effects/startupfeedback/startupfeedback.cpp d700138 
  kwin/effects/taskbarthumbnail/taskbarthumbnail.cpp 0321d6a 
  kwin/effects/thumbnailaside/thumbnailaside.cpp 6a753a4 
  kwin/CMakeLists.txt ad92a79 
  kwin/effects.h 7ccde39 
  kwin/effects.cpp ec1a310 
  kwin/effects/CMakeLists.txt d1e2b0d 
  kwin/effects/blur/blur.cpp 9b40281 
  kwin/effects/coverswitch/coverswitch.cpp 0e2d581 
  kwin/effects/cube/cube.cpp 8b01070 
  kwin/effects/dashboard/dashboard.cpp 6e11d6d 
  kwin/effects/desktopgrid/desktopgrid.cpp 6455c38 
  kwin/effects/diminactive/diminactive.cpp 6a4a287 
  kwin/effects/dimscreen/dimscreen.cpp 8155449 
  kwin/effects/effect_builtins.h PRE-CREATION 
  kwin/effects/effect_builtins.cpp PRE-CREATION 
  kwin/effects/fallapart/fallapart.cpp ace7868 
  kwin/effects/flipswitch/flipswitch.cpp 008fd55 
  kwin/effects/glide/glide.cpp b133858 
  kwin/effects/highlightwindow/highlightwindow.cpp 647b47b 
  kwin/effects/invert/invert.cpp 03ce15d8 
  kwin/effects/kscreen/kscreen.cpp 5747425 
  kwin/effects/logout/logout.cpp 5e19469 
  kwin/effects/lookingglass/lookingglass.cpp 31edd1d 
  kwin/effects/magiclamp/magiclamp.cpp 8595168 
  kwin/effects/magnifier/magnifier.cpp 1159134 
  kwin/effects/minimizeanimation/minimizeanimation.cpp 6d4f388 

Diff: https://git.reviewboard.kde.org/r/115073/diff/


Testing
-------


Thanks,

Martin Gräßlin


--===============7170051323754292578==
Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit




<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="https://git.reviewboard.kde.org/r/115073/">https://git.reviewboard.kde.org/r/115073/</a>
     </td>
    </tr>
   </table>
   <br />



<table bgcolor="#e0e0e0" width="100%" cellpadding="8" style="border: 1px gray solid;">
 <tr>
  <td>
   <h1 style="margin-right: 0.2em; padding: 0; font-size: 10pt;">This change has been marked as submitted.</h1>
  </td>
 </tr>
</table>
<br />


<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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.</div>
<div>By Martin Gräßlin.</div>


<p style="color: grey;"><i>Updated Jan. 24, 2014, 1:17 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kde-workspace
</div>


<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;">As all effects have always been compiled into the same .so file it&#39;s
questionable whether resolving the effects through a library is useful
at all. By linking against the built-in effects we gain the following
advantages:
* don&#39;t have to load/unload the KLibrary
* don&#39;t have to resolve the create, supported and enabled functions
* no version check required
* no dependency resolving (effects don&#39;t use it)
* remove the KWIN_EFFECT macros from the effects

All the effects are now registered in an effects_builtins file which
maps the name to a factory method and supported or enabled by default
methods.

During loading the effects we first check whether there is a built-in
effect by the given name and make a shortcut to create it through that.
If that&#39;s not possible the normal plugin loading is used.

Completely unscientific testing [1] showed an improvement of almost 10
msec during loading all the effects I use.

[1] QElapsedTimer around the loading code, start kwin five times, take
average.</pre>
  </td>
 </tr>
</table>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>kwin/effects/trackmouse/trackmouse.cpp <span style="color: grey">(21ec1db)</span></li>

 <li>kwin/effects/windowgeometry/windowgeometry.cpp <span style="color: grey">(3c20bb2)</span></li>

 <li>kwin/effects/wobblywindows/wobblywindows.cpp <span style="color: grey">(271f7ba)</span></li>

 <li>kwin/effects/zoom/zoom.cpp <span style="color: grey">(97848ae)</span></li>

 <li>kwin/effects/mouseclick/mouseclick.cpp <span style="color: grey">(22b28e7)</span></li>

 <li>kwin/effects/mousemark/mousemark.cpp <span style="color: grey">(88cee2f)</span></li>

 <li>kwin/effects/presentwindows/presentwindows.cpp <span style="color: grey">(a0120b5)</span></li>

 <li>kwin/effects/resize/resize.cpp <span style="color: grey">(86b106f)</span></li>

 <li>kwin/effects/screenedge/screenedgeeffect.cpp <span style="color: grey">(7661ede)</span></li>

 <li>kwin/effects/screenshot/screenshot.cpp <span style="color: grey">(be3c9f0)</span></li>

 <li>kwin/effects/sheet/sheet.cpp <span style="color: grey">(ee5eddf)</span></li>

 <li>kwin/effects/showfps/showfps.cpp <span style="color: grey">(1324165)</span></li>

 <li>kwin/effects/showpaint/showpaint.cpp <span style="color: grey">(5a7a044)</span></li>

 <li>kwin/effects/slide/slide.cpp <span style="color: grey">(7ed5408)</span></li>

 <li>kwin/effects/slideback/slideback.cpp <span style="color: grey">(f280e03)</span></li>

 <li>kwin/effects/slidingpopups/slidingpopups.cpp <span style="color: grey">(823023e)</span></li>

 <li>kwin/effects/snaphelper/snaphelper.cpp <span style="color: grey">(a8cd1a9)</span></li>

 <li>kwin/effects/startupfeedback/startupfeedback.cpp <span style="color: grey">(d700138)</span></li>

 <li>kwin/effects/taskbarthumbnail/taskbarthumbnail.cpp <span style="color: grey">(0321d6a)</span></li>

 <li>kwin/effects/thumbnailaside/thumbnailaside.cpp <span style="color: grey">(6a753a4)</span></li>

 <li>kwin/CMakeLists.txt <span style="color: grey">(ad92a79)</span></li>

 <li>kwin/effects.h <span style="color: grey">(7ccde39)</span></li>

 <li>kwin/effects.cpp <span style="color: grey">(ec1a310)</span></li>

 <li>kwin/effects/CMakeLists.txt <span style="color: grey">(d1e2b0d)</span></li>

 <li>kwin/effects/blur/blur.cpp <span style="color: grey">(9b40281)</span></li>

 <li>kwin/effects/coverswitch/coverswitch.cpp <span style="color: grey">(0e2d581)</span></li>

 <li>kwin/effects/cube/cube.cpp <span style="color: grey">(8b01070)</span></li>

 <li>kwin/effects/dashboard/dashboard.cpp <span style="color: grey">(6e11d6d)</span></li>

 <li>kwin/effects/desktopgrid/desktopgrid.cpp <span style="color: grey">(6455c38)</span></li>

 <li>kwin/effects/diminactive/diminactive.cpp <span style="color: grey">(6a4a287)</span></li>

 <li>kwin/effects/dimscreen/dimscreen.cpp <span style="color: grey">(8155449)</span></li>

 <li>kwin/effects/effect_builtins.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>kwin/effects/effect_builtins.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>kwin/effects/fallapart/fallapart.cpp <span style="color: grey">(ace7868)</span></li>

 <li>kwin/effects/flipswitch/flipswitch.cpp <span style="color: grey">(008fd55)</span></li>

 <li>kwin/effects/glide/glide.cpp <span style="color: grey">(b133858)</span></li>

 <li>kwin/effects/highlightwindow/highlightwindow.cpp <span style="color: grey">(647b47b)</span></li>

 <li>kwin/effects/invert/invert.cpp <span style="color: grey">(03ce15d8)</span></li>

 <li>kwin/effects/kscreen/kscreen.cpp <span style="color: grey">(5747425)</span></li>

 <li>kwin/effects/logout/logout.cpp <span style="color: grey">(5e19469)</span></li>

 <li>kwin/effects/lookingglass/lookingglass.cpp <span style="color: grey">(31edd1d)</span></li>

 <li>kwin/effects/magiclamp/magiclamp.cpp <span style="color: grey">(8595168)</span></li>

 <li>kwin/effects/magnifier/magnifier.cpp <span style="color: grey">(1159134)</span></li>

 <li>kwin/effects/minimizeanimation/minimizeanimation.cpp <span style="color: grey">(6d4f388)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/115073/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>




  </div>
 </body>
</html>


--===============7170051323754292578==--

--===============0080333791330130879==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
kwin mailing list
kwin@kde.org
https://mail.kde.org/mailman/listinfo/kwin

--===============0080333791330130879==--