[Webkit-unassigned] [Bug 47922] Merge ColorSpace and ImageColorSpace enums

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=47922


Nikolas Zimmermann <zimmermann at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #71198|review?                     |review-
               Flag|                            |




--- Comment #2 from Nikolas Zimmermann <zimmermann at kde.org>  2010-10-20 01:40:22 PST ---
(From update of attachment 71198)
View in context: https://bugs.webkit.org/attachment.cgi?id=71198&action=review

Looks good overall, except for some minor things. But I'd really like to see whether we can avoid code duplication, by not using RetainPtr for the colorspace in ImageBuffer.

> WebCore/css/CSSPrimitiveValueMappings.h:2198
> +        default:

default is not added here on purpose, so the compiler warns if anything is missing. If you need to add sth. add the real case value.

> WebCore/platform/graphics/ColorSpace.h:34
> +    sRGBColorSpace,
> +    linearRGBColorSpace

I don't like these names, style guide says "Enum members should user InterCaps with an initial capital letter.".
So rename to SRGB and LinearRGB..

> WebCore/platform/graphics/cg/ImageBufferCG.cpp:83
> +    case DeviceColorSpace:
> +        m_data.m_colorSpace.adoptCF(CGColorSpaceCreateDeviceRGB());
> +        break;

Can you investigate, if it's possible to not use a RetainPtr for m_colorSpace. Then we could use the deviceRGBRef() static methods, instead of duplication code.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.