Revert of experiment: float color components (patchset #6 id:100001 of https://codereview.chromium.org/1617813002/ )
Reason for revert:
broke some colormatrix tests
Original issue's description:
> experiment: float color components
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1617813002
>
> Committed: https://skia.googlesource.com/skia/+/353c148d8e8c9031daca34c6f9d6bcc6f08706c7
TBR=fmalita@chromium.org,mtklein@google.com,herb@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/1623453002
diff --git a/include/core/SkColor.h b/include/core/SkColor.h
index 7794638..1ba1331 100644
--- a/include/core/SkColor.h
+++ b/include/core/SkColor.h
@@ -160,44 +160,4 @@
*/
typedef SkPMColor (*SkXfermodeProc)(SkPMColor src, SkPMColor dst);
-///////////////////////////////////////////////////////////////////////////////////////////////////
-
-/*
- * The float values are 0...1 premultiplied
- */
-struct SK_ATTRIBUTE(aligned(16)) SkPM4f {
- float fVec[4];
-
- float a() const { return fVec[SK_A32_SHIFT/8]; }
-};
-
-/*
- * The float values are 0...1 unpremultiplied
- */
-struct SkColor4f {
- float fA;
- float fR;
- float fG;
- float fB;
-
- bool operator==(const SkColor4f& other) const {
- return fA == other.fA && fR == other.fR && fG == other.fG && fB == other.fB;
- }
- bool operator!=(const SkColor4f& other) const {
- return !(*this == other);
- }
-
- const float* vec() const { return &fA; }
- float* vec() { return &fA; }
-
- static SkColor4f Pin(float a, float r, float g, float b);
- static SkColor4f FromColor(SkColor);
-
- SkColor4f pin() const {
- return Pin(fA, fR, fG, fB);
- }
-
- SkPM4f premul() const;
-};
-
#endif
diff --git a/include/core/SkColorFilter.h b/include/core/SkColorFilter.h
index 211aae6..c5d084a 100644
--- a/include/core/SkColorFilter.h
+++ b/include/core/SkColorFilter.h
@@ -68,13 +68,10 @@
*/
virtual void filterSpan(const SkPMColor src[], int count, SkPMColor result[]) const = 0;
- virtual void filterSpan4f(const SkPM4f src[], int count, SkPM4f result[]) const;
-
enum Flags {
/** If set the filter methods will not change the alpha channel of the colors.
*/
- kAlphaUnchanged_Flag = 1 << 0,
- kSupports4f_Flag = 1 << 1,
+ kAlphaUnchanged_Flag = 0x01,
};
/** Returns the flags for this filter. Override in subclasses to return custom flags.
diff --git a/include/core/SkColorPriv.h b/include/core/SkColorPriv.h
index bd0e040..6347660 100644
--- a/include/core/SkColorPriv.h
+++ b/include/core/SkColorPriv.h
@@ -71,11 +71,6 @@
SK_B32_SHIFT == SK_BGRA_B32_SHIFT)
-#define SK_A_INDEX (SK_A32_SHIFT/8)
-#define SK_R_INDEX (SK_R32_SHIFT/8)
-#define SK_G_INDEX (SK_G32_SHIFT/8)
-#define SK_B_INDEX (SK_B32_SHIFT/8)
-
#if defined(SK_PMCOLOR_IS_RGBA) && !LOCAL_PMCOLOR_SHIFTS_EQUIVALENT_TO_RGBA
#error "SK_PMCOLOR_IS_RGBA does not match SK_*32_SHIFT values"
#endif
diff --git a/include/core/SkShader.h b/include/core/SkShader.h
index bf6d6dd..60ef280 100644
--- a/include/core/SkShader.h
+++ b/include/core/SkShader.h
@@ -81,7 +81,6 @@
shadeSpan().
*/
kConstInY32_Flag = 1 << 1,
- kSupports4f_Flag = 1 << 2,
};
/**
@@ -128,8 +127,6 @@
*/
virtual void shadeSpan(int x, int y, SkPMColor[], int count) = 0;
- virtual void shadeSpan4f(int x, int y, SkPM4f[], int count);
-
/**
* The const void* ctx is only const because all the implementations are const.
* This can be changed to non-const if a new shade proc needs to change the ctx.
diff --git a/include/effects/SkColorMatrixFilter.h b/include/effects/SkColorMatrixFilter.h
index 3edf791..7ffbf11 100644
--- a/include/effects/SkColorMatrixFilter.h
+++ b/include/effects/SkColorMatrixFilter.h
@@ -29,7 +29,6 @@
static SkColorFilter* CreateLightingFilter(SkColor mul, SkColor add);
void filterSpan(const SkPMColor src[], int count, SkPMColor[]) const override;
- void filterSpan4f(const SkPM4f src[], int count, SkPM4f[]) const override;
uint32_t getFlags() const override;
bool asColorMatrix(SkScalar matrix[20]) const override;
SkColorFilter* newComposed(const SkColorFilter*) const override;
diff --git a/src/core/SkColor.cpp b/src/core/SkColor.cpp
index c9f4a14..a21f019 100644
--- a/src/core/SkColor.cpp
+++ b/src/core/SkColor.cpp
@@ -100,37 +100,3 @@
}
return SkColorSetARGB(a, r, g, b);
}
-
-///////////////////////////////////////////////////////////////////////////////////////////////////
-#include "SkNx.h"
-
-SkColor4f SkColor4f::Pin(float a, float r, float g, float b) {
- SkColor4f c4;
- Sk4f::Min(Sk4f::Max(Sk4f(a, r, g, b), Sk4f(0)), Sk4f(1)).store(c4.vec());
- return c4;
-}
-
-SkColor4f SkColor4f::FromColor(SkColor c) {
- Sk4f value = SkNx_shuffle<3,2,1,0>(SkNx_cast<float>(Sk4b::Load((const uint8_t*)&c)));
- SkColor4f c4;
- (value * Sk4f(1.0f / 255)).store(c4.vec());
- return c4;
-}
-
-SkPM4f SkColor4f::premul() const {
- auto src = Sk4f::Load(this->pin().vec());
- float srcAlpha = src.kth<0>(); // need the pinned version of our alpha
- src = src * Sk4f(1, srcAlpha, srcAlpha, srcAlpha);
-
-#ifdef SK_PMCOLOR_IS_BGRA
- // ARGB -> BGRA
- Sk4f dst = SkNx_shuffle<3,2,1,0>(src);
-#else
- // ARGB -> RGBA
- Sk4f dst = SkNx_shuffle<1,2,3,0>(src);
-#endif
-
- SkPM4f pm4;
- dst.store(pm4.fVec);
- return pm4;
-}
diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp
index 4bfacfe..747e5ee 100644
--- a/src/core/SkColorFilter.cpp
+++ b/src/core/SkColorFilter.cpp
@@ -35,10 +35,6 @@
return SkUnPreMultiply::PMColorToColor(dst);
}
-void SkColorFilter::filterSpan4f(const SkPM4f[], int count, SkPM4f[]) const {
- SkASSERT(false && "filterSpan4f called but not implemented");
-}
-
///////////////////////////////////////////////////////////////////////////////////////////////////
/*
diff --git a/src/core/SkColorShader.h b/src/core/SkColorShader.h
index 1c07a3c..25a1d6c 100644
--- a/src/core/SkColorShader.h
+++ b/src/core/SkColorShader.h
@@ -36,10 +36,8 @@
uint32_t getFlags() const override;
void shadeSpan(int x, int y, SkPMColor span[], int count) override;
void shadeSpanAlpha(int x, int y, uint8_t alpha[], int count) override;
- void shadeSpan4f(int x, int y, SkPM4f[], int count) override;
private:
- SkPM4f fPM4f;
SkPMColor fPMColor;
uint32_t fFlags;
diff --git a/src/core/SkMathPriv.h b/src/core/SkMathPriv.h
index 6cca44e..b9184a0 100644
--- a/src/core/SkMathPriv.h
+++ b/src/core/SkMathPriv.h
@@ -78,8 +78,4 @@
return (prod + (prod >> 8)) >> 8;
}
-static inline float SkPinToUnitFloat(float x) {
- return SkTMin(SkTMax(x, 0.0f), 1.0f);
-}
-
#endif
diff --git a/src/core/SkShader.cpp b/src/core/SkShader.cpp
index 856b659..bd3876a 100644
--- a/src/core/SkShader.cpp
+++ b/src/core/SkShader.cpp
@@ -117,10 +117,6 @@
return nullptr;
}
-void SkShader::Context::shadeSpan4f(int x, int y, SkPM4f[], int count) {
- SkASSERT(false && "shadeSpan4f called but not implemented");
-}
-
#include "SkColorPriv.h"
#define kTempColorQuadCount 6 // balance between speed (larger) and saving stack-space
@@ -283,11 +279,7 @@
}
fPMColor = SkPackARGB32(a, r, g, b);
- SkColor4f c4 = SkColor4f::FromColor(shader.fColor);
- c4.fA *= rec.fPaint->getAlpha() / 255.0f;
- fPM4f = c4.premul();
-
- fFlags = kConstInY32_Flag | kSupports4f_Flag;
+ fFlags = kConstInY32_Flag;
if (255 == a) {
fFlags |= kOpaqueAlpha_Flag;
}
@@ -301,12 +293,6 @@
memset(alpha, SkGetPackedA32(fPMColor), count);
}
-void SkColorShader::ColorShaderContext::shadeSpan4f(int x, int y, SkPM4f span[], int count) {
- for (int i = 0; i < count; ++i) {
- span[i] = fPM4f;
- }
-}
-
SkShader::GradientType SkColorShader::asAGradient(GradientInfo* info) const {
if (info) {
if (info->fColors && info->fColorCount >= 1) {
diff --git a/src/effects/SkColorMatrixFilter.cpp b/src/effects/SkColorMatrixFilter.cpp
index 43ce6c2..ce6ca55 100644
--- a/src/effects/SkColorMatrixFilter.cpp
+++ b/src/effects/SkColorMatrixFilter.cpp
@@ -45,11 +45,10 @@
bool usesAlpha = (array[3] || array[8] || array[13]);
if (changesAlpha || usesAlpha) {
- fFlags = changesAlpha ? 0 : kAlphaUnchanged_Flag;
+ fFlags = changesAlpha ? 0 : SkColorFilter::kAlphaUnchanged_Flag;
} else {
- fFlags = kAlphaUnchanged_Flag;
+ fFlags = SkColorFilter::kAlphaUnchanged_Flag;
}
- fFlags |= kSupports4f_Flag;
}
///////////////////////////////////////////////////////////////////////////////
@@ -90,28 +89,28 @@
return c;
}
-template <typename Adaptor, typename T>
-void filter_span(const float array[], const T src[], int count, T dst[]) {
+void SkColorMatrixFilter::filterSpan(const SkPMColor src[], int count, SkPMColor dst[]) const {
// c0-c3 are already in [0,1].
- const Sk4f c0 = Sk4f::Load(array + 0);
- const Sk4f c1 = Sk4f::Load(array + 4);
- const Sk4f c2 = Sk4f::Load(array + 8);
- const Sk4f c3 = Sk4f::Load(array + 12);
+ const Sk4f c0 = Sk4f::Load(fTranspose + 0);
+ const Sk4f c1 = Sk4f::Load(fTranspose + 4);
+ const Sk4f c2 = Sk4f::Load(fTranspose + 8);
+ const Sk4f c3 = Sk4f::Load(fTranspose + 12);
// c4 (the translate vector) is in [0, 255]. Bring it back to [0,1].
- const Sk4f c4 = Sk4f::Load(array + 16)*Sk4f(1.0f/255);
+ const Sk4f c4 = Sk4f::Load(fTranspose + 16)*Sk4f(1.0f/255);
// todo: we could cache this in the constructor...
- T matrix_translate_pmcolor = Adaptor::From4f(premul(clamp_0_1(c4)));
+ SkPMColor matrix_translate_pmcolor = round(premul(clamp_0_1(c4)));
for (int i = 0; i < count; i++) {
- Sk4f srcf = Adaptor::To4f(src[i]);
- float srcA = srcf.kth<SK_A32_SHIFT/8>();
-
- if (0 == srcA) {
+ const SkPMColor src_c = src[i];
+ if (0 == src_c) {
dst[i] = matrix_translate_pmcolor;
continue;
}
- if (1 == srcA) {
+
+ Sk4f srcf = SkNx_cast<float>(Sk4b::Load((const uint8_t*)&src_c)) * Sk4f(1.0f/255);
+
+ if (0xFF != SkGetPackedA32(src_c)) {
srcf = unpremul(srcf);
}
@@ -123,36 +122,11 @@
// apply matrix
Sk4f dst4 = c0 * r4 + c1 * g4 + c2 * b4 + c3 * a4 + c4;
- dst[i] = Adaptor::From4f(premul(clamp_0_1(dst4)));
+ // clamp, re-premul, and write
+ dst[i] = round(premul(clamp_0_1(dst4)));
}
}
-struct SkPMColorAdaptor {
- static SkPMColor From4f(const Sk4f& c4) {
- return round(c4);
- }
- static Sk4f To4f(SkPMColor c) {
- return SkNx_cast<float>(Sk4b::Load((const uint8_t*)&c)) * Sk4f(1.0f/255);
- }
-};
-void SkColorMatrixFilter::filterSpan(const SkPMColor src[], int count, SkPMColor dst[]) const {
- filter_span<SkPMColorAdaptor>(fTranspose, src, count, dst);
-}
-
-struct SkPM4fAdaptor {
- static SkPM4f From4f(const Sk4f& c4) {
- SkPM4f c;
- c4.store(c.fVec);
- return c;
- }
- static Sk4f To4f(const SkPM4f& c) {
- return Sk4f::Load(c.fVec);
- }
-};
-void SkColorMatrixFilter::filterSpan4f(const SkPM4f src[], int count, SkPM4f dst[]) const {
- filter_span<SkPM4fAdaptor>(fTranspose, src, count, dst);
-}
-
///////////////////////////////////////////////////////////////////////////////
void SkColorMatrixFilter::flatten(SkWriteBuffer& buffer) const {
diff --git a/tests/SkColor4fTest.cpp b/tests/SkColor4fTest.cpp
deleted file mode 100644
index b09b0c4..0000000
--- a/tests/SkColor4fTest.cpp
+++ /dev/null
@@ -1,62 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-#include "SkColor.h"
-#include "SkShader.h"
-#include "SkColorMatrixFilter.h"
-#include "Test.h"
-#include "SkRandom.h"
-
-DEF_TEST(SkColor4f_FromColor, reporter) {
- const struct {
- SkColor fC;
- SkColor4f fC4;
- } recs[] = {
- { SK_ColorBLACK, { 1, 0, 0, 0 } },
- { SK_ColorWHITE, { 1, 1, 1, 1 } },
- { SK_ColorRED, { 1, 1, 0, 0 } },
- { SK_ColorGREEN, { 1, 0, 1, 0 } },
- { SK_ColorBLUE, { 1, 0, 0, 1 } },
- { 0, { 0, 0, 0, 0 } },
- { 0x55AAFF00, { 1/3.0f, 2/3.0f, 1, 0 } },
- };
-
- for (const auto& r : recs) {
- SkColor4f c4 = SkColor4f::FromColor(r.fC);
- REPORTER_ASSERT(reporter, c4 == r.fC4);
- }
-}
-
-static bool nearly_equal(float a, float b) {
- const float kTolerance = 1.0f / (1 << 20);
- return fabsf(a - b) < kTolerance;
-}
-
-DEF_TEST(SkColor4f_premul, reporter) {
- SkRandom rand;
-
- for (int i = 0; i < 1000000; ++i) {
- // First just test opaque colors, so that the premul should be exact
- SkColor4f c4 {
- 1, rand.nextUScalar1(), rand.nextUScalar1(), rand.nextUScalar1()
- };
- SkPM4f pm4 = c4.premul();
- REPORTER_ASSERT(reporter, pm4.fVec[SK_A_INDEX] == c4.fA);
- REPORTER_ASSERT(reporter, pm4.fVec[SK_R_INDEX] == c4.fA * c4.fR);
- REPORTER_ASSERT(reporter, pm4.fVec[SK_G_INDEX] == c4.fA * c4.fG);
- REPORTER_ASSERT(reporter, pm4.fVec[SK_B_INDEX] == c4.fA * c4.fB);
-
- // We compare with a tolerance, in case our premul multiply is implemented at slightly
- // different precision than the test code.
- c4.fA = rand.nextUScalar1();
- pm4 = c4.premul();
- REPORTER_ASSERT(reporter, pm4.fVec[SK_A_INDEX] == c4.fA);
- REPORTER_ASSERT(reporter, nearly_equal(pm4.fVec[SK_R_INDEX], c4.fA * c4.fR));
- REPORTER_ASSERT(reporter, nearly_equal(pm4.fVec[SK_G_INDEX], c4.fA * c4.fG));
- REPORTER_ASSERT(reporter, nearly_equal(pm4.fVec[SK_B_INDEX], c4.fA * c4.fB));
- }
-}