<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/101979/">http://git.reviewboard.kde.org/r/101979/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Not sure I would make KWIN_KDEINIT_BACKEND() a macro.
It looks like you could get the same also without a macro (the name KWIN_KDEINIT_BACKEND() is not that self-explaining that you immediately know what it does, in the first moment there is just the add_executable()/add_library() call not there).
You could do something like
if(FOO)
set(kwinLibs ${kwinLibs} ${FOO_LIBRARIES})
endif()
...
kde4_add_executable(kwin ...)
target_link_libraries(kwin ${kwinLibs} )
...
kde4_add_executable(kwin_gles ...)
target_link_libraries(kwin_gles ${kwinLibs} )
This would make the file less "custom" looking, so probably easier approachable for outsiders/newbies.
The KWIN4_GLUTILS_BACKEND(), well, as for proper naming things, macros/functions should have a verb, so I'd make it KWIN4_ADD_GLUTILS_BACKEND() as you already do for the other macro you have.
Alex</pre>
<br />
<p>- Alexander</p>
<br />
<p>On July 17th, 2011, 6 p.m., Martin Gräßlin 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, Plasma and Alexander Neundorf.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated July 17, 2011, 6 p.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;">Major build system change: everything is compiled twice, once with OpenGL (if present) and once with OpenGL ES (if present). To better support this kwinglutils becomes an own library and is splitted out of the kwineffects library. It is build once as kwinglutils and once as kwinglesutils.
The effects are built twice into a kwin4_effect_builtins and kwin4_effect_gles_builtin. And last but not least we build a kwin and a kwin_gles.
The last part I consider as a temporary solution till Arthur finished the part of making scene* completely independent from kwin core. When that is done I want to only compile the compositor twice and load it as a plugin depending on the result of an external test program (so that NVIDIA users cannot get trying to load the non working GLES).
The change to compile everything twice is for the following reasons:
1. Should make life easier for distributions to provide kwin_gles
2. To have the Wayland support only in kwin_gles (does not make sense in kwin+desktop GL)
3. To make testing easier by just starting kwin --replace and kwin_gles --replace
Current issues and things I want to do:
* kwineffects still links libGL as it provides a check whether GLX is available. I think this one can be dropped (should only be used at one place in scene)
* Add another marco to ensure that kwin+gl cannot load GLES effects and vice versa
* Clean up the build files
* think about whether we need XRender when building the gles backend
@Alex: it would be nice if you could have a look at the changes and if they look fine.
@Plasma: Please have a look whether there is something useful for Plasma Active - the patches should (mostly) apply to 4.7</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;">* kwin works with OpenGL and only loads the desktop GL effects
* kwin_gles works with OpenGL ES and only loads the GLES effects</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/kwinconfig.h.cmake <span style="color: grey">(22405a5)</span></li>
<li>kwin/effects.cpp <span style="color: grey">(143b033)</span></li>
<li>kwin/effects/CMakeLists.txt <span style="color: grey">(8683b35)</span></li>
<li>kwin/effects/logout/logout.cpp <span style="color: grey">(d0a5c1d)</span></li>
<li>kwin/effects/showfps/showfps.h <span style="color: grey">(8834956)</span></li>
<li>kwin/effects/showfps/showfps_config.h <span style="color: grey">(b184622)</span></li>
<li>kwin/effects/showfps/showfps_config.cpp <span style="color: grey">(dcbe93b)</span></li>
<li>kwin/kcmkwin/kwincompositing/CMakeLists.txt <span style="color: grey">(57d7750)</span></li>
<li>kwin/kcmkwin/kwinscreenedges/CMakeLists.txt <span style="color: grey">(94e841c)</span></li>
<li>kwin/libkwineffects/CMakeLists.txt <span style="color: grey">(cb9703b)</span></li>
<li>kwin/CMakeLists.txt <span style="color: grey">(4859978)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101979/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>