Fix sRGB textures that must be copied (for NPOT tiling, etc...)

Fix sRGB textures that must be copied (for NPOT tiling, etc...)

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1969923002

Committed: https://skia.googlesource.com/skia/+/97fc9308e31bf03799725f998841212cd6231cdd

[Brian Osman, 2016-05-11 21:48:20]

Description was changed from

==========
Fix sRGB textures that must be copied (for NPOT tiling, etc...)

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

to

==========
Fix sRGB textures that must be copied (for NPOT tiling, etc...)

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

[Brian Osman, 2016-05-11 21:48:31]

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

[commit-bot: I haz the power, 2016-05-11 21:48:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1969923002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1969923002/1

[Brian Osman, 2016-05-11 21:53:45]

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

[Brian Osman, 2016-05-11 21:53:46]

I'm doing some testing to make sure this doesn't break any GMs/SKPs, but this
fixes an issue with the gamma GM on Vulkan right now (Vulkan currently has
NPOT-Tiling marked as not supported).

[egdaniel, 2016-05-11 22:04:46]

I assume vulkan should have no problem with NPOT textures. This just points out
we really need to go back and get all the caps set correctly.

https://codereview.chromium.org/1969923002/diff/1/src/gpu/GrTextureParamsAdju...
File src/gpu/GrTextureParamsAdjuster.cpp (right):

https://codereview.chromium.org/1969923002/diff/1/src/gpu/GrTextureParamsAdju...
src/gpu/GrTextureParamsAdjuster.cpp:75: paint.setGammaCorrect(true);
so you have no conditional around this, so we always want to setGammaCorrect for
true here. If we want all our copies to be gamma correct is this also true for
generic GrPaints? If so why do we need to set this instead of it just being an
implicit property.

[commit-bot: I haz the power, 2016-05-11 22:18:12]

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

[commit-bot: I haz the power, 2016-05-11 22:18:12]

Dry run: This issue passed the CQ dry run.

[Brian Osman, 2016-05-12 01:01:35]

https://codereview.chromium.org/1969923002/diff/1/src/gpu/GrTextureParamsAdju...
File src/gpu/GrTextureParamsAdjuster.cpp (right):

https://codereview.chromium.org/1969923002/diff/1/src/gpu/GrTextureParamsAdju...
src/gpu/GrTextureParamsAdjuster.cpp:75: paint.setGammaCorrect(true);
On 2016/05/11 22:04:46, egdaniel wrote:
> so you have no conditional around this, so we always want to setGammaCorrect
for
> true here. If we want all our copies to be gamma correct is this also true for
> generic GrPaints? If so why do we need to set this instead of it just being an
> implicit property.

This sets two flags on the paint (one that basically allows sRGB inputs, and one
for sRGB output). Normally, those are configured from the drawContext when we
convert an SkPaint to GrPaint. I made the defaults be as "safe" as possible,
such that we would never accidentally break "Legacy" mode (and we would have to
opt-in to being gamma-correct as we discovered locations where it's necessary).
In other places, it could alter the outcome and produce different results.

Here, we're really just copying a texture to another texture with the same
format, so as long as the paint causes the resulting draw operation to be
equivalent to a memcpy, we're good. Unfortunately, the default values on GrPaint
don't do that (allowSRGBInputs is false, so we don't accidentally decode an sRGB
texture in the legacy pipeline, but allowSRGBOutput is true, because you want
that 99.9% of the time, and it's not "dangerous" to the legacy pipeline - it
only affects sRGB render targets which are - by definition - gamma correct).

[bsalomon, 2016-05-12 14:51:29]

lgtm

[Brian Osman, 2016-05-12 14:52:48]

The CQ bit was checked by brianosman@google.com

[commit-bot: I haz the power, 2016-05-12 14:52:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1969923002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1969923002/1

[commit-bot: I haz the power, 2016-05-12 14:53:48]

Description was changed from

==========
Fix sRGB textures that must be copied (for NPOT tiling, etc...)

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...
==========

to

==========
Fix sRGB textures that must be copied (for NPOT tiling, etc...)

BUG=skia:
GOLD_TRYBOT_URL=
https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is...

Committed:
https://skia.googlesource.com/skia/+/97fc9308e31bf03799725f998841212cd6231cdd
==========

[commit-bot: I haz the power, 2016-05-12 14:53:49]

Committed patchset #1 (id:1) as
https://skia.googlesource.com/skia/+/97fc9308e31bf03799725f998841212cd6231cdd