Revert of Delete SkColorSpace::kUnknown_Named, remove fNamed field (patchset #1 id:20001 of https://codereview.chromium.org/2302413002/ )

Reason for revert:
This CL introduced two static initializers (gAdobeRGB and gSRGB) which are causing a sizes regression on Chromium builders:

https://build.chromium.org/p/chromium/builders/Linux%20x64/builds/24981

Original issue's description:
> Delete SkColorSpace::kUnknown_Named, remove fNamed field
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2302413002
>
> Committed: https://skia.googlesource.com/skia/+/54682e856cb66c653bc7e253981a421a2618398e

TBR=reed@google.com,brianosman@google.com,msarett@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=skia:5724

Review-Url: https://codereview.chromium.org/2306313002
This commit is contained in:
mgiuca 2016-09-05 04:58:55 -07:00 committed by Commit bot
parent 3b4bc7465a
commit 008fbd1b8e
4 changed files with 56 additions and 45 deletions

View File

@ -19,21 +19,13 @@ public:
/**
* Common, named profiles that we can recognize.
*/
enum Named : uint8_t {
/**
* By far the most common color space.
* This is the default space for images, unmarked content, and monitors.
*/
enum Named {
kUnknown_Named,
kSRGB_Named,
/**
* Very common wide gamut color space.
* Often used by images and monitors.
*/
kAdobeRGB_Named,
};
enum GammaNamed : uint8_t {
enum GammaNamed {
kLinear_GammaNamed,
/**
@ -124,10 +116,11 @@ public:
static bool Equals(const SkColorSpace* src, const SkColorSpace* dst);
protected:
SkColorSpace(GammaNamed gammaNamed, const SkMatrix44& toXYZD50);
SkColorSpace(GammaNamed gammaNamed, const SkMatrix44& toXYZD50, Named named);
const GammaNamed fGammaNamed;
const SkMatrix44 fToXYZD50;
const Named fNamed;
};
#endif

View File

@ -10,13 +10,14 @@
#include "SkColorSpacePriv.h"
#include "SkOnce.h"
SkColorSpace::SkColorSpace(GammaNamed gammaNamed, const SkMatrix44& toXYZD50)
SkColorSpace::SkColorSpace(GammaNamed gammaNamed, const SkMatrix44& toXYZD50, Named named)
: fGammaNamed(gammaNamed)
, fToXYZD50(toXYZD50)
, fNamed(named)
{}
SkColorSpace_Base::SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& toXYZD50)
: INHERITED(gammaNamed, toXYZD50)
SkColorSpace_Base::SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& toXYZD50, Named named)
: INHERITED(gammaNamed, toXYZD50, named)
, fGammas(nullptr)
, fProfileData(nullptr)
{}
@ -24,7 +25,7 @@ SkColorSpace_Base::SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& to
SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, GammaNamed gammaNamed,
sk_sp<SkGammas> gammas, const SkMatrix44& toXYZD50,
sk_sp<SkData> profileData)
: INHERITED(gammaNamed, toXYZD50)
: INHERITED(gammaNamed, toXYZD50, kUnknown_Named)
, fColorLUT(std::move(colorLUT))
, fGammas(std::move(gammas))
, fProfileData(std::move(profileData))
@ -117,19 +118,18 @@ sk_sp<SkColorSpace> SkColorSpace_Base::NewRGB(GammaNamed gammaNamed, const SkMat
break;
}
return sk_sp<SkColorSpace>(new SkColorSpace_Base(gammaNamed, toXYZD50));
return sk_sp<SkColorSpace>(new SkColorSpace_Base(gammaNamed, toXYZD50, kUnknown_Named));
}
sk_sp<SkColorSpace> SkColorSpace::NewRGB(GammaNamed gammaNamed, const SkMatrix44& toXYZD50) {
return SkColorSpace_Base::NewRGB(gammaNamed, toXYZD50);
}
static sk_sp<SkColorSpace> gAdobeRGB;
static sk_sp<SkColorSpace> gSRGB;
sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
static SkOnce sRGBOnce;
static sk_sp<SkColorSpace> sRGB;
static SkOnce adobeRGBOnce;
static sk_sp<SkColorSpace> adobeRGB;
switch (named) {
case kSRGB_Named: {
@ -139,9 +139,9 @@ sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
// Force the mutable type mask to be computed. This avoids races.
(void)srgbToxyzD50.getType();
gSRGB.reset(new SkColorSpace_Base(kSRGB_GammaNamed, srgbToxyzD50));
sRGB.reset(new SkColorSpace_Base(kSRGB_GammaNamed, srgbToxyzD50, kSRGB_Named));
});
return gSRGB;
return sRGB;
}
case kAdobeRGB_Named: {
adobeRGBOnce([] {
@ -150,9 +150,10 @@ sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
// Force the mutable type mask to be computed. This avoids races.
(void)adobergbToxyzD50.getType();
gAdobeRGB.reset(new SkColorSpace_Base(k2Dot2Curve_GammaNamed, adobergbToxyzD50));
adobeRGB.reset(new SkColorSpace_Base(k2Dot2Curve_GammaNamed, adobergbToxyzD50,
kAdobeRGB_Named));
});
return gAdobeRGB;
return adobeRGB;
}
default:
break;
@ -193,8 +194,8 @@ struct ColorSpaceHeader {
*/
static constexpr uint8_t kFloatGamma_Flag = 1 << 2;
static ColorSpaceHeader Pack(Version version, uint8_t named, uint8_t gammaNamed, uint8_t flags)
{
static ColorSpaceHeader Pack(Version version, SkColorSpace::Named named,
SkColorSpace::GammaNamed gammaNamed, uint8_t flags) {
ColorSpaceHeader header;
SkASSERT(k0_Version == version);
@ -222,17 +223,17 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
// we must have a profile that we can serialize easily.
if (!as_CSB(this)->fProfileData) {
// If we have a named profile, only write the enum.
if (this == gSRGB.get()) {
if (memory) {
*((ColorSpaceHeader*) memory) =
ColorSpaceHeader::Pack(k0_Version, kSRGB_Named, fGammaNamed, 0);
}
return sizeof(ColorSpaceHeader);
} else if (this == gAdobeRGB.get()) {
if (memory) {
*((ColorSpaceHeader*) memory) =
ColorSpaceHeader::Pack(k0_Version, kAdobeRGB_Named, fGammaNamed, 0);
switch (fNamed) {
case kSRGB_Named:
case kAdobeRGB_Named: {
if (memory) {
*((ColorSpaceHeader*) memory) =
ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed, 0);
}
return sizeof(ColorSpaceHeader);
}
default:
break;
}
// If we have a named gamma, write the enum and the matrix.
@ -242,7 +243,7 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
case kLinear_GammaNamed: {
if (memory) {
*((ColorSpaceHeader*) memory) =
ColorSpaceHeader::Pack(k0_Version, 0, fGammaNamed,
ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
ColorSpaceHeader::kMatrix_Flag);
memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
fToXYZD50.as4x3ColMajorf((float*) memory);
@ -253,7 +254,7 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
// Otherwise, write the gamma values and the matrix.
if (memory) {
*((ColorSpaceHeader*) memory) =
ColorSpaceHeader::Pack(k0_Version, 0, fGammaNamed,
ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
ColorSpaceHeader::kFloatGamma_Flag);
memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
@ -280,7 +281,7 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
}
if (memory) {
*((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(k0_Version, 0,
*((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(k0_Version, kUnknown_Named,
kNonStandard_GammaNamed,
ColorSpaceHeader::kICC_Flag);
memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
@ -313,8 +314,12 @@ sk_sp<SkColorSpace> SkColorSpace::Deserialize(const void* data, size_t length) {
ColorSpaceHeader header = *((const ColorSpaceHeader*) data);
data = SkTAddOffset<const void>(data, sizeof(ColorSpaceHeader));
length -= sizeof(ColorSpaceHeader);
if (0 == header.fFlags) {
return NewNamed((Named) header.fNamed);
switch ((Named) header.fNamed) {
case kSRGB_Named:
case kAdobeRGB_Named:
return NewNamed((Named) header.fNamed);
default:
break;
}
switch ((GammaNamed) header.fGammaNamed) {
@ -377,6 +382,17 @@ bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
return false;
}
switch (src->fNamed) {
case kSRGB_Named:
case kAdobeRGB_Named:
return src->fNamed == dst->fNamed;
case kUnknown_Named:
if (kUnknown_Named != dst->fNamed) {
return false;
}
break;
}
SkData* srcData = as_CSB(src)->fProfileData.get();
SkData* dstData = as_CSB(dst)->fProfileData.get();
if (srcData || dstData) {

View File

@ -193,7 +193,7 @@ private:
static sk_sp<SkColorSpace> NewRGB(GammaNamed gammaNamed, const SkMatrix44& toXYZD50);
SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& toXYZ);
SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& toXYZ, Named named);
SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, GammaNamed gammaNamed,
sk_sp<SkGammas> gammas, const SkMatrix44& toXYZ, sk_sp<SkData> profileData);

View File

@ -149,15 +149,17 @@ DEF_TEST(ColorSpaceWriteICC, r) {
DEF_TEST(ColorSpace_Named, r) {
const struct {
SkColorSpace::Named fNamed;
bool fExpectedToSucceed;
bool fIsSRGB;
} recs[] {
{ SkColorSpace::kSRGB_Named, true },
{ SkColorSpace::kAdobeRGB_Named, false },
{ SkColorSpace::kUnknown_Named, false, false },
{ SkColorSpace::kSRGB_Named, true, true },
{ SkColorSpace::kAdobeRGB_Named, true, false },
};
for (auto rec : recs) {
auto cs = SkColorSpace::NewNamed(rec.fNamed);
REPORTER_ASSERT(r, cs);
REPORTER_ASSERT(r, !cs == !rec.fExpectedToSucceed);
if (cs) {
if (rec.fIsSRGB) {
REPORTER_ASSERT(r, SkColorSpace::kSRGB_GammaNamed == cs->gammaNamed());