Skip to content

Commit bf341ae

Browse files
csmartdalton86Skia Commit-Bot
csmartdalton86
authored and
Skia Commit-Bot
committed
Revert "Add MSAA and non-aa modes to GrFillRRect Op"
This reverts commit 16a8e99. Reason for revert: Amd/Radeon bugs and quality concerns Original change's description: > Add MSAA and non-aa modes to GrFillRRect Op > > Adds a non-aa mode and an MSAA mode that uses the sample mask. Also > adds a new cap to decide whether we prefer this new sample mask Op for > large round rects, or whether it's faster to just continue drawing > them as paths like before. > > Bug: skia: > Change-Id: Iea7d9a78766f67c196a02cb833c84a0ac3f1bbac > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/202921 > Reviewed-by: Brian Salomon <[email protected]> > Commit-Queue: Chris Dalton <[email protected]> [email protected],[email protected],[email protected],[email protected] Change-Id: If2c3293bdd8c19307e243d83c44fd328446fded7 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia: Reviewed-on: https://skia-review.googlesource.com/c/skia/+/203961 Reviewed-by: Chris Dalton <[email protected]> Commit-Queue: Chris Dalton <[email protected]>
1 parent 2b7cca4 commit bf341ae

File tree

10 files changed

+171
-540
lines changed

10 files changed

+171
-540
lines changed

‎gm/samplelocations.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ class SampleLocationsTestProcessor::Impl : public GrGLSLGeometryProcessor {
138138
f->sampleOffsets(), coord.fsIn());
139139
f->codeAppendf( "if (all(lessThanEqual(abs(samplecoord), float2(1)))) {");
140140
f->maskOffMultisampleCoverage(
141-
"~(1 << i)", GrGLSLFPFragmentBuilder::ScopeFlags::kInsideLoop);
141+
"~(1 << i)", GrGLSLFragmentShaderBuilder::Scope::kInsideLoopOrBranch);
142142
f->codeAppendf( "}");
143143
f->codeAppendf("}");
144144
}

