<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/124697/">https://git.reviewboard.kde.org/r/124697/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 11th, 2015, 6:45 p.m. CEST, <b>Thomas Lübking</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="https://git.reviewboard.kde.org/r/124697/diff/1/?file=393340#file393340line519" style="color: black; font-weight: bold; text-decoration: underline;">abstract_client.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">519</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">AbstractClient</span><span class="o">::</span><span class="n">setupWindowManagementInterface</span><span class="p">()</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">517</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">AbstractClient</span><span class="o">::</span><span class="n">setupWindowManagementInterface</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Why are these functions in AbstractClient itfp.? Looks like shell-client-only feature.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">No, we also need to expose the X11 Clients to the interface. The idea is that we provide all "managed windows" to Plasma's Wayland Taskmanager, so that Plasma doesn't need to interact with X11 when on Wayland.</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 11th, 2015, 6:45 p.m. CEST, <b>Thomas Lübking</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="https://git.reviewboard.kde.org/r/124697/diff/1/?file=393341#file393341line95" style="color: black; font-weight: bold; text-decoration: underline;">abstract_egl_backend.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">protected:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">95</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">bool</span> <span class="nf">loadShmTexture</span><span class="p">(</span><span class="k">const</span> <span class="n">QPointer</span><span class="o"><</span><span class="n">KWayland</span><span class="o">::</span><span class="n">Server</span><span class="o">::</span><span class="n">BufferInterface</span><span class="o">></span> <span class="o">&</span><span class="n">buffer</span><span class="p">);</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">94</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">bool</span> <span class="nf">loadShmTexture</span><span class="p">(</span><span class="k">const</span> <span class="n">QPointer</span><span class="o"><</span><span class="n">KWayland</span><span class="o">::</span><span class="n">Server</span><span class="o">::</span><span class="n">BufferInterface</span><span class="o">></span> <span class="o">&</span><span class="n">buffer</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">same here?
(also applies to all inner special code - why abstracts if they contain specific code already)</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">That one will become more obvious once we are able to manage Wayland clients on X11 ;-) - this was kind of the idea I had with moving it in the abstract interface: it's able to handle both X11 and Wayland applications, so that we can completely mix.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">(Ideally I would also like the GLX backend to be able to handle Wayland clients, but unfortunately that will be impossible :-( ).</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On August 11th, 2015, 6:45 p.m. CEST, <b>Thomas Lübking</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="https://git.reviewboard.kde.org/r/124697/diff/1/?file=393342#file393342line64" style="color: black; font-weight: bold; text-decoration: underline;">abstract_egl_backend.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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; ">eglQueryWaylandBufferWL_func eglQueryWaylandBufferWL = nullptr;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">64</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">eglUnbindWaylandDisplayWL</span> <span class="o">&&</span> <span class="n">eglDisplay</span><span class="p">()</span> <span class="o">!=</span> <span class="n">EGL_NO_DISPLAY</span><span class="p">)</span> <span class="p">{</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">59</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">eglUnbindWaylandDisplayWL</span> <span class="o">&&</span> <span class="n">eglDisplay</span><span class="p">()</span> <span class="o">!=</span> <span class="n">EGL_NO_DISPLAY</span><span class="p">)</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">eg. cleanup should likely (? doesn't sound that performance critical) be virtual and EglWaylandBackend calls the wayland specific code and then AbstractEglBackend::cleanup()</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm sure there's a reason for this, but it looks a bit like bad design and hacked in :-\</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">same as above - it should in theory also support Wayland applications on X11. Also splitting it out would mean to have it in each of the backend implementations and even more weird in the EglOnXBackend (as that's what is used in the nested Wayland compositor on X11).</p></pre>
<br />
<p>- Martin</p>
<br />
<p>On August 11th, 2015, 2:01 p.m. CEST, Martin Gräßlin wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for kwin and Plasma.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Aug. 11, 2015, 2:01 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwin
</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;">As discussed on release-team ml [1] the following dependencies are
mandatory:
* KF5Wayland
* Wayland::Cursor
* Wayland::Egl
* xkbcommon
[1] https://mail.kde.org/pipermail/release-team/2015-July/008725.html
Drop cmakedefine HAVE_XKB
No longer needed, we always depend on xkbcommon now.
Drop cmakedefine HAVE_WAYLAND_CURSOR
Now a required build-dep.
Drop cmakedefine HAVE_WAYLAND
Now a required build dependency.
Drop cmakedefine HAVE_WAYLAND_EGL
Now a required build dependency.
Make X11_XCB a build dependency of X11 windowed backend
Let's rather not build the plugin if we don't have the dependency
then building it without OpenGL support. Simplifies the code a bit
and makes the backend overall more useful and goes along with e.g.
the Wayland one which has EGL also as a hard dependency for the
plugin.</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>CMakeLists.txt <span style="color: grey">(056801feaa40b8bc24e56b5d132a3e02e66b6782)</span></li>
<li>abstract_backend.cpp <span style="color: grey">(28564e3e03a68f78c58d351b0a2eb1f66a088879)</span></li>
<li>abstract_client.cpp <span style="color: grey">(45976224394a8a467836bbe72596057446c95051)</span></li>
<li>abstract_egl_backend.h <span style="color: grey">(c266521d3a870b4bd110435fa6c5f8e5d4e72e33)</span></li>
<li>abstract_egl_backend.cpp <span style="color: grey">(5ef3adaa7216c314b56c594167805da232f6a499)</span></li>
<li>backends/CMakeLists.txt <span style="color: grey">(809ac64edbce6df001c22395a9a991d324008d92)</span></li>
<li>backends/wayland/CMakeLists.txt <span style="color: grey">(7f643df2e13bc40172ea3ed5140b9aea5563b0b6)</span></li>
<li>backends/wayland/wayland_backend.cpp <span style="color: grey">(57a805eafa470e1fb06cd318bf557c0cd5c2e38c)</span></li>
<li>backends/x11/CMakeLists.txt <span style="color: grey">(9ce72fad1a7800f7e9b0c085a363f8eb256b7d24)</span></li>
<li>backends/x11/x11windowed_backend.cpp <span style="color: grey">(95a5b70ecfa32358af934950f2c723cd9b8a03af)</span></li>
<li>composite.cpp <span style="color: grey">(1db4790b50d99401c175e90852f5e4958f53fc8c)</span></li>
<li>config-kwin.h.cmake <span style="color: grey">(6128c0ccae23453e2f5cf2918dee0d733aaec4d9)</span></li>
<li>effects.cpp <span style="color: grey">(81c6ede571f6f995eee62e999633bcbc07599914)</span></li>
<li>events.cpp <span style="color: grey">(3ce3f917c9a7854613244dd36cf0dc27e908d559)</span></li>
<li>geometry.cpp <span style="color: grey">(f63e85165b9e6c605e5ed740bb61f1ec02acecc7)</span></li>
<li>input.h <span style="color: grey">(c9ef924737b542b9d844fcb8e5b4989e02bf812d)</span></li>
<li>input.cpp <span style="color: grey">(3e464486aa56a7edaa36371747b6c158a03c96c2)</span></li>
<li>layers.cpp <span style="color: grey">(d8328ccafd89706eb463f5dd120b82b7288c86bb)</span></li>
<li>scene.h <span style="color: grey">(9e41cccc2da1d0a9c54adf9d3f412aeb1eece0b3)</span></li>
<li>scene.cpp <span style="color: grey">(09b7ec4a3d5a0af3b976657f9b035a5bfecdc8f3)</span></li>
<li>scene_opengl.cpp <span style="color: grey">(3e9b849ea7e6884386944188d9d6f4d38d74090f)</span></li>
<li>scene_qpainter.cpp <span style="color: grey">(e6829138f00f4529bf13b3733f34b0e694056e9a)</span></li>
<li>screens.cpp <span style="color: grey">(226a2eca05d386b7f8b778b21019dc2d976c1197)</span></li>
<li>scripting/scripting_model.cpp <span style="color: grey">(8b595f7c2ba3132bb574c48bae6d4823d9bc0366)</span></li>
<li>shadow.cpp <span style="color: grey">(56bc97f91c204ec14d8eb8dc6ac5a4ac253e3436)</span></li>
<li>thumbnailitem.cpp <span style="color: grey">(8b984558720ea974afb91aa55269ae514b44479f)</span></li>
<li>toplevel.h <span style="color: grey">(eb46eb4f7571fbde8034308903cd730af2b17854)</span></li>
<li>toplevel.cpp <span style="color: grey">(4740c8f873439dd6ce08d99de7ee8028ec52ebfe)</span></li>
<li>workspace.cpp <span style="color: grey">(9568b83b04112c28cd99ec60d472d5944a9d7f1b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124697/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>