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

List:       kde-panel-devel
Subject:    Re: Review Request: Remove Drawbound Move/Resize Functionality
From:       "Commit Hook" <null () kde ! org>
Date:       2011-06-23 10:21:16
Message-ID: 20110623102116.4973.52790 () 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/101411/#review4093
-----------------------------------------------------------


This review has been submitted with commit dd55fc8a92cbaeea0427d9e4665f4d15=
ab0f325d by Martin Gr=C3=A4=C3=9Flin to branch master.

- Commit


On May 21, 2011, 7:30 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/101411/
> -----------------------------------------------------------
> =

> (Updated May 21, 2011, 7:30 p.m.)
> =

> =

> Review request for kwin and Plasma.
> =

> =

> Summary
> -------
> =

> This patch removes the drawbound functionality for moving/resizing window=
s. Drawbound was/is a feature to only render an outline while resizing/movi=
ng the window. It was a solution for the case that it is too slow to let th=
e client update all the time and the geometry change is moved to the end of=
 the operation.
> =

> With compositing we have for quite some time already the resize effect to=
 replace the drawbound (given that the outline is extremly ugly and IIRC al=
so broken for compositing), so it was only used when using no compositing b=
ut makes the code rather complex and did rather ugly things (grabbing X ser=
ver, using X drawing functionality, requiring an event filter). So I think =
it is justified to remove this functionality for legacy system. Any objecti=
ons?
> =

> This also removes the possibility for decorations to take over drawbound.=
 Given lxr no decoration is actually using it  (b2 implements the method bu=
t the code is ifdefed). I marked the method as deprecated and adjusted the =
documentation. @Thomas: do you use it in Bespin?
> =

> I also removed the options from the KCM. We might consider adding a new o=
ption to control the resize effect (though I don't want to touch that code =
as it doesn't use ui files). I could not remove the rules rows from the rul=
es KCM as my qt-designer currently crashes while starting :-(
> =

> Of course this is for when master is open to feature requests.
> =

> =

> Adding Plasma as it removes functionality for legacy systems and I want a=
 complete workspace decision.
> =

> =

> Diffs
> -----
> =

>   kwin/activation.cpp b43f6ab =

>   kwin/client.h 3b42c29 =

>   kwin/client.cpp f4988ff =

>   kwin/geometry.cpp 499db65 =

>   kwin/kcmkwin/kwinoptions/windows.h dd2b619 =

>   kwin/kcmkwin/kwinoptions/windows.cpp 10b7579 =

>   kwin/kcmkwin/kwinrules/ruleswidget.h 6972e52 =

>   kwin/kcmkwin/kwinrules/ruleswidget.cpp 300191e =

>   kwin/libkdecorations/kdecoration.h 12612bc =

>   kwin/options.h 0ef0b92 =

>   kwin/options.cpp a4cee3c =

>   kwin/rules.h 84a8df1 =

>   kwin/rules.cpp 1587064 =

>   kwin/useractions.cpp d969def =

>   kwin/workspace.h 837e557 =

> =

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

> =

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

> Resizing/Moving still working with both compositing and no-compositing.
> =

> =

> Thanks,
> =

> Martin
> =

>


[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/101411/">http://git.reviewboard.kde.org/r/101411/</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 dd55fc8a92cbaeea0427d9e4665f4d15ab0f325d by Martin Gräßlin to \
branch master.</pre>  <br />







<p>- Commit</p>


<br />
<p>On May 21st, 2011, 7:30 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 and Plasma.</div>
<div>By Martin Gräßlin.</div>


<p style="color: grey;"><i>Updated May 21, 2011, 7:30 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;">This patch removes the drawbound functionality for moving/resizing \
windows. Drawbound was/is a feature to only render an outline while resizing/moving \
the window. It was a solution for the case that it is too slow to let the client \
update all the time and the geometry change is moved to the end of the operation.

With compositing we have for quite some time already the resize effect to replace the \
drawbound (given that the outline is extremly ugly and IIRC also broken for \
compositing), so it was only used when using no compositing but makes the code rather \
complex and did rather ugly things (grabbing X server, using X drawing functionality, \
requiring an event filter). So I think it is justified to remove this functionality \
for legacy system. Any objections?

This also removes the possibility for decorations to take over drawbound. Given lxr \
no decoration is actually using it  (b2 implements the method but the code is \
ifdefed). I marked the method as deprecated and adjusted the documentation. @Thomas: \
do you use it in Bespin?

I also removed the options from the KCM. We might consider adding a new option to \
control the resize effect (though I don&#39;t want to touch that code as it \
doesn&#39;t use ui files). I could not remove the rules rows from the rules KCM as my \
qt-designer currently crashes while starting :-(

Of course this is for when master is open to feature requests.


Adding Plasma as it removes functionality for legacy systems and I want a complete \
workspace decision.</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;">Resizing/Moving still working with both compositing and \
no-compositing.</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/activation.cpp <span style="color: grey">(b43f6ab)</span></li>

 <li>kwin/client.h <span style="color: grey">(3b42c29)</span></li>

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

 <li>kwin/geometry.cpp <span style="color: grey">(499db65)</span></li>

 <li>kwin/kcmkwin/kwinoptions/windows.h <span style="color: \
grey">(dd2b619)</span></li>

 <li>kwin/kcmkwin/kwinoptions/windows.cpp <span style="color: \
grey">(10b7579)</span></li>

 <li>kwin/kcmkwin/kwinrules/ruleswidget.h <span style="color: \
grey">(6972e52)</span></li>

 <li>kwin/kcmkwin/kwinrules/ruleswidget.cpp <span style="color: \
grey">(300191e)</span></li>

 <li>kwin/libkdecorations/kdecoration.h <span style="color: \
grey">(12612bc)</span></li>

 <li>kwin/options.h <span style="color: grey">(0ef0b92)</span></li>

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

 <li>kwin/rules.h <span style="color: grey">(84a8df1)</span></li>

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

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

 <li>kwin/workspace.h <span style="color: grey">(837e557)</span></li>

</ul>

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




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








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



_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


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

Configure | About | News | Add a list | Sponsored by KoreLogic