Remove nearly all use of SkColorSpaceTransferFn
Most interfaces had migrated to skcms_TransferFunction. Having both was
awkward in several places, so this (almost) finishes the migration. Some
clients are calling SkICC::WriteToICC, so that's left intact. After this
CL, those clients can switch to using SkWriteICCProfile directly, and
WriteToICC can be deleted.
Bug: skia:
Change-Id: I46ebaeb1f5b20bf0c620e8a620e73ee323a1de31
Reviewed-on: https://skia-review.googlesource.com/c/186541
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
diff --git a/include/core/SkICC.h b/include/core/SkICC.h
index 8918889..ddc31a5 100644
--- a/include/core/SkICC.h
+++ b/include/core/SkICC.h
@@ -12,9 +12,12 @@
#include "SkMatrix44.h"
#include "SkRefCnt.h"
+struct skcms_Matrix3x3;
+struct skcms_TransferFunction;
struct SkColorSpaceTransferFn;
-SK_API sk_sp<SkData> SkWriteICCProfile(const SkColorSpaceTransferFn&, const float toXYZD50[9]);
+SK_API sk_sp<SkData> SkWriteICCProfile(const skcms_TransferFunction&,
+ const skcms_Matrix3x3& toXYZD50);
namespace SkICC {
SK_API sk_sp<SkData> WriteToICC(const SkColorSpaceTransferFn&, const SkMatrix44&);
diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index b3db194..e85b6e4 100644
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -38,7 +38,7 @@
sk_sp<SkColorSpace> SkColorSpace::MakeRGB(const skcms_TransferFunction& transferFn,
const skcms_Matrix3x3& toXYZ) {
- if (!is_valid_transfer_fn(reinterpret_cast<const SkColorSpaceTransferFn&>(transferFn))) {
+ if (!is_valid_transfer_fn(transferFn)) {
return nullptr;
}
diff --git a/src/core/SkColorSpacePriv.h b/src/core/SkColorSpacePriv.h
index 9ef3089..12379d7 100644
--- a/src/core/SkColorSpacePriv.h
+++ b/src/core/SkColorSpacePriv.h
@@ -31,49 +31,49 @@
return SkTAbs(a - b) < 0.001f;
}
-static inline bool is_valid_transfer_fn(const SkColorSpaceTransferFn& coeffs) {
- if (SkScalarIsNaN(coeffs.fA) || SkScalarIsNaN(coeffs.fB) ||
- SkScalarIsNaN(coeffs.fC) || SkScalarIsNaN(coeffs.fD) ||
- SkScalarIsNaN(coeffs.fE) || SkScalarIsNaN(coeffs.fF) ||
- SkScalarIsNaN(coeffs.fG))
+static inline bool is_valid_transfer_fn(const skcms_TransferFunction& coeffs) {
+ if (SkScalarIsNaN(coeffs.a) || SkScalarIsNaN(coeffs.b) ||
+ SkScalarIsNaN(coeffs.c) || SkScalarIsNaN(coeffs.d) ||
+ SkScalarIsNaN(coeffs.e) || SkScalarIsNaN(coeffs.f) ||
+ SkScalarIsNaN(coeffs.g))
{
return false;
}
- if (coeffs.fD < 0.0f) {
+ if (coeffs.d < 0.0f) {
return false;
}
- if (coeffs.fD == 0.0f) {
+ if (coeffs.d == 0.0f) {
// Y = (aX + b)^g + e for always
- if (0.0f == coeffs.fA || 0.0f == coeffs.fG) {
+ if (0.0f == coeffs.a || 0.0f == coeffs.g) {
SkColorSpacePrintf("A or G is zero, constant transfer function "
"is nonsense");
return false;
}
}
- if (coeffs.fD >= 1.0f) {
+ if (coeffs.d >= 1.0f) {
// Y = cX + f for always
- if (0.0f == coeffs.fC) {
+ if (0.0f == coeffs.c) {
SkColorSpacePrintf("C is zero, constant transfer function is "
"nonsense");
return false;
}
}
- if ((0.0f == coeffs.fA || 0.0f == coeffs.fG) && 0.0f == coeffs.fC) {
+ if ((0.0f == coeffs.a || 0.0f == coeffs.g) && 0.0f == coeffs.c) {
SkColorSpacePrintf("A or G, and C are zero, constant transfer function "
"is nonsense");
return false;
}
- if (coeffs.fC < 0.0f) {
+ if (coeffs.c < 0.0f) {
SkColorSpacePrintf("Transfer function must be increasing");
return false;
}
- if (coeffs.fA < 0.0f || coeffs.fG < 0.0f) {
+ if (coeffs.a < 0.0f || coeffs.g < 0.0f) {
SkColorSpacePrintf("Transfer function must be positive or increasing");
return false;
}
diff --git a/src/core/SkColorSpaceXformSteps.cpp b/src/core/SkColorSpaceXformSteps.cpp
index 5fff910..6e31814 100644
--- a/src/core/SkColorSpaceXformSteps.cpp
+++ b/src/core/SkColorSpaceXformSteps.cpp
@@ -61,8 +61,8 @@
}
// Fill out all the transfer functions we'll use.
- src-> transferFn(&this->srcTF .fG);
- dst->invTransferFn(&this->dstTFInv.fG);
+ src-> transferFn(&this->srcTF .g);
+ dst->invTransferFn(&this->dstTFInv.g);
this->srcTF_is_sRGB = src->gammaCloseToSRGB();
this->dstTF_is_sRGB = dst->gammaCloseToSRGB();
@@ -77,7 +77,7 @@
float dstTF[7];
dst->transferFn(dstTF);
for (int i = 0; i < 7; i++) {
- SkASSERT( (&srcTF.fG)[i] == dstTF[i] && "Hash collision" );
+ SkASSERT( (&srcTF.g)[i] == dstTF[i] && "Hash collision" );
}
#endif
this->flags.linearize = false;
@@ -144,13 +144,13 @@
if (flags.linearize) {
if (src_is_normalized && srcTF_is_sRGB) {
p->append(SkRasterPipeline::from_srgb);
- } else if (srcTF.fA == 1 &&
- srcTF.fB == 0 &&
- srcTF.fC == 0 &&
- srcTF.fD == 0 &&
- srcTF.fE == 0 &&
- srcTF.fF == 0) {
- p->append(SkRasterPipeline::gamma, &srcTF.fG);
+ } else if (srcTF.a == 1 &&
+ srcTF.b == 0 &&
+ srcTF.c == 0 &&
+ srcTF.d == 0 &&
+ srcTF.e == 0 &&
+ srcTF.f == 0) {
+ p->append(SkRasterPipeline::gamma, &srcTF.g);
} else {
p->append(SkRasterPipeline::parametric, &srcTF);
}
@@ -161,13 +161,13 @@
if (flags.encode) {
if (src_is_normalized && dstTF_is_sRGB) {
p->append(SkRasterPipeline::to_srgb);
- } else if (dstTFInv.fA == 1 &&
- dstTFInv.fB == 0 &&
- dstTFInv.fC == 0 &&
- dstTFInv.fD == 0 &&
- dstTFInv.fE == 0 &&
- dstTFInv.fF == 0) {
- p->append(SkRasterPipeline::gamma, &dstTFInv.fG);
+ } else if (dstTFInv.a == 1 &&
+ dstTFInv.b == 0 &&
+ dstTFInv.c == 0 &&
+ dstTFInv.d == 0 &&
+ dstTFInv.e == 0 &&
+ dstTFInv.f == 0) {
+ p->append(SkRasterPipeline::gamma, &dstTFInv.g);
} else {
p->append(SkRasterPipeline::parametric, &dstTFInv);
}
diff --git a/src/core/SkColorSpaceXformSteps.h b/src/core/SkColorSpaceXformSteps.h
index 0953617..9585d1e 100644
--- a/src/core/SkColorSpaceXformSteps.h
+++ b/src/core/SkColorSpaceXformSteps.h
@@ -44,7 +44,7 @@
bool srcTF_is_sRGB,
dstTF_is_sRGB;
- SkColorSpaceTransferFn srcTF, // Apply for linearize.
+ skcms_TransferFunction srcTF, // Apply for linearize.
dstTFInv; // Apply for encode.
float src_to_dst_matrix[9]; // Apply this 3x3 column-major matrix for gamut_transform.
};
diff --git a/src/core/SkICC.cpp b/src/core/SkICC.cpp
index d1db49e..dd441d5 100644
--- a/src/core/SkICC.cpp
+++ b/src/core/SkICC.cpp
@@ -172,25 +172,25 @@
return sk_float_saturate2int((float)floor((double)x * SK_Fixed1 + 0.5));
}
-static void write_xyz_tag(uint32_t* ptr, const float toXYZD50[9], int col) {
+static void write_xyz_tag(uint32_t* ptr, const skcms_Matrix3x3& toXYZD50, int col) {
ptr[0] = SkEndian_SwapBE32(kXYZ_PCSSpace);
ptr[1] = 0;
- ptr[2] = SkEndian_SwapBE32(float_round_to_fixed(toXYZD50[0*3 + col]));
- ptr[3] = SkEndian_SwapBE32(float_round_to_fixed(toXYZD50[1*3 + col]));
- ptr[4] = SkEndian_SwapBE32(float_round_to_fixed(toXYZD50[2*3 + col]));
+ ptr[2] = SkEndian_SwapBE32(float_round_to_fixed(toXYZD50.vals[0][col]));
+ ptr[3] = SkEndian_SwapBE32(float_round_to_fixed(toXYZD50.vals[1][col]));
+ ptr[4] = SkEndian_SwapBE32(float_round_to_fixed(toXYZD50.vals[2][col]));
}
-static void write_trc_tag(uint32_t* ptr, const SkColorSpaceTransferFn& fn) {
+static void write_trc_tag(uint32_t* ptr, const skcms_TransferFunction& fn) {
ptr[0] = SkEndian_SwapBE32(kTAG_ParaCurveType);
ptr[1] = 0;
ptr[2] = (uint32_t) (SkEndian_SwapBE16(kGABCDEF_ParaCurveType));
- ptr[3] = SkEndian_SwapBE32(float_round_to_fixed(fn.fG));
- ptr[4] = SkEndian_SwapBE32(float_round_to_fixed(fn.fA));
- ptr[5] = SkEndian_SwapBE32(float_round_to_fixed(fn.fB));
- ptr[6] = SkEndian_SwapBE32(float_round_to_fixed(fn.fC));
- ptr[7] = SkEndian_SwapBE32(float_round_to_fixed(fn.fD));
- ptr[8] = SkEndian_SwapBE32(float_round_to_fixed(fn.fE));
- ptr[9] = SkEndian_SwapBE32(float_round_to_fixed(fn.fF));
+ ptr[3] = SkEndian_SwapBE32(float_round_to_fixed(fn.g));
+ ptr[4] = SkEndian_SwapBE32(float_round_to_fixed(fn.a));
+ ptr[5] = SkEndian_SwapBE32(float_round_to_fixed(fn.b));
+ ptr[6] = SkEndian_SwapBE32(float_round_to_fixed(fn.c));
+ ptr[7] = SkEndian_SwapBE32(float_round_to_fixed(fn.d));
+ ptr[8] = SkEndian_SwapBE32(float_round_to_fixed(fn.e));
+ ptr[9] = SkEndian_SwapBE32(float_round_to_fixed(fn.f));
}
static bool nearly_equal(float x, float y) {
@@ -205,31 +205,33 @@
return ::fabsf(x - y) <= kTolerance;
}
-static bool nearly_equal(const SkColorSpaceTransferFn& u,
+static bool nearly_equal(const skcms_TransferFunction& u,
const skcms_TransferFunction& v) {
- return nearly_equal(u.fG, v.g)
- && nearly_equal(u.fA, v.a)
- && nearly_equal(u.fB, v.b)
- && nearly_equal(u.fC, v.c)
- && nearly_equal(u.fD, v.d)
- && nearly_equal(u.fE, v.e)
- && nearly_equal(u.fF, v.f);
+ return nearly_equal(u.g, v.g)
+ && nearly_equal(u.a, v.a)
+ && nearly_equal(u.b, v.b)
+ && nearly_equal(u.c, v.c)
+ && nearly_equal(u.d, v.d)
+ && nearly_equal(u.e, v.e)
+ && nearly_equal(u.f, v.f);
}
-static bool nearly_equal(const float u[9], const float v[9]) {
- for (int i = 0; i < 9; i++) {
- if (!nearly_equal(u[i], v[i])) {
- return false;
+static bool nearly_equal(const skcms_Matrix3x3& u, const skcms_Matrix3x3& v) {
+ for (int r = 0; r < 3; r++) {
+ for (int c = 0; c < 3; c++) {
+ if (!nearly_equal(u.vals[r][c], v.vals[r][c])) {
+ return false;
+ }
}
}
return true;
}
// Return nullptr if the color profile doen't have a special name.
-const char* get_color_profile_description(const SkColorSpaceTransferFn& fn,
- const float toXYZD50[9]) {
+const char* get_color_profile_description(const skcms_TransferFunction& fn,
+ const skcms_Matrix3x3& toXYZD50) {
bool srgb_xfer = nearly_equal(fn, SkNamedTransferFn::kSRGB);
- bool srgb_gamut = nearly_equal(toXYZD50, &SkNamedGamut::kSRGB.vals[0][0]);
+ bool srgb_gamut = nearly_equal(toXYZD50, SkNamedGamut::kSRGB);
if (srgb_xfer && srgb_gamut) {
return "sRGB";
}
@@ -241,10 +243,10 @@
if (twoDotTwo && srgb_gamut) {
return "2.2 Transfer with sRGB Gamut";
}
- if (twoDotTwo && nearly_equal(toXYZD50, &SkNamedGamut::kAdobeRGB.vals[0][0])) {
+ if (twoDotTwo && nearly_equal(toXYZD50, SkNamedGamut::kAdobeRGB)) {
return "AdobeRGB";
}
- bool dcip3_gamut = nearly_equal(toXYZD50, &SkNamedGamut::kDCIP3.vals[0][0]);
+ bool dcip3_gamut = nearly_equal(toXYZD50, SkNamedGamut::kDCIP3);
if (srgb_xfer || line_xfer) {
if (srgb_xfer && dcip3_gamut) {
return "sRGB Transfer with DCI-P3 Gamut";
@@ -252,7 +254,7 @@
if (line_xfer && dcip3_gamut) {
return "Linear Transfer with DCI-P3 Gamut";
}
- bool rec2020 = nearly_equal(toXYZD50, &SkNamedGamut::kRec2020.vals[0][0]);
+ bool rec2020 = nearly_equal(toXYZD50, SkNamedGamut::kRec2020);
if (srgb_xfer && rec2020) {
return "sRGB Transfer with Rec-BT-2020 Gamut";
}
@@ -264,8 +266,8 @@
}
static void get_color_profile_tag(char dst[kICCDescriptionTagSize],
- const SkColorSpaceTransferFn& fn,
- const float toXYZD50[9]) {
+ const skcms_TransferFunction& fn,
+ const skcms_Matrix3x3& toXYZD50) {
SkASSERT(dst);
if (const char* description = get_color_profile_description(fn, toXYZD50)) {
SkASSERT(strlen(description) < kICCDescriptionTagSize);
@@ -275,7 +277,7 @@
} else {
strncpy(dst, kDescriptionTagBodyPrefix, sizeof(kDescriptionTagBodyPrefix));
SkMD5 md5;
- md5.write(toXYZD50, 9*sizeof(float));
+ md5.write(&toXYZD50, sizeof(toXYZD50));
static_assert(sizeof(fn) == sizeof(float) * 7, "packed");
md5.write(&fn, sizeof(fn));
SkMD5::Digest digest;
@@ -290,7 +292,8 @@
}
}
-sk_sp<SkData> SkWriteICCProfile(const SkColorSpaceTransferFn& fn, const float toXYZD50[9]) {
+sk_sp<SkData> SkWriteICCProfile(const skcms_TransferFunction& fn,
+ const skcms_Matrix3x3& toXYZD50) {
if (!is_valid_transfer_fn(fn)) {
return nullptr;
}
@@ -353,12 +356,14 @@
toXYZD50.get(3,3) == 1 &&
toXYZD50.get(0,3) == 0 && toXYZD50.get(1,3) == 0 && toXYZD50.get(2,3) == 0) {
- float m33[9];
+ skcms_Matrix3x3 m33;
for (int r = 0; r < 3; r++)
for (int c = 0; c < 3; c++) {
- m33[3*r+c] = toXYZD50.get(r,c);
+ m33.vals[r][c] = toXYZD50.get(r,c);
}
- return SkWriteICCProfile(fn, m33);
+ skcms_TransferFunction tf;
+ memcpy(&tf, &fn, sizeof(tf));
+ return SkWriteICCProfile(tf, m33);
}
return nullptr;
}
diff --git a/src/core/SkRasterPipeline.h b/src/core/SkRasterPipeline.h
index c93fa1e..74e915f 100644
--- a/src/core/SkRasterPipeline.h
+++ b/src/core/SkRasterPipeline.h
@@ -142,11 +142,6 @@
float* read_from = rgba;
};
-// This should line up with the memory layout of SkColorSpaceTransferFn.
-struct SkRasterPipeline_ParametricTransferFunction {
- float G, A,B,C,D,E,F;
-};
-
struct SkRasterPipeline_GradientCtx {
size_t stopCount;
float* fs[4];
diff --git a/src/effects/SkHighContrastFilter.cpp b/src/effects/SkHighContrastFilter.cpp
index 9b0d105..a922545 100644
--- a/src/effects/SkHighContrastFilter.cpp
+++ b/src/effects/SkHighContrastFilter.cpp
@@ -68,9 +68,9 @@
if (!dstCS) {
// In legacy draws this effect approximately linearizes by squaring.
// When non-legacy, we're already (better) linearized.
- auto square = alloc->make<SkRasterPipeline_ParametricTransferFunction>();
- square->G = 2.0f; square->A = 1.0f;
- square->B = square->C = square->D = square->E = square->F = 0;
+ auto square = alloc->make<skcms_TransferFunction>();
+ square->g = 2.0f; square->a = 1.0f;
+ square->b = square->c = square->d = square->e = square->f = 0;
p->append(SkRasterPipeline::parametric, square);
}
@@ -115,9 +115,9 @@
if (!dstCS) {
// See the previous if(!dstCS) { ... }
- auto sqrt = alloc->make<SkRasterPipeline_ParametricTransferFunction>();
- sqrt->G = 0.5f; sqrt->A = 1.0f;
- sqrt->B = sqrt->C = sqrt->D = sqrt->E = sqrt->F = 0;
+ auto sqrt = alloc->make<skcms_TransferFunction>();
+ sqrt->g = 0.5f; sqrt->a = 1.0f;
+ sqrt->b = sqrt->c = sqrt->d = sqrt->e = sqrt->f = 0;
p->append(SkRasterPipeline::parametric, sqrt);
}
diff --git a/src/gpu/glsl/GrGLSLColorSpaceXformHelper.h b/src/gpu/glsl/GrGLSLColorSpaceXformHelper.h
index 397a656..7898e18 100644
--- a/src/gpu/glsl/GrGLSLColorSpaceXformHelper.h
+++ b/src/gpu/glsl/GrGLSLColorSpaceXformHelper.h
@@ -45,13 +45,13 @@
void setData(const GrGLSLProgramDataManager& pdman, const GrColorSpaceXform* colorSpaceXform) {
if (this->applySrcTF()) {
- pdman.set1fv(fSrcTFVar, kNumTransferFnCoeffs, &colorSpaceXform->fSteps.srcTF.fG);
+ pdman.set1fv(fSrcTFVar, kNumTransferFnCoeffs, &colorSpaceXform->fSteps.srcTF.g);
}
if (this->applyGamutXform()) {
pdman.setMatrix3f(fGamutXformVar, colorSpaceXform->fSteps.src_to_dst_matrix);
}
if (this->applyDstTF()) {
- pdman.set1fv(fDstTFVar, kNumTransferFnCoeffs, &colorSpaceXform->fSteps.dstTFInv.fG);
+ pdman.set1fv(fDstTFVar, kNumTransferFnCoeffs, &colorSpaceXform->fSteps.dstTFInv.g);
}
}
diff --git a/src/images/SkImageEncoderFns.h b/src/images/SkImageEncoderFns.h
index c64db2b..60636f2 100644
--- a/src/images/SkImageEncoderFns.h
+++ b/src/images/SkImageEncoderFns.h
@@ -167,12 +167,10 @@
return nullptr;
}
- SkColorSpaceTransferFn fn;
+ skcms_TransferFunction fn;
skcms_Matrix3x3 toXYZD50;
if (cs->isNumericalTransferFn(&fn) && cs->toXYZD50(&toXYZD50)) {
- SkMatrix44 m44;
- m44.set3x3RowMajorf(&toXYZD50.vals[0][0]);
- return SkICC::WriteToICC(fn, m44);
+ return SkWriteICCProfile(fn, toXYZD50);
}
return nullptr;
}
diff --git a/src/opts/SkRasterPipeline_opts.h b/src/opts/SkRasterPipeline_opts.h
index 4c02958..cf81986 100644
--- a/src/opts/SkRasterPipeline_opts.h
+++ b/src/opts/SkRasterPipeline_opts.h
@@ -1532,13 +1532,13 @@
return bit_cast<F>(sign | bit_cast<U32>(x));
}
-STAGE(parametric, const SkRasterPipeline_ParametricTransferFunction* ctx) {
+STAGE(parametric, const skcms_TransferFunction* ctx) {
auto fn = [&](F v) {
U32 sign;
v = strip_sign(v, &sign);
- F r = if_then_else(v <= ctx->D, mad(ctx->C, v, ctx->F)
- , approx_powf(mad(ctx->A, v, ctx->B), ctx->G) + ctx->E);
+ F r = if_then_else(v <= ctx->d, mad(ctx->c, v, ctx->f)
+ , approx_powf(mad(ctx->a, v, ctx->b), ctx->g) + ctx->e);
return apply_sign(r, sign);
};
r = fn(r);
diff --git a/tests/ParametricStageTest.cpp b/tests/ParametricStageTest.cpp
index e3c5b73..9dcaa00 100644
--- a/tests/ParametricStageTest.cpp
+++ b/tests/ParametricStageTest.cpp
@@ -9,7 +9,7 @@
#include "SkRasterPipeline.h"
#include "Test.h"
-static void check_error(skiatest::Reporter* r, float limit, SkColorSpaceTransferFn fn) {
+static void check_error(skiatest::Reporter* r, float limit, skcms_TransferFunction fn) {
float in[256], out[256];
for (int i = 0; i < 256; i++) {
in [i] = i / 255.0f;
@@ -28,8 +28,8 @@
for (int i = 0; i < 256; i++) {
- float want = (in[i] <= fn.fD) ? fn.fC * in[i] + fn.fF
- : powf(in[i] * fn.fA + fn.fB, fn.fG) + fn.fE;
+ float want = (in[i] <= fn.d) ? fn.c * in[i] + fn.f
+ : powf(in[i] * fn.a + fn.b, fn.g) + fn.e;
if (i % 4 == 3) { // alpha should stay unchanged.
want = in[i];
}
@@ -41,9 +41,9 @@
}
static void check_error(skiatest::Reporter* r, float limit, float gamma) {
- SkColorSpaceTransferFn fn = {0,0,0,0,0,0,0};
- fn.fG = gamma;
- fn.fA = 1;
+ skcms_TransferFunction fn = {0,0,0,0,0,0,0};
+ fn.g = gamma;
+ fn.a = 1;
check_error(r, limit, fn);
}