commit 396cd1dfabdd40d3c7be7287c96e2581a8fb4de2
author Derek Sollenberger <djsollen@google.com> Fri Apr 02 15:44:36 2021 +0000
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Fri Apr 02 15:44:47 2021 +0000
tree be1732d18dafcbb879b84febd91858edab1455c1
parent 348ee387a96d7d94733d46ad9e82b19cb890dd16

Revert "Lift atlas clip FP creation out of GrClip::apply"

This reverts commit 92b35673c5bb97ffad6f88c3d237db51cdce39bb.

Reason for revert: breaking the android roll (see ag/14076257)

Original change's description:
> Lift atlas clip FP creation out of GrClip::apply
>
> Atlas clips always had a potential point of failure: If the SDC's
> opsTask ever got closed between GrClip::apply and
> GrOpsTask::addDrawOp, their mask would have gotten sent to the wrong
> opsTask. It didn't _look_ like this could happen with the current
> code, but it could have also been inadvertently changed quite easily.
>
> This CL adds a "pathsForClipAtlas" array for GrClip::apply to fill out
> instead of creating FPs. The SDC then generates the actual clip atlas
> FPs once it knows exactly which opsTask they will belong in.
>
> Bug: chromium:928984
> Change-Id: I507ab13b2b5e8c3c3c1916d97611297dbbd8a522
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/389926
> Commit-Queue: Chris Dalton <csmartdalton@google.com>
> Reviewed-by: Michael Ludwig <michaelludwig@google.com>

TBR=robertphillips@google.com,csmartdalton@google.com,michaelludwig@google.com,adlai@google.com

Change-Id: I9597d822a9f31b7070aee691ddf3a52f4f424a24
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:928984
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392339
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Derek Sollenberger <djsollen@google.com>

src/gpu/GrSurfaceDrawContext.cpp

diff --git a/src/gpu/GrSurfaceDrawContext.cpp b/src/gpu/GrSurfaceDrawContext.cpp
index b53257a..5f8c49b 100644
--- a/src/gpu/GrSurfaceDrawContext.cpp
+++ b/src/gpu/GrSurfaceDrawContext.cpp
@@ -48,7 +48,6 @@
 #include "src/gpu/GrStyle.h"
 #include "src/gpu/GrTracing.h"
 #include "src/gpu/SkGr.h"
-#include "src/gpu/ccpr/GrCoverageCountingPathRenderer.h"
 #include "src/gpu/effects/GrBicubicEffect.h"
 #include "src/gpu/effects/GrBlendFragmentProcessor.h"
 #include "src/gpu/effects/GrRRectEffect.h"
@@ -1865,7 +1864,8 @@
     }
 }
 
-void GrSurfaceDrawContext::addDrawOp(const GrClip* clip, GrOp::Owner op,
+void GrSurfaceDrawContext::addDrawOp(const GrClip* clip,
+                                     GrOp::Owner op,
                                      const std::function<WillAddOpFn>& willAddFn) {
     ASSERT_SINGLE_OWNER
     if (fContext->abandoned()) {
@@ -1880,8 +1880,6 @@
     SkRect bounds;
     op_bounds(&bounds, op.get());
     GrAppliedClip appliedClip(this->dimensions(), this->asSurfaceProxy()->backingStoreDimensions());
-    SkSTArray<4, SkPath> pathsForClipAtlas;
-    auto* ccpr = this->drawingManager()->getCoverageCountingPathRenderer();
     GrDrawOp::FixedFunctionFlags fixedFunctionFlags = drawOp->fixedFunctionFlags();
     bool usesHWAA = fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesHWAA;
     bool usesUserStencilBits = fixedFunctionFlags & GrDrawOp::FixedFunctionFlags::kUsesStencil;
@@ -1899,9 +1897,8 @@
         } else {
             aaType = op->hasAABloat() ? GrAAType::kCoverage : GrAAType::kNone;
         }
-        auto clipEffect = clip->apply(fContext, this, aaType, usesUserStencilBits, &appliedClip,
-                                      &bounds, (ccpr) ? &pathsForClipAtlas : nullptr);
-        skipDraw = (clipEffect == GrClip::Effect::kClippedOut);
+        skipDraw = clip->apply(fContext, this, aaType, usesUserStencilBits,
+                               &appliedClip, &bounds) == GrClip::Effect::kClippedOut;
     } else {
         // No clipping, so just clip the bounds against the logical render target dimensions
         skipDraw = !bounds.intersect(this->asSurfaceProxy()->getBoundsRect());
@@ -1911,27 +1908,6 @@
         return;
     }
 
-    // Create atlas clip paths now that we know exactly which opsTask their atlas will come from.
-    if (!pathsForClipAtlas.empty()) {
-        std::unique_ptr<GrFragmentProcessor> clipFP;
-        if (appliedClip.hasCoverageFragmentProcessor()) {
-            clipFP = appliedClip.detachCoverageFragmentProcessor();
-        }
-        for (const SkPath& clipPath : pathsForClipAtlas) {
-            bool success;
-            // FIXME: ccpr shouldn't be given an opsTaskID until after the potential call to
-            // setupDstProxyView.
-            std::tie(success, clipFP) = ccpr->makeClipProcessor(std::move(clipFP),
-                                                                this->getOpsTask()->uniqueID(),
-                                                                clipPath, bounds.roundOut(),
-                                                                *this->caps());
-            if (!success) {
-                return;  // Clipped completely out.
-            }
-        }
-        appliedClip.addCoverageFP(std::move(clipFP));
-    }
-
     bool willUseStencil = usesUserStencilBits || appliedClip.hasStencilClip();
     SkASSERT(!willUseStencil || fNumStencilSamples > 0);
 
@@ -1958,17 +1934,12 @@
     }
 
     auto opsTask = this->getOpsTask();
-
     if (willAddFn) {
         willAddFn(op.get(), opsTask->uniqueID());
     }
     opsTask->addDrawOp(this->drawingManager(), std::move(op), analysis, std::move(appliedClip),
                        dstProxyView, GrTextureResolveManager(this->drawingManager()),
                        *this->caps());
-
-    // Our opsTask should not have changed or closed between the previous getOpsTask call and now.
-    SkASSERT(!opsTask->isClosed());
-    SkASSERT(this->getOpsTask() == opsTask);
 }
 
 bool GrSurfaceDrawContext::setupDstProxyView(const GrOp& op,