[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request 109090: OpenGLPaintRedirector updates textures directly
From: Fredrik_Höglund <fredrik () kde ! org>
Date: 2013-02-28 4:50:43
Message-ID: 20130228045043.31822.90953 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On Feb. 27, 2013, 6:27 a.m., Fredrik Höglund wrote:
> > > GLES supports the UNPACK_FOO enums only since 3.0
> >
> > Not true; they've been added to GLES2 as GL_EXT_unpack_subimage.
> > BGRA textures are also supported with the GL_EXT_texture_format_BGRA8888 \
> > extension.
> > Both extensions are supported by Mesa.
> >
>
> Martin Gräßlin wrote:
> when did the GL_EXT_unpack_subimage got added to Mesa? for me
> static bool s_supportsUnpack = hasGLExtension("GL_EXT_unpack_subimage");
> still evolves to false
According to git log it was added on March 27 2012.
> On Feb. 27, 2013, 6:27 a.m., Fredrik Höglund wrote:
> > kwin/libkwineffects/kwingltexture.cpp, line 242
> > <http://git.reviewboard.kde.org/r/109090/diff/3/?file=115302#file115302line242>
> >
> > We should always do this copy on big endian. Otherwise we end up converting the \
> > whole source image below.
>
> Martin Gräßlin wrote:
> given the API doc of QImage that seems to be internally handled?
What I'm referring to is that convertToGLFormat() always swaps the bytes on big \
endian systems, so there's always a conversion.
Presumably this is because QImage uses packed pixel formats, while we pass the pixels \
to glTex(Sub)Image() as an array of unsigned bytes. If we specified the type as \
GL_UNSIGNED_INT_8_8_8_8_REV instead, we would match the QImage format regardless of \
the CPU architecture.
- Fredrik
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109090/#review28187
-----------------------------------------------------------
On Feb. 27, 2013, 7:54 a.m., Martin Gräßlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109090/
> -----------------------------------------------------------
>
> (Updated Feb. 27, 2013, 7:54 a.m.)
>
>
> Review request for kwin.
>
>
> Description
> -------
>
> OpenGLPaintRedirector updates textures directly
>
> Ownership of decoration textures is moved from SceneOpenGL::Window to
> OpenGLPaintRedirector. The PaintRedirector is responsible for updating
> the textures whenever they change. For this GLTexture is extended by an
> update(QImage, QPoint) method which uses glTexSubImage2D to update only
> the changed parts.
>
> The big advantage compared to before is that if e.g. only a button is
> animated only the button part is updated instead of the complete deco
> part.
>
>
> Diffs
> -----
>
> kwin/libkwineffects/kwingltexture.h 7a9df727701b78684225e1775fbe50d1502c3bdd
> kwin/libkwineffects/kwingltexture.cpp c1c3e9409f0a98ee2066be32bf6918dd89318e7a
> kwin/libkwineffects/kwingltexture_p.h 9675ec60acf96c487e1e8e2f3694db13ee70d02e
> kwin/paintredirector.h c5b3364456d01a0935deedcb3fa2b53d36c38341
> kwin/paintredirector.cpp c8892c8f1576058e9674fe3e2192c4f7fd30da9d
> kwin/scene_opengl.h 7971c8314f57907c6f275b30a3d6647a245df731
> kwin/scene_opengl.cpp 3185c9eca7784e69c9a27a03904dccd984c4a2e3
>
> Diff: http://git.reviewboard.kde.org/r/109090/diff/
>
>
> Testing
> -------
>
> OpenGL1: yes
> OpenGL2: yes
> GLES: yes
>
>
> Thanks,
>
> Martin Gräßlin
>
>
[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/109090/">http://git.reviewboard.kde.org/r/109090/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On February 27th, 2013, 6:27 a.m. UTC, <b>Fredrik \
Höglund</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;">> GLES supports the UNPACK_FOO enums only since 3.0
Not true; they've been added to GLES2 as GL_EXT_unpack_subimage.
BGRA textures are also supported with the GL_EXT_texture_format_BGRA8888 extension.
Both extensions are supported by Mesa.
</pre>
</blockquote>
<p>On February 27th, 2013, 7:15 a.m. UTC, <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;">when did the \
GL_EXT_unpack_subimage got added to Mesa? for me static bool s_supportsUnpack = \
hasGLExtension("GL_EXT_unpack_subimage"); still evolves to false</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">According to git log it \
was added on March 27 2012.</pre> <br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On February 27th, 2013, 6:27 a.m. UTC, <b>Fredrik \
Höglund</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/109090/diff/3/?file=115302#file115302line242" \
style="color: black; font-weight: bold; text-decoration: \
underline;">kwin/libkwineffects/kwingltexture.cpp</a> <span style="font-weight: \
normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<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">242</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="n">tmpImage</span> <span class="o">=</span> <span \
class="n">image</span><span class="p">.</span><span class="n">copy</span><span \
class="p">(</span><span class="n">src</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;">We should always do this \
copy on big endian. Otherwise we end up converting the whole source image \
below.</pre> </blockquote>
<p>On February 27th, 2013, 7:15 a.m. UTC, <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;">given the API doc of \
QImage that seems to be internally handled?</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;">What \
I'm referring to is that convertToGLFormat() always swaps the bytes on big endian \
systems, so there's always a conversion.
Presumably this is because QImage uses packed pixel formats, while we pass the pixels \
to glTex(Sub)Image() as an array of unsigned bytes. If we specified the type as \
GL_UNSIGNED_INT_8_8_8_8_REV instead, we would match the QImage format regardless of \
the CPU architecture. </pre>
<br />
<p>- Fredrik</p>
<br />
<p>On February 27th, 2013, 7:54 a.m. UTC, Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://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.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Feb. 27, 2013, 7:54 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;">OpenGLPaintRedirector updates textures directly
Ownership of decoration textures is moved from SceneOpenGL::Window to
OpenGLPaintRedirector. The PaintRedirector is responsible for updating
the textures whenever they change. For this GLTexture is extended by an
update(QImage, QPoint) method which uses glTexSubImage2D to update only
the changed parts.
The big advantage compared to before is that if e.g. only a button is
animated only the button part is updated instead of the complete deco
part.</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;">OpenGL1: yes
OpenGL2: yes
GLES: yes</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/kwingltexture.h <span style="color: \
grey">(7a9df727701b78684225e1775fbe50d1502c3bdd)</span></li>
<li>kwin/libkwineffects/kwingltexture.cpp <span style="color: \
grey">(c1c3e9409f0a98ee2066be32bf6918dd89318e7a)</span></li>
<li>kwin/libkwineffects/kwingltexture_p.h <span style="color: \
grey">(9675ec60acf96c487e1e8e2f3694db13ee70d02e)</span></li>
<li>kwin/paintredirector.h <span style="color: \
grey">(c5b3364456d01a0935deedcb3fa2b53d36c38341)</span></li>
<li>kwin/paintredirector.cpp <span style="color: \
grey">(c8892c8f1576058e9674fe3e2192c4f7fd30da9d)</span></li>
<li>kwin/scene_opengl.h <span style="color: \
grey">(7971c8314f57907c6f275b30a3d6647a245df731)</span></li>
<li>kwin/scene_opengl.cpp <span style="color: \
grey">(3185c9eca7784e69c9a27a03904dccd984c4a2e3)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/109090/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