Revert "[M85 Cherry-pick] Fix bug(s) in combining and chaining GrTextureOps"
This reverts commit c3f094e9773746f91fc18c83a617a54e42274763.
Bug: 1108475, 1112259
Change-Id: Ibbcc3305fd7dd4226867a03e3e9bd1811a11a874
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308695
Reviewed-by: Greg Daniel <egdaniel@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
diff --git a/src/gpu/ops/GrTextureOp.cpp b/src/gpu/ops/GrTextureOp.cpp
index 94bd158..ee71291 100644
--- a/src/gpu/ops/GrTextureOp.cpp
+++ b/src/gpu/ops/GrTextureOp.cpp
@@ -740,45 +740,34 @@
}
#ifdef SK_DEBUG
- static int validate_op(GrTextureType textureType,
- GrAAType aaType,
- GrSwizzle swizzle,
- const TextureOp* op) {
- SkASSERT(op->fMetadata.fSwizzle == swizzle);
-
- int quadCount = 0;
- for (unsigned p = 0; p < op->fMetadata.fProxyCount; ++p) {
- auto* proxy = op->fViewCountPairs[p].fProxy->asTextureProxy();
- quadCount += op->fViewCountPairs[p].fQuadCnt;
- SkASSERT(proxy);
- SkASSERT(proxy->textureType() == textureType);
- }
-
- SkASSERT(aaType == op->fMetadata.aaType());
- return quadCount;
- }
-
void validate() const override {
// NOTE: Since this is debug-only code, we use the virtual asTextureProxy()
auto textureType = fViewCountPairs[0].fProxy->asTextureProxy()->textureType();
GrAAType aaType = fMetadata.aaType();
- GrSwizzle swizzle = fMetadata.fSwizzle;
- int quadCount = validate_op(textureType, aaType, swizzle, this);
+ int quadCount = 0;
+ for (const auto& op : ChainRange<TextureOp>(this)) {
+ SkASSERT(op.fMetadata.fSwizzle == fMetadata.fSwizzle);
- for (const GrOp* tmp = this->prevInChain(); tmp; tmp = tmp->prevInChain()) {
- quadCount += validate_op(textureType, aaType, swizzle,
- static_cast<const TextureOp*>(tmp));
- }
+ for (unsigned p = 0; p < op.fMetadata.fProxyCount; ++p) {
+ auto* proxy = op.fViewCountPairs[p].fProxy->asTextureProxy();
+ quadCount += op.fViewCountPairs[p].fQuadCnt;
+ SkASSERT(proxy);
+ SkASSERT(proxy->textureType() == textureType);
+ }
- for (const GrOp* tmp = this->nextInChain(); tmp; tmp = tmp->nextInChain()) {
- quadCount += validate_op(textureType, aaType, swizzle,
- static_cast<const TextureOp*>(tmp));
+ // Each individual op must be a single aaType. kCoverage and kNone ops can chain
+ // together but kMSAA ones do not.
+ if (aaType == GrAAType::kCoverage || aaType == GrAAType::kNone) {
+ SkASSERT(op.fMetadata.aaType() == GrAAType::kCoverage ||
+ op.fMetadata.aaType() == GrAAType::kNone);
+ } else {
+ SkASSERT(aaType == GrAAType::kMSAA && op.fMetadata.aaType() == GrAAType::kMSAA);
+ }
}
SkASSERT(quadCount == this->numChainedQuads());
}
-
#endif
#if GR_TEST_UTILS
@@ -786,8 +775,6 @@
#endif
void characterize(Desc* desc) const {
- SkDEBUGCODE(this->validate();)
-
GrQuad::Type quadType = GrQuad::Type::kAxisAligned;
ColorType colorType = ColorType::kNone;
GrQuad::Type srcQuadType = GrQuad::Type::kAxisAligned;
@@ -939,32 +926,11 @@
SkASSERT(numDraws == fDesc->fNumProxies);
}
- void propagateCoverageAAThroughoutChain() {
- fMetadata.fAAType = static_cast<uint16_t>(GrAAType::kCoverage);
-
- for (GrOp* tmp = this->prevInChain(); tmp; tmp = tmp->prevInChain()) {
- TextureOp* tex = static_cast<TextureOp*>(tmp);
- SkASSERT(tex->fMetadata.aaType() == GrAAType::kCoverage ||
- tex->fMetadata.aaType() == GrAAType::kNone);
- tex->fMetadata.fAAType = static_cast<uint16_t>(GrAAType::kCoverage);
- }
-
- for (GrOp* tmp = this->nextInChain(); tmp; tmp = tmp->nextInChain()) {
- TextureOp* tex = static_cast<TextureOp*>(tmp);
- SkASSERT(tex->fMetadata.aaType() == GrAAType::kCoverage ||
- tex->fMetadata.aaType() == GrAAType::kNone);
- tex->fMetadata.fAAType = static_cast<uint16_t>(GrAAType::kCoverage);
- }
- }
-
CombineResult onCombineIfPossible(GrOp* t, GrRecordingContext::Arenas*,
const GrCaps& caps) override {
TRACE_EVENT0("skia.gpu", TRACE_FUNC);
const auto* that = t->cast<TextureOp>();
- SkDEBUGCODE(this->validate();)
- SkDEBUGCODE(that->validate();)
-
if (fDesc || that->fDesc) {
// This should never happen (since only DDL recorded ops should be prePrepared)
// but, in any case, we should never combine ops that that been prePrepared
@@ -1010,16 +976,7 @@
thisProxy != thatProxy) {
// We can't merge across different proxies. Check if 'this' can be chained with 'that'.
if (GrTextureProxy::ProxiesAreCompatibleAsDynamicState(thisProxy, thatProxy) &&
- caps.dynamicStateArrayGeometryProcessorTextureSupport() &&
- fMetadata.aaType() == that->fMetadata.aaType()) {
- // We only allow chaining when the aaTypes match bc otherwise the AA type
- // reported by the chain can be inconsistent. That is, since chaining doesn't
- // propagate revised AA information throughout the chain, the head of the chain
- // could have an AA setting of kNone while the chain as a whole could have a
- // setting of kCoverage. This inconsistency would then interfere with the validity
- // of the CombinedQuadCountWillOverflow calls.
- // This problem doesn't occur w/ merging bc we do propagate the AA information
- // (in propagateCoverageAAThroughoutChain) below.
+ caps.dynamicStateArrayGeometryProcessorTextureSupport()) {
return CombineResult::kMayChain;
}
return CombineResult::kCannotCombine;
@@ -1027,20 +984,18 @@
fMetadata.fSubset |= that->fMetadata.fSubset;
fMetadata.fColorType = std::max(fMetadata.fColorType, that->fMetadata.fColorType);
+ if (upgradeToCoverageAAOnMerge) {
+ fMetadata.fAAType = static_cast<uint16_t>(GrAAType::kCoverage);
+ }
// Concatenate quad lists together
fQuads.concat(that->fQuads);
fViewCountPairs[0].fQuadCnt += that->fQuads.count();
fMetadata.fTotalQuadCount += that->fQuads.count();
- if (upgradeToCoverageAAOnMerge) {
- this->propagateCoverageAAThroughoutChain();
- }
-
- SkDEBUGCODE(this->validate();)
-
return CombineResult::kMerged;
}
+
GrQuadBuffer<ColorSubsetAndAA> fQuads;
sk_sp<GrColorSpaceXform> fTextureColorSpaceXform;
// Most state of TextureOp is packed into these two field to minimize the op's size.
@@ -1263,8 +1218,7 @@
for (int i = 0; i < state.numLeft(); ++i) {
int absIndex = state.baseIndex() + i;
- if (set[absIndex].fAAFlags != GrQuadAAFlags::kNone ||
- runningAA == GrAAType::kCoverage) {
+ if (set[absIndex].fAAFlags != GrQuadAAFlags::kNone) {
if (i >= GrResourceProvider::MaxNumAAQuads()) {
// Here we either need to boost the AA type to kCoverage, but doing so with
diff --git a/tests/BulkRectTest.cpp b/tests/BulkRectTest.cpp
index 71532b5..c65d8ae 100644
--- a/tests/BulkRectTest.cpp
+++ b/tests/BulkRectTest.cpp
@@ -4,12 +4,10 @@
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
-
#include "src/core/SkBlendModePriv.h"
#include "src/gpu/GrContextPriv.h"
#include "src/gpu/GrProxyProvider.h"
#include "src/gpu/GrRenderTargetContext.h"
-#include "src/gpu/GrRenderTargetContextPriv.h"
#include "src/gpu/ops/GrFillRectOp.h"
#include "src/gpu/ops/GrTextureOp.h"
#include "tests/Test.h"
@@ -32,26 +30,15 @@
typedef GrQuadAAFlags (*PerQuadAAFunc)(int i);
-typedef void (*BulkRectTest)(skiatest::Reporter*,
- GrContext*,
- PerQuadAAFunc,
- GrAAType overallAA,
- SkBlendMode,
- bool addOneByOne,
- bool allUniqueProxies,
- int requestedTotNumQuads,
- int expectedNumOps);
+typedef void (*BulkRectTest)(skiatest::Reporter* reporter, GrContext* context,
+ PerQuadAAFunc perQuadAA, GrAAType overallAA, SkBlendMode blendMode,
+ int requestedTotNumQuads, int expectedNumOps);
//-------------------------------------------------------------------------------------------------
-static void fillrectop_creation_test(skiatest::Reporter* reporter, GrContext* context,
- PerQuadAAFunc perQuadAA, GrAAType overallAA,
- SkBlendMode blendMode, bool addOneByOne,
- bool allUniqueProxies,
- int requestedTotNumQuads, int expectedNumOps) {
-
- if (addOneByOne || allUniqueProxies) {
- return;
- }
+static void bulk_fill_rect_create_test(skiatest::Reporter* reporter, GrContext* context,
+ PerQuadAAFunc perQuadAA, GrAAType overallAA,
+ SkBlendMode blendMode,
+ int requestedTotNumQuads, int expectedNumOps) {
std::unique_ptr<GrRenderTargetContext> rtc = new_RTC(context);
@@ -66,7 +53,6 @@
GrPaint paint;
paint.setXPFactory(SkBlendMode_AsXPFactory(blendMode));
-
GrFillRectOp::AddFillRectOps(rtc.get(), nullptr, context, std::move(paint), overallAA,
SkMatrix::I(), quads, requestedTotNumQuads);
@@ -91,42 +77,24 @@
}
//-------------------------------------------------------------------------------------------------
-static void textureop_creation_test(skiatest::Reporter* reporter, GrContext* context,
- PerQuadAAFunc perQuadAA, GrAAType overallAA,
- SkBlendMode blendMode, bool addOneByOne,
- bool allUniqueProxies,
- int requestedTotNumQuads, int expectedNumOps) {
+static void bulk_texture_rect_create_test(skiatest::Reporter* reporter, GrContext* context,
+ PerQuadAAFunc perQuadAA, GrAAType overallAA,
+ SkBlendMode blendMode,
+ int requestedTotNumQuads, int expectedNumOps) {
std::unique_ptr<GrRenderTargetContext> rtc = new_RTC(context);
- GrSurfaceProxyView proxyViewA, proxyViewB;
-
- if (!allUniqueProxies) {
- sk_sp<GrSurfaceProxy> proxyA = create_proxy(context);
- sk_sp<GrSurfaceProxy> proxyB = create_proxy(context);
- proxyViewA = GrSurfaceProxyView(std::move(proxyA),
- kTopLeft_GrSurfaceOrigin,
- GrSwizzle::RGBA());
- proxyViewB = GrSurfaceProxyView(std::move(proxyB),
- kTopLeft_GrSurfaceOrigin,
- GrSwizzle::RGBA());
- }
+ sk_sp<GrSurfaceProxy> proxyA = create_proxy(context);
+ sk_sp<GrSurfaceProxy> proxyB = create_proxy(context);
+ GrSurfaceProxyView proxyViewA(std::move(proxyA), kTopLeft_GrSurfaceOrigin, GrSwizzle::RGBA());
+ GrSurfaceProxyView proxyViewB(std::move(proxyB), kTopLeft_GrSurfaceOrigin, GrSwizzle::RGBA());
auto set = new GrRenderTargetContext::TextureSetEntry[requestedTotNumQuads];
for (int i = 0; i < requestedTotNumQuads; ++i) {
- if (!allUniqueProxies) {
- // Alternate between two proxies to prevent op merging if the batch API was forced to
- // submit one op at a time (to work, this does require that all fDstRects overlap).
- set[i].fProxyView = i % 2 == 0 ? proxyViewA : proxyViewB;
- } else {
- // Each op gets its own proxy to force chaining only
- sk_sp<GrSurfaceProxy> proxyA = create_proxy(context);
- set[i].fProxyView = GrSurfaceProxyView(std::move(proxyA),
- kTopLeft_GrSurfaceOrigin,
- GrSwizzle::RGBA());
- }
-
+ // Alternate between two proxies to prevent op merging if the batch API was forced to submit
+ // one op at a time (to work, this does require that all fDstRects overlap).
+ set[i].fProxyView = i % 2 == 0 ? proxyViewA : proxyViewB;
set[i].fSrcAlphaType = kPremul_SkAlphaType;
set[i].fSrcRect = SkRect::MakeWH(100.0f, 100.0f);
set[i].fDstRect = SkRect::MakeWH(100.5f, 100.5f); // prevent the int non-AA optimization
@@ -136,42 +104,14 @@
set[i].fAAFlags = perQuadAA(i);
}
- if (addOneByOne) {
- for (int i = 0; i < requestedTotNumQuads; ++i) {
- DrawQuad quad;
-
- quad.fDevice = GrQuad::MakeFromRect(set[i].fDstRect, SkMatrix::I());
- quad.fLocal = GrQuad(set[i].fSrcRect);
- quad.fEdgeFlags = set[i].fAAFlags;
-
- std::unique_ptr<GrDrawOp> op = GrTextureOp::Make(context,
- set[i].fProxyView,
- set[i].fSrcAlphaType,
- nullptr,
- GrSamplerState::Filter::kNearest,
- set[i].fColor,
- GrTextureOp::Saturate::kYes,
- blendMode,
- overallAA,
- &quad,
- nullptr);
- rtc->priv().testingOnly_addDrawOp(nullptr, std::move(op));
- }
- } else {
- GrTextureOp::AddTextureSetOps(rtc.get(),
- nullptr,
- context,
- set,
- requestedTotNumQuads,
- requestedTotNumQuads, // We alternate so proxyCnt == cnt
- GrSamplerState::Filter::kNearest,
- GrTextureOp::Saturate::kYes,
- blendMode,
- overallAA,
- SkCanvas::kStrict_SrcRectConstraint,
- SkMatrix::I(),
- nullptr);
- }
+ GrTextureOp::AddTextureSetOps(rtc.get(), nullptr, context, set, requestedTotNumQuads,
+ requestedTotNumQuads, // We alternate so proxyCnt == cnt
+ GrSamplerState::Filter::kNearest,
+ GrTextureOp::Saturate::kYes,
+ blendMode,
+ overallAA,
+ SkCanvas::kStrict_SrcRectConstraint,
+ SkMatrix::I(), nullptr);
GrOpsTask* opsTask = rtc->testingOnly_PeekLastOpsTask();
int actualNumOps = opsTask->numOpChains();
@@ -189,12 +129,9 @@
: GrFillRectOp::ClassID();
for (int i = 0; i < actualNumOps; ++i) {
const GrOp* tmp = opsTask->getChain(i);
- REPORTER_ASSERT(reporter, allUniqueProxies || tmp->isChainTail());
- while (tmp) {
- REPORTER_ASSERT(reporter, tmp->classID() == expectedOpID);
- actualTotNumQuads += ((GrDrawOp*) tmp)->numQuads();
- tmp = tmp->nextInChain();
- }
+ REPORTER_ASSERT(reporter, tmp->classID() == expectedOpID);
+ REPORTER_ASSERT(reporter, tmp->isChainTail());
+ actualTotNumQuads += ((GrDrawOp*) tmp)->numQuads();
}
REPORTER_ASSERT(reporter, expectedNumOps == actualNumOps);
@@ -207,7 +144,6 @@
//-------------------------------------------------------------------------------------------------
static void run_test(GrContext* context, skiatest::Reporter* reporter, BulkRectTest test) {
-
// This is the simple case where there is no AA at all. We expect 2 non-AA clumps of quads.
{
auto noAA = [](int i) -> GrQuadAAFlags {
@@ -217,7 +153,7 @@
static const int kNumExpectedOps = 2;
test(reporter, context, noAA, GrAAType::kNone, SkBlendMode::kSrcOver,
- false, false, 2*GrResourceProvider::MaxNumNonAAQuads(), kNumExpectedOps);
+ 2*GrResourceProvider::MaxNumNonAAQuads(), kNumExpectedOps);
}
// This is the same as the above case except the overall AA is kCoverage. However, since
@@ -230,7 +166,7 @@
static const int kNumExpectedOps = 2;
test(reporter, context, noAA, GrAAType::kCoverage, SkBlendMode::kSrcOver,
- false, false, 2*GrResourceProvider::MaxNumNonAAQuads(), kNumExpectedOps);
+ 2*GrResourceProvider::MaxNumNonAAQuads(), kNumExpectedOps);
}
// This case has an overall AA of kCoverage but the per-quad AA alternates.
@@ -244,7 +180,7 @@
GrResourceProvider::MaxNumAAQuads();
test(reporter, context, alternateAA, GrAAType::kCoverage, SkBlendMode::kSrcOver,
- false, false, 2*GrResourceProvider::MaxNumNonAAQuads(), numExpectedOps);
+ 2*GrResourceProvider::MaxNumNonAAQuads(), numExpectedOps);
}
// In this case we have a run of MaxNumAAQuads non-AA quads and then AA quads. This
@@ -259,7 +195,7 @@
static const int kNumExpectedOps = 2;
test(reporter, context, runOfNonAA, GrAAType::kCoverage, SkBlendMode::kSrcOver,
- false, false, 2*GrResourceProvider::MaxNumAAQuads(), kNumExpectedOps);
+ 2*GrResourceProvider::MaxNumAAQuads(), kNumExpectedOps);
}
// In this case we use a blend mode other than src-over, which hits the GrFillRectOp fallback
@@ -274,42 +210,14 @@
static const int kNumExpectedOps = 2;
test(reporter, context, fixedAA, GrAAType::kCoverage, SkBlendMode::kSrcATop,
- false, false, 2*GrResourceProvider::MaxNumAAQuads(), kNumExpectedOps);
+ 2*GrResourceProvider::MaxNumAAQuads(), kNumExpectedOps);
}
-
- // This repros crbug.com/1108475, where we create 1024 non-AA texture ops w/ one coverage-AA
- // texture op in the middle. Because each op has its own texture, all the texture ops
- // get chained together so the quad count can exceed the AA maximum.
- {
- auto onlyOneAA = [](int i) -> GrQuadAAFlags {
- return i == 256 ? GrQuadAAFlags::kAll : GrQuadAAFlags::kNone;
- };
-
- static const int kNumExpectedOps = 3;
-
- test(reporter, context, onlyOneAA, GrAAType::kCoverage, SkBlendMode::kSrcOver,
- true, true, 1024, kNumExpectedOps);
- }
-
- // This repros a problem related to crbug.com/1108475. In this case, the bulk creation
- // method had no way to break up the set of texture ops at the AA quad limit.
- {
- auto onlyOneAA = [](int i) -> GrQuadAAFlags {
- return i == 256 ? GrQuadAAFlags::kAll : GrQuadAAFlags::kNone;
- };
-
- static const int kNumExpectedOps = 2;
-
- test(reporter, context, onlyOneAA, GrAAType::kCoverage, SkBlendMode::kSrcOver,
- false, true, 1024, kNumExpectedOps);
- }
-
}
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(BulkFillRectTest, reporter, ctxInfo) {
- run_test(ctxInfo.grContext(), reporter, fillrectop_creation_test);
+ run_test(ctxInfo.grContext(), reporter, bulk_fill_rect_create_test);
}
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(BulkTextureRectTest, reporter, ctxInfo) {
- run_test(ctxInfo.grContext(), reporter, textureop_creation_test);
+ run_test(ctxInfo.grContext(), reporter, bulk_texture_rect_create_test);
}
