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 <johnstiles@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/gm/bleed.cpp b/gm/bleed.cpp
index 005510c..4eeae73 100644
--- a/gm/bleed.cpp
+++ b/gm/bleed.cpp
@@ -219,9 +219,9 @@
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;
@@ -229,58 +229,56 @@
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();
}
diff --git a/src/gpu/ops/GrTextureOp.cpp b/src/gpu/ops/GrTextureOp.cpp
index 13999dc..f07148e 100644
--- a/src/gpu/ops/GrTextureOp.cpp
+++ b/src/gpu/ops/GrTextureOp.cpp
@@ -185,6 +185,32 @@
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 @@
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 @@
!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,11 +568,6 @@
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());
@@ -556,6 +575,7 @@
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 @@
// 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.
