Review Request 112755: Reimplement KXUtils::createPixmapFromHandle with XCB

[Fredrik Höglund]

> On Oct. 29, 2013, 2:42 a.m., Fredrik Höglund wrote:
> > Looks much better, but it doesn't handle depth 30 pixmaps.
> 
> Martin Gräßlin wrote:
>     I'm lacking ideas on how to test this. Do you know any application which uses 30 bit pixmaps?
> 
> Fredrik Höglund wrote:
>     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.
>
> 
> Martin Gräßlin wrote:
>     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.

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.


- Fredrik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112755/#review42568
-----------------------------------------------------------


On Nov. 4, 2013, 8:14 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112755/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2013, 8:14 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> Implements the createPixmapFromHandle by getting the image for the pixmaps and using it as either the Pixmap or the bitmap mask.
> 
> 
> Diffs
> -----
> 
>   tier1/kwindowsystem/src/kxutils.cpp 33bd678 
>   tier1/kwindowsystem/src/kxutils_p.h 84d639b 
>   tier1/kwindowsystem/tests/CMakeLists.txt 0060903 
>   tier1/kwindowsystem/tests/createpixmapfromhandletest.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/112755/diff/
> 
> 
> Testing
> -------
> 
> Adjusted KWin to take this codepath and say thanks to Iceweasel for having a mask
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131105/a6ff0a9b/attachment.html>