From 088f4b8eb31b2af26e66dcbaae062f40b3955059 Mon Sep 17 00:00:00 2001 From: reed Date: Thu, 28 Aug 2014 14:46:19 -0700 Subject: [PATCH] Revert of Revert of Revert of add gamma/sRGB to SkImageInfo (patchset #1 of https://codereview.chromium.org/511363002/) Reason for revert: android builder in chrome reveals the caller using { } initialization for SkImageInfo, which this CL now makes illegal. Will have to fix the call site before I can re-land this Original issue's description: > Revert of Revert of add gamma/sRGB to SkImageInfo (patchset #1 of https://codereview.chromium.org/512243002/) > > Reason for revert: > this revert was premature -- the chrome canary I looked at is old, and newer ones were green. So trying again... > > Original issue's description: > > Revert of add gamma/sRGB to SkImageInfo (patchset #4 of https://codereview.chromium.org/514753002/) > > > > Reason for revert: > > breaks linker on chrome -- may need SK_API, but not sure. > > > > Original issue's description: > > > add gamma/sRGB to SkImageInfo > > > > > > BUG=skia: > > > > > > Committed: https://skia.googlesource.com/skia/+/615c369777258231054840a88cdb74c68c382485 > > > > TBR=bungeman@google.com,bsalomon@google.com > > NOTREECHECKS=true > > NOTRY=true > > BUG=skia: > > > > Committed: https://skia.googlesource.com/skia/+/dc53701216c0e9f98eacf6c0cda769ebbd200b59 > > TBR=bungeman@google.com,bsalomon@google.com > NOTREECHECKS=true > NOTRY=true > BUG=skia: > > Committed: https://skia.googlesource.com/skia/+/e759a2662273fdf52ebbb3822229945eda317e46 R=bungeman@google.com, bsalomon@google.com TBR=bsalomon@google.com, bungeman@google.com NOTREECHECKS=true NOTRY=true BUG=skia: Author: reed@google.com Review URL: https://codereview.chromium.org/517113002 --- gyp/tests.gypi | 1 - include/core/SkImageInfo.h | 88 +++++++------------------ src/core/SkImageInfo.cpp | 132 ++++++------------------------------- tests/ImageInfoTest.cpp | 64 ------------------ 4 files changed, 45 insertions(+), 240 deletions(-) delete mode 100644 tests/ImageInfoTest.cpp diff --git a/gyp/tests.gypi b/gyp/tests.gypi index e4f891663..c3b559e88 100644 --- a/gyp/tests.gypi +++ b/gyp/tests.gypi @@ -119,7 +119,6 @@ '../tests/ImageDecodingTest.cpp', '../tests/ImageFilterTest.cpp', '../tests/ImageGeneratorTest.cpp', - '../tests/ImageInfoTest.cpp', '../tests/ImageIsOpaqueTest.cpp', '../tests/ImageNewShaderTest.cpp', '../tests/InfRectTest.cpp', diff --git a/include/core/SkImageInfo.h b/include/core/SkImageInfo.h index 82d24753a..3d82dc805 100644 --- a/include/core/SkImageInfo.h +++ b/include/core/SkImageInfo.h @@ -136,69 +136,36 @@ bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType, * Describe an image's dimensions and pixel type. */ struct SkImageInfo { -public: - SkImageInfo() {} - int fWidth; int fHeight; SkColorType fColorType; SkAlphaType fAlphaType; -private: - enum Profile { - kUnknown_Profile, - kSRGB_Profile, - kExponential_Profile, - }; - - uint32_t fProfile; - float fGamma; - - SkImageInfo(int width, int height, SkColorType ct, SkAlphaType at, Profile p, float g) - : fWidth(width) - , fHeight(height) - , fColorType(ct) - , fAlphaType(at) - , fProfile(p) - , fGamma(g) - {} - -public: - /* - * Return an info with the specified attributes, tagged as sRGB. Note that if the requested - * color type does not make sense with sRGB (e.g. kAlpha_8) then the sRGB request is ignored. - * - * You can call isSRGB() on the returned info to determine if the request was fulfilled. - */ - static SkImageInfo MakeSRGB(int width, int height, SkColorType ct, SkAlphaType at); - - /* - * Return an info with the specified attributes, tagged with a specific gamma. - * Note that if the requested gamma is unsupported for the requested color type, then the gamma - * value will be set to 1.0 (the default). - * - * You can call gamma() to query the resulting gamma value. - */ - static SkImageInfo MakeWithGamma(int width, int height, SkColorType ct, SkAlphaType at, - float gamma); - static SkImageInfo Make(int width, int height, SkColorType ct, SkAlphaType at) { - return MakeWithGamma(width, height, ct, at, 1); + SkImageInfo info = { + width, height, ct, at + }; + return info; } /** * Sets colortype to the native ARGB32 type. */ static SkImageInfo MakeN32(int width, int height, SkAlphaType at) { - return SkImageInfo(width, height, kN32_SkColorType, at, kExponential_Profile, 1); + SkImageInfo info = { + width, height, kN32_SkColorType, at + }; + return info; } /** * Sets colortype to the native ARGB32 type, and the alphatype to premul. */ static SkImageInfo MakeN32Premul(int width, int height) { - return SkImageInfo(width, height, kN32_SkColorType, kPremul_SkAlphaType, - kExponential_Profile, 1); + SkImageInfo info = { + width, height, kN32_SkColorType, kPremul_SkAlphaType + }; + return info; } /** @@ -209,17 +176,24 @@ public: } static SkImageInfo MakeA8(int width, int height) { - return SkImageInfo(width, height, kAlpha_8_SkColorType, kPremul_SkAlphaType, - kUnknown_Profile, 0); + SkImageInfo info = { + width, height, kAlpha_8_SkColorType, kPremul_SkAlphaType + }; + return info; } static SkImageInfo MakeUnknown(int width, int height) { - return SkImageInfo(width, height, kUnknown_SkColorType, kIgnore_SkAlphaType, - kUnknown_Profile, 0); + SkImageInfo info = { + width, height, kUnknown_SkColorType, kIgnore_SkAlphaType + }; + return info; } static SkImageInfo MakeUnknown() { - return SkImageInfo(0, 0, kUnknown_SkColorType, kIgnore_SkAlphaType, kUnknown_Profile, 0); + SkImageInfo info = { + 0, 0, kUnknown_SkColorType, kIgnore_SkAlphaType + }; + return info; } int width() const { return fWidth; } @@ -262,11 +236,8 @@ public: return 0 != memcmp(this, &other, sizeof(other)); } - // DEPRECATED : use the static Unflatten void unflatten(SkReadBuffer&); void flatten(SkWriteBuffer&) const; - - static SkImageInfo Unflatten(SkReadBuffer&); int64_t getSafeSize64(size_t rowBytes) const { if (0 == fHeight) { @@ -285,17 +256,6 @@ public: } SkDEBUGCODE(void validate() const;) - - /** - * If the Info was tagged to be sRGB, return true, else return false. - */ - bool isSRGB() const { return kSRGB_Profile == fProfile; } - - /** - * If this was tagged with an explicit gamma value, return that value, else return 0. - * If this was tagged as sRGB, return 0. - */ - float gamma() const { return fGamma; } }; #endif diff --git a/src/core/SkImageInfo.cpp b/src/core/SkImageInfo.cpp index 44fd808dc..e61cd7d45 100644 --- a/src/core/SkImageInfo.cpp +++ b/src/core/SkImageInfo.cpp @@ -9,53 +9,34 @@ #include "SkReadBuffer.h" #include "SkWriteBuffer.h" -static bool color_type_supports_sRGB(SkColorType colorType) { - switch (colorType) { - case kRGBA_8888_SkColorType: - case kBGRA_8888_SkColorType: - return true; - default: - return false; - } +static bool alpha_type_is_valid(SkAlphaType alphaType) { + return (alphaType >= 0) && (alphaType <= kLastEnum_SkAlphaType); } -static bool color_type_supports_gamma(SkColorType colorType) { - switch (colorType) { - case kRGBA_8888_SkColorType: - case kBGRA_8888_SkColorType: - // case kLuminance ... - return true; - default: - return false; - } +static bool color_type_is_valid(SkColorType colorType) { + return (colorType >= 0) && (colorType <= kLastEnum_SkColorType); } -static float pin_gamma_to_legal(float gamma) { - if (!SkScalarIsFinite(gamma)) { - return 1; - } - // these limits are just made up -- feel free to change them within reason - const float min_gamma = 0.01f; - const float max_gamma = 4.0; - return SkScalarPin(gamma, min_gamma, max_gamma); +void SkImageInfo::unflatten(SkReadBuffer& buffer) { + fWidth = buffer.read32(); + fHeight = buffer.read32(); + + uint32_t packed = buffer.read32(); + SkASSERT(0 == (packed >> 16)); + fAlphaType = (SkAlphaType)((packed >> 8) & 0xFF); + fColorType = (SkColorType)((packed >> 0) & 0xFF); + buffer.validate(alpha_type_is_valid(fAlphaType) && + color_type_is_valid(fColorType)); } -SkImageInfo SkImageInfo::MakeSRGB(int width, int height, SkColorType ct, SkAlphaType at) { - Profile p = color_type_supports_sRGB(ct) ? kSRGB_Profile : kUnknown_Profile; - return SkImageInfo(width, height, ct, at, p, 0); -} +void SkImageInfo::flatten(SkWriteBuffer& buffer) const { + buffer.write32(fWidth); + buffer.write32(fHeight); -SkImageInfo SkImageInfo::MakeWithGamma(int width, int height, SkColorType ct, SkAlphaType at, - float gamma) { - Profile p; - if (color_type_supports_gamma(ct)) { - gamma = pin_gamma_to_legal(gamma); - p = kExponential_Profile; - } else { - p = kUnknown_Profile; - gamma = 0; - } - return SkImageInfo(width, height, ct, at, p, gamma); + SkASSERT(0 == (fAlphaType & ~0xFF)); + SkASSERT(0 == (fColorType & ~0xFF)); + uint32_t packed = (fAlphaType << 8) | fColorType; + buffer.write32(packed); } bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType, @@ -88,74 +69,3 @@ bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType, } return true; } - -void SkImageInfo::unflatten(SkReadBuffer& buffer) { - *this = Unflatten(buffer); -} - -//////////////////////////////////////////////////////////////////////////////////////////// - -static bool alpha_type_is_valid(SkAlphaType alphaType) { - return (alphaType >= 0) && (alphaType <= kLastEnum_SkAlphaType); -} - -static bool color_type_is_valid(SkColorType colorType) { - return (colorType >= 0) && (colorType <= kLastEnum_SkColorType); -} - -static float igamma_to_gamma(int gamma3dot9) { - return gamma3dot9 / 512.0f; -} - -static unsigned gamma_to_igamma(float gamma) { - SkASSERT(gamma >= 0 && gamma < 8); - int igamma = SkScalarRoundToInt(gamma * 512); - SkASSERT(igamma >= 0 && igamma <= 0xFFF); - return igamma; -} - -SkImageInfo SkImageInfo::Unflatten(SkReadBuffer& buffer) { - int width = buffer.read32(); - int height = buffer.read32(); - uint32_t packed = buffer.read32(); - - SkColorType ct = (SkColorType)((packed >> 0) & 0xFF); // 8 bits for colortype - SkAlphaType at = (SkAlphaType)((packed >> 8) & 0xFF); // 8 bits for alphatype - if (!alpha_type_is_valid(at) || !color_type_is_valid(ct)) { - return MakeUnknown(); - } - - // Earlier formats always stored 0 in the upper 16 bits. That corresponds to - // days before we had gamma/profile. That happens to correspond to kUnknown_Profile, - // which means we can just ignore the gamma value anyways. - // - int iprofile = ((packed >> 16) & 0xF); // 4 bits for profile - - switch (iprofile) { - case kUnknown_Profile: - return Make(width, height, ct, at); - case kSRGB_Profile: - return MakeSRGB(width, height, ct, at); - case kExponential_Profile: { - int igamma = packed >> 20; // 12 bits for gamma 3.9 - float gamma = igamma_to_gamma(igamma); - return MakeWithGamma(width, height, ct, at, gamma); - } - default: - (void)buffer.validate(false); - return MakeUnknown(); - } -} - -void SkImageInfo::flatten(SkWriteBuffer& buffer) const { - buffer.write32(fWidth); - buffer.write32(fHeight); - - SkASSERT(0 == (fColorType & ~0xFF)); // 8 bits for colortype - SkASSERT(0 == (fAlphaType & ~0xFF)); // 8 bits for alphatype - SkASSERT(0 == (fProfile & ~0xF)); // 4 bits for profile - int igamma = gamma_to_igamma(fGamma); // 12 bits for gamma (if needed) - - uint32_t packed = (igamma << 20) | (fProfile << 16) | (fAlphaType << 8) | fColorType; - buffer.write32(packed); -} diff --git a/tests/ImageInfoTest.cpp b/tests/ImageInfoTest.cpp deleted file mode 100644 index 679b302ce..000000000 --- a/tests/ImageInfoTest.cpp +++ /dev/null @@ -1,64 +0,0 @@ -/* - * Copyright 2014 Google Inc. - * - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - */ - -#include "SkImageInfo.h" - -#include "Test.h" - -struct ImageInfoRec { - int fWidth; - int fHeight; - SkColorType fColorType; - SkAlphaType fAlphaType; - float fGamma; - bool fIsSRGB; -}; - -static void check_info(skiatest::Reporter* reporter, - const ImageInfoRec& expected, const SkImageInfo& info) { - REPORTER_ASSERT(reporter, info.width() == expected.fWidth); - REPORTER_ASSERT(reporter, info.height() == expected.fHeight); - REPORTER_ASSERT(reporter, info.colorType() == expected.fColorType); - REPORTER_ASSERT(reporter, info.alphaType() == expected.fAlphaType); - REPORTER_ASSERT(reporter, info.gamma() == expected.fGamma); - REPORTER_ASSERT(reporter, info.isSRGB() == expected.fIsSRGB); -} - -DEF_TEST(ImageInfo, reporter) { - const float nan = SK_ScalarNaN; - const float nice_gamma = 1.5f; - const int W = 100; - const int H = 200; - SkImageInfo info; - - const ImageInfoRec rec[] = { - { 0, 0, kUnknown_SkColorType, kIgnore_SkAlphaType, 0, false }, // MakeUnknown() - { W, H, kUnknown_SkColorType, kIgnore_SkAlphaType, 0, false }, // MakeUnknown(...) - { W, H, kN32_SkColorType, kPremul_SkAlphaType, 1, false }, // MakeN32Premul(...) - { W, H, kN32_SkColorType, kOpaque_SkAlphaType, 1, false }, // MakeN32(...) - { W, H, kAlpha_8_SkColorType, kPremul_SkAlphaType, 0, false }, // MakeA8() - { W, H, kRGBA_8888_SkColorType, kUnpremul_SkAlphaType, 1, false }, // Make() - { W, H, kBGRA_8888_SkColorType, kPremul_SkAlphaType, 1, false }, // Make() - { W, H, kBGRA_8888_SkColorType, kPremul_SkAlphaType, 0, true }, // MakeSRGB() - { W, H, kN32_SkColorType, kPremul_SkAlphaType, 1, false }, // MakeWithGamma() NaN - { W, H, kAlpha_8_SkColorType, kPremul_SkAlphaType, 0, false }, // MakeWithGamma() bad ct for gamma - { W, H, kN32_SkColorType, kPremul_SkAlphaType, nice_gamma, false }, // MakeWithGamma() good - }; - - check_info(reporter, rec[ 0], SkImageInfo::MakeUnknown()); - check_info(reporter, rec[ 1], SkImageInfo::MakeUnknown(W, H)); - check_info(reporter, rec[ 2], SkImageInfo::MakeN32Premul(W, H)); - check_info(reporter, rec[ 3], SkImageInfo::MakeN32(W, H, rec[3].fAlphaType)); - check_info(reporter, rec[ 4], SkImageInfo::MakeA8(W, H)); - check_info(reporter, rec[ 5], SkImageInfo::Make(W, H, rec[5].fColorType, rec[5].fAlphaType)); - check_info(reporter, rec[ 6], SkImageInfo::Make(W, H, rec[6].fColorType, rec[6].fAlphaType)); - check_info(reporter, rec[ 7], SkImageInfo::MakeSRGB(W, H, rec[7].fColorType, rec[7].fAlphaType)); - check_info(reporter, rec[ 8], SkImageInfo::MakeWithGamma(W, H, rec[8].fColorType, rec[8].fAlphaType, nan)); - check_info(reporter, rec[ 9], SkImageInfo::MakeWithGamma(W, H, rec[9].fColorType, rec[9].fAlphaType, nice_gamma)); - check_info(reporter, rec[10], SkImageInfo::MakeWithGamma(W, H, rec[10].fColorType, rec[10].fAlphaType, rec[10].fGamma)); -} -