Create special surfaces according to original device (not always in N32)

Change SkSpecialImage::makeSurface and makeTightSurface to take output
properties (color space), bounds, and (optional) alphaType.

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space. All call sites were saying N32.

What we want to do is propagate the original device's color space, and
pick a good format from that. Rather than force all the clients to
jump through hoops constructing an SkImageInfo that meets our criteria,
just have them supply the few bits we care about, and do everything else
internally.

This also lets us always use RGBA on GPU, but N32 on raster.

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

Committed: https://skia.googlesource.com/skia/+/53c38087949252d27cde668368a3eeb59cc2eb00
Committed: https://skia.googlesource.com/skia/+/eed6b0e1d865a1f93143c09961debba0aca592ca

[Brian Osman, 2016-09-20 21:31:54]

Description was changed from

==========
Change SkSpecialImage to just take bounds and (optional) alphaType

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space.

What we want to do is propagate our color space, and pick a good format
from that. Rather than force all the clients to jump through hoops
constructing an SkImageInfo that meets our criteria, just have them supply
the few bits we care about, and do everything else internally.

This also lets us always use RGBA on GPU, but N32 on raster.

With this (and the dependent CL), gpusrgb and gpuf16 now render much
more correctly on about 30 GMs.

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

to

==========
Change SkSpecialImage to just take bounds and (optional) alphaType

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space.

What we want to do is propagate our color space, and pick a good format
from that. Rather than force all the clients to jump through hoops
constructing an SkImageInfo that meets our criteria, just have them supply
the few bits we care about, and do everything else internally.

This also lets us always use RGBA on GPU, but N32 on raster.

With this (and the dependent CL), gpusrgb and gpuf16 now render much
more correctly on about 30 GMs.

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

[Brian Osman, 2016-09-20 21:32:50]

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

[commit-bot: I haz the power, 2016-09-20 21:33:02]

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

[Brian Osman, 2016-09-20 21:33:45]

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

[Brian Osman, 2016-09-20 21:33:45]

I think I need to make a similar change to makeTightSurface?

[commit-bot: I haz the power, 2016-09-20 21:39:32]

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

[commit-bot: I haz the power, 2016-09-20 21:39:33]

Dry run: Try jobs failed on following builders:
  Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Debug-GN-Trybot on master.client.skia
