Revert "Add IORef capability to GrSurfaceProxy objects"
This reverts commit 286b96f876953c94c178e3abbeb4eab186ad1fef.
Reason for revert:
Failure on Mac Mini:
FAILURE: ../../../tests/ProxyRefTest.cpp:59 proxy->getPendingWriteCnt_TestOnly() == expectedNumWrites
../../../tests/ProxyRefTest.cpp:64: fatal error: "assert(proxy->getPendingWriteCnt_TestOnly() == expectedNumWrites)"
Original change's description:
> Add IORef capability to GrSurfaceProxy objects
>
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4734
>
> Change-Id: If10fbe555e9fa3331bfa01065028e1afe82adb78
> Reviewed-on: https://skia-review.googlesource.com/4734
> Commit-Queue: Robert Phillips <robertphillips@google.com>
> Reviewed-by: Brian Salomon <bsalomon@google.com>
>
TBR=bsalomon@google.com,robertphillips@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
Change-Id: I5e1c201e027d4e9687b01f28a418b1884d3f9ece
Reviewed-on: https://skia-review.googlesource.com/4776
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
diff --git a/gn/tests.gni b/gn/tests.gni
index a035b9e..d828297 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -158,7 +158,6 @@
"$_tests/PremulAlphaRoundTripTest.cpp",
"$_tests/PrimitiveProcessorTest.cpp",
"$_tests/ProxyConversionTest.cpp",
- "$_tests/ProxyRefTest.cpp",
"$_tests/ProxyTest.cpp",
"$_tests/QuickRejectTest.cpp",
"$_tests/RandomTest.cpp",
diff --git a/include/gpu/GrGpuResource.h b/include/gpu/GrGpuResource.h
index 29d33df..4597464 100644
--- a/include/gpu/GrGpuResource.h
+++ b/include/gpu/GrGpuResource.h
@@ -93,8 +93,6 @@
bool internalHasRef() const { return SkToBool(fRefCnt); }
private:
- friend class GrIORefProxy; // needs to forward on wrapped IO calls
-
void addPendingRead() const {
this->validate();
++fPendingReads;
diff --git a/include/private/GrSurfaceProxy.h b/include/private/GrSurfaceProxy.h
index 3273f65..dd5ece1 100644
--- a/include/private/GrSurfaceProxy.h
+++ b/include/private/GrSurfaceProxy.h
@@ -50,29 +50,13 @@
void validate() const {
#ifdef SK_DEBUG
- SkASSERT(fRefCnt >= 1);
- SkASSERT(fPendingReads >= 0);
- SkASSERT(fPendingWrites >= 0);
- SkASSERT(fRefCnt + fPendingReads + fPendingWrites >= 1);
-
- if (fTarget) {
- SkASSERT(!fPendingReads && !fPendingWrites);
- // The backing GrSurface can have more refs than the proxy if the proxy
- // started off wrapping an external resource (that came in with refs).
- // The GrSurface should never have fewer refs than the proxy however.
- SkASSERT(fTarget->fRefCnt >= fRefCnt);
- }
+ SkASSERT(fRefCnt >= 0);
#endif
}
- int32_t getProxyRefCnt_TestOnly() const;
- int32_t getBackingRefCnt_TestOnly() const;
- int32_t getPendingReadCnt_TestOnly() const;
- int32_t getPendingWriteCnt_TestOnly() const;
-
protected:
- GrIORefProxy() : fTarget(nullptr), fRefCnt(1), fPendingReads(0), fPendingWrites(0) {}
- GrIORefProxy(sk_sp<GrSurface> surface) : fRefCnt(1), fPendingReads(0), fPendingWrites(0) {
+ GrIORefProxy() : fRefCnt(1), fTarget(nullptr) {}
+ GrIORefProxy(sk_sp<GrSurface> surface) : fRefCnt(1) {
// Since we're manually forwarding on refs & unrefs we don't want sk_sp doing
// anything extra.
fTarget = surface.release();
@@ -82,76 +66,13 @@
// have forwarded on the unref call that got use here.
}
- // This GrIORefProxy was deferred before but has just been instantiated. To
- // make all the reffing & unreffing work out we now need to transfer any deferred
- // refs & unrefs to the new GrSurface
- void transferRefs() {
- SkASSERT(fTarget);
-
- fTarget->fRefCnt += fRefCnt;
- fTarget->fPendingReads += fPendingReads;
- fTarget->fPendingWrites += fPendingWrites;
-
- fPendingReads = 0;
- fPendingWrites = 0;
- }
+ // TODO: add the IO ref counts. Although if we can delay shader creation to flush time
+ // we may not even need to do that.
+ mutable int32_t fRefCnt;
// For deferred proxies this will be null. For wrapped proxies it will point to the
// wrapped resource.
GrSurface* fTarget;
-
-private:
- // This class is used to manage conversion of refs to pending reads/writes.
- friend class GrGpuResourceRef;
- template <typename, GrIOType> friend class GrPendingIOResource;
-
- void addPendingRead() const {
- this->validate();
-
- if (fTarget) {
- fTarget->addPendingRead();
- return;
- }
-
- ++fPendingReads;
- }
-
- void completedRead() const {
- this->validate();
-
- if (fTarget) {
- fTarget->completedRead();
- return;
- }
-
- SkFAIL("How was the read completed if the Proxy hasn't been instantiated?");
- }
-
- void addPendingWrite() const {
- this->validate();
-
- if (fTarget) {
- fTarget->addPendingWrite();
- return;
- }
-
- ++fPendingWrites;
- }
-
- void completedWrite() const {
- this->validate();
-
- if (fTarget) {
- fTarget->completedWrite();
- return;
- }
-
- SkFAIL("How was the write completed if the Proxy hasn't been instantiated?");
- }
-
- mutable int32_t fRefCnt;
- mutable int32_t fPendingReads;
- mutable int32_t fPendingWrites;
};
class GrSurfaceProxy : public GrIORefProxy {
diff --git a/src/gpu/GrSurfaceProxy.cpp b/src/gpu/GrSurfaceProxy.cpp
index 9de41ef..831cd32 100644
--- a/src/gpu/GrSurfaceProxy.cpp
+++ b/src/gpu/GrSurfaceProxy.cpp
@@ -43,8 +43,6 @@
return nullptr;
}
- this->INHERITED::transferRefs();
-
#ifdef SK_DEBUG
if (kInvalidGpuMemorySize != this->getRawGpuMemorySize_debugOnly()) {
SkASSERT(fTarget->gpuMemorySize() <= this->getRawGpuMemorySize_debugOnly());
diff --git a/tests/ProxyRefTest.cpp b/tests/ProxyRefTest.cpp
deleted file mode 100644
index bbbb02c..0000000
--- a/tests/ProxyRefTest.cpp
+++ /dev/null
@@ -1,200 +0,0 @@
-/*
- * Copyright 2016 Google Inc.
- *
- * Use of this source code is governed by a BSD-style license that can be
- * found in the LICENSE file.
- */
-
-// This is a GPU-backend specific test.
-
-#include "Test.h"
-
-#if SK_SUPPORT_GPU
-#include "GrSurfaceProxy.h"
-#include "GrTextureProxy.h"
-#include "GrRenderTargetPriv.h"
-#include "GrRenderTargetProxy.h"
-
-static const int kWidthHeight = 128;
-
-int32_t GrIORefProxy::getProxyRefCnt_TestOnly() const {
- return fRefCnt;
-}
-
-int32_t GrIORefProxy::getBackingRefCnt_TestOnly() const {
- if (fTarget) {
- return fTarget->fRefCnt;
- }
-
- return fRefCnt;
-}
-
-int32_t GrIORefProxy::getPendingReadCnt_TestOnly() const {
- if (fTarget) {
- SkASSERT(!fPendingReads);
- return fTarget->fPendingReads;
- }
-
- return fPendingReads;
-}
-
-int32_t GrIORefProxy::getPendingWriteCnt_TestOnly() const {
- if (fTarget) {
- SkASSERT(!fPendingWrites);
- return fTarget->fPendingWrites;
- }
-
- return fPendingWrites;
-}
-
-static void check_refs(skiatest::Reporter* reporter,
- GrSurfaceProxy* proxy,
- int32_t expectedProxyRefs,
- int32_t expectedBackingRefs,
- int32_t expectedNumReads,
- int32_t expectedNumWrites) {
- REPORTER_ASSERT(reporter, proxy->getProxyRefCnt_TestOnly() == expectedProxyRefs);
- REPORTER_ASSERT(reporter, proxy->getBackingRefCnt_TestOnly() == expectedBackingRefs);
- REPORTER_ASSERT(reporter, proxy->getPendingReadCnt_TestOnly() == expectedNumReads);
- REPORTER_ASSERT(reporter, proxy->getPendingWriteCnt_TestOnly() == expectedNumWrites);
-
- SkASSERT(proxy->getProxyRefCnt_TestOnly() == expectedProxyRefs);
- SkASSERT(proxy->getBackingRefCnt_TestOnly() == expectedBackingRefs);
- SkASSERT(proxy->getPendingReadCnt_TestOnly() == expectedNumReads);
- SkASSERT(proxy->getPendingWriteCnt_TestOnly() == expectedNumWrites);
-}
-
-static sk_sp<GrSurfaceProxy> make_deferred(const GrCaps& caps, GrTextureProvider* provider) {
- GrSurfaceDesc desc;
- desc.fFlags = kRenderTarget_GrSurfaceFlag;
- desc.fWidth = kWidthHeight;
- desc.fHeight = kWidthHeight;
- desc.fConfig = kRGBA_8888_GrPixelConfig;
-
- return GrSurfaceProxy::MakeDeferred(caps, desc, SkBackingFit::kApprox, SkBudgeted::kYes);
-}
-
-static sk_sp<GrSurfaceProxy> make_wrapped(const GrCaps& caps, GrTextureProvider* provider) {
- GrSurfaceDesc desc;
- desc.fFlags = kRenderTarget_GrSurfaceFlag;
- desc.fWidth = kWidthHeight;
- desc.fHeight = kWidthHeight;
- desc.fConfig = kRGBA_8888_GrPixelConfig;
-
- sk_sp<GrTexture> tex(provider->createTexture(desc, SkBudgeted::kNo));
-
- // Flush the IOWrite from the initial discard or it will confuse the later ref count checks
- tex->flushWrites();
-
- return GrSurfaceProxy::MakeWrapped(std::move(tex));
-}
-
-DEF_GPUTEST_FOR_RENDERING_CONTEXTS(ProxyRefTest, reporter, ctxInfo) {
- GrTextureProvider* provider = ctxInfo.grContext()->textureProvider();
- const GrCaps& caps = *ctxInfo.grContext()->caps();
-
- for (auto make : { make_deferred, make_wrapped }) {
-
- // A single write
- {
- sk_sp<GrSurfaceProxy> sProxy((*make)(caps, provider));
-
- GrPendingIOResource<GrSurfaceProxy, kWrite_GrIOType> fWrite(sProxy.get());
-
- check_refs(reporter, sProxy.get(), 1, 1, 0, 1);
-
- // In the deferred case, the discard batch created on instantiation adds an
- // extra ref and write
- int expectedRefs = !sProxy->isWrapped_ForTesting() ? 2 : 1;
-
- sProxy->instantiate(provider);
-
- // In the deferred case, this checks that the refs transfered to the GrSurface
- check_refs(reporter, sProxy.get(), 1, expectedRefs, 0, expectedRefs);
- }
-
- // A single read
- {
- sk_sp<GrSurfaceProxy> sProxy((*make)(caps, provider));
-
- GrPendingIOResource<GrSurfaceProxy, kRead_GrIOType> fRead(sProxy.get());
-
- check_refs(reporter, sProxy.get(), 1, 1, 1, 0);
-
- // In the deferred case, the discard batch created on instantiation adds an
- // extra ref and write
- int expectedBackingRefs = !sProxy->isWrapped_ForTesting() ? 2 : 1;
- int expectedWrites = !sProxy->isWrapped_ForTesting() ? 1 : 0;
-
- sProxy->instantiate(provider);
-
- // In the deferred case, this checks that the refs transfered to the GrSurface
- check_refs(reporter, sProxy.get(), 1, expectedBackingRefs, 1, expectedWrites);
- }
-
- // A single read/write pair
- {
- sk_sp<GrSurfaceProxy> sProxy((*make)(caps, provider));
-
- GrPendingIOResource<GrSurfaceProxy, kRW_GrIOType> fRW(sProxy.get());
-
- check_refs(reporter, sProxy.get(), 1, 1, 1, 1);
-
- // In the deferred case, the discard batch created on instantiation adds an
- // extra ref and write
- int expectedBackingRefs = !sProxy->isWrapped_ForTesting() ? 2 : 1;
- int expectedWrites = !sProxy->isWrapped_ForTesting() ? 2 : 1;
-
- sProxy->instantiate(provider);
-
- // In the deferred case, this checks that the refs transfered to the GrSurface
- check_refs(reporter, sProxy.get(), 1, expectedBackingRefs, 1, expectedWrites);
- }
-
- // Multiple normal refs
- {
- sk_sp<GrSurfaceProxy> sProxy((*make)(caps, provider));
- sProxy->ref();
- sProxy->ref();
-
- check_refs(reporter, sProxy.get(), 3, 3, 0, 0);
-
- int expectedBackingRefs = !sProxy->isWrapped_ForTesting() ? 4 : 3;
- int expectedWrites = !sProxy->isWrapped_ForTesting() ? 1 : 0;
-
- sProxy->instantiate(provider);
-
- // In the deferred case, this checks that the refs transfered to the GrSurface
- check_refs(reporter, sProxy.get(), 3, expectedBackingRefs, 0, expectedWrites);
-
- sProxy->unref();
- sProxy->unref();
- }
-
- // Continue using (reffing) proxy after instantiation
- {
- sk_sp<GrSurfaceProxy> sProxy((*make)(caps, provider));
- sProxy->ref();
-
- GrPendingIOResource<GrSurfaceProxy, kWrite_GrIOType> fWrite(sProxy.get());
-
- check_refs(reporter, sProxy.get(), 2, 2, 0, 1);
-
- int expectedBackingRefs = !sProxy->isWrapped_ForTesting() ? 3 : 2;
- int expectedWrites = !sProxy->isWrapped_ForTesting() ? 2 : 1;
-
- sProxy->instantiate(provider);
-
- // In the deferred case, this checks that the refs transfered to the GrSurface
- check_refs(reporter, sProxy.get(), 2, expectedBackingRefs, 0, expectedWrites);
-
- sProxy->unref();
- check_refs(reporter, sProxy.get(), 1, expectedBackingRefs-1, 0, expectedWrites);
-
- GrPendingIOResource<GrSurfaceProxy, kRead_GrIOType> fRead(sProxy.get());
- check_refs(reporter, sProxy.get(), 1, expectedBackingRefs-1, 1, expectedWrites);
- }
- }
-}
-
-#endif