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/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.