Fix strict-constraint bleed in strict_constraint_[batch_]no_red_allowed.

GrTextureOp was attempting to detect subset-rectangles that wouldn't
affect the rendering output and could be ignored. Unfortunately, this
optimization attempt had various flaws--small, one-pixel cracks on
the edge of the border when AA was off, and highly-visible red fuzz on
the edges of textures when MSAA was enabled. This CL limits the
optimization to cases where the source and destination quads are
axis-aligned rectangles, or cases where the inset is more than a half-
pixel deep.

This fix was made for both the single-image and batch drawing path, and
generalized as much as possible to allow the code to be shared.

This CL also cleans up the test code slightly.

Bug: skia:10263, skia:10277
Change-Id: I200aaab47737b5ba0f559182ef4d0dfe0b719d50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291197
Commit-Queue: John Stiles <[email protected]>
Reviewed-by: Michael Ludwig <[email protected]>
Auto-Submit: John Stiles <[email protected]>

Author: johnstiles-google

gm/bleed.cpp

@@ -219,68 +219,66 @@ class SrcRectConstraintGM : public skiagm::GM {
 
     void onDraw(SkCanvas* canvas) override {
         canvas->clear(SK_ColorGRAY);
-        SkTDArray<SkMatrix> matrices;
+        std::vector<SkMatrix> matrices;
         // Draw with identity
-        *matrices.append() = SkMatrix::I();
+        matrices.push_back(SkMatrix::I());
 
         // Draw with rotation and scale down in x, up in y.
         SkMatrix m;
         constexpr SkScalar kBottom = SkIntToScalar(kRow4Y + kBlockSize + kBlockSpacing);
         m.setTranslate(0, kBottom);
         m.preRotate(15.f, 0, kBottom + kBlockSpacing);
         m.preScale(0.71f, 1.22f);
-        *matrices.append() = m;
+        matrices.push_back(m);
 
         // Align the next set with the middle of the previous in y, translated to the right in x.
-        SkPoint corners[] = {{0, 0}, { 0, kBottom }, { kWidth, kBottom }, {kWidth, 0} };
-        matrices[matrices.count()-1].mapPoints(corners, 4);
+        SkPoint corners[] = {{0, 0}, {0, kBottom}, {kWidth, kBottom}, {kWidth, 0}};
+        matrices.back().mapPoints(corners, 4);
         SkScalar y = (corners[0].fY + corners[1].fY + corners[2].fY + corners[3].fY) / 4;
-        SkScalar x = std::max(std::max(corners[0].fX, corners[1].fX),
-                              std::max(corners[2].fX, corners[3].fX));
+        SkScalar x = std::max({corners[0].fX, corners[1].fX, corners[2].fX, corners[3].fX});
         m.setTranslate(x, y);
         m.preScale(0.2f, 0.2f);
-        *matrices.append() = m;
+        matrices.push_back(m);
 
         SkScalar maxX = 0;
-        for (int antiAlias = 0; antiAlias < 2; ++antiAlias) {
+        for (bool antiAlias : {false, true}) {
             canvas->save();
             canvas->translate(maxX, 0);
-            for (int m = 0; m < matrices.count(); ++m) {
+            for (const SkMatrix& matrix : matrices) {
                 canvas->save();
-                canvas->concat(matrices[m]);
-                bool aa = SkToBool(antiAlias);
+                canvas->concat(matrix);
 
                 // First draw a column with no filtering
-                this->drawCase1(canvas, kCol0X, kRow0Y, aa, kNone_SkFilterQuality);
-                this->drawCase2(canvas, kCol0X, kRow1Y, aa, kNone_SkFilterQuality);
-                this->drawCase3(canvas, kCol0X, kRow2Y, aa, kNone_SkFilterQuality);
-                this->drawCase4(canvas, kCol0X, kRow3Y, aa, kNone_SkFilterQuality);
-                this->drawCase5(canvas, kCol0X, kRow4Y, aa, kNone_SkFilterQuality);
+                this->drawCase1(canvas, kCol0X, kRow0Y, antiAlias, kNone_SkFilterQuality);
+                this->drawCase2(canvas, kCol0X, kRow1Y, antiAlias, kNone_SkFilterQuality);
+                this->drawCase3(canvas, kCol0X, kRow2Y, antiAlias, kNone_SkFilterQuality);
+                this->drawCase4(canvas, kCol0X, kRow3Y, antiAlias, kNone_SkFilterQuality);
+                this->drawCase5(canvas, kCol0X, kRow4Y, antiAlias, kNone_SkFilterQuality);
 
                 // Then draw a column with low filtering
-                this->drawCase1(canvas, kCol1X, kRow0Y, aa, kLow_SkFilterQuality);
-                this->drawCase2(canvas, kCol1X, kRow1Y, aa, kLow_SkFilterQuality);
-                this->drawCase3(canvas, kCol1X, kRow2Y, aa, kLow_SkFilterQuality);
-                this->drawCase4(canvas, kCol1X, kRow3Y, aa, kLow_SkFilterQuality);
-                this->drawCase5(canvas, kCol1X, kRow4Y, aa, kLow_SkFilterQuality);
+                this->drawCase1(canvas, kCol1X, kRow0Y, antiAlias, kLow_SkFilterQuality);
+                this->drawCase2(canvas, kCol1X, kRow1Y, antiAlias, kLow_SkFilterQuality);
+                this->drawCase3(canvas, kCol1X, kRow2Y, antiAlias, kLow_SkFilterQuality);
+                this->drawCase4(canvas, kCol1X, kRow3Y, antiAlias, kLow_SkFilterQuality);
+                this->drawCase5(canvas, kCol1X, kRow4Y, antiAlias, kLow_SkFilterQuality);
 
                 // Then draw a column with high filtering. Skip it if in kStrict mode and MIP
                 // mapping will be used. On GPU we allow bleeding at non-base levels because
                 // building a new MIP chain for the subset is expensive.
                 SkScalar scales[2];
-                SkAssertResult(matrices[m].getMinMaxScales(scales));
+                SkAssertResult(matrix.getMinMaxScales(scales));
                 if (fConstraint != SkCanvas::kStrict_SrcRectConstraint || scales[0] >= 1.f) {
-                    this->drawCase1(canvas, kCol2X, kRow0Y, aa, kHigh_SkFilterQuality);
-                    this->drawCase2(canvas, kCol2X, kRow1Y, aa, kHigh_SkFilterQuality);
-                    this->drawCase3(canvas, kCol2X, kRow2Y, aa, kHigh_SkFilterQuality);
-                    this->drawCase4(canvas, kCol2X, kRow3Y, aa, kHigh_SkFilterQuality);
-                    this->drawCase5(canvas, kCol2X, kRow4Y, aa, kHigh_SkFilterQuality);
+                    this->drawCase1(canvas, kCol2X, kRow0Y, antiAlias, kHigh_SkFilterQuality);
+                    this->drawCase2(canvas, kCol2X, kRow1Y, antiAlias, kHigh_SkFilterQuality);
+                    this->drawCase3(canvas, kCol2X, kRow2Y, antiAlias, kHigh_SkFilterQuality);
+                    this->drawCase4(canvas, kCol2X, kRow3Y, antiAlias, kHigh_SkFilterQuality);
+                    this->drawCase5(canvas, kCol2X, kRow4Y, antiAlias, kHigh_SkFilterQuality);
                 }
 
-                SkPoint corners[] = { { 0, 0 },{ 0, kBottom },{ kWidth, kBottom },{ kWidth, 0 } };
-                matrices[m].mapPoints(corners, 4);
-                SkScalar x = kBlockSize + std::max(std::max(corners[0].fX, corners[1].fX),
-                                                   std::max(corners[2].fX, corners[3].fX));
+                SkPoint innerCorners[] = {{0, 0}, {0, kBottom}, {kWidth, kBottom}, {kWidth, 0}};
+                matrix.mapPoints(innerCorners, 4);
+                SkScalar x = kBlockSize + std::max({innerCorners[0].fX, innerCorners[1].fX,
+                                                    innerCorners[2].fX, innerCorners[3].fX});
                 maxX = std::max(maxX, x);
                 canvas->restore();
             }

src/gpu/ops/GrTextureOp.cpp

@@ -185,6 +185,32 @@ static int proxy_run_count(const GrRenderTargetContext::TextureSetEntry set[], i
     return actualProxyRunCount;
 }
 
+static bool safe_to_ignore_subset_rect(GrAAType aaType, GrSamplerState::Filter filter,
+                                       const DrawQuad& quad, const SkRect& subsetRect) {
+    // If both the device and local quad are both axis-aligned, and filtering is off, the local quad
+    // can push all the way up to the edges of the the subset rect and the sampler shouldn't
+    // overshoot. Unfortunately, antialiasing adds enough jitter that we can only rely on this in
+    // the non-antialiased case.
+    SkRect localBounds = quad.fLocal.bounds();
+    if (aaType == GrAAType::kNone &&
+        filter == GrSamplerState::Filter::kNearest &&
+        quad.fDevice.quadType() == GrQuad::Type::kAxisAligned &&
+        quad.fLocal.quadType() == GrQuad::Type::kAxisAligned &&
+        subsetRect.contains(localBounds)) {
+
+        return true;
+    }
+
+    // If the subset rect is inset by at least 0.5 pixels into the local quad's bounds, the
+    // sampler shouldn't overshoot, even when antialiasing and filtering is taken into account.
+    if (subsetRect.makeInset(0.5f, 0.5f).contains(localBounds)) {
+        return true;
+    }
+
+    // The subset rect cannot be ignored safely.
+    return false;
+}
+
 /**
  * Op that implements GrTextureOp::Make. It draws textured quads. Each quad can modulate against a
  * the texture by color. The blend with the destination is always src-over. The edges are non-AA.
@@ -417,9 +443,7 @@ class TextureOp final : public GrMeshDrawOp {
         void allocatePrePreparedVertices(SkArenaAlloc* arena) {
             fPrePreparedVertices = arena->makeArrayDefault<char>(this->totalSizeInBytes());
         }
-
     };
-
     // If subsetRect is not null it will be used to apply a strict src rect-style constraint.
     TextureOp(GrSurfaceProxyView proxyView,
               sk_sp<GrColorSpaceXform> textureColorSpaceXform,
@@ -446,12 +470,12 @@ class TextureOp final : public GrMeshDrawOp {
                  !subsetRect->contains(proxyView.proxy()->backingStoreBoundsRect()));
 
         // We may have had a strict constraint with nearest filter solely due to possible AA bloat.
-        // If we don't have (or determined we don't need) coverage AA then we can skip using a
-        // subset.
-        if (subsetRect && filter == GrSamplerState::Filter::kNearest &&
-            aaType != GrAAType::kCoverage) {
-            subsetRect = nullptr;
-            fMetadata.fSubset = static_cast<uint16_t>(Subset::kNo);
+        // Try to identify cases where the subsetting isn't actually necessary, and skip it.
+        if (subsetRect) {
+            if (safe_to_ignore_subset_rect(aaType, filter, *quad, *subsetRect)) {
+                subsetRect = nullptr;
+                fMetadata.fSubset = static_cast<uint16_t>(Subset::kNo);
+            }
         }
 
         // Normalize src coordinates and the subset (if set)
@@ -544,18 +568,14 @@ class TextureOp final : public GrMeshDrawOp {
                 netFilter = GrSamplerState::Filter::kBilerp;
             }
 
-            // Normalize the src quads and apply origin
-            NormalizationParams proxyParams = proxy_normalization_params(
-                    curProxy, set[q].fProxyView.origin());
-            normalize_src_quad(proxyParams, &quad.fLocal);
-
             // Update overall bounds of the op as the union of all quads
             bounds.joinPossiblyEmptyRect(quad.fDevice.bounds());
 
             // Determine the AA type for the quad, then merge with net AA type
             GrAAType aaForQuad;
             GrQuadUtils::ResolveAAType(aaType, set[q].fAAFlags, quad.fDevice,
                                        &aaForQuad, &quad.fEdgeFlags);
+
             // Resolve sets aaForQuad to aaType or None, there is never a change between aa methods
             SkASSERT(aaForQuad == GrAAType::kNone || aaForQuad == aaType);
             if (netAAType == GrAAType::kNone && aaForQuad != GrAAType::kNone) {
@@ -565,17 +585,21 @@ class TextureOp final : public GrMeshDrawOp {
             // Calculate metadata for the entry
             const SkRect* subsetForQuad = nullptr;
             if (constraint == SkCanvas::kStrict_SrcRectConstraint) {
-                // Check (briefly) if the strict constraint is needed for this set entry
-                if (!set[q].fSrcRect.contains(curProxy->backingStoreBoundsRect()) &&
-                    (filter == GrSamplerState::Filter::kBilerp ||
-                     aaForQuad == GrAAType::kCoverage)) {
-                    // Can't rely on hardware clamping and the draw will access outer texels
-                    // for AA and/or bilerp. Unlike filter quality, this op still has per-quad
-                    // control over AA so that can check aaForQuad, not netAAType.
-                    netSubset = Subset::kYes;
-                    subsetForQuad = &set[q].fSrcRect;
+                // Check (briefly) if the subset rect is actually needed for this set entry.
+                SkRect* subsetRect = &set[q].fSrcRect;
+                if (!subsetRect->contains(curProxy->backingStoreBoundsRect())) {
+                    if (!safe_to_ignore_subset_rect(aaForQuad, filter, quad, *subsetRect)) {
+                        netSubset = Subset::kYes;
+                        subsetForQuad = subsetRect;
+                    }
                 }
             }
+
+            // Normalize the src quads and apply origin
+            NormalizationParams proxyParams = proxy_normalization_params(
+                    curProxy, set[q].fProxyView.origin());
+            normalize_src_quad(proxyParams, &quad.fLocal);
+
             // This subset may represent a no-op, otherwise it will have the origin and dimensions
             // of the texture applied to it. Insetting for bilinear filtering is deferred until
             // on[Pre]Prepare so that the overall filter can be lazily determined.