(JOB_FAILED,
http://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2...)

[bsalomon, 2016-09-21 13:30:55]

https://codereview.chromium.org/2349373004/diff/1/src/core/SkSpecialImage.h
File src/core/SkSpecialImage.h (right):

https://codereview.chromium.org/2349373004/diff/1/src/core/SkSpecialImage.h#n...
src/core/SkSpecialImage.h:89: sk_sp<SkSpecialSurface> makeSurface(const SkIRect&
size,
SkISize?

[robertphillips, 2016-09-21 13:58:09]

lgtm % Brian's comment

[Brian Osman, 2016-09-23 13:42:45]

Description was changed from

==========
Change SkSpecialImage to just take bounds and (optional) alphaType

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space.

What we want to do is propagate our color space, and pick a good format
from that. Rather than force all the clients to jump through hoops
constructing an SkImageInfo that meets our criteria, just have them supply
the few bits we care about, and do everything else internally.

This also lets us always use RGBA on GPU, but N32 on raster.

With this (and the dependent CL), gpusrgb and gpuf16 now render much
more correctly on about 30 GMs.

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

to

==========
Change SkSpecialImage::makeSurface and makeTightSurface to take output
properties (color space), bounds, and (optional) alphaType.

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space. All call sites were saying N32.

What we want to do is propagate the original device's color space, and
pick a good format from that. Rather than force all the clients to
jump through hoops constructing an SkImageInfo that meets our criteria,
just have them supply the few bits we care about, and do everything else
internally.

This also lets us always use RGBA on GPU, but N32 on raster.

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

[Brian Osman, 2016-09-23 13:44:08]

Much better version of this change, based on our discussion. Includes a hack so
that raster continues to produce N32 surfaces in all cases.

https://codereview.chromium.org/2349373004/diff/140001/src/effects/SkDisplace...
File src/effects/SkDisplacementMapEffect.cpp (right):

https://codereview.chromium.org/2349373004/diff/140001/src/effects/SkDisplace...
src/effects/SkDisplacementMapEffect.cpp:282: // Creation of the displacement map
should happen in a non-colorspace aware context:
This is an arbitrary decision after a long discussion between Rob & I. The
correct answer here is ... not obvious.

[bsalomon, 2016-09-23 13:54:59]

lgtm

[robertphillips, 2016-09-23 14:11:29]

https://codereview.chromium.org/2349373004/diff/140001/src/core/SkImageFilter...
File src/core/SkImageFilter.cpp (right):

https://codereview.chromium.org/2349373004/diff/140001/src/core/SkImageFilter...
src/core/SkImageFilter.cpp:350: int newWidth, int newHeight, int offX, int offY)
{
Although SpecialImages are inextricably tied to ImageFilters it still seems odd
for makeSurface to be so reliant on a struct from SkImageFilter (i.e.,
OutputProperties).

https://codereview.chromium.org/2349373004/diff/140001/src/core/SkSpecialImag...
File src/core/SkSpecialImage.cpp (right):

https://codereview.chromium.org/2349373004/diff/140001/src/core/SkSpecialImag...
src/core/SkSpecialImage.cpp:61: const SkISize& size,
Do we need the default parameter here ?

https://codereview.chromium.org/2349373004/diff/140001/src/effects/SkDisplace...
File src/effects/SkDisplacementMapEffect.cpp (right):

https://codereview.chromium.org/2349373004/diff/140001/src/effects/SkDisplace...
src/effects/SkDisplacementMapEffect.cpp:282: // Creation of the displacement map
should happen in a non-colorspace aware context:
Maybe expand on this to mention some of the consequences, when it matters ?

[Brian Osman, 2016-09-23 14:33:51]

The CQ bit was checked by brianosman@google.com

[Brian Osman, 2016-09-23 14:33:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertphillips@google.com, bsalomon@google.com
Link to the patchset: https://codereview.chromium.org/2349373004/#ps160001
(title: "Remove copy-pasted default parameter, expand comment")

[commit-bot: I haz the power, 2016-09-23 14:34:02]

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-09-23 15:11:57]

Description was changed from

==========
Change SkSpecialImage::makeSurface and makeTightSurface to take output
properties (color space), bounds, and (optional) alphaType.

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space. All call sites were saying N32.

What we want to do is propagate the original device's color space, and
pick a good format from that. Rather than force all the clients to
jump through hoops constructing an SkImageInfo that meets our criteria,
just have them supply the few bits we care about, and do everything else
internally.

This also lets us always use RGBA on GPU, but N32 on raster.

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

to

==========
Change SkSpecialImage::makeSurface and makeTightSurface to take output
properties (color space), bounds, and (optional) alphaType.

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space. All call sites were saying N32.

What we want to do is propagate the original device's color space, and
pick a good format from that. Rather than force all the clients to
jump through hoops constructing an SkImageInfo that meets our criteria,
just have them supply the few bits we care about, and do everything else
internally.

This also lets us always use RGBA on GPU, but N32 on raster.

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

Committed:
https://skia.googlesource.com/skia/+/53c38087949252d27cde668368a3eeb59cc2eb00
==========

[commit-bot: I haz the power, 2016-09-23 15:11:57]

Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/53c38087949252d27cde668368a3eeb59cc2eb00

[Brian Osman, 2016-09-23 15:49:30]

A revert of this CL (patchset #9 id:160001) has been created in
https://codereview.chromium.org/2366723004/ by brianosman@google.com.

The reason for reverting is: DM crash and/or TSAN failure.

[Brian Osman, 2016-09-23 20:00:35]

Description was changed from

==========
Change SkSpecialImage::makeSurface and makeTightSurface to take output
properties (color space), bounds, and (optional) alphaType.

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space. All call sites were saying N32.

What we want to do is propagate the original device's color space, and
pick a good format from that. Rather than force all the clients to
jump through hoops constructing an SkImageInfo that meets our criteria,
just have them supply the few bits we care about, and do everything else
internally.

This also lets us always use RGBA on GPU, but N32 on raster.

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

Committed:
https://skia.googlesource.com/skia/+/53c38087949252d27cde668368a3eeb59cc2eb00
==========

to

==========
Change SkSpecialImage::makeSurface and makeTightSurface to take output
properties (color space), bounds, and (optional) alphaType.

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space. All call sites were saying N32.

What we want to do is propagate the original device's color space, and
pick a good format from that. Rather than force all the clients to
jump through hoops constructing an SkImageInfo that meets our criteria,
just have them supply the few bits we care about, and do everything else
internally.

This also lets us always use RGBA on GPU, but N32 on raster.

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

Committed:
https://skia.googlesource.com/skia/+/53c38087949252d27cde668368a3eeb59cc2eb00
==========

[Brian Osman, 2016-09-23 20:02:40]

The CQ bit was checked by brianosman@google.com

[commit-bot: I haz the power, 2016-09-23 20:02:50]

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-09-23 20:04:07]

Description was changed from

==========
Change SkSpecialImage::makeSurface and makeTightSurface to take output
properties (color space), bounds, and (optional) alphaType.

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space. All call sites were saying N32.

What we want to do is propagate the original device's color space, and
pick a good format from that. Rather than force all the clients to
jump through hoops constructing an SkImageInfo that meets our criteria,
just have them supply the few bits we care about, and do everything else
internally.

This also lets us always use RGBA on GPU, but N32 on raster.

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

Committed:
https://skia.googlesource.com/skia/+/53c38087949252d27cde668368a3eeb59cc2eb00
==========

to

==========
Change SkSpecialImage::makeSurface and makeTightSurface to take output
properties (color space), bounds, and (optional) alphaType.

We were being pretty inconsistent before. Raster was honoring all
components of the info. GPU was using the supplied color type, but
propagating the source's color space. All call sites were saying N32.

What we want to do is propagate the original device's color space, and
pick a good format from that. Rather than force all the clients to
jump through hoops constructing an SkImageInfo that meets our criteria,
just have them supply the few bits we care about, and do everything else
internally.

This also lets us always use RGBA on GPU, but N32 on raster.

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

Committed:
https://skia.googlesource.com/skia/+/53c38087949252d27cde668368a3eeb59cc2eb00
Committed:
https://skia.googlesource.com/skia/+/eed6b0e1d865a1f93143c09961debba0aca592ca
==========

[commit-bot: I haz the power, 2016-09-23 20:04:08]

Committed patchset #9 (id:160001) as
https://skia.googlesource.com/skia/+/eed6b0e1d865a1f93143c09961debba0aca592ca