<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/115225/">https://git.reviewboard.kde.org/r/115225/</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 23rd, 2014, 3:47 p.m. CET, <b>Alexander Richardson</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;">Wouldn't this be easier using methods with the subclass as a template parameter instead of the enum. I.e:
class KWindowSystemPrivate {
//....
template <class Impl>
inline unsigned long state() const
{
// use a different name to prevent infinite recursion in case this is called with KWindowSystemPrivate as a template parameter
return static_cast<Impl*>(this)->stateImpl();
}
//....
};
class KWindowSystemPrivateDummy : public KWindowSystemPrivate {
//....
inline unsigned long stateImpl() const {
return 0;
}
//...
};
class KWindowSystemPrivateX11 : public KWindowSystemPrivate {
//....
inline unsigned long stateImpl() const {
return m_info->state();
}
//...
}
Then there is no more need for #define X11 static_cast<const KWindowInfoPrivateX11*>(this) anymore.
DELEGATE would look like this then:
#define DELEGATE(name, args) \
switch (d->platform()) { \
case KWindowInfoPrivate::XcbPlatform: \
return d->name<KWindowInfoPrivateX11>( args ); \
default: \
return d->name<KWindowInfoPrivateDummy>( args ); \
}
This should also work, however I am just typing it from my head without having it compiled.</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;">return static_cast<Impl*>(this)->stateImpl();
This obviously has to be static_cast<const Impl*>(this)</pre>
<br />
<p>- Alexander</p>
<br />
<p>On January 23rd, 2014, 9:09 a.m. CET, Martin Gräßlin 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 KDE Frameworks and kdewin.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Jan. 23, 2014, 9:09 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kwindowsystem
</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;">Add runtime platform support to KWindowInfo
Main idea of this change is to only pick the X11 implementation in case
that the application is running on the X11 platform. So far it was a
compile time switch which meant that if compiled with X11 support but
not running on the X11 platform it would have caused runtime errors.
To make this possible a KWindowInfoPrivate class with a dummy
implementation is provided. This is used as d-ptr for KWindowInfo.
Thus there is one generic implementation and the implementation of
KWindowInfo is no longer ifdefed for the supported platforms.
The platform specific code can inherit from KWindowInfoPrivate and
overwrite the dummy method implementation. KWindowInfoPrivate provides
a factory method where the platform specific implementation can be
hooked into. There we can have both compile time and runtime checking.
If there is no platfom specific implementation available the dummy
implementation is used.
NOTE: THIS CHANGE BREAKS THE WINDOWS AND MAC BACKEND!
Windows and Mac is excluded from build. At the moment they get the
dummy implementation. Unfortunately I don't have the possibility to
compile the changes and thus don't dare to touch the code. Fixes from
the teams are highly appreciated.</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;">Unit test from https://git.reviewboard.kde.org/r/115190/ is still working. Now you can guess why I wrote that test ;-)</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>src/CMakeLists.txt <span style="color: grey">(e32a1150a2c190f23ad456ca8218b012c5d71507)</span></li>
<li>src/kwindowinfo.h <span style="color: grey">(171f441ff329a5356ccf560341272199e20c837a)</span></li>
<li>src/kwindowinfo.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/kwindowinfo_p.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/kwindowinfo_p_x11.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/kwindowinfo_x11.cpp <span style="color: grey">(865d8bed085e987f97f479ea8aa0e6de8567586f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115225/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>