After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 676362 - _gdk_quartz_image_copy_to_image always returns black for the root window
_gdk_quartz_image_copy_to_image always returns black for the root window
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Quartz
2.24.x
Other Mac OS
: Normal normal
: ---
Assigned To: gtk-quartz maintainers
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2012-05-19 09:01 UTC by Daniel Sabo
Modified: 2012-08-23 16:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GDK Quartz: Special case for get image on the root window (3.35 KB, patch)
2012-05-19 09:01 UTC, Daniel Sabo
none Details | Review
patch that works for me (3.04 KB, patch)
2012-06-09 18:52 UTC, Kristian Rietveld
none Details | Review
Use correct rect type for CGWindowListCreateImage (1.16 KB, patch)
2012-06-13 15:13 UTC, Daniel Sabo
committed Details | Review

Description Daniel Sabo 2012-05-19 09:01:17 UTC
Created attachment 214438 [details] [review]
GDK Quartz: Special case for get image on the root window

The MyPaint color picker always returns black when using the GDK quartz backend. This happens because it calls gdk_drawable_get_image to grab a pixel of the root window, which eventually calls _gdk_quartz_image_copy_to_image, which fails because [view lockFocusIfCanDraw] fails on the root window.

I've attached a patch that does special handling of the root window, but 100% sure it's assumptions about bit depth are accurate. I also haven't tested it with multiple monitors.
Comment 1 Michael Natterer 2012-05-27 20:48:03 UTC
Same is true for the GIMP color pickers, will look into this patch.
Comment 2 Michael Natterer 2012-05-27 20:48:57 UTC
*** Bug 576124 has been marked as a duplicate of this bug. ***
Comment 3 Kristian Rietveld 2012-06-05 20:56:02 UTC
I fully agree with the approach taken, but the patch is not working for me.  I played around with it a bit to try to get it to work.  I have almost managed to get it going.  What needs to be fixed is e.g.:

 - even for [res hasAlpha] == FALSE, I get 32 bits per pixel ([rep bitsPerPixel]).  We seem to really want to increment src based on [rep bitsPerPixel] / 8 instead of hardcoded 3 or 4.

 - this seems to be the case because [rep bitmapFormat] == 1 (premultipled alpha).


With these changes I appear to get correct color values in _gdk_quartz_image_copy_image() but not in the GdkPixbuf that is read by the color picking code.  I don't understand yet what is going wrong with the color conversion here.
Comment 4 Daniel Sabo 2012-06-06 13:06:33 UTC
I was hoping the format of the desktop would be consistent between computers, but getting wrong colors might mean it's also necessary to check ([rep bitmapFormat] & NSAlphaFirstBitmapFormat). NSAlphaFirstBitmapFormat would mean that the data is ARGB instead of RGBA. If you pick from splotches of pure red/green/blue does one of them come back as black?
Comment 5 Kristian Rietveld 2012-06-06 13:35:02 UTC
(In reply to comment #4)
> I was hoping the format of the desktop would be consistent between computers,
> but getting wrong colors might mean it's also necessary to check ([rep
> bitmapFormat] & NSAlphaFirstBitmapFormat)

Right, from what I remember I was getting the premultiplied alpha format yesterday (don't have that  computer around at the moment).  I would have expected consistency as well btw.


> . NSAlphaFirstBitmapFormat would mean
> that the data is ARGB instead of RGBA. If you pick from splotches of pure
> red/green/blue does one of them come back as black?

I don't remember that, what I do remember from dumping src[0], src[1], src[2] and src[3] to terminal was that src[0] was always 255 (so prolly the alpha), src[1..3] contained color values.
Comment 6 Kristian Rietveld 2012-06-09 18:51:39 UTC
Double checked now, and the format I get here is also NSAlphaFirstBitmapFormat.  So forget about the pre-multipled alpha :)
Comment 7 Kristian Rietveld 2012-06-09 18:52:54 UTC
Created attachment 216043 [details] [review]
patch that works for me

I tinkered around with the patch -- this patch actually works for me and picks the right colors.


Could you check whether this patch also works for you?  If so -- we can probably commit it.
Comment 8 Daniel Sabo 2012-06-11 10:14:00 UTC
Your patch picks colors correctly for me, looks good.
Comment 9 Kristian Rietveld 2012-06-11 19:12:30 UTC
Thanks for testing.  Pushed into gtk-2-24.



commit 2f706868ff3c4a3d25aa85e233590161a55c9657
Author: Daniel Sabo <DanielSabo@gmail.com>
Date:   Mon Jun 11 21:06:58 2012 +0200

    Bug 676362 - _gdk_quartz_image_copy_to_image always returns black...
    
    Implement a special case for the root window, which has to be handled
    differently on OS X.
    
    Contains some bit fiddling corrections by Kristian Rietveld.
Comment 10 John Ralls 2012-06-13 10:03:15 UTC
Unfortunately, this breaks the build when compiling for arch i386 with SDK 10.5:

gdkimage-quartz.c: In function '_gdk_quartz_image_copy_to_image':
gdkimage-quartz.c:147: error: incompatible type for argument 1 of 'CGWindowListCreateImage'
Comment 11 Daniel Sabo 2012-06-13 15:13:47 UTC
Created attachment 216293 [details] [review]
Use correct rect type for CGWindowListCreateImage

A shot in the dark, I'll need to set up another jhbuild prefix to properly test this.
Comment 12 John Ralls 2012-06-13 16:29:05 UTC
Comment on attachment 216293 [details] [review]
Use correct rect type for CGWindowListCreateImage

Yes, that works.
Comment 13 John Ralls 2012-06-13 16:44:33 UTC
Comment on attachment 216293 [details] [review]
Use correct rect type for CGWindowListCreateImage

Pushed to gtk-2-24
Comment 14 John Ralls 2012-08-22 20:10:51 UTC
Is there another way to get the CGImageRef besides CGWindowListCreateImage? That function -- in fact, all of CGWindow.h -- was added in 10.5, so it breaks building in 10.4.
Comment 15 Daniel Sabo 2012-08-23 16:04:50 UTC
It may be possible to do something with transparent windows ( example code here: http://cocoadev.com/wiki/ScreenShotCode ) but my quick attempt to use this didn't work and I haven't had time to investigate further.