From 3722d3195be5ddba7f3d44095565628dbe65452b Mon Sep 17 00:00:00 2001 From: Chris Dalton Date: Wed, 23 Jun 2021 17:52:09 -0600 Subject: [PATCH] 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 Reviewed-by: Brian Salomon --- src/gpu/GrSurfaceDrawContext.cpp | 16 ++++++++-------- src/gpu/gl/GrGLCaps.cpp | 17 +++++++++++++++++ src/gpu/gl/GrGLCaps.h | 4 +--- src/gpu/ops/GrDashLinePathRenderer.cpp | 12 +++--------- 4 files changed, 29 insertions(+), 20 deletions(-) diff --git a/src/gpu/GrSurfaceDrawContext.cpp b/src/gpu/GrSurfaceDrawContext.cpp index 7dd213b9c1..2240135156 100644 --- a/src/gpu/GrSurfaceDrawContext.cpp +++ b/src/gpu/GrSurfaceDrawContext.cpp @@ -297,10 +297,10 @@ void GrSurfaceDrawContext::willReplaceOpsTask(GrOpsTask* prevTask, GrOpsTask* ne } 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. + if (fCanUseDynamicMSAA && !this->caps()->multisampleDisableSupport()) { + // Trigger dmsaa by default if we have it and we can't disable multisample. 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 GrAAType::kMSAA; } if (GrAA::kNo == aa) { @@ -730,7 +730,7 @@ void GrSurfaceDrawContext::drawRect(const GrClip* clip, !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 @@ -1070,7 +1070,7 @@ void GrSurfaceDrawContext::drawRRect(const GrClip* origClip, op = GrFillRRectOp::Make(fContext, std::move(paint), viewMatrix, rrect, rrect.rect(), GrAA(aaType != GrAAType::kNone)); } - if (!op && (GrAAType::kCoverage == aaType || fCanUseDynamicMSAA)) { + if (!op && GrAAType::kCoverage == aaType) { assert_alive(paint); op = GrOvalOpFactory::MakeRRectOp( fContext, std::move(paint), viewMatrix, rrect, stroke, this->caps()->shaderCaps()); @@ -1383,7 +1383,7 @@ void GrSurfaceDrawContext::drawOval(const GrClip* clip, op = GrFillRRectOp::Make(fContext, std::move(paint), viewMatrix, SkRRect::MakeOval(oval), oval, GrAA(aaType != GrAAType::kNone)); } - if (!op && (GrAAType::kCoverage == aaType || fCanUseDynamicMSAA)) { + if (!op && GrAAType::kCoverage == aaType) { assert_alive(paint); op = GrOvalOpFactory::MakeOvalOp(fContext, std::move(paint), viewMatrix, oval, style, this->caps()->shaderCaps()); @@ -1416,7 +1416,7 @@ void GrSurfaceDrawContext::drawArc(const GrClip* clip, AutoCheckFlush acf(this->drawingManager()); GrAAType aaType = this->chooseAAType(aa); - if (GrAAType::kCoverage == aaType || fCanUseDynamicMSAA) { + if (GrAAType::kCoverage == aaType) { const GrShaderCaps* shaderCaps = this->caps()->shaderCaps(); GrOp::Owner op = GrOvalOpFactory::MakeArcOp(fContext, std::move(paint), diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp index bf9103f744..9eb5dd4f8f 100644 --- a/src/gpu/gl/GrGLCaps.cpp +++ b/src/gpu/gl/GrGLCaps.cpp @@ -4571,6 +4571,23 @@ GrDstSampleFlags GrGLCaps::onGetDstSampleFlagsForProxy(const GrRenderTargetProxy 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 84cd3a5d32..d35b699709 100644 --- a/src/gpu/gl/GrGLCaps.h +++ b/src/gpu/gl/GrGLCaps.h @@ -505,9 +505,7 @@ private: 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 dfa2967335..eda3392a70 100644 --- a/src/gpu/ops/GrDashLinePathRenderer.cpp +++ b/src/gpu/ops/GrDashLinePathRenderer.cpp @@ -37,15 +37,9 @@ bool GrDashLinePathRenderer::onDrawPath(const DrawPathArgs& args) { 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. - aaMode = GrDashOp::AAMode::kCoverage; - } else { - // In this mode we will use aa between dashes but the outer border uses MSAA. - // Otherwise, we can wind up with external edges antialiased and internal edges - // unantialiased. - aaMode = GrDashOp::AAMode::kCoverageWithMSAA; - } + // In this mode we will use aa between dashes but the outer border uses MSAA. Otherwise, + // we can wind up with external edges antialiased and internal edges unantialiased. + aaMode = GrDashOp::AAMode::kCoverageWithMSAA; break; case GrAAType::kCoverage: aaMode = GrDashOp::AAMode::kCoverage;