flag imageinfo as srgb

flag imageinfo as srgb

intended uses:
- flag a SkSurface as sRGB (only supported by Ganesh for now)
- flag images (e.g. png or jpeg) as sRGB if the codec tells us that

wins:
- faster gamma-correct text (esp. w/ distance-fields) when we can use sRGB for text
- better color fidelity when the screen really is sRGB

Committed: https://skia.googlesource.com/skia/+/7c74885e017a2473383fed72bd629cc07c773942

[reed1, 2014-10-24 20:16:15]

reed@google.com changed reviewers:
+ bsalomon@google.com, bungeman@google.com, djsollen@google.com

[djsollen, 2014-10-24 20:22:16]

WooHoo...lgtm

[reed1, 2014-10-24 20:33:57]

Ben is worried that sRGB applied to text may not make everyone happy, but I'd
sure like to try it and look at the quality.

[reed1, 2014-11-07 15:16:25]

reed@google.com changed reviewers:
+ robertphillips@google.com

[reed1, 2014-11-07 15:16:26]

any pointers on where I would wack GL to tell it that the uploaded texture is
sRGB?

[bsalomon, 2014-11-07 18:26:19]

On 2014/11/07 15:16:26, reed1 wrote:
> any pointers on where I would wack GL to tell it that the uploaded texture is
> sRGB?

You can try making GrGpuGL::configToGLFormats always return 0x8C42 for
internalFormat and externalFormat when the config is either rgba_8888 or
bgra_8888. I'm not totally sure that will work, but it might.

[robertphillips, 2014-11-07 18:38:56]

https://codereview.chromium.org/676883003/diff/60001/src/core/SkImageInfo.cpp
File src/core/SkImageInfo.cpp (right):

https://codereview.chromium.org/676883003/diff/60001/src/core/SkImageInfo.cpp...
src/core/SkImageInfo.cpp:28: uint32_t packed = buffer.read32();
Seems like this assert could give us trouble in the future.

[reed1, 2014-11-07 19:48:10]

https://codereview.chromium.org/676883003/diff/60001/src/core/SkImageInfo.cpp
File src/core/SkImageInfo.cpp (right):

https://codereview.chromium.org/676883003/diff/60001/src/core/SkImageInfo.cpp...
src/core/SkImageInfo.cpp:28: uint32_t packed = buffer.read32();
On 2014/11/07 18:38:56, robertphillips wrote:
> Seems like this assert could give us trouble in the future.

When we knowingly add more bits, we will change the assert.

[reed1, 2014-11-07 19:50:39]

https://codereview.chromium.org/676883003/diff/60001/src/core/SkImageInfo.cpp
File src/core/SkImageInfo.cpp (right):

https://codereview.chromium.org/676883003/diff/60001/src/core/SkImageInfo.cpp...
src/core/SkImageInfo.cpp:28: uint32_t packed = buffer.read32();
On 2014/11/07 19:48:10, reed1 wrote:
> On 2014/11/07 18:38:56, robertphillips wrote:
> > Seems like this assert could give us trouble in the future.
> 
> When we knowingly add more bits, we will change the assert.

I'm an idiot. Will now write a test (and then fix it). You're right.

[reed1, 2014-11-07 20:30:02]

ptal -- added test (and fix)

[robertphillips, 2014-11-10 13:45:59]

lgtm modulo test fixes.

https://codereview.chromium.org/676883003/diff/100001/tests/ImageIsOpaqueTest...
File tests/ImageIsOpaqueTest.cpp (right):

https://codereview.chromium.org/676883003/diff/100001/tests/ImageIsOpaqueTest...
tests/ImageIsOpaqueTest.cpp:28: 
set -> sets ?

