[prev in list] [next in list] [prev in thread] [next in thread]
List: kwin
Subject: Re: Review Request: Port Input Window handling for Effects to XCB
From: Thomas_Lübking <thomas.luebking () web ! de>
Date: 2012-12-24 13:56:15
Message-ID: 20121224135615.1081.72748 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107817/#review23952
-----------------------------------------------------------
Random thoughts:
kwin/x11utils.h
<http://git.reviewboard.kde.org/r/107817/#comment18256>
namespace Xcb? Xqb? XQB? XCB?
X_C_B? >-)
(XKB is unfortunately a bit "taken")
Oh, and in that namespace sth. like
typedef xcb_window_t Window; // ahhh... much better now ;-)
kwin/x11utils.h
<http://git.reviewboard.kde.org/r/107817/#comment18254>
brace operator? ;-)
private:
Cookie m_cookie;
Reply *m_reply;
bool m_stub;
public:
inline Reply *operator->() const {
if (m_stub)
m_reply = replyFunc(connection(), m_cookie, NULL);
return m_reply; // can be 0
}
inline bool isValid() {
if (m_stub)
m_reply = replyFunc(connection(), m_cookie, NULL);
return bool(m_reply);
}
// do we need this? needs isValid for deref.
inline T &operator*() const {
if (m_stub)
m_reply = replyFunc(connection(), m_cookie, NULL);
return *m_reply; // can be 0
}
~XcbWrapper() { // "Helper is sooooo generic ;-)
if (m_reply) free(m_reply);
}
kwin/x11utils.h
<http://git.reviewboard.kde.org/r/107817/#comment18255>
Since the constructor takes one argument, being "xcb_winddow_t" (nur echt mit \
deppen_unter_strich) it's not gonna be the attributes of your wife or geometry of germany - ie. \
do we need the explicit "Window" tag in this context (and namespace)?
kwin/x11utils.h
<http://git.reviewboard.kde.org/r/107817/#comment18257>
WindowGeometry someGeometry(window);
QRect r = someGeometry().toRect(); // we hopefully don't have a KWin::someGeometry() ...
---
Geometry someGeometry(window);
QRect r = someGeometry->rect();
- Thomas Lübking
On Dec. 24, 2012, 8:05 a.m., Martin Gräßlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107817/
> -----------------------------------------------------------
>
> (Updated Dec. 24, 2012, 8:05 a.m.)
>
>
> Review request for kwin and Fredrik Höglund.
>
>
> Description
> -------
>
> Port Input Window handling for Effects to XCB
>
>
> Introduce helper classes to perform common xcb requests
>
> Two function objects WindowGeometry and WindowAttributes which can be
> used to request the geometry and attributes more easily.
>
> The ctor performs the async request and the operator() provides the
> reply and will block. The WindowGeometry class also provides a convenient
> method to provide the result as a QRect.
>
> It's the task of the callee to free the returned object.
>
> These functors are implemented by using a templated class called
> XcbHelper. WindowAttributes is in fact just a typedef with all template
> parameters being specified and WindowGeometry inherits to be able to add
> another function. The templated approach should make it easy to add new
> function object classes when needed.
>
>
> Diffs
> -----
>
> kwin/effects.h d985de4e4f76ecb53099b36d22e7678ba199b9f3
> kwin/effects.cpp 70258835054cb579625bc42ea7f077a718d06104
> kwin/libkwineffects/kwineffects.h 6aea85d20772a8a69855b81ef687b91dc93b98ac
> kwin/libkwineffects/kwineffects.cpp 4895ca0e1e4fce2ca38330cf6d5cd8a350baad94
> kwin/x11utils.h PRE-CREATION
>
> Diff: http://git.reviewboard.kde.org/r/107817/diff/
>
>
> Testing
> -------
>
>
> 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/107817/">http://git.reviewboard.kde.org/r/107817/</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;">Random thoughts:</pre> <br />
<div>
<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/107817/diff/5/?file=100910#file100910line33" \
style="color: black; font-weight: bold; text-decoration: underline;">kwin/x11utils.h</a> <span \
style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace \
KWin</pre></td>
</tr>
</tbody>
<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">33</font></th> \
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"><span class="n">namespace</span> <span class="n">X11</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">namespace Xcb? Xqb? XQB? XCB? \
X_C_B? >-) (XKB is unfortunately a bit "taken")
Oh, and in that namespace sth. like
typedef xcb_window_t Window; // ahhh... much better now ;-)</pre>
</div>
<br />
<div>
<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/107817/diff/5/?file=100910#file100910line46" \
style="color: black; font-weight: bold; text-decoration: underline;">kwin/x11utils.h</a> <span \
style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace \
KWin</pre></td>
</tr>
</tbody>
<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">46</font></th> \
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="n">Reply</span> <span class="o">*</span><span class="n">operator</span><span \
class="p">()()</span> <span class="p">{</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">brace operator? ;-)
private:
Cookie m_cookie;
Reply *m_reply;
bool m_stub;
public:
inline Reply *operator->() const {
if (m_stub)
m_reply = replyFunc(connection(), m_cookie, NULL);
return m_reply; // can be 0
}
inline bool isValid() {
if (m_stub)
m_reply = replyFunc(connection(), m_cookie, NULL);
return bool(m_reply);
}
// do we need this? needs isValid for deref.
inline T &operator*() const {
if (m_stub)
m_reply = replyFunc(connection(), m_cookie, NULL);
return *m_reply; // can be 0
}
~XcbWrapper() { // "Helper is sooooo generic ;-)
if (m_reply) free(m_reply);
}</pre>
</div>
<br />
<div>
<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/107817/diff/5/?file=100910#file100910line53" \
style="color: black; font-weight: bold; text-decoration: underline;">kwin/x11utils.h</a> <span \
style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace \
KWin</pre></td>
</tr>
</tbody>
<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">53</font></th> \
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"><span class="k">typedef</span> <span class="n">XcbHelper</span><span \
class="o"><</span><span class="n">xcb_get_window_attributes_reply_t</span><span \
class="p">,</span> <span class="n">xcb_get_window_attributes_cookie_t</span><span \
class="p">,</span> <span class="o">&</span><span \
class="n">xcb_get_window_attributes_reply</span><span class="p">,</span> <span \
class="o">&</span><span class="n">xcb_get_window_attributes_unchecked</span><span \
class="o">></span> <span class="n">WindowAttributes</span><span \
class="p">;</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Since the constructor takes one \
argument, being "xcb_winddow_t" (nur echt mit deppen_unter_strich) it's not gonna \
be the attributes of your wife or geometry of germany - ie. do we need the explicit \
"Window" tag in this context (and namespace)?</pre> </div>
<br />
<div>
<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/107817/diff/5/?file=100910#file100910line61" \
style="color: black; font-weight: bold; text-decoration: underline;">kwin/x11utils.h</a> <span \
style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">namespace \
KWin</pre></td>
</tr>
</tbody>
<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">61</font></th> \
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> \
<span class="n">QRect</span> <span class="n">toRect</span><span class="p">()</span> <span \
class="p">{</span></pre></td> </tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">WindowGeometry \
someGeometry(window); QRect r = someGeometry().toRect(); // we hopefully don't have a \
KWin::someGeometry() ...
---
Geometry someGeometry(window);
QRect r = someGeometry->rect();</pre>
</div>
<br />
<p>- Thomas</p>
<br />
<p>On December 24th, 2012, 8:05 a.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 and Fredrik Höglund.</div>
<div>By Martin Gräßlin.</div>
<p style="color: grey;"><i>Updated Dec. 24, 2012, 8:05 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;">Port Input Window \
handling for Effects to XCB
Introduce helper classes to perform common xcb requests
Two function objects WindowGeometry and WindowAttributes which can be
used to request the geometry and attributes more easily.
The ctor performs the async request and the operator() provides the
reply and will block. The WindowGeometry class also provides a convenient
method to provide the result as a QRect.
It's the task of the callee to free the returned object.
These functors are implemented by using a templated class called
XcbHelper. WindowAttributes is in fact just a typedef with all template
parameters being specified and WindowGeometry inherits to be able to add
another function. The templated approach should make it easy to add new
function object classes when needed.</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/effects.h <span style="color: \
grey">(d985de4e4f76ecb53099b36d22e7678ba199b9f3)</span></li>
<li>kwin/effects.cpp <span style="color: \
grey">(70258835054cb579625bc42ea7f077a718d06104)</span></li>
<li>kwin/libkwineffects/kwineffects.h <span style="color: \
grey">(6aea85d20772a8a69855b81ef687b91dc93b98ac)</span></li>
<li>kwin/libkwineffects/kwineffects.cpp <span style="color: \
grey">(4895ca0e1e4fce2ca38330cf6d5cd8a350baad94)</span></li>
<li>kwin/x11utils.h <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/107817/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