Issue 6117006: Mac: Explicitly set the colorspace on SkBitmap -> CGImageRef conversions.

Issue 6117006: Mac: Explicitly set the colorspace on SkBitmap -> CGImageRef conversions. (Closed)

Created:
9 years, 11 months ago by Nico

Modified:
9 years, 7 months ago

Reviewers:
viettrungluu

CC:
chromium-reviews, pam+watch_chromium.org, Robert Sesek, TVL

Base URL:
svn://svn.chromium.org/chrome/trunk/src

Visibility:
Public.

Description

Mac: Explicitly set the colorspace on SkBitmap -> CGImageRef conversions. The color space was hardcoded as "generic rgb" in skia. This is not always correct, also skia is changing this color space around a lot currently. To protect us from their unreliable default, hardcode "generic rgb" as default on our side for now, but make it possible for clients to provide their own color space. Use this to let tabpose and the favicon code pass in the device colorspace. BUG=24267, 50307 TEST=Open tabpose. Delayed thumbnails should look like backing-store backed thumbnails. The colors of favicons should now match other browsers. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71208

Patch Set 1 #

Patch Set 2 : '' #

Total comments: 3

Created: 9 years, 11 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+26 lines, -7 lines)			Patch
M	chrome/browser/ui/cocoa/tab_strip_controller.mm	View	1	2 chunks	+7 lines, -2 lines	0 comments	Download
M	chrome/browser/ui/cocoa/tabpose_window.mm	View		1 chunk	+3 lines, -2 lines	0 comments	Download
M	skia/ext/skia_utils_mac.h	View		1 chunk	+6 lines, -1 line	1 comment	Download
M	skia/ext/skia_utils_mac.mm	View	1	2 chunks	+10 lines, -2 lines	2 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

viettrungluu

"BUG=XXX"? LGTM with nits. http://codereview.chromium.org/6117006/diff/3001/skia/ext/skia_utils_mac.h File skia/ext/skia_utils_mac.h (right): http://codereview.chromium.org/6117006/diff/3001/skia/ext/skia_utils_mac.h#newcode68 skia/ext/skia_utils_mac.h:68: // DEPRECATED, use SkBitmapToNSImageWithColorSpace() instead. ...

9 years, 11 months ago (2011-01-12 19:02:11 UTC) #2

Nico

9 years, 11 months ago (2011-01-12 19:07:54 UTC) #3

All fixed, thanks. Submitting.

On Wed, Jan 12, 2011 at 11:02 AM,  <viettrungluu@chromium.org> wrote:
> "BUG=XXX"?
>
> LGTM with nits.
>
>
> http://codereview.chromium.org/6117006/diff/3001/skia/ext/skia_utils_mac.h
> File skia/ext/skia_utils_mac.h (right):
>
>
http://codereview.chromium.org/6117006/diff/3001/skia/ext/skia_utils_mac.h#ne...
> skia/ext/skia_utils_mac.h:68: // DEPRECATED, use
> SkBitmapToNSImageWithColorSpace() instead.
> If it's deprecated, you should file a bug (to remove the uses), add a
> TODO (for yourself) here, and then fix that bug/do that TODO.
>
> http://codereview.chromium.org/6117006/diff/3001/skia/ext/skia_utils_mac.mm
> File skia/ext/skia_utils_mac.mm (right):
>
>
http://codereview.chromium.org/6117006/diff/3001/skia/ext/skia_utils_mac.mm#n...
> skia/ext/skia_utils_mac.mm:177: skiaBitmap, colorSpace);
> I believe you can format this more nicely.
>
>
http://codereview.chromium.org/6117006/diff/3001/skia/ext/skia_utils_mac.mm#n...
> skia/ext/skia_utils_mac.mm:189: NSImage* SkBitmapToNSImage(const
> SkBitmap& skiaBitmap) {
> Do you put implementations in random order just to make erg and beng
> mad?
>
> http://codereview.chromium.org/6117006/
>

Expand Messages | Collapse Messages