Remove debug quadType() checks
This was originally in place to prevent gross programmer error when
writing the rectangle draw ops. Given how sensitive the quadType()
functions are to numerical precision issues, it created many false
positives. The quadType() is only used for minor optimizations that
don't really impact what is drawn, so there is no real risk when the
type determined via matrix differs from the type calculated from the
coordinates.
Bug: chromium:935563, chromium:935503
Change-Id: I4c9798758971dc853289975335506ade141441df
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/207882
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/gpu/GrQuad.cpp b/src/gpu/GrQuad.cpp
index c741827..34ee630 100644
--- a/src/gpu/GrQuad.cpp
+++ b/src/gpu/GrQuad.cpp
@@ -9,84 +9,6 @@
#include "GrTypesPriv.h"
-///////////////////////////////////////////////////////////////////////////////////////////////////
-// Functions for identifying the quad type from its coordinates, which are kept debug-only since
-// production code should rely on the matrix to derive the quad type more efficiently. These are
-// useful in asserts that the quad type is as expected.
-///////////////////////////////////////////////////////////////////////////////////////////////////
-
-#ifdef SK_DEBUG
-// Allow some tolerance from floating point matrix transformations, but SkScalarNearlyEqual doesn't
-// support comparing infinity, and coords_form_rect should return true for infinite edges
-#define NEARLY_EQUAL(f1, f2) (f1 == f2 || SkScalarNearlyEqual(f1, f2, 1e-5f))
-// Similarly, support infinite rectangles by looking at the sign of infinities
-static bool dot_nearly_zero(const SkVector& e1, const SkVector& e2) {
- static constexpr auto dot = SkPoint::DotProduct;
- static constexpr auto sign = SkScalarSignAsScalar;
-
- SkScalar dotValue = dot(e1, e2);
- if (SkScalarIsNaN(dotValue)) {
- // Form vectors from the signs of infinities, and check their dot product
- dotValue = dot({sign(e1.fX), sign(e1.fY)}, {sign(e2.fX), sign(e2.fY)});
- }
-
- return SkScalarNearlyZero(dotValue, 1e-3f);
-}
-
-// This is not the most performance critical function; code using GrQuad should rely on the faster
-// quad type from matrix path, so this will only be called as part of SkASSERT.
-static bool coords_form_rect(const float xs[4], const float ys[4]) {
- return (NEARLY_EQUAL(xs[0], xs[1]) && NEARLY_EQUAL(xs[2], xs[3]) &&
- NEARLY_EQUAL(ys[0], ys[2]) && NEARLY_EQUAL(ys[1], ys[3])) ||
- (NEARLY_EQUAL(xs[0], xs[2]) && NEARLY_EQUAL(xs[1], xs[3]) &&
- NEARLY_EQUAL(ys[0], ys[1]) && NEARLY_EQUAL(ys[2], ys[3]));
-}
-
-static bool coords_rectilinear(const float xs[4], const float ys[4]) {
- SkVector e0{xs[1] - xs[0], ys[1] - ys[0]}; // connects to e1 and e2(repeat)
- SkVector e1{xs[3] - xs[1], ys[3] - ys[1]}; // connects to e0(repeat) and e3
- SkVector e2{xs[0] - xs[2], ys[0] - ys[2]}; // connects to e0 and e3(repeat)
- SkVector e3{xs[2] - xs[3], ys[2] - ys[3]}; // connects to e1(repeat) and e2
-
- e0.normalize();
- e1.normalize();
- e2.normalize();
- e3.normalize();
-
- return dot_nearly_zero(e0, e1) && dot_nearly_zero(e1, e3) &&
- dot_nearly_zero(e2, e0) && dot_nearly_zero(e3, e2);
-}
-
-GrQuadType GrQuad::quadType() const {
- // Since GrQuad applies any perspective information at construction time, there's only two
- // types to choose from.
- if (coords_form_rect(fX, fY)) {
- return GrQuadType::kRect;
- } else if (coords_rectilinear(fX, fY)) {
- return GrQuadType::kRectilinear;
- } else {
- return GrQuadType::kStandard;
- }
-}
-
-GrQuadType GrPerspQuad::quadType() const {
- if (this->hasPerspective()) {
- return GrQuadType::kPerspective;
- } else {
- // Rect or standard quad, can ignore w since they are all ones
- if (coords_form_rect(fX, fY)) {
- return GrQuadType::kRect;
- } else if (coords_rectilinear(fX, fY)) {
- return GrQuadType::kRectilinear;
- } else {
- return GrQuadType::kStandard;
- }
- }
-}
-#endif
-
-///////////////////////////////////////////////////////////////////////////////////////////////////
-
static bool aa_affects_rect(float ql, float qt, float qr, float qb) {
return !SkScalarIsInt(ql) || !SkScalarIsInt(qr) || !SkScalarIsInt(qt) || !SkScalarIsInt(qb);
}
@@ -239,7 +161,6 @@
}
bool GrQuad::aaHasEffectOnRect() const {
- SkASSERT(this->quadType() == GrQuadType::kRect);
return aa_affects_rect(fX[0], fY[0], fX[3], fY[3]);
}
@@ -275,7 +196,6 @@
}
bool GrPerspQuad::aaHasEffectOnRect() const {
- SkASSERT(this->quadType() == GrQuadType::kRect);
// If rect, ws must all be 1s so no need to divide
return aa_affects_rect(fX[0], fY[0], fX[3], fY[3]);
}
diff --git a/src/gpu/GrQuad.h b/src/gpu/GrQuad.h
index 675a088..f0fc1fd 100644
--- a/src/gpu/GrQuad.h
+++ b/src/gpu/GrQuad.h
@@ -34,9 +34,7 @@
};
static const int kGrQuadTypeCount = static_cast<int>(GrQuadType::kLast) + 1;
-// If an SkRect is transformed by this matrix, what class of quad is required to represent it. Since
-// quadType() is only provided on Gr[Persp]Quad in debug builds, production code should use this
-// to efficiently determine quad types.
+// If an SkRect is transformed by this matrix, what class of quad is required to represent it.
GrQuadType GrQuadTypeForTransformedRect(const SkMatrix& matrix);
// Perform minimal analysis of 'pts' (which are suitable for MakeFromSkQuad), and determine a
// quad type that will be as minimally general as possible.
@@ -97,13 +95,9 @@
Sk4f x4f() const { return Sk4f::Load(fX); }
Sk4f y4f() const { return Sk4f::Load(fY); }
- // True if anti-aliasing affects this quad. Requires quadType() == kRect_QuadType
+ // True if anti-aliasing affects this quad. Only valid when quadType == kRect_QuadType
bool aaHasEffectOnRect() const;
-#ifdef SK_DEBUG
- GrQuadType quadType() const;
-#endif
-
private:
template<typename T>
friend class GrQuadListBase;
@@ -145,8 +139,6 @@
SkPoint3 point(int i) const { return {fX[i], fY[i], fW[i]}; }
SkRect bounds(GrQuadType type) const {
- SkASSERT(this->quadType() <= type);
-
Sk4f x = this->x4f();
Sk4f y = this->y4f();
if (type == GrQuadType::kPerspective) {
@@ -170,13 +162,9 @@
bool hasPerspective() const { return (w4f() != Sk4f(1.f)).anyTrue(); }
- // True if anti-aliasing affects this quad. Requires quadType() == kRect_QuadType
+ // True if anti-aliasing affects this quad. Only valid when quadType == kRect_QuadType
bool aaHasEffectOnRect() const;
-#ifdef SK_DEBUG
- GrQuadType quadType() const;
-#endif
-
private:
template<typename T>
friend class GrQuadListBase;
@@ -261,8 +249,6 @@
// Returns the added item data so that its metadata can be initialized if T is not void
QuadData<T>& pushBackImpl(const GrQuad& quad, GrQuadType type) {
- SkASSERT(quad.quadType() <= type);
-
this->upgradeType(type);
QuadData<T>& item = fXYs.push_back();
memcpy(item.fX, quad.fX, 4 * sizeof(float));
@@ -274,8 +260,6 @@
}
QuadData<T>& pushBackImpl(const GrPerspQuad& quad, GrQuadType type) {
- SkASSERT(quad.quadType() <= type);
-
this->upgradeType(type);
QuadData<T>& item = fXYs.push_back();
memcpy(item.fX, quad.fX, 4 * sizeof(float));
diff --git a/src/gpu/ops/GrFillRectOp.cpp b/src/gpu/ops/GrFillRectOp.cpp
index 6810a6f..ef4f20a 100644
--- a/src/gpu/ops/GrFillRectOp.cpp
+++ b/src/gpu/ops/GrFillRectOp.cpp
@@ -299,8 +299,6 @@
void addQuad(const GrPerspQuad& deviceQuad, const GrPerspQuad& localQuad,
GrQuadType localQuadType, const SkPMColor4f& color, GrQuadAAFlags edgeAA,
GrAAType aaType) {
- SkASSERT(deviceQuad.quadType() <= fDeviceQuads.quadType());
-
// The new quad's aa type should be the same as the first quad's or none, except when the
// first quad's aa type was already downgraded to none, in which case the stored type must
// be lifted to back to the requested type.
