Add SkColorSpace to GrDrawContext

Add SkColorSpace to GrDrawContext

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2164363002

Committed: https://skia.googlesource.com/skia/+/dfe4f2e4fe5b162d4adb4486fe751f1e3b30bea7

[Brian Osman, 2016-07-21 17:26:43]

Description was changed from

==========
Add SkColorSpace to GrDrawContext

BUG=skia:
==========

to

==========
Add SkColorSpace to GrDrawContext

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2164363002
==========

[Brian Osman, 2016-07-21 17:26:54]

The CQ bit was checked by brianosman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-07-21 17:27:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-21 18:00:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-21 18:00:23]

Dry run: This issue passed the CQ dry run.

[Brian Osman, 2016-07-21 18:36:57]

brianosman@google.com changed reviewers:
+ bsalomon@google.com, robertphillips@google.com

[Brian Osman, 2016-07-21 18:36:58]

Sorry that this is still fairly big. The actual diffs are much less intrusive
than in the previous (SkImageInfo) version, and there are fewer loose ends,
thanks to the SkSpecialImage work.

[robertphillips, 2016-07-21 19:22:42]

https://codereview.chromium.org/2164363002/diff/1/src/effects/SkAlphaThreshol...
File src/effects/SkAlphaThresholdFilter.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/effects/SkAlphaThreshol...
src/effects/SkAlphaThresholdFilter.cpp:104: 
// In this case we don't ever care about the color space b.c. we're rendering a
mask

?

https://codereview.chromium.org/2164363002/diff/1/src/effects/SkBlurMaskFilte...
File src/effects/SkBlurMaskFilter.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/effects/SkBlurMaskFilte...
src/effects/SkBlurMaskFilter.cpp:1249: bool isNormalBlur = (kNormal_SkBlurStyle
== fBlurStyle);
// same comment about how we don't care about color space here ?

https://codereview.chromium.org/2164363002/diff/1/src/effects/SkXfermodeImage...
File src/effects/SkXfermodeImageFilter.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/effects/SkXfermodeImage...
src/effects/SkXfermodeImageFilter.cpp:243: 
What if foreground & background have different color spaces ?

source is probably the correct answer here.

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrBlurUtils.cpp
File src/gpu/GrBlurUtils.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrBlurUtils.cpp#new...
src/gpu/GrBlurUtils.cpp:111: 
// ??

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrClipMaskManager.cpp
File src/gpu/GrClipMaskManager.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrClipMaskManager.c...
src/gpu/GrClipMaskManager.cpp:473: 
// ??

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrContext.cpp
File src/gpu/GrContext.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrContext.cpp#newco...
src/gpu/GrContext.cpp:360: // TODO: Supply color space?
Soup up this comment

e.g., this matches the none-draw paths. Do we want to pass in the desired color
space?

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrTextureToYUVPlane...
File src/gpu/GrTextureToYUVPlanes.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrTextureToYUVPlane...
src/gpu/GrTextureToYUVPlanes.cpp:70: // and U and V are the same but Y differs
(2 draws, one for Y, one for UV).
What's up with the 'colorSpace' parameter ?

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrYUVProvider.cpp
File src/gpu/GrYUVProvider.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrYUVProvider.cpp#n...
src/gpu/GrYUVProvider.cpp:115: 
I question the ':'

https://codereview.chromium.org/2164363002/diff/1/tests/GLProgramsTest.cpp
File tests/GLProgramsTest.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/tests/GLProgramsTest.cpp#ne...
tests/GLProgramsTest.cpp:156: 
Should we start to randomly create a color space too ?

https://codereview.chromium.org/2164363002/diff/1/tests/GLProgramsTest.cpp#ne...
tests/GLProgramsTest.cpp:345: // Validate that GrFPs work correctly without an
input.
Any idea why we don't call random_draw_context here?

https://codereview.chromium.org/2164363002/diff/1/tests/ImageFilterTest.cpp
File tests/ImageFilterTest.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/tests/ImageFilterTest.cpp#n...
tests/ImageFilterTest.cpp:364: 
It seems like we should be augmenting a lot of these tests to ensure the result
we get after filtering is in the expected color space (esp. the DAGs with
SkImageSources)

[bsalomon, 2016-07-21 19:40:28]

lgtm

[Brian Osman, 2016-07-21 19:51:44]

Just wanted to confirm that - yes - all of the mask
drawing functions are "correct" at this point with
nullptr color space...

https://codereview.chromium.org/2164363002/diff/1/src/effects/SkXfermodeImage...
File src/effects/SkXfermodeImageFilter.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/effects/SkXfermodeImage...
src/effects/SkXfermodeImageFilter.cpp:243: 
On 2016/07/21 19:22:41, robertphillips wrote:
> What if foreground & background have different color spaces ?
> 
> source is probably the correct answer here.

Yes. Output should be in color space of source. I'll be plumbing
colorspace of FG/BG to the FP for this draw, so we convert both
to the destination space before combining them.

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrTextureToYUVPlane...
File src/gpu/GrTextureToYUVPlanes.cpp (right):

https://codereview.chromium.org/2164363002/diff/1/src/gpu/GrTextureToYUVPlane...
src/gpu/GrTextureToYUVPlanes.cpp:70: // and U and V are the same but Y differs
(2 draws, one for Y, one for UV).
On 2016/07/21 19:22:41, robertphillips wrote:
> What's up with the 'colorSpace' parameter ?

That defines the transform between RGB (in any color space)
and YUV. (There are multiple such YUV transforms defined).
It's basically orthogonal to the RGB color-space. JPEG uses
one YUV conversion matrix, but video has a couple different
standard ones, and we support those, too. In this case, the
YUV data is actually "linear" from the SkColorSpace
perspective, so I think nullptr is fully correct here.

[Brian Osman, 2016-07-21 19:51:50]

The CQ bit was checked by brianosman@google.com

[Brian Osman, 2016-07-21 19:51:51]

The patchset sent to the CQ was uploaded after l-g-t-m from bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/2164363002/#ps40001
(title: "Remove ':' from comment")

[commit-bot: I haz the power, 2016-07-21 19:51:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-21 20:28:38]

Description was changed from

==========
Add SkColorSpace to GrDrawContext

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2164363002
==========

to

==========
Add SkColorSpace to GrDrawContext

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2164363002

Committed:
https://skia.googlesource.com/skia/+/dfe4f2e4fe5b162d4adb4486fe751f1e3b30bea7
==========

[commit-bot: I haz the power, 2016-07-21 20:28:39]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/dfe4f2e4fe5b162d4adb4486fe751f1e3b30bea7