GNOME Bugzilla – Bug 57608
color picker doesn't work
Last modified: 2011-02-04 16:09:32 UTC
Hi, the color picker of the colorsel dialog doesn't work, because of a bug in X11 implementation of gdk_image_get, which would always place the image data at an offset instead of at (0,0), where it should be. The attached patch fixes this and thus makes the color picker work.
Created attachment 745 [details] [review] the patch
Looking at it _gdk_x11_get_image() has all sorts of problems: - It needs to handle bitmaps when visual == NULL - It should return an image with undefined contents when the window is completely off-screen rather than NULL. - It doesn't properly translate the window to root window coordinates before clipping to the screen size - It doesn't use the results of clipping to the screen size.
Yes, but I think my patch is nevertheless needed to get get_image to work properly in the few cases where it has a chance to (ie if we're asking for a on-screen rectangle of the root window). We don't want to put the subimage we're getting at an offset in a large destination image, thus we should use 0,0 as dest_x, dest_y.
Created attachment 764 [details] [review] this (mostly untested) patch tries to address some of the issues (except for bitmaps)
Looks all good to me. I'd like to see this merged with the bitmap patch Ron Steinke is working and get the whole thing committed as one unit, to avoid excessive conflicts.
This has been fixed together with #57604.
Testing revealed that the patch didn't work as nicely as intended. Here is an additional (tested) patch which makes _gtk_x11_get_image actually work.
Created attachment 813 [details] [review] the patch
Sorry for missing the second patch - Havoc committed a change this morning that is almost identical to this, though it fixes one extra thing - the code before the fix still had a bit of window/screen coordinate confusion. It translated the screen position to window coordinate and window position to to screen coordinates...