Revert of Introduce SkGammas type to represent ICC gamma curves (patchset #7 id:40002 of https://codereview.chromium.org/1928123002/ )

Reason for revert:
Breaks TSAN
https://build.chromium.org/p/client.skia/builders/Test-Ubuntu-GCC-GCE-CPU-AVX2-x86_64-Release-TSAN/builds/6387/steps/dm/logs/stdio

Original issue's description:
> Introduce SkGammas type to represent ICC gamma curves
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1928123002
>
> Committed: https://skia.googlesource.com/skia/+/7b2c6dd8c918209cb92e1338905d511c68da3eb2

TBR=scroggo@google.com,reed@google.com,brianosman@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

Review-Url: https://codereview.chromium.org/1933863002
This commit is contained in:
msarett 2016-04-29 11:12:22 -07:00 committed by Commit bot
parent 7b2c6dd8c9
commit a2ffa98323
4 changed files with 102 additions and 164 deletions

View File

@ -210,7 +210,7 @@ sk_sp<SkColorSpace> read_color_space(png_structp png_ptr, png_infop info_ptr) {
png_fixed_point XYZ[9];
SkFloat3x3 toXYZD50;
png_fixed_point gamma;
SkColorSpace::SkGammas gammas;
SkFloat3 gammas;
if (png_get_cHRM_XYZ_fixed(png_ptr, info_ptr, &XYZ[0], &XYZ[1], &XYZ[2], &XYZ[3], &XYZ[4],
&XYZ[5], &XYZ[6], &XYZ[7], &XYZ[8])) {
@ -225,17 +225,16 @@ sk_sp<SkColorSpace> read_color_space(png_structp png_ptr, png_infop info_ptr) {
}
if (PNG_INFO_gAMA == png_get_gAMA_fixed(png_ptr, info_ptr, &gamma)) {
float value = png_inverted_fixed_point_to_float(gamma);
gammas = SkColorSpace::SkGammas(value, value, value);
gammas.fVec[0] = gammas.fVec[1] = gammas.fVec[2] =
png_inverted_fixed_point_to_float(gamma);
} else {
// Default to sRGB (gamma = 2.2f) if the image has color space information,
// but does not specify gamma.
gammas = SkColorSpace::SkGammas(2.2f, 2.2f, 2.2f);
// If the image does not specify gamma, let's choose linear. Should we default
// to sRGB? Most images are intended to be sRGB (gamma = 2.2f).
gammas.fVec[0] = gammas.fVec[1] = gammas.fVec[2] = 1.0f;
}
return SkColorSpace::NewRGB(toXYZD50, std::move(gammas));
return SkColorSpace::NewRGB(toXYZD50, gammas);
}
// Last, check for gamma.
@ -248,10 +247,9 @@ sk_sp<SkColorSpace> read_color_space(png_structp png_ptr, png_infop info_ptr) {
toXYZD50.fMat[0] = toXYZD50.fMat[4] = toXYZD50.fMat[8] = 1.0f;
// Set the gammas.
float value = png_inverted_fixed_point_to_float(gamma);
gammas = SkColorSpace::SkGammas(value, value, value);
gammas.fVec[0] = gammas.fVec[1] = gammas.fVec[2] = png_inverted_fixed_point_to_float(gamma);
return SkColorSpace::NewRGB(toXYZD50, std::move(gammas));
return SkColorSpace::NewRGB(toXYZD50, gammas);
}
#endif // LIBPNG >= 1.6

View File

@ -23,29 +23,29 @@ void SkFloat3x3::dump() const {
static int32_t gUniqueColorSpaceID;
SkColorSpace::SkColorSpace(SkGammas gammas, const SkFloat3x3& toXYZD50, Named named)
: fGammas(std::move(gammas))
SkColorSpace::SkColorSpace(const SkFloat3& gamma, const SkFloat3x3& toXYZD50, Named named)
: fGamma(gamma)
, fToXYZD50(toXYZD50)
, fToXYZOffset({{ 0.0f, 0.0f, 0.0f }})
, fUniqueID(sk_atomic_inc(&gUniqueColorSpaceID))
, fNamed(named)
{}
SkColorSpace::SkColorSpace(SkColorLookUpTable colorLUT, SkGammas gammas,
SkColorSpace::SkColorSpace(SkColorLookUpTable colorLUT, const SkFloat3& gamma,
const SkFloat3x3& toXYZD50, const SkFloat3& toXYZOffset)
: fColorLUT(std::move(colorLUT))
, fGammas(std::move(gammas))
, fGamma(gamma)
, fToXYZD50(toXYZD50)
, fToXYZOffset(toXYZOffset)
, fUniqueID(sk_atomic_inc(&gUniqueColorSpaceID))
, fNamed(kUnknown_Named)
{}
sk_sp<SkColorSpace> SkColorSpace::NewRGB(const SkFloat3x3& toXYZD50, SkGammas gammas) {
return sk_sp<SkColorSpace>(new SkColorSpace(std::move(gammas), toXYZD50, kUnknown_Named));
sk_sp<SkColorSpace> SkColorSpace::NewRGB(const SkFloat3x3& toXYZD50, const SkFloat3& gamma) {
return sk_sp<SkColorSpace>(new SkColorSpace(gamma, toXYZD50, kUnknown_Named));
}
SkColorSpace::SkGammas gSRGB_gamma(2.2f, 2.2f, 2.2f);
const SkFloat3 gSRGB_gamma {{ 2.2f, 2.2f, 2.2f }};
const SkFloat3x3 gSRGB_toXYZD50 {{
0.4358f, 0.2224f, 0.0139f, // * R
0.3853f, 0.7170f, 0.0971f, // * G
@ -55,8 +55,7 @@ const SkFloat3x3 gSRGB_toXYZD50 {{
sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
switch (named) {
case kSRGB_Named:
return sk_sp<SkColorSpace>(new SkColorSpace(std::move(gSRGB_gamma), gSRGB_toXYZD50,
kSRGB_Named));
return sk_sp<SkColorSpace>(new SkColorSpace(gSRGB_gamma, gSRGB_toXYZD50, kSRGB_Named));
default:
break;
}
@ -265,8 +264,9 @@ bool load_xyz(float dst[3], const uint8_t* src, size_t len) {
static const uint32_t kTAG_CurveType = SkSetFourByteTag('c', 'u', 'r', 'v');
static const uint32_t kTAG_ParaCurveType = SkSetFourByteTag('p', 'a', 'r', 'a');
bool SkColorSpace::LoadGammas(SkGammaCurve* gammas, uint32_t numGammas, const uint8_t* src,
size_t len) {
// FIXME (msarett):
// We need to handle the possibility that the gamma curve does not correspond to 2.2f.
static bool load_gammas(float* gammas, uint32_t numGammas, const uint8_t* src, size_t len) {
for (uint32_t i = 0; i < numGammas; i++) {
if (len < 12) {
// FIXME (msarett):
@ -289,7 +289,7 @@ bool SkColorSpace::LoadGammas(SkGammaCurve* gammas, uint32_t numGammas, const ui
// Some tags require a gamma curve, but the author doesn't actually want
// to transform the data. In this case, it is common to see a curve with
// a count of 0.
gammas[i].fValue = 1.0f;
gammas[i] = 1.0f;
break;
} else if (len < 12 + 2 * count) {
SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
@ -298,31 +298,27 @@ bool SkColorSpace::LoadGammas(SkGammaCurve* gammas, uint32_t numGammas, const ui
const uint16_t* table = (const uint16_t*) (src + 12);
if (1 == count) {
// The table entry is the gamma (with a bias of 256).
// Table entry is the exponent (bias 256).
uint16_t value = read_big_endian_short((const uint8_t*) table);
gammas[i].fValue = value / 256.0f;
gammas[i] = value / 256.0f;
SkColorSpacePrintf("gamma %d %g\n", value, *gamma);
break;
}
// Fill in the interpolation table.
// FIXME (msarett):
// We should recognize commonly occurring tables and just set gamma to 2.2f.
gammas[i].fTableSize = count;
gammas[i].fTable = std::unique_ptr<float[]>(new float[count]);
// Print the interpolation table. For now, we ignore this and guess 2.2f.
for (uint32_t j = 0; j < count; j++) {
gammas[i].fTable[j] =
(read_big_endian_short((const uint8_t*) &table[j])) / 65535.0f;
SkColorSpacePrintf("curve[%d] %d\n", j,
read_big_endian_short((const uint8_t*) &table[j]));
}
gammas[i] = 2.2f;
break;
}
case kTAG_ParaCurveType:
// Guess 2.2f.
// FIXME (msarett): Handle parametric curves.
SkColorSpacePrintf("parametric curve\n");
gammas[i].fValue = 2.2f;
gammas[i] = 2.2f;
// Determine the size of the parametric curve tag.
switch(read_big_endian_short(src + 8)) {
case 0:
tagBytes = 12 + 4;
@ -362,19 +358,28 @@ bool SkColorSpace::LoadGammas(SkGammaCurve* gammas, uint32_t numGammas, const ui
}
}
return true;
// If all of the gammas we encounter are 1.0f, indicate that we failed to load gammas.
// There is no need to apply a gamma of 1.0f.
for (uint32_t i = 0; i < numGammas; i++) {
if (1.0f != gammas[i]) {
return true;
}
}
return false;
}
static const uint32_t kTAG_AtoBType = SkSetFourByteTag('m', 'A', 'B', ' ');
bool SkColorSpace::LoadColorLUT(SkColorLookUpTable* colorLUT, uint32_t inputChannels,
uint32_t outputChannels, const uint8_t* src, size_t len) {
bool load_color_lut(SkColorLookUpTable* colorLUT, uint32_t inputChannels, uint32_t outputChannels,
const uint8_t* src, size_t len) {
if (len < 20) {
SkColorSpacePrintf("Color LUT tag is too small (%d bytes).", len);
return false;
}
SkASSERT(inputChannels <= SkColorLookUpTable::kMaxChannels && 3 == outputChannels);
SkASSERT(inputChannels <= SkColorLookUpTable::kMaxChannels &&
outputChannels <= SkColorLookUpTable::kMaxChannels);
colorLUT->fInputChannels = inputChannels;
colorLUT->fOutputChannels = outputChannels;
uint32_t numEntries = 1;
@ -436,8 +441,8 @@ bool load_matrix(SkFloat3x3* toXYZ, SkFloat3* toXYZOffset, const uint8_t* src, s
return true;
}
bool SkColorSpace::LoadA2B0(SkColorLookUpTable* colorLUT, SkGammas* gammas, SkFloat3x3* toXYZ,
SkFloat3* toXYZOffset, const uint8_t* src, size_t len) {
bool load_a2b0(SkColorLookUpTable* colorLUT, SkFloat3* gamma, SkFloat3x3* toXYZ,
SkFloat3* toXYZOffset, const uint8_t* src, size_t len) {
if (len < 32) {
SkColorSpacePrintf("A to B tag is too small (%d bytes).", len);
return false;
@ -455,7 +460,7 @@ bool SkColorSpace::LoadA2B0(SkColorLookUpTable* colorLUT, SkGammas* gammas, SkFl
uint8_t inputChannels = src[8];
uint8_t outputChannels = src[9];
if (0 == inputChannels || inputChannels > SkColorLookUpTable::kMaxChannels ||
3 != outputChannels) {
0 < outputChannels || outputChannels > SkColorLookUpTable::kMaxChannels) {
// The color LUT assumes that there are at most 16 input channels. For RGB
// profiles, output channels should be 3.
SkColorSpacePrintf("Too many input or output channels in A to B tag.\n");
@ -478,16 +483,16 @@ bool SkColorSpace::LoadA2B0(SkColorLookUpTable* colorLUT, SkGammas* gammas, SkFl
uint32_t offsetToColorLUT = read_big_endian_int(src + 24);
if (0 != offsetToColorLUT && offsetToColorLUT < len) {
if (!SkColorSpace::LoadColorLUT(colorLUT, inputChannels, outputChannels,
src + offsetToColorLUT, len - offsetToColorLUT)) {
if (!load_color_lut(colorLUT, inputChannels, outputChannels, src + offsetToColorLUT,
len - offsetToColorLUT)) {
SkColorSpacePrintf("Failed to read color LUT from A to B tag.\n");
}
}
uint32_t offsetToMCurves = read_big_endian_int(src + 20);
if (0 != offsetToMCurves && offsetToMCurves < len) {
if (!SkColorSpace::LoadGammas(&gammas->fRed, outputChannels, src + offsetToMCurves,
len - offsetToMCurves)) {
if (!load_gammas(gamma->fVec, outputChannels, src + offsetToMCurves, len - offsetToMCurves))
{
SkColorSpacePrintf("Failed to read M curves from A to B tag.\n");
}
}
@ -562,39 +567,42 @@ sk_sp<SkColorSpace> SkColorSpace::NewICC(const void* base, size_t len) {
// It is not uncommon to see missing or empty gamma tags. This indicates
// that we should use unit gamma.
SkGammas gammas;
SkFloat3 gamma {{ 1.0f, 1.0f, 1.0f }};
r = ICCTag::Find(tags.get(), tagCount, kTAG_rTRC);
g = ICCTag::Find(tags.get(), tagCount, kTAG_gTRC);
b = ICCTag::Find(tags.get(), tagCount, kTAG_bTRC);
if (!r || !SkColorSpace::LoadGammas(&gammas.fRed, 1,
r->addr((const uint8_t*) base), r->fLength)) {
if (!r ||
!load_gammas(&gamma.fVec[0], 1, r->addr((const uint8_t*) base), r->fLength))
{
SkColorSpacePrintf("Failed to read R gamma tag.\n");
}
if (!g || !SkColorSpace::LoadGammas(&gammas.fGreen, 1,
g->addr((const uint8_t*) base), g->fLength)) {
if (!g ||
!load_gammas(&gamma.fVec[1], 1, g->addr((const uint8_t*) base), g->fLength))
{
SkColorSpacePrintf("Failed to read G gamma tag.\n");
}
if (!b || !SkColorSpace::LoadGammas(&gammas.fBlue, 1,
b->addr((const uint8_t*) base), b->fLength)) {
if (!b ||
!load_gammas(&gamma.fVec[2], 1, b->addr((const uint8_t*) base), b->fLength))
{
SkColorSpacePrintf("Failed to read B gamma tag.\n");
}
return SkColorSpace::NewRGB(toXYZ, std::move(gammas));
return SkColorSpace::NewRGB(toXYZ, gamma);
}
// Recognize color profile specified by A2B0 tag.
const ICCTag* a2b0 = ICCTag::Find(tags.get(), tagCount, kTAG_A2B0);
if (a2b0) {
SkColorLookUpTable colorLUT;
SkGammas gammas;
SkFloat3 gamma;
SkFloat3x3 toXYZ;
SkFloat3 toXYZOffset;
if (!SkColorSpace::LoadA2B0(&colorLUT, &gammas, &toXYZ, &toXYZOffset,
a2b0->addr((const uint8_t*) base), a2b0->fLength)) {
if (!load_a2b0(&colorLUT, &gamma, &toXYZ, &toXYZOffset,
a2b0->addr((const uint8_t*) base), a2b0->fLength)) {
return_null("Failed to parse A2B0 tag");
}
return sk_sp<SkColorSpace>(new SkColorSpace(std::move(colorLUT), std::move(gammas),
toXYZ, toXYZOffset));
return sk_sp<SkColorSpace>(new SkColorSpace(std::move(colorLUT), gamma, toXYZ,
toXYZOffset));
}
}

View File

@ -35,121 +35,57 @@ struct SkFloat3x3 {
void dump() const;
};
struct SkColorLookUpTable {
static const uint8_t kMaxChannels = 16;
uint8_t fInputChannels;
uint8_t fOutputChannels;
uint8_t fGridPoints[kMaxChannels];
std::unique_ptr<float[]> fTable;
SkColorLookUpTable() {
memset(this, 0, sizeof(struct SkColorLookUpTable));
}
SkColorLookUpTable(SkColorLookUpTable&& that)
: fInputChannels(that.fInputChannels)
, fOutputChannels(that.fOutputChannels)
, fTable(std::move(that.fTable))
{
memcpy(fGridPoints, that.fGridPoints, kMaxChannels);
}
};
class SkColorSpace : public SkRefCnt {
private:
struct SkGammaCurve {
bool isValue() const {
bool result = (0.0f != fValue);
SkASSERT(!result || (0 == fTableSize));
return result;
}
bool isTable() const {
bool result = (0 != fTableSize);
SkASSERT(!result || (0.0f == fValue));
SkASSERT(!result || fTable);
return result;
}
bool isParametric() const { return false; }
// We have three different ways to represent gamma.
// (1) A single value:
float fValue;
// (2) A lookup table:
uint32_t fTableSize;
std::unique_ptr<float[]> fTable;
// (3) Parameters for a curve:
// FIXME (msarett): Handle parametric curves.
SkGammaCurve() {
memset(this, 0, sizeof(struct SkGammaCurve));
}
SkGammaCurve(float value)
: fValue(value)
, fTableSize(0)
, fTable(nullptr)
{}
};
struct SkColorLookUpTable {
static const uint8_t kMaxChannels = 16;
uint8_t fInputChannels;
uint8_t fOutputChannels;
uint8_t fGridPoints[kMaxChannels];
std::unique_ptr<float[]> fTable;
SkColorLookUpTable() {
memset(this, 0, sizeof(struct SkColorLookUpTable));
}
};
public:
enum Named {
kUnknown_Named,
kSRGB_Named,
};
struct SkGammas {
public:
SkGammas(float red, float green, float blue)
: fRed(red)
, fGreen(green)
, fBlue(blue)
{}
SkGammas() {}
SkDEBUGCODE(float red() const { return fRed.fValue; })
SkDEBUGCODE(float green() const { return fGreen.fValue; })
SkDEBUGCODE(float blue() const { return fBlue.fValue; })
private:
SkGammaCurve fRed;
SkGammaCurve fGreen;
SkGammaCurve fBlue;
friend class SkColorSpace;
};
/**
* Return a colorspace instance, given a 3x3 transform from linear_RGB to D50_XYZ
* and the src-gamma, return a ColorSpace
*/
static sk_sp<SkColorSpace> NewRGB(const SkFloat3x3& toXYZD50, SkGammas gammas);
static sk_sp<SkColorSpace> NewRGB(const SkFloat3x3& toXYZD50, const SkFloat3& gamma);
static sk_sp<SkColorSpace> NewNamed(Named);
static sk_sp<SkColorSpace> NewICC(const void*, size_t);
const SkGammas& gammas() const { return fGammas; }
SkFloat3 gamma() const { return fGamma; }
SkFloat3x3 xyz() const { return fToXYZD50; }
SkFloat3 xyzOffset() const { return fToXYZOffset; }
Named named() const { return fNamed; }
uint32_t uniqueID() const { return fUniqueID; }
private:
SkColorSpace(const SkFloat3& gamma, const SkFloat3x3& toXYZ, Named);
static bool LoadGammas(SkGammaCurve* gammas, uint32_t num, const uint8_t* src, size_t len);
static bool LoadColorLUT(SkColorLookUpTable* colorLUT, uint32_t inputChannels,
uint32_t outputChannels, const uint8_t* src, size_t len);
static bool LoadA2B0(SkColorLookUpTable* colorLUT, SkGammas* gammas, SkFloat3x3* toXYZ,
SkFloat3* toXYZOffset, const uint8_t* src, size_t len);
SkColorSpace(SkGammas gammas, const SkFloat3x3& toXYZ, Named);
SkColorSpace(SkColorLookUpTable colorLUT, SkGammas gammas,
const SkFloat3x3& toXYZ, const SkFloat3& toXYZOffset);
SkColorSpace(SkColorLookUpTable colorLUT, const SkFloat3& gamma, const SkFloat3x3& toXYZ,
const SkFloat3& toXYZOffset);
const SkColorLookUpTable fColorLUT;
const SkGammas fGammas;
const SkFloat3 fGamma;
const SkFloat3x3 fToXYZD50;
const SkFloat3 fToXYZOffset;

View File

@ -35,14 +35,12 @@ DEF_TEST(ColorSpaceParsePngICCProfile, r) {
SkColorSpace* colorSpace = codec->getColorSpace();
REPORTER_ASSERT(r, nullptr != colorSpace);
// The color profile that we have extracted has represents gamma with a lookup table.
// So we expect the gamma value to be zero.
#ifdef SK_DEBUG
const SkColorSpace::SkGammas& gammas = colorSpace->gammas();
REPORTER_ASSERT(r, 0.0f == gammas.red());
REPORTER_ASSERT(r, 0.0f == gammas.green());
REPORTER_ASSERT(r, 0.0f == gammas.blue());
#endif
// No need to use almost equal here. The color profile that we have extracted
// actually has a table of gammas. And our current implementation guesses 2.2f.
SkFloat3 gammas = colorSpace->gamma();
REPORTER_ASSERT(r, 2.2f == gammas.fVec[0]);
REPORTER_ASSERT(r, 2.2f == gammas.fVec[1]);
REPORTER_ASSERT(r, 2.2f == gammas.fVec[2]);
// These nine values were extracted from the color profile in isolation (before
// we embedded it in the png). Here we check that we still extract the same values.
@ -77,12 +75,10 @@ DEF_TEST(ColorSpaceParseJpegICCProfile, r) {
// It's important to use almost equal here. This profile sets gamma as
// 563 / 256, which actually comes out to about 2.19922.
#ifdef SK_DEBUG
const SkColorSpace::SkGammas& gammas = colorSpace->gammas();
REPORTER_ASSERT(r, almost_equal(2.2f, gammas.red()));
REPORTER_ASSERT(r, almost_equal(2.2f, gammas.green()));
REPORTER_ASSERT(r, almost_equal(2.2f, gammas.blue()));
#endif
SkFloat3 gammas = colorSpace->gamma();
REPORTER_ASSERT(r, almost_equal(2.2f, gammas.fVec[0]));
REPORTER_ASSERT(r, almost_equal(2.2f, gammas.fVec[1]));
REPORTER_ASSERT(r, almost_equal(2.2f, gammas.fVec[2]));
// These nine values were extracted from the color profile. Until we know any
// better, we'll assume these are the right values and test that we continue