[Kde-graphics-devel] Review Request 114178: XCB port and port away from window-system specific functionality in QPixmap

[Fredrik Höglund]

> On Jan. 30, 2014, 4:41 p.m., Fredrik Höglund wrote:
> > The X server sets the depth in the GetImage reply to the depth of the drawable,
> > so I don't know what the problem is.
> > 
> > However, unlike pixmaps, windows have visuals, so instead of relying on the
> > depth I think the code should always look at the visual description, or
> > better yet the render pictformat that corresponds to the visual, and match
> > the red/green/blue/alpha masks against the QImage format.
> > 
> > (sorry for the late reply)
> >
> 
> Martin Gräßlin wrote:
>     oh wow, I have a solution and I'm not sure whether I like it ;-)
>     
>     Looking at the visual didn't help at all and after looking at the code in more depth it's not surprising. KSnapshot always captures the root window, so we get the default visual. I then started to look at how the QPixmap::grabWindow worked in Qt 4.8 (create pixmap, create gc, copy area to pixmap, destroy gc) but using the same pattern didn't result in success. As I don't like working with the gc I gave a try to do the same with xrender and succeeded with:
>     
>     xcb_render_composite(c, XCB_RENDER_PICT_OP_CLEAR, source, XCB_RENDER_PICTURE_NONE, destination, x, y, 0, 0, 0, 0, width, height);
>     xcb_render_composite(c, XCB_RENDER_PICT_OP_ADD, source, XCB_RENDER_PICTURE_NONE, destination, x, y, 0, 0, 0, 0, width, height);
>     
>     with source being a picture for the window and destination being a picture for the created temp pixmap.
>     
>     If that gives you an idea why I need to go such a way to get xterm captured correctly, please let me know and I give it a try. Otherwise I'll just upload the new version here.
> 
> Fredrik Höglund wrote:
>     Does it also work if you substitute those two calls with:
>     
>     xcb_render_composite(c, XCB_RENDER_PICT_OP_SRC, source, XCB_RENDER_PICTURE_NONE, destination, x, y, 0, 0, 0, 0, width, height);
>     
>     I can't say exactly why it works without looking at the code, but while CopyArea is equivalent to a memcpy,
>     RenderComposite does data conversion if the source and destination formats don't match.
>
> 
> Martin Gräßlin wrote:
>     no, PICT_OP_SRC doesn't work (was the first thing I tested), it behaves exactly like CopyArea.

That's really strange. PICT_OP_CLEAR doesn't read from either
the source or the destination, so your code equates to:

destination = 0;
destination = destination + source;

And this should produce exactly the same result as:

destination = source;


- Fredrik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/114178/#review48642
-----------------------------------------------------------


On Jan. 24, 2014, 10:32 a.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114178/
> -----------------------------------------------------------
> 
> (Updated Jan. 24, 2014, 10:32 a.m.)
> 
> 
> Review request for KDE Graphics and Richard Moore.
> 
> 
> Repository: ksnapshot
> 
> 
> Description
> -------
> 
> Patch set is available in kde:clones/ksnapshot/graesslin/ksnapshot branch xcb. Direct link to latest commit: http://commits.kde.org/clones/ksnapshot/graesslin/ksnapshot/6e41a393337c198dd9cebe8c597014b638ea64f4
> 
> The branch prepares ksnapshot for two things: changes in QPixmap API in Qt5 and being able to support multiple platforms (X11/Wayland) at runtime which cannot be solved by pure ifdefs.
> 
> In summary the changes do the following:
> * move X11 specific code from WindowGrabber into a windowgrabber_x11.cpp. Same needs to be done with the windows specific code, but I'm not able to do that due to lack of that OS
> * port the X11 specific code from WindowGrabber to xcb. For that FindXCB and FindX11XCB is copied from ECM (to be deleted in the Qt 5 branch) and I added some of the helper classes we use in KWin.
> * Port the xfixes specific code in ksnapshot.cpp to xcb
> * Adds a PixmapHelper class with an X11 backend to provide the functionality which got removed in Qt5.
> 
> I'm quite confident that the code base after those changes will compile against Qt 5, though I have not tried yet. Wanted to get out this review request first.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 9b2dc6e 
>   cmake/modules/FindX11_XCB.cmake PRE-CREATION 
>   cmake/modules/FindXCB.cmake PRE-CREATION 
>   config-ksnapshot.h.cmake deca85f 
>   freeregiongrabber.cpp f7f1924 
>   kbackgroundsnapshot.cpp 9d81ba1 
>   ksnapshot.cpp ca308bb 
>   ksnapshotobject.cpp 608f7dc 
>   pixmaphelper.h PRE-CREATION 
>   pixmaphelper.cpp PRE-CREATION 
>   pixmaphelper_x11.h PRE-CREATION 
>   pixmaphelper_x11.cpp PRE-CREATION 
>   regiongrabber.cpp fb038a3 
>   windowgrabber.cpp 21a9531 
>   windowgrabber_x11.cpp PRE-CREATION 
>   xcbutils.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/114178/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-graphics-devel/attachments/20140211/8dc58e90/attachment.html>