[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 115902: Force backgroundcontrast during slidingpopup animations
From: Thomas_Lübking <thomas.luebking () gmail ! com>
Date: 2014-02-20 13:14:19
Message-ID: 20140220131419.1865.26147 () probe ! kde ! org
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115902/#review50363
-----------------------------------------------------------
kwin/effects/slidingpopups/slidingpopups.cpp
<https://git.reviewboard.kde.org/r/115902/#comment35427>
please prefer the alpha check
kwin/effects/slidingpopups/slidingpopups.cpp
<https://git.reviewboard.kde.org/r/115902/#comment35428>
i'll remind that this does not solve pot. races but relies on aligned runtimes.
if the hint can expected to be set by different effects, you might still run \
into: "start short, start long, short exits, flicker" -> the hint should really be \
additive (in a second patch)
- Thomas Lübking
On Feb. 20, 2014, 1:11 p.m., Sebastian Kügler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115902/
> -----------------------------------------------------------
>
> (Updated Feb. 20, 2014, 1:11 p.m.)
>
>
> Review request for kwin and Plasma.
>
>
> Repository: kde-workspace
>
>
> Description
> -------
>
> Force backgroundcontrast during slidingpopup animations
>
> This fixes the sliding popups losing their contrast effect when
> animating, less flicker.
>
> In this patch, we temporarily force the contrast effect on, but only if
> it hasn't been explicitely disabled. As soon as the animation stops, the
> force flag is disabled again. For disappearing windows, we just set the
> flag in the same way, but skip over the bookkeeping, since the window is
> going to be deleted, anyway.
>
>
> Diffs
> -----
>
> kwin/effects/slidingpopups/slidingpopups.h f66e42e
> kwin/effects/slidingpopups/slidingpopups.cpp 4f0a9ea
>
> Diff: https://git.reviewboard.kde.org/r/115902/diff/
>
>
> Testing
> -------
>
> Slowed down effects, made sure the contrast effect is applied correctly, made sure \
> the bookkeeping is OK. No visible side-effects observed.
>
> Thanks,
>
> Sebastian Kügler
>
>
[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="https://git.reviewboard.kde.org/r/115902/">https://git.reviewboard.kde.org/r/115902/</a>
</td>
</tr>
</table>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/115902/diff/4/?file=245031#file245031line263" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/effects/slidingpopups/slidingpopups.cpp</a> <span \
style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void \
SlidingPopupsEffect::postPaintWindow(EffectWindow* w)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">263</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k">if</span> <span class="p">(</span><span class="o">!</span><span \
class="n">w</span><span class="o">-></span><span class="n">data</span><span \
class="p">(</span><span class="n">WindowForceBackgroundContrastRole</span><span \
class="p">).</span><span class="n">isValid</span><span class="p">()</span> <span \
class="o">&&</span> <span class="n">w</span><span class="o">-></span><span \
class="n">hasAlpha</span><span class="p">())</span> <span \
class="p">{</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">please \
prefer the alpha check</pre> </div>
<br />
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="https://git.reviewboard.kde.org/r/115902/diff/4/?file=245031#file245031line264" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/effects/slidingpopups/slidingpopups.cpp</a> <span \
style="font-weight: normal;">
(Diff revision 4)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void \
SlidingPopupsEffect::postPaintWindow(EffectWindow* w)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">264</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="n">w</span><span class="o">-></span><span \
class="n">setData</span><span class="p">(</span><span \
class="n">WindowForceBackgroundContrastRole</span><span class="p">,</span> <span \
class="n">QVariant</span><span class="p">(</span><span class="nb">true</span><span \
class="p">));</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">i'll \
remind that this does not solve pot. races but relies on aligned runtimes.
if the hint can expected to be set by different effects, you might still run into:
"start short, start long, short exits, flicker" -> the hint should \
really be additive (in a second patch)</pre> </div>
<br />
<p>- Thomas Lübking</p>
<br />
<p>On February 20th, 2014, 1:11 p.m. UTC, Sebastian Kügler wrote:</p>
<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 and Plasma.</div>
<div>By Sebastian Kügler.</div>
<p style="color: grey;"><i>Updated Feb. 20, 2014, 1:11 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;">Force backgroundcontrast during slidingpopup animations
This fixes the sliding popups losing their contrast effect when
animating, less flicker.
In this patch, we temporarily force the contrast effect on, but only if
it hasn't been explicitely disabled. As soon as the animation stops, the
force flag is disabled again. For disappearing windows, we just set the
flag in the same way, but skip over the bookkeeping, since the window is
going to be deleted, anyway.</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;">Slowed down effects, made sure the contrast effect is applied correctly, \
made sure the bookkeeping is OK. No visible side-effects observed.</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/slidingpopups/slidingpopups.h <span style="color: \
grey">(f66e42e)</span></li>
<li>kwin/effects/slidingpopups/slidingpopups.cpp <span style="color: \
grey">(4f0a9ea)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115902/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