Bug 37621 – Chromium/mac: Image thumbnails don't work for indexed images

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Nico Weber]

WebKit bug for http://crbug.com/41512

[Nico Weber]

Created attachment 53382 [details]
Patch

[Nico Weber]

The CoreGraphics docs say that CGCreateContext doesn't work with indexed color spaces. Always using RGB is probably better for CMYK images too.

[Avi Drissman]

> Index: WebCore/platform/chromium/DragImageChromiumMac.cpp
> ===================================================================
> +    CGColorSpaceRef deviceRGB = CGColorSpaceCreateDeviceRGB();

RetainPtr?

> +    CGContextRef context = CGBitmapContextCreate(0, width, height, 8, width * 4, deviceRGB, kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host);
> +    CGColorSpaceRelease(deviceRGB);
> +

BTW, the comment about CGCreateContext not working for indexed color spaces is IMHO comment-worthy in the code. Just me.

[Avi Drissman]

> Index: WebCore/platform/chromium/DragImageChromiumMac.cpp
> ===================================================================
> +    CGColorSpaceRef deviceRGB = CGColorSpaceCreateDeviceRGB();

BTW, CGColorSpaceCreateDeviceRGB hasn't created a device RGB color space since 10.3, so the variable name, technically speaking, is inaccurate. Since CGColorSpaceCreateDeviceRGB() == CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), you should switch to the latter as it's clearer what's it's doing.

[Nico Weber]

Created attachment 53464 [details]
Patch

Did the RetainPtr thing and even the CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB) suggestion.

I didn't add a comment though, because just saying that createcontext doesn't work with indexed images misses the point. We really want RGB here, and not whatever weird color space the image might be in.

(Also, the WebKit way apparently is to do `svn annotate` and then look at revision log/bug afaiu).

[Avi Drissman]

(In reply to comment #5)
> (Also, the WebKit way apparently is to do `svn annotate` and then look at
> revision log/bug afaiu).

Fine with me.

Code LGTM.

[Dimitri Glazkov (Google)]

Comment on attachment 53464 [details]
Patch

motownavi can't be wrong. rs=me.

[WebKit Commit Bot]

Comment on attachment 53464 [details]
Patch

Clearing flags on attachment: 53464

Committed r57687: <http://trac.webkit.org/changeset/57687>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.