Add IORef capability to GrSurfaceProxy objects - take 2
Re-landing of https://skia-review.googlesource.com/c/4734/ (Add IORef capability to GrSurfaceProxy objects) with fixes for:
platforms that don't support discards.
memory leak due to copying over of GrSurfaceProxy's creation ref to instantiated GrSurface in instantiate()
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4827
BUG=skia:
Change-Id: If0b695d5cafc5ef3fdd251c38262849d09e5d27f
Review-Url: https://codereview.chromium.org/2502923003
diff --git a/gn/tests.gni b/gn/tests.gni
index d888c6a..2cf9d33 100644
--- a/gn/tests.gni
+++ b/gn/tests.gni
@@ -158,6 +158,7 @@
"$_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 4597464..29d33df 100644
--- a/include/gpu/GrGpuResource.h
+++ b/include/gpu/GrGpuResource.h
@@ -93,6 +93,8 @@
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 dd5ece1..1ff6df8 100644
--- a/include/private/GrSurfaceProxy.h
+++ b/include/private/GrSurfaceProxy.h
@@ -50,13 +50,29 @@
void validate() const {
#ifdef SK_DEBUG
- SkASSERT(fRefCnt >= 0);
+ 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);
+ }
#endif
}
+ int32_t getProxyRefCnt_TestOnly() const;
+ int32_t getBackingRefCnt_TestOnly() const;
+ int32_t getPendingReadCnt_TestOnly() const;
+ int32_t getPendingWriteCnt_TestOnly() const;
+
protected:
- GrIORefProxy() : fRefCnt(1), fTarget(nullptr) {}
- GrIORefProxy(sk_sp<GrSurface> surface) : fRefCnt(1) {
+ GrIORefProxy() : fTarget(nullptr), fRefCnt(1), fPendingReads(0), fPendingWrites(0) {}
+ GrIORefProxy(sk_sp<GrSurface> surface) : fRefCnt(1), fPendingReads(0), fPendingWrites(0) {
// Since we're manually forwarding on refs & unrefs we don't want sk_sp doing
// anything extra.
fTarget = surface.release();
@@ -66,13 +82,76 @@
// have forwarded on the unref call that got use here.
}
- // 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;
+ // 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-1); // don't xfer the proxy's creation ref
+ fTarget->fPendingReads += fPendingReads;
+ fTarget->fPendingWrites += fPendingWrites;
+
+ fPendingReads = 0;
+ fPendingWrites = 0;
+ }
// 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 831cd32..9de41ef 100644
--- a/src/gpu/GrSurfaceProxy.cpp
+++ b/src/gpu/GrSurfaceProxy.cpp
@@ -43,6 +43,8 @@
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
new file mode 100644
index 0000000..4f6a3e3
--- /dev/null
+++ b/tests/ProxyRefTest.cpp
@@ -0,0 +1,205 @@
+/*
+ * 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
+ bool proxyGetsDiscardRef = !sProxy->isWrapped_ForTesting() &&
+ caps.discardRenderTargetSupport();
+ int expectedWrites = proxyGetsDiscardRef ? 2 : 1;
+
+ sProxy->instantiate(provider);
+
+ // In the deferred case, this checks that the refs transfered to the GrSurface
+ check_refs(reporter, sProxy.get(), 1, 1, 0, expectedWrites);
+ }
+
+ // 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
+ bool proxyGetsDiscardRef = !sProxy->isWrapped_ForTesting() &&
+ caps.discardRenderTargetSupport();
+ int expectedWrites = proxyGetsDiscardRef ? 1 : 0;
+
+ sProxy->instantiate(provider);
+
+ // In the deferred case, this checks that the refs transfered to the GrSurface
+ check_refs(reporter, sProxy.get(), 1, 1, 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
+ bool proxyGetsDiscardRef = !sProxy->isWrapped_ForTesting() &&
+ caps.discardRenderTargetSupport();
+ int expectedWrites = proxyGetsDiscardRef ? 2 : 1;
+
+ sProxy->instantiate(provider);
+
+ // In the deferred case, this checks that the refs transfered to the GrSurface
+ check_refs(reporter, sProxy.get(), 1, 1, 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);
+
+ bool proxyGetsDiscardRef = !sProxy->isWrapped_ForTesting() &&
+ caps.discardRenderTargetSupport();
+ int expectedWrites = proxyGetsDiscardRef ? 1 : 0;
+
+ sProxy->instantiate(provider);
+
+ // In the deferred case, this checks that the refs transfered to the GrSurface
+ check_refs(reporter, sProxy.get(), 3, 3, 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);
+
+ bool proxyGetsDiscardRef = !sProxy->isWrapped_ForTesting() &&
+ caps.discardRenderTargetSupport();
+ int expectedWrites = proxyGetsDiscardRef ? 2 : 1;
+
+ sProxy->instantiate(provider);
+
+ // In the deferred case, this checks that the refs transfered to the GrSurface
+ check_refs(reporter, sProxy.get(), 2, 2, 0, expectedWrites);
+
+ sProxy->unref();
+ check_refs(reporter, sProxy.get(), 1, 1, 0, expectedWrites);
+
+ GrPendingIOResource<GrSurfaceProxy, kRead_GrIOType> fRead(sProxy.get());
+ check_refs(reporter, sProxy.get(), 1, 1, 1, expectedWrites);
+ }
+ }
+}
+
+#endif