From 64ba5fa1ff428858f803523257cd862f8b33423b Mon Sep 17 00:00:00 2001 From: reed Date: Fri, 29 Aug 2014 07:50:28 -0700 Subject: Revert of Add gamma/sRGB tags to SkImageInfo (patchset #1 of https://codereview.chromium.org/517123002/) Reason for revert: Seems to be triggering assert in blink SSLUITest.TestRedirectHTTPToBadHTTPS (run #1): [ RUN ] SSLUITest.TestRedirectHTTPToBadHTTPS HTTP server started on http://127.0.0.1:58000... sending server_data: {"host": "127.0.0.1", "port": 58000} (36 bytes) HTTPS server started on https://127.0.0.1:58009... sending server_data: {"host": "127.0.0.1", "port": 58009} (36 bytes) ASSERTION FAILED: info.fAlphaType == m_imageInfo.fAlphaType ../../third_party/WebKit/Source/platform/graphics/DecodingImageGenerator.cpp(78) : virtual bool blink::DecodingImageGenerator::onGetPixels(const SkImageInfo &, void *, size_t, SkPMColor *, int *) 1 0x77eb0d3 blink::DecodingImageGenerator::onGetPixels(SkImageInfo const&, void*, unsigned long, unsigned int*, int*) 2 0x92edddc SkImageGenerator::getPixels(SkImageInfo const&, void*, unsigned long, unsigned int*, int*) 3 0x92adf78 SkDiscardablePixelRef::onNewLockPixels(SkPixelRef::LockRec*) 4 0x9369283 SkPixelRef::lockPixels(SkPixelRef::LockRec*) 5 0x9369433 SkPixelRef::lockPixels() 6 0x9213344 SkBitmap::lockPixels() const 7 0x921ca57 SkAutoLockPixels::SkAutoLockPixels(SkBitmap const&, bool) 8 0x921ad80 SkAutoLockPixels::SkAutoLockPixels(SkBitmap const&, bool) 9 0x92b7125 SkDraw::drawBitmap(SkBitmap const&, SkMatrix const&, SkPaint const&) const 10 0x921f4fb SkBitmapDevice::drawBitmap(SkDraw const&, SkBitmap const&, SkMatrix const&, SkPaint const&) 11 0x921f8c7 SkBitmapDevice::drawBitmapRect(SkDraw const&, SkBitmap const&, SkRect const*, SkRect const&, SkPaint const&, SkCanvas::DrawBitmapRectFlags) 12 0x9288e12 SkCanvas::internalDrawBitmapRect(SkBitmap const&, SkRect const*, SkRect const&, SkPaint const*, SkCanvas::DrawBitmapRectFlags) 13 0x9288ee9 SkCanvas::drawBitmapRectToRect(SkBitmap const&, SkRect const*, SkRect const&, SkPaint const*, SkCanvas::DrawBitmapRectFlags) Original issue's description: > Add gamma/sRGB tags to SkImageInfo > > requires this CL to land in chrome > https://codereview.chromium.org/517803002/ > > BUG=skia: > > Committed: https://skia.googlesource.com/skia/+/228b285ba14a6e9b6d1cc95ea1583caab30168a1 R=fmalita@google.com, fmalita@chromium.org, reed@chromium.org TBR=fmalita@chromium.org, fmalita@google.com, reed@chromium.org NOTREECHECKS=true NOTRY=true BUG=skia: Author: reed@google.com Review URL: https://codereview.chromium.org/519583004 --- gyp/tests.gypi | 1 - include/core/SkImageInfo.h | 89 +++++++++--------------------- src/core/SkImageInfo.cpp | 132 ++++++++------------------------------------- tests/ImageInfoTest.cpp | 64 ---------------------- 4 files changed, 46 insertions(+), 240 deletions(-) delete mode 100644 tests/ImageInfoTest.cpp diff --git a/gyp/tests.gypi b/gyp/tests.gypi index e4f8916637..c3b559e884 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 7a56fb4557..3d82dc805c 100644 --- a/include/core/SkImageInfo.h +++ b/include/core/SkImageInfo.h @@ -135,50 +135,37 @@ bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType, /** * Describe an image's dimensions and pixel type. */ -struct SK_API SkImageInfo { -public: - SkImageInfo() {} - +struct SkImageInfo { int fWidth; int fHeight; SkColorType fColorType; SkAlphaType fAlphaType; - /* - * 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; } /** @@ -189,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; } @@ -242,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) { @@ -265,36 +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; } - -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) - {} }; #endif diff --git a/src/core/SkImageInfo.cpp b/src/core/SkImageInfo.cpp index 44fd808dc8..e61cd7d45f 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(); -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); + 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::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); +void SkImageInfo::flatten(SkWriteBuffer& buffer) const { + buffer.write32(fWidth); + buffer.write32(fHeight); + + 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 679b302ceb..0000000000 --- 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)); -} - -- cgit v1.2.3