<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/112755/">http://git.reviewboard.kde.org/r/112755/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 29th, 2013, 2:42 a.m. UTC, <b>Fredrik Höglund</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;">Looks much better, but it doesn't handle depth 30 pixmaps.</pre>
 </blockquote>




 <p>On November 4th, 2013, 8:05 a.m. UTC, <b>Martin Gräßlin</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;">I'm lacking ideas on how to test this. Do you know any application which uses 30 bit pixmaps?</pre>
 </blockquote>





 <p>On November 4th, 2013, 8:03 p.m. UTC, <b>Fredrik Höglund</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;">The default depth is 30 when you're using the NVIDIA driver and the monitor
supports 30 bits. This is the reason why it's important.

The X server can be told to use a 30 bit visual even when the monitor only supports
24 bits by starting it with -depth 30. But this isn't supported by all drivers.

Note that the raster graphics system is completely broken when the default depth
is 30 since Qt assumes that any depth >= 24 means 8 bits per channel.
</pre>
 </blockquote>





 <p>On November 5th, 2013, 7:58 a.m. UTC, <b>Martin Gräßlin</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;">thanks, that did work. I had tried Xephyr yesterday which didn't work and didn't think of running X with -depth 30.

I have some code now, but have no idea whether that's correct (bit manipulation is not what I'm very used to ;-)

case 30: {
    // Qt doesn't have a matching image format. We need to convert manually
    uint8_t *origData = xcb_get_image_data(xImage.data());
    uint8_t *converted = new uint8_t[xcb_get_image_data_length(xImage.data())];
    for (int i = 0; i < xcb_get_image_data_length(xImage.data()); i += 4) {
        uint32_t origWord = 0x00000000;
        for (int j = 0; j < 4; ++j) {
            if (j > 0) {
                origWord = origWord << 8;
            }
            origWord |= origData[i+j];
        }
        converted[i]   = uint8_t((float(origWord & 0xc0000000) / 0xc0000000) * 0xff);
        converted[i+1] = uint8_t((float(origWord & 0x3ff00000) / 0x3ff00000) * 0xff);
        converted[i+2] = uint8_t((float(origWord & 0x000ffc00) / 0x000ffc00) * 0xff);
        converted[i+3] = uint8_t((float(origWord & 0x000003ff) / 0x000003ff) * 0xff);
    }
    QImage image(converted, geo->width, geo->height,
                 xcb_get_image_data_length(xImage.data())/geo->height, QImage::Format_ARGB32_Premultiplied);
    if (image.isNull()) {
        return T();
    }
    T result = T::fromImage(image);
    delete[] converted;
    return result;
}

Testing is rather difficult. For Qt applications I'm rather sceptic that the icon has a correct color in the first place given the very visible brokeness and all GTK apps I tried just had an empty icon and looked equally broken as the Qt apps.</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;">I would simplify this to:

    uint32_t *pixels = reinterpret_cast<uint32_t *>(xcb_get_image_data(xImage.data()));
    for (int i = 0; i < xImage.data()->length; i++) {
        int r = (pixels[i] >> 22) & 0xff;
        int g = (pixels[i] >> 12) & 0xff;
        int b = (pixels[i] >> 2)  & 0xff;

        pixels[i] = qRgba(r, g, b, 0xff);
    }

The conversion can be done in-place, and the alpha bits are likely zero.

This algorithm obviously just truncates the values, but it's probably
good enough for things like icons. For extra credit you would use an
ordered dither map.
</pre>
<br />










<p>- Fredrik</p>


<br />
<p>On November 4th, 2013, 8:14 a.m. UTC, Martin Gräßlin wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://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.</div>
<div>By Martin Gräßlin.</div>


<p style="color: grey;"><i>Updated Nov. 4, 2013, 8:14 a.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</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;">Implements the createPixmapFromHandle by getting the image for the pixmaps and using it as either the Pixmap or the bitmap mask.</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;">Adjusted KWin to take this codepath and say thanks to Iceweasel for having a mask</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>tier1/kwindowsystem/src/kxutils.cpp <span style="color: grey">(33bd678)</span></li>

 <li>tier1/kwindowsystem/src/kxutils_p.h <span style="color: grey">(84d639b)</span></li>

 <li>tier1/kwindowsystem/tests/CMakeLists.txt <span style="color: grey">(0060903)</span></li>

 <li>tier1/kwindowsystem/tests/createpixmapfromhandletest.cpp <span style="color: grey">(PRE-CREATION)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/112755/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>