[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request: Fix some issues with resizing panels
From: "Aaron Seigo" <aseigo () kde ! org>
Date: 2010-06-28 16:49:47
Message-ID: 20100628164947.17412.77775 () localhost
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4463/#review6314
-----------------------------------------------------------
Ship it!
looks good and the improvement is quite noticeable. great work. i'll commit=
and backport to the 4.5 branch shortly. and yes, please consider getting a=
n svn account: http://techbase.kde.org/Contribute/Get_a_SVN_Account
:)
- Aaron
On 2010-06-27 11:52:59, Anthony Bryant wrote:
> =
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4463/
> -----------------------------------------------------------
> =
> (Updated 2010-06-27 11:52:59)
> =
> =
> Review request for Plasma.
> =
> =
> Summary
> -------
> =
> Resizing a horizontal panel's height or a vertical panel's width currentl=
y has some problems, which make it look like the size is randomly adjusting=
while you drag it.
> The cause turns out to be a sequence of calls resulting in setContainment=
() being called on every resize:
> 1. Containment::resizeEvent() - from Qt
> 2. ContainmentPrivate::positionPanel()
> 3. Containment::setPos() - to Qt
> 4. Containment::geometryChanged() signal - from Qt
> 5. ViewPrivate::updateSceneRect() slot
> 6. View::sceneRectAboutToChange() signal
> 7. PanelView::pinchContainmentToCurrentScreen() slot
> 8. PanelView::pinchContainment()
> 9. PanelController::setContainment()
> Whenever setContainment() is called, some of the tool buttons in the cont=
roller are removed and re-added. Since this happens on every resize event, =
resizing the panel is very slow and jerky, and if local coordinates from th=
e mouse event are being compared it turns out in the wrong place.
> =
> To fix this, I have used global coordinates to find the new position of t=
he controller, and removed the call to setContainment() from pinchContainme=
nt() - it should only be called on initialization afaics.
> =
> Another issue this fixes is to always resize with something, even if the =
mouse is out of the allowed range. In this case, it bounds the new size to =
the allowed range, so that resizing past the limit works as expected.
> =
> =
> Diffs
> -----
> =
> /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelcontroller.h 114=
3346 =
> /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelcontroller.cpp 1=
143346 =
> /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelview.cpp 1143346 =
> =
> Diff: http://reviewboard.kde.org/r/4463/diff
> =
> =
> Testing
> -------
> =
> Tried moving and resizing both horizontal and vertical panels. I noticed =
some visible relayouting of the controller and repainting issues in the pan=
el, but they seem to have existed before this patch as well.
> =
> =
> Thanks,
> =
> Anthony
> =
>
[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://reviewboard.kde.org/r/4463/">http://reviewboard.kde.org/r/4463/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre>looks good and the improvement is quite noticeable. great work. i'll commit \
and backport to the 4.5 branch shortly. and yes, please consider getting an svn \
account: http://techbase.kde.org/Contribute/Get_a_SVN_Account
> )</pre>
<br />
<p>- Aaron</p>
<br />
<p>On June 27th, 2010, 11:52 a.m., Anthony Bryant wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://reviewboard.kde.orgrb/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 Plasma.</div>
<div>By Anthony Bryant.</div>
<p style="color: grey;"><i>Updated 2010-06-27 11:52:59</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;">Resizing a horizontal panel's height or a \
vertical panel's width currently has some problems, which make it look like the \
size is randomly adjusting while you drag it. The cause turns out to be a sequence of \
calls resulting in setContainment() being called on every resize: 1. \
Containment::resizeEvent() - from Qt 2. ContainmentPrivate::positionPanel()
3. Containment::setPos() - to Qt
4. Containment::geometryChanged() signal - from Qt
5. ViewPrivate::updateSceneRect() slot
6. View::sceneRectAboutToChange() signal
7. PanelView::pinchContainmentToCurrentScreen() slot
8. PanelView::pinchContainment()
9. PanelController::setContainment()
Whenever setContainment() is called, some of the tool buttons in the controller are \
removed and re-added. Since this happens on every resize event, resizing the panel is \
very slow and jerky, and if local coordinates from the mouse event are being compared \
it turns out in the wrong place.
To fix this, I have used global coordinates to find the new position of the \
controller, and removed the call to setContainment() from pinchContainment() - it \
should only be called on initialization afaics.
Another issue this fixes is to always resize with something, even if the mouse is out \
of the allowed range. In this case, it bounds the new size to the allowed range, so \
that resizing past the limit works as expected.</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;">Tried moving and resizing both horizontal and \
vertical panels. I noticed some visible relayouting of the controller and repainting \
issues in the panel, but they seem to have existed before this patch as well.</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>/trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelcontroller.h <span \
style="color: grey">(1143346)</span></li>
<li>/trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelcontroller.cpp <span \
style="color: grey">(1143346)</span></li>
<li>/trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelview.cpp <span \
style="color: grey">(1143346)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4463/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