Reland "Remove all instances of incorrect coverage with DMSAA"
This is a reland of 3722d3195be5ddba7f3d44095565628dbe65452b
Original change's description:
> Remove all instances of incorrect coverage with DMSAA
>
> Here we pivot and handle DMSAA differently depending on platform:
>
> 1) Desktop GL and EXT_multisample_compatibility: Here it's easy to use
> all our existing coverage ops because we can just call
> glDisable(GL_MULTISAMPLE). So we only trigger MSAA for paths and
> use the coverage ops for everything else with MSAA disabled.
>
> 2) EXT_multisampled_render_to_to_texture (86% adoption on Android):
> The assumption here is that MSAA is almost free. So we just allow
> MSAA to be triggered often and don't worry about it.
>
> Devices that neither support #1 nor #2 just don't get DMSAA for now.
>
> Bug: skia:11396
> Change-Id: I53ad840216ea6d88ae69eece6f7a062f9e82dad7
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/421019
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
Bug: skia:11396
Change-Id: I536d365efc3c6890b45f14b7d188e0ef5ec36e19
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/421825
Reviewed-by: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrSurfaceDrawContext.cpp b/src/gpu/GrSurfaceDrawContext.cpp
index df4f1c7..09b7dc1 100644
--- a/src/gpu/GrSurfaceDrawContext.cpp
+++ b/src/gpu/GrSurfaceDrawContext.cpp
@@ -298,10 +298,10 @@
inline GrAAType GrSurfaceDrawContext::chooseAAType(GrAA aa) {
if (fCanUseDynamicMSAA) {
- // Trigger dmsaa by default if the render target has it. Coverage ops that know how to
- // handle both single and multisample targets without popping will do so without calling
- // chooseAAType.
- return GrAAType::kMSAA;
+ // Use coverage AA if we can disable multisample. Otherwise always trigger dmsaa. The few
+ // coverage ops we have that know how to handle both single and multisample targets without
+ // popping will do so without calling chooseAAType.
+ return this->caps()->multisampleDisableSupport() ? GrAAType::kCoverage : GrAAType::kMSAA;
}
if (GrAA::kNo == aa) {
// On some devices we cannot disable MSAA if it is enabled so we make the AA type reflect
@@ -730,7 +730,7 @@
!this->caps()->reducedShaderMode()) {
// Only use the StrokeRectOp for non-empty rectangles. Empty rectangles will be processed by
// GrStyledShape to handle stroke caps and dashing properly.
- GrAAType aaType = (fCanUseDynamicMSAA) ? GrAAType::kCoverage : this->chooseAAType(aa);
+ GrAAType aaType = this->chooseAAType(aa);
GrOp::Owner op = GrStrokeRectOp::Make(
fContext, std::move(paint), aaType, viewMatrix, rect, stroke);
// op may be null if the stroke is not supported or if using coverage aa and the view matrix
@@ -957,7 +957,7 @@
AutoCheckFlush acf(this->drawingManager());
SkASSERT(vertices);
- GrAAType aaType = this->chooseAAType(GrAA::kNo);
+ GrAAType aaType = fCanUseDynamicMSAA ? GrAAType::kMSAA : this->chooseAAType(GrAA::kNo);
GrOp::Owner op =
GrDrawVerticesOp::Make(fContext, std::move(paint), std::move(vertices), matrixProvider,
aaType, this->colorInfo().refColorSpaceXformFromSRGB(),
@@ -1049,28 +1049,28 @@
AutoCheckFlush acf(this->drawingManager());
GrAAType aaType = this->chooseAAType(aa);
- bool preferFillRRectOverCircle = aaType == GrAAType::kCoverage &&
- this->caps()->drawInstancedSupport() &&
- this->caps()->reducedShaderMode();
GrOp::Owner op;
- if (!preferFillRRectOverCircle &&
- GrAAType::kCoverage == aaType &&
- rrect.isSimple() &&
- rrect.getSimpleRadii().fX == rrect.getSimpleRadii().fY &&
- viewMatrix.rectStaysRect() && viewMatrix.isSimilarity()) {
- // In coverage mode, we draw axis-aligned circular roundrects with the GrOvalOpFactory
- // to avoid perf regressions on some platforms.
- assert_alive(paint);
- op = GrOvalOpFactory::MakeCircularRRectOp(
- fContext, std::move(paint), viewMatrix, rrect, stroke, this->caps()->shaderCaps());
+ if (aaType == GrAAType::kCoverage) {
+ bool preferFillRRectOverCircle = this->caps()->drawInstancedSupport() &&
+ (fCanUseDynamicMSAA || this->caps()->reducedShaderMode());
+ if (!preferFillRRectOverCircle &&
+ rrect.isSimple() &&
+ rrect.getSimpleRadii().fX == rrect.getSimpleRadii().fY &&
+ viewMatrix.rectStaysRect() && viewMatrix.isSimilarity()) {
+ // In coverage mode, we draw axis-aligned circular roundrects with the GrOvalOpFactory
+ // to avoid perf regressions on some platforms.
+ assert_alive(paint);
+ op = GrOvalOpFactory::MakeCircularRRectOp(fContext, std::move(paint), viewMatrix, rrect,
+ stroke, this->caps()->shaderCaps());
+ }
}
if (!op && style.isSimpleFill()) {
assert_alive(paint);
op = GrFillRRectOp::Make(fContext, std::move(paint), viewMatrix, rrect, rrect.rect(),
GrAA(aaType != GrAAType::kNone));
}
- if (!op && (GrAAType::kCoverage == aaType || fCanUseDynamicMSAA)) {
+ if (!op && aaType == GrAAType::kCoverage) {
assert_alive(paint);
op = GrOvalOpFactory::MakeRRectOp(
fContext, std::move(paint), viewMatrix, rrect, stroke, this->caps()->shaderCaps());
@@ -1325,9 +1325,11 @@
return this->drawPath(clip, std::move(paint), aa, viewMatrix, path, style);
}
- GrAAType aaType = this->chooseAAType(GrAA::kNo);
- GrOp::Owner op = GrRegionOp::Make(fContext, std::move(paint), viewMatrix, region,
- aaType, ss);
+ GrAAType aaType = (this->numSamples() > 1 &&
+ !this->caps()->multisampleDisableSupport())
+ ? GrAAType::kMSAA
+ : GrAAType::kNone;
+ GrOp::Owner op = GrRegionOp::Make(fContext, std::move(paint), viewMatrix, region, aaType, ss);
this->addDrawOp(clip, std::move(op));
}
@@ -1357,21 +1359,20 @@
GrAAType aaType = this->chooseAAType(aa);
- bool preferFillRRectOverCircle = aaType == GrAAType::kCoverage &&
- this->caps()->drawInstancedSupport() &&
- this->caps()->reducedShaderMode();
-
GrOp::Owner op;
- if (!preferFillRRectOverCircle &&
- GrAAType::kCoverage == aaType &&
- oval.width() > SK_ScalarNearlyZero &&
- oval.width() == oval.height() &&
- viewMatrix.isSimilarity()) {
- // We don't draw true circles as round rects in coverage mode, because it can
- // cause perf regressions on some platforms as compared to the dedicated circle Op.
- assert_alive(paint);
- op = GrOvalOpFactory::MakeCircleOp(fContext, std::move(paint), viewMatrix, oval, style,
- this->caps()->shaderCaps());
+ if (aaType == GrAAType::kCoverage) {
+ bool preferFillRRectOverCircle = this->caps()->drawInstancedSupport() &&
+ (fCanUseDynamicMSAA || this->caps()->reducedShaderMode());
+ if (!preferFillRRectOverCircle &&
+ oval.width() > SK_ScalarNearlyZero &&
+ oval.width() == oval.height() &&
+ viewMatrix.isSimilarity()) {
+ // We don't draw true circles as round rects in coverage mode, because it can
+ // cause perf regressions on some platforms as compared to the dedicated circle Op.
+ assert_alive(paint);
+ op = GrOvalOpFactory::MakeCircleOp(fContext, std::move(paint), viewMatrix, oval, style,
+ this->caps()->shaderCaps());
+ }
}
if (!op && style.isSimpleFill()) {
// GrFillRRectOp has special geometry and a fragment-shader branch to conditionally evaluate
@@ -1383,7 +1384,7 @@
op = GrFillRRectOp::Make(fContext, std::move(paint), viewMatrix, SkRRect::MakeOval(oval),
oval, GrAA(aaType != GrAAType::kNone));
}
- if (!op && (GrAAType::kCoverage == aaType || fCanUseDynamicMSAA)) {
+ if (!op && aaType == GrAAType::kCoverage) {
assert_alive(paint);
op = GrOvalOpFactory::MakeOvalOp(fContext, std::move(paint), viewMatrix, oval, style,
this->caps()->shaderCaps());
@@ -1416,7 +1417,7 @@
AutoCheckFlush acf(this->drawingManager());
GrAAType aaType = this->chooseAAType(aa);
- if (GrAAType::kCoverage == aaType || fCanUseDynamicMSAA) {
+ if (aaType == GrAAType::kCoverage) {
const GrShaderCaps* shaderCaps = this->caps()->shaderCaps();
GrOp::Owner op = GrOvalOpFactory::MakeArcOp(fContext,
std::move(paint),
@@ -1750,7 +1751,8 @@
SkIRect clipConservativeBounds = get_clip_bounds(this, clip);
- GrAAType aaType = this->chooseAAType(aa);
+ // Always allow paths to trigger DMSAA.
+ GrAAType aaType = fCanUseDynamicMSAA ? GrAAType::kMSAA : this->chooseAAType(aa);
GrPathRenderer::CanDrawPathArgs canDrawArgs;
canDrawArgs.fCaps = this->caps();
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index d4942ae..ff608c8 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -4612,6 +4612,23 @@
return GrDstSampleFlags::kNone;
}
+bool GrGLCaps::onSupportsDynamicMSAA(const GrRenderTargetProxy* rtProxy) const {
+ if (fDisallowDynamicMSAA) {
+ return false;
+ }
+ // We only allow DMSAA in two cases:
+ //
+ // 1) Desktop GL and EXT_multisample_compatibility: Here it's easy to use all our existing
+ // coverage ops because we can just call glDisable(GL_MULTISAMPLE). So we only trigger
+ // MSAA for paths and use the coverage ops for everything else with MSAA disabled.
+ //
+ // 2) EXT_multisampled_render_to_to_texture (86% adoption on Android): The assumption here
+ // is that MSAA is almost free. So we just allow MSAA to be triggered often and don't
+ // worry about it.
+ return fMultisampleDisableSupport ||
+ (fMSAAResolvesAutomatically && rtProxy->asTextureProxy());
+}
+
uint64_t GrGLCaps::computeFormatKey(const GrBackendFormat& format) const {
auto glFormat = format.asGLFormat();
return (uint64_t)(glFormat);
diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index 7b11851..15765cb 100644
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -506,9 +506,7 @@
GrDstSampleFlags onGetDstSampleFlagsForProxy(const GrRenderTargetProxy*) const override;
- bool onSupportsDynamicMSAA(const GrRenderTargetProxy*) const override {
- return !fDisallowDynamicMSAA;
- }
+ bool onSupportsDynamicMSAA(const GrRenderTargetProxy*) const override;
GrGLStandard fStandard = kNone_GrGLStandard;
diff --git a/src/gpu/ops/GrDashLinePathRenderer.cpp b/src/gpu/ops/GrDashLinePathRenderer.cpp
index dfa2967..d880283 100644
--- a/src/gpu/ops/GrDashLinePathRenderer.cpp
+++ b/src/gpu/ops/GrDashLinePathRenderer.cpp
@@ -37,8 +37,9 @@
aaMode = GrDashOp::AAMode::kNone;
break;
case GrAAType::kMSAA:
- if (args.fSurfaceDrawContext->canUseDynamicMSAA()) {
- // In DMSAA we avoid using MSAA, in order to reduce the number of MSAA triggers.
+ if (args.fSurfaceDrawContext->canUseDynamicMSAA() &&
+ args.fContext->priv().caps()->multisampleDisableSupport()) {
+ // The coverage-only mode works as well as MSAA, so try to avoid DMSAA triggers.
aaMode = GrDashOp::AAMode::kCoverage;
} else {
// In this mode we will use aa between dashes but the outer border uses MSAA.