'Re: Review Request: Port Input Window handling for Effects to XCB'

[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? &gt;-)
(XKB is unfortunately a bit &quot;taken&quot;)

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-&gt;() 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 &amp;operator*() const {
   if (m_stub)
      m_reply = replyFunc(connection(), m_cookie, NULL);
      return *m_reply; // can be 0
}

~XcbWrapper() { // &quot;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">&lt;</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">&amp;</span><span \
class="n">xcb_get_window_attributes_reply</span><span class="p">,</span> <span \
class="o">&amp;</span><span class="n">xcb_get_window_attributes_unchecked</span><span \
class="o">&gt;</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 &quot;xcb_winddow_t&quot; (nur echt mit \
deppen_unter_strich) it&#39;s not gonna be the attributes of your wife or geometry of \
germany - ie. do we need the explicit &quot;Window&quot; 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&#39;t have a KWin::someGeometry() ...

---

Geometry someGeometry(window);
QRect r = someGeometry-&gt;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&#39;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] 