From 0d60676556f2d24b3e1c1d57fba20ef441d9bc1a Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Fri, 25 Jan 2019 09:58:38 -0500 Subject: [PATCH] Actually reuse GrTexture if SkPromiseImageTexture used with multiple images. Change-Id: Id68d2f2a4c0012b2219a505f1259a9c9bd014c65 Reviewed-on: https://skia-review.googlesource.com/c/186700 Reviewed-by: Robert Phillips Commit-Queue: Brian Salomon --- include/private/GrSurfaceProxy.h | 13 ++++++------- src/image/SkImage_GpuBase.cpp | 30 ++++++++++++++++++++---------- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/include/private/GrSurfaceProxy.h b/include/private/GrSurfaceProxy.h index 5dd139ab06..1c7b01f961 100644 --- a/include/private/GrSurfaceProxy.h +++ b/include/private/GrSurfaceProxy.h @@ -64,14 +64,13 @@ public: SkASSERT(0 == fPendingReads); SkASSERT(0 == fPendingWrites); - SkASSERT(fRefCnt == fTarget->fRefCnt); - SkASSERT(!fTarget->internalHasPendingIO()); // In the current hybrid world, the proxy and backing surface are ref/unreffed in - // synchrony. In this instance we're deInstantiating the proxy so, regardless of the - // number of refs on the backing surface, we're going to remove it. If/when the proxy - // is re-instantiated all the refs on the proxy (presumably due to multiple uses in ops) - // will be transfered to the new surface. - for (int refs = fTarget->fRefCnt; refs; --refs) { + // synchrony. Each ref we've added or removed to the proxy was mirrored to the backing + // surface. Though, that backing surface could be owned by other proxies as well. Remove + // a ref from the backing surface for each ref the proxy has since we are about to remove + // our pointer to the surface. If this proxy is reinstantiated then all the proxy's refs + // get transferred to the (possibly new) backing surface. + for (int refs = fRefCnt; refs; --refs) { fTarget->unref(); } fTarget = nullptr; diff --git a/src/image/SkImage_GpuBase.cpp b/src/image/SkImage_GpuBase.cpp index 330808222b..7c731cf2a2 100644 --- a/src/image/SkImage_GpuBase.cpp +++ b/src/image/SkImage_GpuBase.cpp @@ -459,21 +459,31 @@ sk_sp SkImage_GpuBase::MakePromiseImageLazyProxy( return sk_sp(); } - auto tex = resourceProvider->wrapBackendTexture(backendTexture, kBorrow_GrWrapOwnership, - GrWrapCacheable::kYes, kRead_GrIOType); - if (!tex) { - // Even though we failed to wrap the backend texture, we must call the release - // proc to keep our contract of always calling Fulfill and Release in pairs. - fReleaseContext->release(); - return sk_sp(); - } - // The texture gets a ref, which is balanced when the idle callback is called. - this->addToIdleContext(tex.get()); static const GrUniqueKey::Domain kDomain = GrUniqueKey::GenerateDomain(); GrUniqueKey::Builder builder(&fLastFulfilledKey, kDomain, 2, "promise"); builder[0] = promiseTexture->uniqueID(); builder[1] = fConfig; builder.finish(); + // A texture with this key may already exist from a different instance of this lazy + // callback. This could happen if the client fulfills a promise image with a texture + // that was previously used to fulfill a different promise image. + sk_sp tex; + if (auto surf = resourceProvider->findByUniqueKey(fLastFulfilledKey)) { + tex = sk_ref_sp(surf->asTexture()); + SkASSERT(tex); + } else { + if ((tex = resourceProvider->wrapBackendTexture( + backendTexture, kBorrow_GrWrapOwnership, GrWrapCacheable::kYes, + kRead_GrIOType))) { + tex->resourcePriv().setUniqueKey(fLastFulfilledKey); + } else { + // Even though we failed to wrap the backend texture, we must call the release + // proc to keep our contract of always calling Fulfill and Release in pairs. + fReleaseContext->release(); + return sk_sp(); + } + } + this->addToIdleContext(tex.get()); tex->resourcePriv().setUniqueKey(fLastFulfilledKey); SkASSERT(fContextID == SK_InvalidUniqueID || fContextID == tex->getContext()->uniqueID());