https://codereview.chromium.org/676883003/diff/100001/tests/ImageIsOpaqueTest...
tests/ImageIsOpaqueTest.cpp:37: DEF_TEST(ImageInfo_flattening, reporter) {
kLastEnum_SkColorType ?

https://codereview.chromium.org/676883003/diff/100001/tests/ImageIsOpaqueTest...
tests/ImageIsOpaqueTest.cpp:38: for (int ct = 0; ct <=
kLastEnum_SkColorProfileType; ++ct) {
kLastEnum_SkAlphaType ?

[reed1, 2014-11-10 16:48:30]

https://codereview.chromium.org/676883003/diff/100001/tests/ImageIsOpaqueTest...
File tests/ImageIsOpaqueTest.cpp (right):

https://codereview.chromium.org/676883003/diff/100001/tests/ImageIsOpaqueTest...
tests/ImageIsOpaqueTest.cpp:28: 
On 2014/11/10 13:45:59, robertphillips wrote:
> set -> sets ?

Done.

https://codereview.chromium.org/676883003/diff/100001/tests/ImageIsOpaqueTest...
tests/ImageIsOpaqueTest.cpp:37: DEF_TEST(ImageInfo_flattening, reporter) {
On 2014/11/10 13:45:59, robertphillips wrote:
> kLastEnum_SkColorType ?

eeek, the bane of copy/paste

https://codereview.chromium.org/676883003/diff/100001/tests/ImageIsOpaqueTest...
tests/ImageIsOpaqueTest.cpp:38: for (int ct = 0; ct <=
kLastEnum_SkColorProfileType; ++ct) {
On 2014/11/10 13:45:59, robertphillips wrote:
> kLastEnum_SkAlphaType ?

Done.

[reed1, 2014-11-10 16:48:34]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-11-10 16:49:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/676883003/120001

[commit-bot: I haz the power, 2014-11-10 16:57:25]

Committed patchset #7 (id:120001) as
https://skia.googlesource.com/skia/+/7c74885e017a2473383fed72bd629cc07c773942

[mtklein, 2014-11-11 14:35:48]

On 2014/11/10 16:57:25, I haz the power (commit-bot) wrote:
> Committed patchset #7 (id:120001) as
> https://skia.googlesource.com/skia/+/7c74885e017a2473383fed72bd629cc07c773942

Have you noticed this from the iOS build?

FAILED: /usr/bin/clang++ -MMD -MF obj/src/ports/images.SkImageDecoder_CG.o.d
-DSK_INTERNAL -DSK_GAMMA_SRGB -DSK_GAMMA_APPLY_TO_A8
-DSK_SCALAR_TO_FLOAT_EXCLUDED -DSK_ALLOW_STATIC_GLOBAL_INITIALIZERS=1
-DSK_SUPPORT_GPU=1 -DSK_SUPPORT_OPENCL=0 -DSK_FORCE_DISTANCE_FIELD_TEXT=0
-DSK_SCALAR_IS_FLOAT -DSK_BUILD_FOR_IOS -DSK_USE_POSIX_THREADS
-DSK_CRASH_HANDLER -DSK_DEVELOPER=1 -I../../include/images -I../../src/lazy
-I../../src/core -I../../src/image -I../../include/utils/mac -I../../include/c
-I../../include/config -I../../include/core -I../../include/pathops
-I../../include/pipe -I../../include/utils/ios
-I../../third_party/externals/libjpeg -I../../third_party/etc1
-I../../third_party/ktx -I../../third_party/externals/libwebp/src
-I../../include/utils -I../../src/utils -isysroot
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS6.1.sdk
-fasm-blocks -mpascal-strings -O0 -gdwarf-2 -miphoneos-version-min=6.0 -arch
armv7 -Wno-invalid-offsetof -fvisibility=hidden -fvisibility-inlines-hidden
-Werror  -c ../../src/ports/SkImageDecoder_CG.cpp -o
obj/src/ports/images.SkImageDecoder_CG.o
../../src/ports/SkImageDecoder_CG.cpp:140:30: error: call to unavailable
function 'CGColorSpaceCopyICCProfile': not available on iOS
            CFDataRef data = CGColorSpaceCopyICCProfile(cs);
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS6.1.sdk/System/Library/Frameworks/CoreGraphics.framework/Headers/CGColorSpace.h:247:21:
note: candidate function has been explicitly made unavailable
CG_EXTERN CFDataRef CGColorSpaceCopyICCProfile(CGColorSpaceRef space)
                    ^

[reed1, 2014-11-11 14:50:08]

On 2014/11/11 14:35:48, mtklein wrote:
> On 2014/11/10 16:57:25, I haz the power (commit-bot) wrote:
> > Committed patchset #7 (id:120001) as
> >
https://skia.googlesource.com/skia/+/7c74885e017a2473383fed72bd629cc07c773942
> 
> Have you noticed this from the iOS build?
> 
> FAILED: /usr/bin/clang++ -MMD -MF obj/src/ports/images.SkImageDecoder_CG.o.d
> -DSK_INTERNAL -DSK_GAMMA_SRGB -DSK_GAMMA_APPLY_TO_A8
> -DSK_SCALAR_TO_FLOAT_EXCLUDED -DSK_ALLOW_STATIC_GLOBAL_INITIALIZERS=1
> -DSK_SUPPORT_GPU=1 -DSK_SUPPORT_OPENCL=0 -DSK_FORCE_DISTANCE_FIELD_TEXT=0
> -DSK_SCALAR_IS_FLOAT -DSK_BUILD_FOR_IOS -DSK_USE_POSIX_THREADS
> -DSK_CRASH_HANDLER -DSK_DEVELOPER=1 -I../../include/images -I../../src/lazy
> -I../../src/core -I../../src/image -I../../include/utils/mac -I../../include/c
> -I../../include/config -I../../include/core -I../../include/pathops
> -I../../include/pipe -I../../include/utils/ios
> -I../../third_party/externals/libjpeg -I../../third_party/etc1
> -I../../third_party/ktx -I../../third_party/externals/libwebp/src
> -I../../include/utils -I../../src/utils -isysroot
>
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS6.1.sdk
> -fasm-blocks -mpascal-strings -O0 -gdwarf-2 -miphoneos-version-min=6.0 -arch
> armv7 -Wno-invalid-offsetof -fvisibility=hidden -fvisibility-inlines-hidden
> -Werror  -c ../../src/ports/SkImageDecoder_CG.cpp -o
> obj/src/ports/images.SkImageDecoder_CG.o
> ../../src/ports/SkImageDecoder_CG.cpp:140:30: error: call to unavailable
> function 'CGColorSpaceCopyICCProfile': not available on iOS
>             CFDataRef data = CGColorSpaceCopyICCProfile(cs);
>                              ^~~~~~~~~~~~~~~~~~~~~~~~~~
>
/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS6.1.sdk/System/Library/Frameworks/CoreGraphics.framework/Headers/CGColorSpace.h:247:21:
> note: candidate function has been explicitly made unavailable
> CG_EXTERN CFDataRef CGColorSpaceCopyICCProfile(CGColorSpaceRef space)
>                     ^

Ah, thanks for pointing this out.

How can we know when we're building for iOS? I think in that world we can
actually assume that ALL images are already sRGB (or so I've heard).

[mtklein, 2014-11-11 14:51:40]

> How can we know when we're building for iOS? I think in that world we can
> actually assume that ALL images are already sRGB (or so I've heard).

SK_BUILD_FOR_IOS?