[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwin
Subject:    Re: Review Request: Refactoring of Window/ScreenPaintData
From:       "Commit Hook" <null () kde ! org>
Date:       2012-07-19 16:29:52
Message-ID: 20120719162952.19659.83433 () 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/105141/#review16133
-----------------------------------------------------------


This review has been submitted with commit 8d6f6b3f3f73d67f955c1335a05a672d=
6cc994bf by Martin Gr=C3=A4=C3=9Flin to branch master.

- Commit Hook


On July 12, 2012, 9:08 p.m., Martin Gr=C3=A4=C3=9Flin wrote:
> =

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

> (Updated July 12, 2012, 9:08 p.m.)
> =

> =

> Review request for kwin.
> =

> =

> Description
> -------
> =

> Getter/setters for opacity, saturation and brightness in WindowPaintData
> =

> The public member variables for opacity, saturation and brightness
> are removed in favor for getter and setters. The variables are
> moved into a private class. Those are now qreal instead of double.
> =

> To make usage inside the effects easier a multiply method is added
> which multiplies the current value with passed in factor and returns
> the new value in a functional programming style.
> =

> Drop support for explicit contents opacity from WindowPaintData
> =

> No effect has ever made use of contents opacity. Which means it
> is not needed. Removing means faster effects as we used to
> multiply the value (always 1.0) with the opacity in each frame
> for each window.
> =

> Adding unit test for ScreenPaintData and WindowPaintData
> =

> =

> Split out common part of ScreenPaintData and WindowPaintData
> =

> New common d-pointered class PaintData is added which cannot
> be instantiated.
> =

> Replace translation by QVector3D in Screen/Window PaintData
> =

> =

> Use QGraphicsScale for scale information in ScreenPaintData
> =

> =

> Replace custom RotationData with QGraphicsRotation
> =

> =

> Diffs
> -----
> =

>   kwin/effects/blur/blur.cpp feba0361281e533b7c9672de5026ff79659af1f6 =

>   kwin/effects/boxswitch/boxswitch.cpp a24173613d20ea9a60ed3bb3d1926449e7=
562b29 =

>   kwin/effects/coverswitch/coverswitch.cpp ba79799b6986400f6548806548a594=
8482905ccc =

>   kwin/effects/cube/cube.cpp 43bdb8d47a284d9607313a3c02d582c7646446cb =

>   kwin/effects/cube/cubeslide.cpp be9ec69a6edb81d67e94d2dfbe27d0599d61b80=
8 =

>   kwin/effects/dashboard/dashboard.cpp f5832f4b9999418352b41fb88157839c09=
6d5c15 =

>   kwin/effects/desktopgrid/desktopgrid.cpp 7e0afc17f7394b80ec6904dff3c212=
f6a0644306 =

>   kwin/effects/dialogparent/dialogparent.cpp bf9e24b018f790142b57557c6179=
6c0599d5d868 =

>   kwin/effects/diminactive/diminactive.cpp e1f8f6eced3d4dbae66248e107aa49=
5d69fff1ff =

>   kwin/effects/dimscreen/dimscreen.cpp 6563f47368e170a91c2972c4f7ff499364=
025d09 =

>   kwin/effects/explosion/explosion.cpp 9d55b9356405feb497db23849ef8e52b68=
5d24b8 =

>   kwin/effects/flipswitch/flipswitch.cpp 5037f93a829eb502b597decb0e837a29=
d0ad142c =

>   kwin/effects/glide/glide.cpp 2dcdfc381ef17e6c1d321840f00dd7716d2e8474 =

>   kwin/effects/highlightwindow/highlightwindow.cpp 816526d59bbfe0c735a277=
7834cc098d59ad44fe =

>   kwin/effects/login/login.cpp cbbda1b2a379a3accd937cd4570870c29a00fe96 =

>   kwin/effects/logout/logout.cpp 1d68f2fc4494b29128dd8cba905327f8073c1b3d =

>   kwin/effects/minimizeanimation/minimizeanimation.cpp eb7f6f1d19a9cbb0dd=
1ea8c31d5275ac069de294 =

>   kwin/effects/presentwindows/presentwindows.cpp 4ffa3b06f96d7ecef096a9d8=
c7c3794e42b8f066 =

>   kwin/effects/resize/resize.cpp 62ed6e72c185ab13119c3b0a3ffc80c91f71e23e =

>   kwin/effects/scalein/scalein.cpp 17cf105e881c6dddf3e3482e73a90938b720af=
99 =

>   kwin/effects/screenshot/screenshot.cpp 943bddf2a810d6a439d37cd76731298b=
62f60140 =

>   kwin/effects/sheet/sheet.cpp 622403a8c5a67b86714a970e4acfc713e5515e83 =

>   kwin/effects/slide/slide.cpp dec7499e0a757b696f39f55b23575bffec82edd3 =

>   kwin/effects/slidingpopups/slidingpopups.cpp 137ff97fab52b178089525295f=
e47a756c2ff04f =

>   kwin/effects/taskbarthumbnail/taskbarthumbnail.cpp aff217bfd0c7a697d111=
fee6516eb50f0cfd3231 =

>   kwin/effects/thumbnailaside/thumbnailaside.cpp 10160193d9c568c9cfca80b7=
da52426fbfbe13b5 =

>   kwin/effects/translucency/translucency.cpp f176ff3b52a3d8c2c4250ab262d6=
0b771653b429 =

>   kwin/effects/zoom/zoom.cpp 1aec16176ddfb912fe3b4b1997889deb95a078c2 =

>   kwin/lanczosfilter.cpp 295e946e20dadde83d18d5cb67430ae31669a7f5 =

>   kwin/libkwineffects/kwinanimationeffect.cpp 363cab08157eecb5a5ee05f9241=
8cfbb848ff79f =

>   kwin/libkwineffects/kwineffects.h b952999d4770caec444d61b071e6c60b1b8f6=
58a =

>   kwin/libkwineffects/kwineffects.cpp 863cf9e8338d8cc718e609b5d3837a5d054=
74bf9 =

>   kwin/scene.cpp 0c5f6339a94f45f635eea697e70c5b167a465fef =

>   kwin/scene_opengl.cpp 6ca15ddb402a5c4a7c8f6997003164b002040bc3 =

>   kwin/scene_xrender.cpp 142d29f11d67e34d8bd02e1be46233d12d4d3572 =

>   kwin/tests/CMakeLists.txt 77555061ddd640beea4cb9e3f59fc43550b9b6b8 =

>   kwin/tests/test_screen_paint_data.cpp PRE-CREATION =

>   kwin/tests/test_window_paint_data.cpp PRE-CREATION =

> =

> Diff: http://git.reviewboard.kde.org/r/105141/diff/
> =

> =

> Testing
> -------
> =

> Base functionality ensured through unit tests.
> =

> CoverSwitch seems to have an error during start/stop animation. FlipSwitc=
h and Cube seem fine.
> =

> =

> Thanks,
> =

> Martin Gr=C3=A4=C3=9Flin
> =

>


[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/105141/">http://git.reviewboard.kde.org/r/105141/</a>
  </td>
    </tr>
   </table>
   <br />





 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This review has been \
submitted with commit 8d6f6b3f3f73d67f955c1335a05a672d6cc994bf by Martin Gräßlin to \
branch master.</pre>  <br />







<p>- Commit</p>


<br />
<p>On July 12th, 2012, 9:08 p.m., Martin Gräßlin wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.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 July 12, 2012, 9:08 p.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;">Getter/setters for opacity, saturation and brightness in WindowPaintData

The public member variables for opacity, saturation and brightness
are removed in favor for getter and setters. The variables are
moved into a private class. Those are now qreal instead of double.

To make usage inside the effects easier a multiply method is added
which multiplies the current value with passed in factor and returns
the new value in a functional programming style.

Drop support for explicit contents opacity from WindowPaintData

No effect has ever made use of contents opacity. Which means it
is not needed. Removing means faster effects as we used to
multiply the value (always 1.0) with the opacity in each frame
for each window.

Adding unit test for ScreenPaintData and WindowPaintData


Split out common part of ScreenPaintData and WindowPaintData

New common d-pointered class PaintData is added which cannot
be instantiated.

Replace translation by QVector3D in Screen/Window PaintData


Use QGraphicsScale for scale information in ScreenPaintData


Replace custom RotationData with QGraphicsRotation</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;">Base functionality ensured through unit tests.

CoverSwitch seems to have an error during start/stop animation. FlipSwitch and Cube \
seem fine.</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/blur/blur.cpp <span style="color: \
grey">(feba0361281e533b7c9672de5026ff79659af1f6)</span></li>

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

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

 <li>kwin/effects/cube/cube.cpp <span style="color: \
grey">(43bdb8d47a284d9607313a3c02d582c7646446cb)</span></li>

 <li>kwin/effects/cube/cubeslide.cpp <span style="color: \
grey">(be9ec69a6edb81d67e94d2dfbe27d0599d61b808)</span></li>

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

 <li>kwin/effects/desktopgrid/desktopgrid.cpp <span style="color: \
grey">(7e0afc17f7394b80ec6904dff3c212f6a0644306)</span></li>

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

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

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

 <li>kwin/effects/explosion/explosion.cpp <span style="color: \
grey">(9d55b9356405feb497db23849ef8e52b685d24b8)</span></li>

 <li>kwin/effects/flipswitch/flipswitch.cpp <span style="color: \
grey">(5037f93a829eb502b597decb0e837a29d0ad142c)</span></li>

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

 <li>kwin/effects/highlightwindow/highlightwindow.cpp <span style="color: \
grey">(816526d59bbfe0c735a2777834cc098d59ad44fe)</span></li>

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

 <li>kwin/effects/logout/logout.cpp <span style="color: \
grey">(1d68f2fc4494b29128dd8cba905327f8073c1b3d)</span></li>

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

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

 <li>kwin/effects/resize/resize.cpp <span style="color: \
grey">(62ed6e72c185ab13119c3b0a3ffc80c91f71e23e)</span></li>

 <li>kwin/effects/scalein/scalein.cpp <span style="color: \
grey">(17cf105e881c6dddf3e3482e73a90938b720af99)</span></li>

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

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

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

 <li>kwin/effects/slidingpopups/slidingpopups.cpp <span style="color: \
grey">(137ff97fab52b178089525295fe47a756c2ff04f)</span></li>

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

 <li>kwin/effects/thumbnailaside/thumbnailaside.cpp <span style="color: \
grey">(10160193d9c568c9cfca80b7da52426fbfbe13b5)</span></li>

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

 <li>kwin/effects/zoom/zoom.cpp <span style="color: \
grey">(1aec16176ddfb912fe3b4b1997889deb95a078c2)</span></li>

 <li>kwin/lanczosfilter.cpp <span style="color: \
grey">(295e946e20dadde83d18d5cb67430ae31669a7f5)</span></li>

 <li>kwin/libkwineffects/kwinanimationeffect.cpp <span style="color: \
grey">(363cab08157eecb5a5ee05f92418cfbb848ff79f)</span></li>

 <li>kwin/libkwineffects/kwineffects.h <span style="color: \
grey">(b952999d4770caec444d61b071e6c60b1b8f658a)</span></li>

 <li>kwin/libkwineffects/kwineffects.cpp <span style="color: \
grey">(863cf9e8338d8cc718e609b5d3837a5d05474bf9)</span></li>

 <li>kwin/scene.cpp <span style="color: \
grey">(0c5f6339a94f45f635eea697e70c5b167a465fef)</span></li>

 <li>kwin/scene_opengl.cpp <span style="color: \
grey">(6ca15ddb402a5c4a7c8f6997003164b002040bc3)</span></li>

 <li>kwin/scene_xrender.cpp <span style="color: \
grey">(142d29f11d67e34d8bd02e1be46233d12d4d3572)</span></li>

 <li>kwin/tests/CMakeLists.txt <span style="color: \
grey">(77555061ddd640beea4cb9e3f59fc43550b9b6b8)</span></li>

 <li>kwin/tests/test_screen_paint_data.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>kwin/tests/test_window_paint_data.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/105141/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