I think we should merge the ColorSpace and ImageColorSpace enumerations. This mean adding linearRGB to ColorSpace and will be necessary for implementing color-interpolation as well as color-interpolation-filters on SVG.
Created attachment 71198 [details]
Comment on attachment 71198 [details]
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.
> + 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.
> + 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..
> + 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.
Created attachment 71272 [details]
Attachment 71272 [details] did not build on qt:
Build output: http://queues.webkit.org/results/4516016
Comment on attachment 71272 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=71272&action=review
Other than that, looks good to me!
> + ASSERT_NOT_REACHED();
You should comment here, that linearRgb is not available, through css "-webkit-color-correction". Otherwhise this is a bit confusing.
Created attachment 71274 [details]
Comment on attachment 71274 [details]
Looks great, r=me.
Please do a leaks run first, as I'm not 100% certain about the removal of RetainPtr for m_colorSpace. Looks fine according to the CoreGraphics docs though, but better be safe than sorry.
I checked. No leaks found. *Phew* ;-)
Committed r70143: <http://trac.webkit.org/changeset/70143>
http://trac.webkit.org/changeset/70143 might have broken Qt Linux Release
The following tests are not passing: