[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request: Abort color correction initialization and disable it in case of errors
From: "Casian Andrei" <skeletk13 () gmail ! com>
Date: 2013-01-05 8:50:23
Message-ID: 20130105085023.22397.92458 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On Jan. 4, 2013, 9:59 a.m., Martin Gräßlin wrote:
> > kwin/libkwineffects/kwinglcolorcorrection.cpp, line 623
> > <http://git.reviewboard.kde.org/r/107754/diff/4/?file=104379#file104379line623>
> >
> > note: that will return false in case there has been a gl error somewhere else \
> > before calling that method. I would at the start of the method clear the error \
> > stack.
>
> Martin Gräßlin wrote:
> latest patch is still showing this problem (in fact I saw it in multiple methods). \
> As you have marked it as resolved: forgot something in the patch you uploaded?
Oops, forgot the other places - will do the same in the other places
- Casian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107754/#review24644
-----------------------------------------------------------
On Jan. 5, 2013, 2:40 a.m., Casian Andrei wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107754/
> -----------------------------------------------------------
>
> (Updated Jan. 5, 2013, 2:40 a.m.)
>
>
> Review request for kwin, Thomas Lübking and Martin Gräßlin.
>
>
> Description
> -------
>
> Abort color correction initialization and disable it in case of errors
>
> Checks are now performed for GL errors and in case of errors everything
> is aborted. The error handling mechanism introduced for this purpose
> somewhat improves the color correction code.
>
> Tracked down and fixed a GL invalid operation when first attempting to set the \
> lookup texture uniform for color correction.
>
> Diffs
> -----
>
> kwin/libkwineffects/kwinglcolorcorrection.h ec080c0
> kwin/libkwineffects/kwinglcolorcorrection.cpp b5127e6
> kwin/libkwineffects/kwinglcolorcorrection_p.h f54aa49
> kwin/options.h f9f623f
> kwin/scene_opengl.cpp 147a248
>
> Diff: http://git.reviewboard.kde.org/r/107754/diff/
>
>
> Testing
> -------
>
> Tested when a GL invalid operation error occurs - color correction is disabled and \
> will not be enabled again because of the error. However, the check box in KCM \
> remains checked - but this is ok I think.
> Tested when no GL invalid operation is detected when doing color correction stuff. \
> Everything works fine from my point of view.
>
> Thanks,
>
> Casian Andrei
>
>
[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/107754/">http://git.reviewboard.kde.org/r/107754/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On January 4th, 2013, 9:59 a.m., <b>Martin \
Gräßlin</b> wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;">
<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="http://git.reviewboard.kde.org/r/107754/diff/4/?file=104379#file104379line623" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/libkwineffects/kwinglcolorcorrection.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; \
">QByteArray ColorCorrection::prepareFragmentShader(const QByteArray \
&sourceCode)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">580</font></th> <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">checkGLError</span><span class="p">(</span><span \
class="s">"deleteCCTextures"</span><span class="p">);</span></pre></td> \
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">622</font></th> <td bgcolor="#fdfebc" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="k"><span class="hl">return</span></span><span class="hl"> </span><span \
class="o"><span class="hl">!</span></span><span class="n">checkGLError</span><span \
class="p">(</span><span class="s">"deleteCCTextures"</span><span \
class="p">);</span></pre></td> </tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">note: that will return \
false in case there has been a gl error somewhere else before calling that method. I \
would at the start of the method clear the error stack.</pre> </blockquote>
<p>On January 5th, 2013, 6:04 a.m., <b>Martin Gräßlin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">latest patch is still \
showing this problem (in fact I saw it in multiple methods). As you have marked it as \
resolved: forgot something in the patch you uploaded?</pre> </blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Oops, \
forgot the other places - will do the same in the other places</pre> <br />
<p>- Casian</p>
<br />
<p>On January 5th, 2013, 2:40 a.m., Casian Andrei 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, Thomas Lübking and Martin Gräßlin.</div>
<div>By Casian Andrei.</div>
<p style="color: grey;"><i>Updated Jan. 5, 2013, 2:40 a.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;">Abort color correction initialization and disable it in case of errors
Checks are now performed for GL errors and in case of errors everything
is aborted. The error handling mechanism introduced for this purpose
somewhat improves the color correction code.
Tracked down and fixed a GL invalid operation when first attempting to set the lookup \
texture uniform for color correction.</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;">Tested when a GL invalid operation error occurs - color correction is \
disabled and will not be enabled again because of the error. However, the check box \
in KCM remains checked - but this is ok I think.
Tested when no GL invalid operation is detected when doing color correction stuff. \
Everything works fine from my point of view. </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/libkwineffects/kwinglcolorcorrection.h <span style="color: \
grey">(ec080c0)</span></li>
<li>kwin/libkwineffects/kwinglcolorcorrection.cpp <span style="color: \
grey">(b5127e6)</span></li>
<li>kwin/libkwineffects/kwinglcolorcorrection_p.h <span style="color: \
grey">(f54aa49)</span></li>
<li>kwin/options.h <span style="color: grey">(f9f623f)</span></li>
<li>kwin/scene_opengl.cpp <span style="color: grey">(147a248)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107754/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