GrTextureEffect handles insets for float rects as well as integer rects.
Fix insets for clamp/nearest case.
Change-Id: I1fa2f6d172fdac59836ae73c7a5fec0d0cde020c
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/268397
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/gpu/GrTextureProducer.cpp b/src/gpu/GrTextureProducer.cpp
index 44600e4..d41c12b 100644
--- a/src/gpu/GrTextureProducer.cpp
+++ b/src/gpu/GrTextureProducer.cpp
@@ -110,7 +110,9 @@
return kNoDomain_DomainMode;
}
- // Get the domain inset based on sampling mode (or bail if mipped)
+ // Get the domain inset based on sampling mode (or bail if mipped). This is used
+ // to evaluate whether we will read outside a non-exact proxy's dimensions.
+ // TODO: Let GrTextureEffect handle this.
SkScalar filterHalfWidth = 0.f;
if (filterModeOrNullForBicubic) {
switch (*filterModeOrNullForBicubic) {
@@ -136,15 +138,8 @@
filterHalfWidth = 1.5f;
}
- // Both bilerp and bicubic use bilinear filtering and so need to be clamped to the center
- // of the edge texel. Pinning to the texel center has no impact on nearest mode and MIP-maps
-
- static const SkScalar kDomainInset = 0.5f;
- // Figure out the limits of pixels we're allowed to sample from.
- // Unless we know the amount of outset and the texture matrix we have to conservatively enforce
- // the domain.
if (restrictFilterToRect) {
- *domainRect = constraintRect.makeInset(kDomainInset, kDomainInset);
+ *domainRect = constraintRect;
} else if (!proxyIsExact) {
// If we got here then: proxy is not exact, the coords are limited to the
// constraint rect, and we're allowed to filter across the constraint rect boundary. So
@@ -157,32 +152,34 @@
// rect in order to avoid having to add a domain.
bool needContentAreaConstraint = false;
if (proxyBounds.fRight - filterHalfWidth < constraintRect.fRight) {
- domainRect->fRight = proxyBounds.fRight - kDomainInset;
+ domainRect->fRight = proxyBounds.fRight;
needContentAreaConstraint = true;
}
if (proxyBounds.fBottom - filterHalfWidth < constraintRect.fBottom) {
- domainRect->fBottom = proxyBounds.fBottom - kDomainInset;
+ domainRect->fBottom = proxyBounds.fBottom;
needContentAreaConstraint = true;
}
if (!needContentAreaConstraint) {
return kNoDomain_DomainMode;
}
- } else {
- // Our sample coords for the texture are allowed to be outside the constraintRect so we
- // don't consider it when computing the domain.
- domainRect->fRight = proxyBounds.fRight - kDomainInset;
- domainRect->fBottom = proxyBounds.fBottom - kDomainInset;
}
} else {
return kNoDomain_DomainMode;
}
- if (domainRect->fLeft > domainRect->fRight) {
- domainRect->fLeft = domainRect->fRight = SkScalarAve(domainRect->fLeft, domainRect->fRight);
+ if (!filterModeOrNullForBicubic) {
+ // Bicubic doesn't yet rely on GrTextureEffect to do this insetting.
+ domainRect->inset(0.5f, 0.5f);
+ if (domainRect->fLeft > domainRect->fRight) {
+ domainRect->fLeft = domainRect->fRight =
+ SkScalarAve(domainRect->fLeft, domainRect->fRight);
+ }
+ if (domainRect->fTop > domainRect->fBottom) {
+ domainRect->fTop = domainRect->fBottom =
+ SkScalarAve(domainRect->fTop, domainRect->fBottom);
+ }
}
- if (domainRect->fTop > domainRect->fBottom) {
- domainRect->fTop = domainRect->fBottom = SkScalarAve(domainRect->fTop, domainRect->fBottom);
- }
+
return kDomain_DomainMode;
}
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index 330deb6..8e7249e4 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -937,29 +937,15 @@
const auto& caps = *this->caps();
if (needsTextureDomain && (SkCanvas::kStrict_SrcRectConstraint == constraint)) {
- // Use a constrained texture domain to avoid color bleeding
- SkRect domain;
- if (srcRect.width() > SK_Scalar1) {
- domain.fLeft = srcRect.fLeft + 0.5f;
- domain.fRight = srcRect.fRight - 0.5f;
- } else {
- domain.fLeft = domain.fRight = srcRect.centerX();
- }
- if (srcRect.height() > SK_Scalar1) {
- domain.fTop = srcRect.fTop + 0.5f;
- domain.fBottom = srcRect.fBottom - 0.5f;
- } else {
- domain.fTop = domain.fBottom = srcRect.centerY();
- }
if (bicubic) {
static constexpr auto kDir = GrBicubicEffect::Direction::kXY;
- fp = GrBicubicEffect::Make(std::move(proxy), texMatrix, domain, kDir, srcAlphaType);
+ fp = GrBicubicEffect::Make(std::move(proxy), texMatrix, srcRect, kDir, srcAlphaType);
} else {
GrSurfaceOrigin origin = proxy->origin();
GrSwizzle swizzle = proxy->textureSwizzle();
GrSurfaceProxyView view(std::move(proxy), origin, swizzle);
- fp = GrTextureEffect::MakeSubset(std::move(view), srcAlphaType, texMatrix, samplerState,
- domain, caps);
+ fp = GrTextureEffect::MakeSubset(std::move(view), srcAlphaType, texMatrix,
+ samplerState, srcRect, caps);
}
} else if (bicubic) {
SkASSERT(GrSamplerState::Filter::kNearest == samplerState.filter());
diff --git a/src/gpu/effects/GrTextureEffect.cpp b/src/gpu/effects/GrTextureEffect.cpp
index 7b4a292..298cf47 100644
--- a/src/gpu/effects/GrTextureEffect.cpp
+++ b/src/gpu/effects/GrTextureEffect.cpp
@@ -78,7 +78,6 @@
GrTextureEffect::Sampling::Sampling(const GrSurfaceProxy& proxy,
GrSamplerState sampler,
const SkRect& subset,
- bool adjustForFilter,
const SkRect* domain,
const GrCaps& caps) {
using Mode = GrSamplerState::WrapMode;
@@ -91,9 +90,8 @@
Filter fFilter;
};
- auto resolve = [adjustForFilter, filter = sampler.filter(), &caps](int size, Mode mode,
- Span subset, Span domain) {
- float inset;
+ auto resolve = [filter = sampler.filter(), &caps](int size, Mode mode, Span subset,
+ Span domain) {
Result1D r;
r.fFilter = filter;
bool canDoHW = (mode != Mode::kClampToBorder || caps.clampToBorderSupport()) &&
@@ -104,10 +102,27 @@
return r;
}
- inset = (adjustForFilter && filter != Filter::kNearest) ? 0.5 : 0;
- auto insetSubset = subset.makeInset(inset);
-
- if (canDoHW && insetSubset.contains(domain)) {
+ bool domainIsSafe = false;
+ Span insetSubset;
+ if (filter == Filter::kNearest) {
+ Span isubset{sk_float_floor(subset.fA), sk_float_ceil(subset.fB)};
+ if (domain.fA > isubset.fA && domain.fB < isubset.fB) {
+ domainIsSafe = true;
+ }
+ if (mode == Mode::kClamp) {
+ // This inset prevents sampling neighboring texels that could occur when
+ // texture coords fall exactly at texel boundaries (depending on precision
+ // and GPU-specific snapping at the boundary).
+ insetSubset = isubset.makeInset(0.5f);
+ } else {
+ // TODO: Handle other modes properly in this case.
+ insetSubset = subset;
+ }
+ } else {
+ insetSubset = subset.makeInset(0.5f);
+ domainIsSafe = insetSubset.contains(domain);
+ }
+ if (canDoHW && domainIsSafe) {
r.fShaderMode = ShaderMode::kNone;
r.fHWMode = mode;
return r;
@@ -173,8 +188,7 @@
GrSamplerState sampler,
const SkIRect& subset,
const GrCaps& caps) {
- Sampling sampling(*view.proxy(), sampler, SkRect::Make(subset), true, nullptr, caps);
-
+ Sampling sampling(*view.proxy(), sampler, SkRect::Make(subset), nullptr, caps);
return std::unique_ptr<GrFragmentProcessor>(
new GrTextureEffect(std::move(view), alphaType, matrix, sampling));
}
@@ -186,7 +200,7 @@
const SkIRect& subset,
const SkRect& domain,
const GrCaps& caps) {
- Sampling sampling(*view.proxy(), sampler, SkRect::Make(subset), true, &domain, caps);
+ Sampling sampling(*view.proxy(), sampler, SkRect::Make(subset), &domain, caps);
return std::unique_ptr<GrFragmentProcessor>(
new GrTextureEffect(std::move(view), alphaType, matrix, sampling));
}
@@ -197,7 +211,7 @@
GrSamplerState sampler,
const SkRect& subset,
const GrCaps& caps) {
- Sampling sampling(*view.proxy(), sampler, subset, false, nullptr, caps);
+ Sampling sampling(*view.proxy(), sampler, subset, nullptr, caps);
return std::unique_ptr<GrFragmentProcessor>(
new GrTextureEffect(std::move(view), alphaType, matrix, sampling));
}
@@ -209,7 +223,7 @@
const SkRect& subset,
const SkRect& domain,
const GrCaps& caps) {
- Sampling sampling(*view.proxy(), sampler, subset, false, &domain, caps);
+ Sampling sampling(*view.proxy(), sampler, subset, &domain, caps);
return std::unique_ptr<GrFragmentProcessor>(
new GrTextureEffect(std::move(view), alphaType, matrix, sampling));
}
diff --git a/src/gpu/effects/GrTextureEffect.h b/src/gpu/effects/GrTextureEffect.h
index 513bd21..85bad15 100644
--- a/src/gpu/effects/GrTextureEffect.h
+++ b/src/gpu/effects/GrTextureEffect.h
@@ -61,9 +61,6 @@
/**
* This is similar to MakeTexelSubset but takes a float rather than integer subset rect.
- * No adjustment is done for filtering, the texture coordinates are limited to the
- * unmodified subset. The subset should be unnormalized. The effect will apply texture
- * coordinate normalization after subset restriction (logically).
*/
static std::unique_ptr<GrFragmentProcessor> MakeSubset(GrSurfaceProxyView,
SkAlphaType,
@@ -108,7 +105,6 @@
Sampling(const GrSurfaceProxy& proxy,
GrSamplerState sampler,
const SkRect&,
- bool,
const SkRect*,
const GrCaps&);
inline bool usesDecal() const;
diff --git a/src/gpu/effects/GrYUVtoRGBEffect.cpp b/src/gpu/effects/GrYUVtoRGBEffect.cpp
index 569be09..9183b67 100644
--- a/src/gpu/effects/GrYUVtoRGBEffect.cpp
+++ b/src/gpu/effects/GrYUVtoRGBEffect.cpp
@@ -70,10 +70,6 @@
GrSurfaceProxyView view(proxies[i], origin, swizzle);
if (domain) {
SkASSERT(planeFilter != GrSamplerState::Filter::kMipMap);
- if (planeFilter != GrSamplerState::Filter::kNearest) {
- // Inset by half a pixel for bilerp, after scaling to the size of the plane
- planeDomain.inset(0.5f, 0.5f);
- }
planeFPs[i] = GrTextureEffect::MakeSubset(std::move(view), kUnknown_SkAlphaType,
*planeMatrix, planeFilter, planeDomain, caps);
} else {
diff --git a/src/gpu/ops/GrTextureOp.cpp b/src/gpu/ops/GrTextureOp.cpp
index feae314..49b1590 100644
--- a/src/gpu/ops/GrTextureOp.cpp
+++ b/src/gpu/ops/GrTextureOp.cpp
@@ -1071,17 +1071,14 @@
std::unique_ptr<GrFragmentProcessor> fp;
if (domain) {
- // Update domain to match what GrTextureOp would do for bilerp, but don't do any
- // normalization since GrTextureEffect handles that and the origin.
- SkRect correctedDomain = normalize_domain(filter, {1.f, 1.f, 0.f}, domain);
const auto& caps = *context->priv().caps();
SkRect localRect;
if (localQuad.asRect(&localRect)) {
- fp = GrTextureEffect::MakeSubset(std::move(proxyView), alphaType, SkMatrix::I(),
- filter, correctedDomain, localRect, caps);
+ fp = GrTextureEffect::MakeSubset(std::move(proxyView), alphaType, SkMatrix::I(), filter,
+ *domain, localRect, caps);
} else {
- fp = GrTextureEffect::MakeSubset(std::move(proxyView), alphaType, SkMatrix::I(),
- filter, correctedDomain, caps);
+ fp = GrTextureEffect::MakeSubset(std::move(proxyView), alphaType, SkMatrix::I(), filter,
+ *domain, caps);
}
} else {
fp = GrTextureEffect::Make(std::move(proxyView), alphaType, SkMatrix::I(), filter);