[wip] image color correction

[wip] image color correction

BUG=None

[Stephen White, 2014-07-03 18:38:17]

Some random comments.

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/Col...
File Source/platform/graphics/ColorSpaceFilter.cpp (right):

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/Col...
Source/platform/graphics/ColorSpaceFilter.cpp:156:
sender.writeFunctionPtr(const_cast<ColorSpaceTransformFilter*>(this)->m_transform.release().leakRef());
This definitely won't work out-of-process. For now, you should probably assert
!SkWriteBuffer.isCrossProcess(). Even in-process, it feels pretty
dangerous/hacky.

The safer thing would be to actually serialize all of the qcms_transform's
parameters individually via sender.writeFoo(). It looks like the deserialization
could be accomplished with qcms_profile_from_memory(), which you could pass to
SkReadBuffer::readByteArray() in the constructor. However, there doesn't seem to
be a corresponding qcms_profile_to_memory().

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/Gra...
File Source/platform/graphics/GraphicsLayer.cpp (right):

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/Gra...
Source/platform/graphics/GraphicsLayer.cpp:1019: OwnPtr<ImageBuffer> buffer =
ImageBuffer::create(image->size(), opaque ? Opaque : NonOpaque);
Rather than create an ImageBuffer here (incurring another draw pass, and not
necessarily being GPU-friendly), I think another way to accomplish this would be
to create an SkColorFilterImageFilter to wrap your color profile colorfilter,
create a WebFilterOperations for it, and call appendReferenceFilter() on that
with your SkColorFilterImageFilter.

Of course, things might be a bit trickier if the layer already has a filter of
its own. In that case, you might have to insert the color transformation at the
head of the list as part of GraphicsLayer::setFilters().

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/ski...
File Source/platform/graphics/skia/NativeImageSkia.cpp (right):

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/ski...
Source/platform/graphics/skia/NativeImageSkia.cpp:359: //    software filters to
co-exist with the color space filter.
Actually I don't think CSS or SVG filters ever hit this particular path directly
-- they draw all images and text to a separate buffer and then apply filters to
that. So there should be no conflict with those.

For the existing color filters here, you should probably chat with fmalita@
about how to make them work. It could be done by turning them both into image
filters via SkColorFilterImageFilter adapters, and then concatenating them. That
would incur an extra pass, though, so you might want to only do that if both
filters are present.

[Noel Gordon, 2014-08-18 14:56:22]

On 2014/07/03 18:38:17, Stephen White wrote:
> Some random comments.

Didn't seem random to me; they were right on the money and useful to me.  Before
we discuss filters and whether they are the one-true-way (TM), I gonna carve off
some vestigial parts of the patch to slim it down some.

[Noel Gordon, 2014-08-18 14:58:33]

First, factor out some ImageSource.cpp changes.  Submitted
https://src.chromium.org/viewvc/blink?revision=180434&view=revision for that.

[Noel Gordon, 2014-08-18 15:01:48]

The webp decoder does not update correctly when a tab changes monitor. Filed
https://code.google.com/p/chromium/issues/detail?id=404544 about that.

[Noel Gordon, 2014-08-18 15:04:24]

Factor out clearColorTransform() changed to JPEG/PNG/WEBP decoders. Filed
https://code.google.com/p/chromium/issues/detail?id=404639 about that.

[Noel Gordon, 2014-08-19 05:34:04]

On 2014/08/18 15:04:24, Noel Gordon wrote:
> Factor out clearColorTransform() changed to JPEG/PNG/WEBP decoders. Filed
> https://code.google.com/p/chromium/issues/detail?id=404639 about that.

Landed https://src.chromium.org/viewvc/blink?revision=180521&view=revision

[Noel Gordon, 2014-08-19 13:43:22]

Fix include order ImageDecoder.h
https://src.chromium.org/viewvc/blink?view=rev&revision=180535

[Noel Gordon, 2014-08-19 13:43:56]

Give BitmapImage::draw "bm" a name
https://src.chromium.org/viewvc/blink?view=rev&revision=180540

[Noel Gordon, 2014-08-19 13:44:44]

Remove a comment from isSizeAvailable (for urvang)
https://src.chromium.org/viewvc/blink?view=rev&revision=180531

[Noel Gordon, 2014-08-22 05:09:06]

Directly composited images are images: ASSERT that
https://src.chromium.org/viewvc/blink?view=rev&revision=180721

