Fix bug in op chaining

I created this problem with my fix to crbug.com/1108475

Bug: 1112259

Change-Id: Ieb001fa61bd9ed68dcae43e7d511b4581f16ef0f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308638
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Michael Ludwig <michaelludwig@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This commit is contained in:
Robert Phillips 2020-08-12 11:57:07 -04:00 committed by Skia Commit-Bot
parent 9b7f30c7d1
commit 089b7c96ca
5 changed files with 146 additions and 2 deletions

View File

@ -301,6 +301,7 @@ tests_sources = [
"$_tests/TestUtils.h",
"$_tests/TextBlobCacheTest.cpp",
"$_tests/TextBlobTest.cpp",
"$_tests/TextureOpTest.cpp",
"$_tests/TextureProxyTest.cpp",
"$_tests/TextureStripAtlasManagerTest.cpp",
"$_tests/Time.cpp",

View File

@ -293,6 +293,7 @@ private:
ExpectedOutcome onMakeClosed(const GrCaps& caps, SkIRect* targetUpdateBounds) override;
friend class OpsTaskTestingAccess;
friend class GrRenderTargetContextPriv; // for stencil clip state. TODO: this is invasive
// The RTC and OpsTask have to work together to handle buffer clears. In most cases, buffer

View File

@ -993,7 +993,7 @@ private:
CombineResult onCombineIfPossible(GrOp* t, GrRecordingContext::Arenas*,
const GrCaps& caps) override {
TRACE_EVENT0("skia.gpu", TRACE_FUNC);
const auto* that = t->cast<TextureOp>();
auto* that = t->cast<TextureOp>();
SkDEBUGCODE(this->validate();)
SkDEBUGCODE(that->validate();)
@ -1070,7 +1070,16 @@ private:
fMetadata.fTotalQuadCount += that->fQuads.count();
if (upgradeToCoverageAAOnMerge) {
// This merger may be the start of a concatenation of two chains. When one
// of the chains mutates its AA the other must follow suit or else the above AA
// check may prevent later ops from chaining together. A specific example of this is
// when chain2 is prepended onto chain1:
// chain1 (that): opA (non-AA/mergeable) opB (non-AA/non-mergeable)
// chain2 (this): opC (cov-AA/non-mergeable) opD (cov-AA/mergeable)
// W/o this propagation, after opD & opA merge, opB and opC would say they couldn't
// chain - which would stop the concatenation process.
this->propagateCoverageAAThroughoutChain();
that->propagateCoverageAAThroughoutChain();
}
SkDEBUGCODE(this->validate();)

View File

@ -20,7 +20,7 @@ static std::unique_ptr<GrRenderTargetContext> new_RTC(GrRecordingContext* rConte
rContext, GrColorType::kRGBA_8888, nullptr, SkBackingFit::kExact, {128, 128});
}
sk_sp<GrSurfaceProxy> create_proxy(GrRecordingContext* rContext) {
static sk_sp<GrSurfaceProxy> create_proxy(GrRecordingContext* rContext) {
static constexpr SkISize kDimensions = {128, 128};
const GrBackendFormat format = rContext->priv().caps()->getDefaultBackendFormat(

133
tests/TextureOpTest.cpp Normal file
View File

@ -0,0 +1,133 @@
/*
* Copyright 2020 Google LLC
*
* Use of this source code is governed by a BSD-style license that can be
* found in the LICENSE file.
*/
#include "include/gpu/GrDirectContext.h"
#include "include/gpu/GrRecordingContext.h"
#include "src/gpu/GrContextPriv.h"
#include "src/gpu/GrProxyProvider.h"
#include "src/gpu/GrRecordingContextPriv.h"
#include "src/gpu/ops/GrTextureOp.h"
#include "src/gpu/ops/GrTextureOp.h"
#include "tests/Test.h"
class OpsTaskTestingAccess {
public:
typedef GrOpsTask::OpChain OpChain;
};
static void check_chain(OpsTaskTestingAccess::OpChain* chain, SkRect firstRect, SkRect lastRect,
int expectedNumOps) {
int actualNumOps = 0;
for (const auto& op : GrOp::ChainRange<>(chain->head())) {
++actualNumOps;
if (actualNumOps == 1) {
SkAssertResult(op.bounds() == firstRect.makeOutset(0.5f, 0.5f));
} else if (actualNumOps == expectedNumOps) {
SkAssertResult(op.bounds() == lastRect.makeOutset(0.5f, 0.5f));
}
}
SkAssertResult(actualNumOps == expectedNumOps);
}
static sk_sp<GrSurfaceProxy> create_proxy(GrRecordingContext* rContext) {
const GrCaps* caps = rContext->priv().caps();
static constexpr SkISize kDimensions = {16, 16};
const GrBackendFormat format = caps->getDefaultBackendFormat(GrColorType::kRGBA_8888,
GrRenderable::kYes);
return rContext->priv().proxyProvider()->createProxy(
format, kDimensions, GrRenderable::kYes, 1, GrMipmapped::kNo, SkBackingFit::kExact,
SkBudgeted::kNo, GrProtected::kNo, GrInternalSurfaceFlags::kNone);
}
static std::unique_ptr<GrDrawOp> create_op(GrDirectContext* dContext, SkRect rect,
const GrSurfaceProxyView& proxyView, bool isAA) {
DrawQuad quad;
quad.fDevice = GrQuad::MakeFromRect(rect.makeOutset(0.5f, 0.5f), SkMatrix::I());
quad.fLocal = GrQuad(rect);
quad.fEdgeFlags = isAA ? GrQuadAAFlags::kAll : GrQuadAAFlags::kNone;
return GrTextureOp::Make(dContext,
proxyView,
kPremul_SkAlphaType,
nullptr,
GrSamplerState::Filter::kNearest,
GrSamplerState::MipmapMode::kNone,
{1.f, 1.f, 1.f, 1.f},
GrTextureOp::Saturate::kYes,
SkBlendMode::kSrcOver,
isAA ? GrAAType::kCoverage
: GrAAType::kNone,
&quad,
nullptr);
}
// This unit test exercises the crbug.com/1112259 case.
DEF_GPUTEST_FOR_RENDERING_CONTEXTS(TextureOpTest, reporter, ctxInfo) {
GrDirectContext* dContext = ctxInfo.directContext();
const GrCaps* caps = dContext->priv().caps();
GrRecordingContext::Arenas arenas = dContext->priv().arenas();
auto auditTrail = dContext->priv().auditTrail();
if (!caps->dynamicStateArrayGeometryProcessorTextureSupport()) {
// This test requires chaining
return;
}
GrSurfaceProxyView proxyViewA(create_proxy(dContext),
kTopLeft_GrSurfaceOrigin,
GrSwizzle::RGBA());
GrSurfaceProxyView proxyViewB(create_proxy(dContext),
kTopLeft_GrSurfaceOrigin,
GrSwizzle::RGBA());
GrSurfaceProxyView proxyViewC(create_proxy(dContext),
kTopLeft_GrSurfaceOrigin,
GrSwizzle::RGBA());
static const SkRect kOpARect{ 0, 0, 16, 16 };
static const SkRect kOpBRect{ 32, 0, 48, 16 };
static const SkRect kOpCRect{ 0, 32, 16, 48 };
static const SkRect kOpDRect{ 32, 32, 48, 48 };
// opA & opB can chain together but can't merge bc they have different proxies
std::unique_ptr<GrDrawOp> opA = create_op(dContext, kOpARect, proxyViewA, false);
std::unique_ptr<GrDrawOp> opB = create_op(dContext, kOpBRect, proxyViewB, false);
GrAppliedClip noClip = GrAppliedClip::Disabled();
OpsTaskTestingAccess::OpChain chain1(std::move(opA), GrProcessorSet::EmptySetAnalysis(),
&noClip, nullptr);
chain1.appendOp(std::move(opB), GrProcessorSet::EmptySetAnalysis(), nullptr, &noClip, *caps,
&arenas, dContext->priv().auditTrail());
check_chain(&chain1, kOpARect, kOpBRect, 2);
// opC & opD can also chain together but can't merge (bc, again, they have different
// proxies). Note, however, that opA and opD do share a proxy so can be merged if opA's
// anti-aliasing is upgraded to coverage.
std::unique_ptr<GrDrawOp> opC = create_op(dContext, kOpCRect, proxyViewC, true);
std::unique_ptr<GrDrawOp> opD = create_op(dContext, kOpDRect, proxyViewA, true);
OpsTaskTestingAccess::OpChain chain2(std::move(opC), GrProcessorSet::EmptySetAnalysis(),
&noClip, nullptr);
chain2.appendOp(std::move(opD), GrProcessorSet::EmptySetAnalysis(), nullptr, &noClip, *caps,
&arenas, auditTrail);
check_chain(&chain2, kOpCRect, kOpDRect, 2);
// opA and opD, now in separate chains, can merge when the two chains are combined while
// opB and opC can still only chain.
chain1.prependChain(&chain2, *caps, &arenas, auditTrail);
// We should end up with the chain
// opC - opD/opA - opB
check_chain(&chain1, kOpCRect, kOpBRect, 3);
chain1.deleteOps(arenas.opMemoryPool());
}