‎src/gpu/GrCaps.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ GrCaps::GrCaps(const GrContextOptions& options) {
7474

7575
fPreferVRAMUseOverFlushes = true;
7676

77-
fPreferTrianglesOverSampleMask = false;
78-
7977
// Default to true, allow older versions of OpenGL to disable explicitly
8078
fClampToBorderSupport = true;
8179

@@ -212,8 +210,6 @@ void GrCaps::dumpJSON(SkJSONWriter* writer) const {
212210
writer->appendBool("Blacklist Coverage Counting Path Renderer [workaround]",
213211
fBlacklistCoverageCounting);
214212
writer->appendBool("Prefer VRAM Use over flushes [workaround]", fPreferVRAMUseOverFlushes);
215-
writer->appendBool("Prefer more triangles over sample mask [MSAA only]",
216-
fPreferTrianglesOverSampleMask);
217213
writer->appendBool("Avoid stencil buffers [workaround]", fAvoidStencilBuffers);
218214

219215
if (this->advancedBlendEquationSupport()) {

‎src/gpu/GrCaps.h‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,6 @@ class GrCaps : public SkRefCnt {
7373

7474
bool preferVRAMUseOverFlushes() const { return fPreferVRAMUseOverFlushes; }
7575

76-
bool preferTrianglesOverSampleMask() const { return fPreferTrianglesOverSampleMask; }
77-
7876
bool blacklistCoverageCounting() const { return fBlacklistCoverageCounting; }
7977

8078
bool avoidStencilBuffers() const { return fAvoidStencilBuffers; }
@@ -360,9 +358,6 @@ class GrCaps : public SkRefCnt {
360358
// ANGLE performance workaround
361359
bool fPreferVRAMUseOverFlushes : 1;
362360

363-
// On some platforms it's better to make more triangles than to use the sample mask (MSAA only).
364-
bool fPreferTrianglesOverSampleMask : 1;
365-
366361
// TODO: this may need to be an enum to support different fence types
367362
bool fFenceSyncSupport : 1;
368363

‎src/gpu/GrRenderTargetContext.cpp‎

Lines changed: 29 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,22 +1225,21 @@ void GrRenderTargetContext::drawRRect(const GrClip& origClip,
12251225
AutoCheckFlush acf(this->drawingManager());
12261226

12271227
GrAAType aaType = this->chooseAAType(aa);
1228+
if (GrAAType::kCoverage == aaType) {
1229+
std::unique_ptr<GrDrawOp> op;
1230+
if (style.isSimpleFill()) {
1231+
op = GrFillRRectOp::Make(fContext, viewMatrix, rrect, *this->caps(), std::move(paint));
1232+
}
1233+
if (!op) {
1234+
assert_alive(paint);
1235+
op = GrOvalOpFactory::MakeRRectOp(fContext, std::move(paint), viewMatrix, rrect, stroke,
1236+
this->caps()->shaderCaps());
1237+
}
12281238

1229-
std::unique_ptr<GrDrawOp> op;
1230-
if (style.isSimpleFill()) {
1231-
assert_alive(paint);
1232-
op = GrFillRRectOp::Make(
1233-
fContext, aaType, viewMatrix, rrect, *this->caps(), std::move(paint));
1234-
}
1235-
if (!op && GrAAType::kCoverage == aaType) {
1236-
assert_alive(paint);
1237-
op = GrOvalOpFactory::MakeRRectOp(
1238-
fContext, std::move(paint), viewMatrix, rrect, stroke, this->caps()->shaderCaps());
1239-
1240-
}
1241-
if (op) {
1242-
this->addDrawOp(*clip, std::move(op));
1243-
return;
1239+
if (op) {
1240+
this->addDrawOp(*clip, std::move(op));
1241+
return;
1242+
}
12441243
}
12451244

12461245
assert_alive(paint);
@@ -1631,31 +1630,29 @@ void GrRenderTargetContext::drawOval(const GrClip& clip,
16311630
AutoCheckFlush acf(this->drawingManager());
16321631

16331632
GrAAType aaType = this->chooseAAType(aa);
1634-
1635-
std::unique_ptr<GrDrawOp> op;
1636-
if (style.isSimpleFill()) {
1633+
if (GrAAType::kCoverage == aaType) {
1634+
std::unique_ptr<GrDrawOp> op;
16371635
// GrFillRRectOp has special geometry and a fragment-shader branch to conditionally evaluate
16381636
// the arc equation. This same special geometry and fragment branch also turn out to be a
16391637
// substantial optimization for drawing ovals (namely, by not evaluating the arc equation
16401638
// inside the oval's inner diamond). Given these optimizations, it's a clear win to draw
16411639
// ovals the exact same way we do round rects.
16421640
//
1643-
// However, we still don't draw true circles as round rects in coverage mode, because it can
1644-
// cause perf regressions on some platforms as compared to the dedicated circle Op.
1645-
if (GrAAType::kCoverage != aaType || oval.height() != oval.width()) {
1641+
// However, we still don't draw true circles as round rects, because it can cause perf
1642+
// regressions on some platforms as compared to the dedicated circle Op.
1643+
if (style.isSimpleFill() && oval.height() != oval.width()) {
1644+
op = GrFillRRectOp::Make(
1645+
fContext, viewMatrix, SkRRect::MakeOval(oval), *this->caps(), std::move(paint));
1646+
}
1647+
if (!op) {
16461648
assert_alive(paint);
1647-
op = GrFillRRectOp::Make(fContext, aaType, viewMatrix, SkRRect::MakeOval(oval),
1648-
*this->caps(), std::move(paint));
1649+
op = GrOvalOpFactory::MakeOvalOp(fContext, std::move(paint), viewMatrix, oval, style,
1650+
this->caps()->shaderCaps());
1651+
}
1652+
if (op) {
1653+
this->addDrawOp(clip, std::move(op));
1654+
return;
16491655
}
1650-
}
1651-
if (!op && GrAAType::kCoverage == aaType) {
1652-
assert_alive(paint);
1653-
op = GrOvalOpFactory::MakeOvalOp(fContext, std::move(paint), viewMatrix, oval, style,
1654-
this->caps()->shaderCaps());
1655-
}
1656-
if (op) {
1657-
this->addDrawOp(clip, std::move(op));
1658-
return;
16591656
}
16601657

16611658
assert_alive(paint);

‎src/gpu/gl/GrGLCaps.cpp‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -526,12 +526,6 @@ void GrGLCaps::init(const GrContextOptions& contextOptions,
526526
fPreferVRAMUseOverFlushes = !isANGLE;
527527
#endif
528528

529-
if (kARM_GrGLVendor == ctxInfo.vendor()) {
530-
// ARM seems to do better with larger quantities of fine triangles, as opposed to using the
531-
// sample mask. (At least in our current round rect op.)
532-
fPreferTrianglesOverSampleMask = true;
533-
}
534-
535529
if (kChromium_GrGLDriver == ctxInfo.driver()) {
536530
fMustClearUploadedBufferData = true;
537531
}

‎src/gpu/glsl/GrGLSLFragmentShaderBuilder.cpp‎

Lines changed: 10 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,7 @@ const char* GrGLSLFragmentShaderBuilder::sampleOffsets() {
9191
return "_sampleOffsets";
9292
}
9393

94-
void GrGLSLFragmentShaderBuilder::maskOffMultisampleCoverage(
95-
const char* mask, ScopeFlags scopeFlags) {
94+
void GrGLSLFragmentShaderBuilder::maskOffMultisampleCoverage(const char* mask, Scope scope) {
9695
const GrShaderCaps& shaderCaps = *fProgramBuilder->shaderCaps();
9796
if (!shaderCaps.sampleVariablesSupport()) {
9897
SkDEBUGFAIL("Attempted to mask sample coverage without support.");
@@ -102,58 +101,16 @@ void GrGLSLFragmentShaderBuilder::maskOffMultisampleCoverage(
102101
this->addFeature(1 << kSampleVariables_GLSLPrivateFeature, extension);
103102
}
104103

105-
if (!fHasModifiedSampleMask) {
106-
fHasModifiedSampleMask = true;
107-
if (ScopeFlags::kTopLevel != scopeFlags) {
108-
this->codePrependf("gl_SampleMask[0] = ~0;");
109-
}
110-
if (!(ScopeFlags::kInsideLoop & scopeFlags)) {
111-
this->codeAppendf("gl_SampleMask[0] = (%s);", mask);
112-
return;
113-
}
104+
if (!fHasInitializedSampleMask && Scope::kTopLevel == scope) {
105+
this->codeAppendf("gl_SampleMask[0] = (%s);", mask);
106+
fHasInitializedSampleMask = true;
107+
return;
114108
}
115-
116-
this->codeAppendf("gl_SampleMask[0] &= (%s);", mask);
117-
}
118-
119-
void GrGLSLFragmentShaderBuilder::applyFnToMultisampleMask(
120-
const char* fn, const char* grad, ScopeFlags scopeFlags) {
121-
SkASSERT(CustomFeatures::kSampleLocations & fProgramBuilder->header().processorFeatures());
122-
SkDEBUGCODE(fUsedProcessorFeaturesThisStage_DebugOnly |= CustomFeatures::kSampleLocations);
123-
SkDEBUGCODE(fUsedProcessorFeaturesAllStages_DebugOnly |= CustomFeatures::kSampleLocations);
124-
125-
int sampleCnt = fProgramBuilder->effectiveSampleCnt();
126-
SkASSERT(sampleCnt > 1);
127-
128-
this->codeAppendf("{");
129-
130-
if (!grad) {
131-
SkASSERT(fProgramBuilder->shaderCaps()->shaderDerivativeSupport());
132-
// In order to use HW derivatives, our neighbors within the same primitive must also be
133-
// executing the same code. A per-pixel branch makes this pre-condition impossible to
134-
// fulfill.
135-
SkASSERT(!(ScopeFlags::kInsidePerPixelBranch & scopeFlags));
136-
this->codeAppendf("float2 grad = float2(dFdx(fn), dFdy(fn));");
137-
this->codeAppendf("float fnwidth = fwidth(fn);");
138-
grad = "grad";
139-
} else {
140-
this->codeAppendf("float fnwidth = abs(%s.x) + abs(%s.y);", grad, grad);
109+
if (!fHasInitializedSampleMask) {
110+
this->codePrependf("gl_SampleMask[0] = ~0;");
111+
fHasInitializedSampleMask = true;
141112
}
142-
143-
this->codeAppendf("int mask = 0;");
144-
this->codeAppendf("if (%s*2 < fnwidth) {", fn); // Are ANY samples inside the implicit fn?
145-
this->codeAppendf( "if (%s*-2 >= fnwidth) {", fn); // Are ALL samples inside the implicit?
146-
this->codeAppendf( "mask = ~0;");
147-
this->codeAppendf( "} else for (int i = 0; i < %i; ++i) {", sampleCnt);
148-
this->codeAppendf( "float fnsample = dot(%s, _sampleOffsets[i]) + %s;", grad, fn);
149-
this->codeAppendf( "if (fnsample < 0) {");
150-
this->codeAppendf( "mask |= (1 << i);");
151-
this->codeAppendf( "}");
152-
this->codeAppendf( "}");
153-
this->codeAppendf("}");
154-
this->maskOffMultisampleCoverage("mask", scopeFlags);
155-
156-
this->codeAppendf("}");
113+
this->codeAppendf("gl_SampleMask[0] &= (%s);", mask);
157114
}
158115

159116
const char* GrGLSLFragmentShaderBuilder::dstColor() {
@@ -260,10 +217,10 @@ void GrGLSLFragmentShaderBuilder::onFinalize() {
260217
== fUsedProcessorFeaturesAllStages_DebugOnly);
261218

262219
if (CustomFeatures::kSampleLocations & fProgramBuilder->header().processorFeatures()) {
220+
this->definitions().append("const float2 _sampleOffsets[] = float2[](");
263221
const GrPipeline& pipeline = fProgramBuilder->pipeline();
264222
const SkTArray<SkPoint>& sampleLocations =
265223
fProgramBuilder->renderTarget()->renderTargetPriv().getSampleLocations(pipeline);
266-
this->definitions().append("const float2 _sampleOffsets[] = float2[](");
267224
for (int i = 0; i < sampleLocations.count(); ++i) {
268225
SkPoint offset = sampleLocations[i] - SkPoint::Make(.5f, .5f);
269226
if (kBottomLeft_GrSurfaceOrigin == this->getSurfaceOrigin()) {

‎src/gpu/glsl/GrGLSLFragmentShaderBuilder.h‎

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,9 @@ class GrGLSLFPFragmentBuilder : virtual public GrGLSLFragmentBuilder {
5454
*/
5555
virtual const char* sampleOffsets() = 0;
5656

57-
enum class ScopeFlags {
58-
// Every fragment will always execute this code, and will do it exactly once.
59-
kTopLevel = 0,
60-
// Either all fragments in a given primitive, or none, will execute this code.
61-
kInsidePerPrimitiveBranch = (1 << 0),
62-
// Any given fragment may or may not execute this code.
63-
kInsidePerPixelBranch = (1 << 1),
64-
// This code will be executed more than once.
65-
kInsideLoop = (1 << 2)
57+
enum class Scope : bool {
58+
kTopLevel,
59+
kInsideLoopOrBranch
6660
};
6761

6862
/**
@@ -74,21 +68,7 @@ class GrGLSLFPFragmentBuilder : virtual public GrGLSLFragmentBuilder {
7468
*
7569
* Requires MSAA and GLSL support for sample variables.
7670
*/
77-
virtual void maskOffMultisampleCoverage(const char* mask, ScopeFlags) = 0;
78-
79-
/**
80-
* Turns off coverage at each sample where the implicit function fn > 0.
81-
*
82-
* The provided "fn" value represents the implicit function at pixel center. We then approximate
83-
* the implicit at each sample by riding the gradient, "grad", linearly from pixel center to
84-
* each sample location.
85-
*
86-
* If "grad" is null, we approximate the gradient using HW derivatives.
87-
*
88-
* Requires MSAA and GLSL support for sample variables. Also requires HW derivatives if not
89-
* providing a gradient.
90-
*/
91-
virtual void applyFnToMultisampleMask(const char* fn, const char* grad, ScopeFlags) = 0;
71+
virtual void maskOffMultisampleCoverage(const char* mask, Scope) = 0;
9272

9373
/**
9474
* Fragment procs with child procs should call these functions before/after calling emitCode
@@ -102,8 +82,6 @@ class GrGLSLFPFragmentBuilder : virtual public GrGLSLFragmentBuilder {
10282
virtual void forceHighPrecision() = 0;
10383
};
10484

105-
GR_MAKE_BITFIELD_CLASS_OPS(GrGLSLFPFragmentBuilder::ScopeFlags);
106-
10785
/*
10886
* This class is used by Xfer processors to build their fragment code.
10987
*/
@@ -141,8 +119,7 @@ class GrGLSLFragmentShaderBuilder : public GrGLSLFPFragmentBuilder, public GrGLS
141119

142120
// GrGLSLFPFragmentBuilder interface.
143121
const char* sampleOffsets() override;
144-
void maskOffMultisampleCoverage(const char* mask, ScopeFlags) override;
145-
void applyFnToMultisampleMask(const char* fn, const char* grad, ScopeFlags) override;
122+
void maskOffMultisampleCoverage(const char* mask, Scope) override;
146123
const SkString& getMangleString() const override { return fMangleString; }
147124
void onBeforeChildProcEmitCode() override;
148125
void onAfterChildProcEmitCode() override;
@@ -210,7 +187,7 @@ class GrGLSLFragmentShaderBuilder : public GrGLSLFPFragmentBuilder, public GrGLS
210187
bool fHasCustomColorOutput = false;
211188
int fCustomColorOutputIndex = -1;
212189
bool fHasSecondaryOutput = false;
213-
bool fHasModifiedSampleMask = false;
190+
bool fHasInitializedSampleMask = false;
214191
bool fForceHighPrecision = false;
215192

216193
friend class GrGLSLProgramBuilder;

0 commit comments

Comments
 (0)