[Stephen White, 2014-08-22 13:53:41]

I realize this is still WIP, but you should probably break out the Page and
ImageResource changes into a separate CL (+japhet) and the compositing changes
into a third (+vollick) so they can be reviewed in isolation by the appropriate
folks.

[Noel Gordon, 2014-08-22 13:55:42]

Yeap, I'm breaking the compositer changes for vollick as we speak.

[Noel Gordon, 2014-08-22 17:26:38]

Fix include order pre-submit nit in NativeImageSkia.cpp
https://src.chromium.org/viewvc/blink?view=rev&revision=180780
Doing compositor prep change again post-revert
https://src.chromium.org/viewvc/blink?view=rev&revision=180779

[Noel Gordon, 2014-08-26 12:57:41]

Directly composited images are images: ASSERT that
https://codereview.chromium.org/491773002
(appears to have stuck this time, fingers crossed).

[Noel Gordon, 2014-08-26 12:58:15]

Add directly composited image dynamic CSS filter test
https://src.chromium.org/viewvc/blink?view=rev&revision=180826

[Noel Gordon, 2014-08-26 12:58:45]

Inline BitmpaImage::isAllDataReceived
https://src.chromium.org/viewvc/blink?view=rev&revision=180832

[Noel Gordon, 2014-08-26 12:59:22]

Add BitmapImage resetDecoder() method
https://src.chromium.org/viewvc/blink?view=rev&revision=180835

[sugoi1, 2014-08-29 18:51:11]

sugoi@chromium.org changed reviewers:
+ sugoi@chromium.org

[sugoi1, 2014-08-29 18:51:12]

I read this mostly for myself, but here are some minor comments.

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/Bit...
File Source/platform/graphics/BitmapImage.cpp (right):

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/Bit...
Source/platform/graphics/BitmapImage.cpp:302: RefPtr<SkColorFilter>
colorTransform = createColorSpaceFilter(source.get(), screen.get());
Note: If you put "RefPtr<SkColorFilter> colorTransform;" above the if statement
(and still assign it here), you could add the colorTransform argument at line
315 and remove lines 303 to 308 to avoid duplicating code.

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/Bit...
Source/platform/graphics/BitmapImage.cpp:335: RefPtr<SkColorFilter>
colorTransform = createColorSpaceFilter(source.get(), screen.get());
Same here

[Noel Gordon, 2014-09-01 16:48:39]

On 2014/08/29 18:51:12, sugoi1 wrote:
> I read this mostly for myself, but here are some minor
> comments.

Thanks for reading. Wondered if I could move the color profile cache down into
Skia to hook up with your work.

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/Bit...
File Source/platform/graphics/BitmapImage.cpp (right):

https://codereview.chromium.org/352873002/diff/1/Source/platform/graphics/Bit...
Source/platform/graphics/BitmapImage.cpp:302: RefPtr<SkColorFilter>
colorTransform = createColorSpaceFilter(source.get(), screen.get());
Yeah possible. I wrote it this way because colorProfile() inlines a nullptr in
the "if" for ports that don't support color profiles, so the compiler could
optimize the if-clause out on those ports.

[Noel Gordon, 2014-09-01 16:50:06]

CSS clip-path should prevent direct image compositing
https://src.chromium.org/viewvc/blink?view=rev&revision=181138

[Noel Gordon, 2014-09-01 16:50:59]

Composited images should respond to dynamic style changes
https://src.chromium.org/viewvc/blink?view=rev&revision=181154

[Noel Gordon, 2014-09-05 14:44:13]

On 2014/08/22 13:53:41, Stephen White wrote:
> I realize this is still WIP, but you should probably break out the Page and
> ImageResource changes into a separate CL (+japhet) and the compositing changes
> into a third (+vollick)

Compositing changes done, ImageResource changes done

Add ImageResource update bitmap images api
https://codereview.chromium.org/526153002

> so they can be reviewed in isolation by the appropriate folks.

[Noel Gordon, 2014-09-15 06:16:01]

On 2014/08/22 13:53:41, Stephen White wrote:
> I realize this is still WIP, but you should probably break out the Page and
> ImageResource changes into a separate CL (+japhet) and the compositing changes
> into a third (+vollick)

Page changes done.

Add RenderView / Widget setDeviceColorProfile interface to Page
https://src.chromium.org/viewvc/blink?view=rev&revision=181899

> so they can be reviewed in isolation by the appropriate folks.