Make GrTexture caching for SkPromiseImageTexture work when same texture
fulfills a different SkImage.
Bug: skia:8613
Change-Id: I7ee14112c69f8aaef223a37dda455259b501a2bf
Reviewed-on: https://skia-review.googlesource.com/c/184440
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 8f6e407..5e65b50 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -1978,6 +1978,9 @@
DDLTileHelper tiles(canvas, viewport, fNumDivisions);
// Second, reinflate the compressed picture individually for each thread
+ // This recreates the promise SkImages on each replay iteration. We are currently
+ // relying on this to test using a SkPromiseImageTexture to fulfill different
+ // SkImages. On each replay the promise SkImages are recreated in createSKPPerTile.
tiles.createSKPPerTile(compressedPictureData.get(), promiseImageHelper);
// Third, create the DDLs in parallel
diff --git a/include/gpu/GrTexture.h b/include/gpu/GrTexture.h
index 286b7ce..80dd3f3 100644
--- a/include/gpu/GrTexture.h
+++ b/include/gpu/GrTexture.h
@@ -70,6 +70,7 @@
*/
using IdleProc = void(void*);
virtual void setIdleProc(IdleProc, void* context) = 0;
+ virtual void* idleContext() const = 0;
/** Access methods that are only to be used within Skia code. */
inline GrTexturePriv texturePriv();
diff --git a/src/core/SkPromiseImageTexture.cpp b/src/core/SkPromiseImageTexture.cpp
index f4801bb..5c55a9a 100644
--- a/src/core/SkPromiseImageTexture.cpp
+++ b/src/core/SkPromiseImageTexture.cpp
@@ -27,6 +27,11 @@
void SkPromiseImageTexture::addKeyToInvalidate(uint32_t contextID, const GrUniqueKey& key) {
SkASSERT(contextID != SK_InvalidUniqueID);
SkASSERT(key.isValid());
+ for (const auto& msg : fMessages) {
+ if (msg.contextID() == contextID && msg.key() == key) {
+ return;
+ }
+ }
fMessages.emplace_back(key, contextID);
}
diff --git a/src/gpu/gl/GrGLTexture.h b/src/gpu/gl/GrGLTexture.h
index db83eea..f9d4acd 100644
--- a/src/gpu/gl/GrGLTexture.h
+++ b/src/gpu/gl/GrGLTexture.h
@@ -86,6 +86,7 @@
fIdleProc = proc;
fIdleProcContext = context;
}
+ void* idleContext() const override { return fIdleProcContext; }
// These functions are used to track the texture parameters associated with the texture.
GrGpu::ResetTimestamp getCachedParamsTimestamp() const { return fParamsTimestamp; }
diff --git a/src/gpu/mock/GrMockTexture.h b/src/gpu/mock/GrMockTexture.h
index ac5eab0..3674de6 100644
--- a/src/gpu/mock/GrMockTexture.h
+++ b/src/gpu/mock/GrMockTexture.h
@@ -52,6 +52,7 @@
fIdleProc = proc;
fIdleProcContext = context;
}
+ void* idleContext() const override { return fIdleProcContext; }
protected:
// constructor for subclasses
diff --git a/src/gpu/mtl/GrMtlTexture.h b/src/gpu/mtl/GrMtlTexture.h
index 8037c68..2cdb92b 100644
--- a/src/gpu/mtl/GrMtlTexture.h
+++ b/src/gpu/mtl/GrMtlTexture.h
@@ -47,6 +47,7 @@
fIdleProc = proc;
fIdleProcContext = context;
}
+ void* idleContext() const override { return fIdleProcContext; }
protected:
GrMtlTexture(GrMtlGpu*, const GrSurfaceDesc&, id<MTLTexture>, GrMipMapsStatus);
diff --git a/src/gpu/vk/GrVkTexture.h b/src/gpu/vk/GrVkTexture.h
index d913aed..0592fc8 100644
--- a/src/gpu/vk/GrVkTexture.h
+++ b/src/gpu/vk/GrVkTexture.h
@@ -46,6 +46,7 @@
}
void setIdleProc(IdleProc, void* context) override;
+ void* idleContext() const override { return fIdleProcContext; }
protected:
GrVkTexture(GrVkGpu*, const GrSurfaceDesc&, const GrVkImageInfo&, sk_sp<GrVkImageLayout>,
diff --git a/src/image/SkImage_GpuBase.cpp b/src/image/SkImage_GpuBase.cpp
index 8d90be5..0e74923 100644
--- a/src/image/SkImage_GpuBase.cpp
+++ b/src/image/SkImage_GpuBase.cpp
@@ -17,6 +17,7 @@
#include "SkImage_Gpu.h"
#include "SkPromiseImageTexture.h"
#include "SkReadPixelsRec.h"
+#include "SkTLList.h"
#include "effects/GrYUVtoRGBEffect.h"
SkImage_GpuBase::SkImage_GpuBase(sk_sp<GrContext> context, int width, int height, uint32_t uniqueID,
@@ -405,18 +406,11 @@
: fFulfillProc(fulfillProc)
, fConfig(config) {
auto doneHelper = sk_make_sp<GrReleaseProcHelper>(doneProc, context);
- fIdleContext = sk_make_sp<IdleContext>(releaseProc, context, std::move(doneHelper));
- static std::atomic<uint32_t> gUniqueID;
- fUniqueID = gUniqueID.fetch_add(1) + 1;
+ fReleaseContext = sk_make_sp<IdleContext::PromiseImageReleaseContext>(
+ releaseProc, context, std::move(doneHelper));
}
- ~PromiseLazyInstantiateCallback() {
- // Remove the key from the texture so that the texture will be removed from the cache.
- if (fLastFulfilledKey.isValid()) {
- SkMessageBus<GrUniqueKeyInvalidatedMessage>::Post(
- GrUniqueKeyInvalidatedMessage(fLastFulfilledKey, fContextID));
- }
- }
+ ~PromiseLazyInstantiateCallback() = default;
sk_sp<GrSurface> operator()(GrResourceProvider* resourceProvider) {
if (!resourceProvider) {
@@ -434,24 +428,22 @@
}
// If the release callback hasn't been called already by releasing the GrTexture
// then we can be sure that won't happen so long as we have a ref to the texture.
- if (cachedTexture && !fIdleContext->wasReleased()) {
+ if (cachedTexture && !fReleaseContext->isReleased()) {
return std::move(cachedTexture);
}
GrBackendTexture backendTexture;
sk_sp<SkPromiseImageTexture> promiseTexture =
- fFulfillProc(fIdleContext->textureContext());
- fIdleContext->reset();
+ fFulfillProc(fReleaseContext->textureContext());
+ fReleaseContext->notifyWasFulfilled();
if (!promiseTexture) {
- IdleContext::IdleProc(fIdleContext.get());
+ fReleaseContext->release();
return sk_sp<GrTexture>();
}
bool same = promiseTexture->uniqueID() == fLastFulfillID;
SkASSERT(!same || fLastFulfilledKey.isValid());
if (same && cachedTexture) {
- SkASSERT(fIdleContext->unique());
- // Reset the purgeable context so that we balance the new fulfill with a release.
- fIdleContext->ref();
- cachedTexture->setIdleProc(IdleContext::IdleProc, fIdleContext.get());
+ SkASSERT(fReleaseContext->unique());
+ this->addToIdleContext(cachedTexture.get());
return std::move(cachedTexture);
} else if (cachedTexture) {
cachedTexture->resourcePriv().removeUniqueKey();
@@ -463,7 +455,7 @@
if (!backendTexture.isValid()) {
// Even though the GrBackendTexture is not valid, we must call the release
// proc to keep our contract of always calling Fulfill and Release in pairs.
- IdleContext::IdleProc(fIdleContext.get());
+ fReleaseContext->release();
return sk_sp<GrTexture>();
}
@@ -472,16 +464,15 @@
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.
- IdleContext::IdleProc(fIdleContext.get());
+ fReleaseContext->release();
return sk_sp<GrTexture>();
}
// The texture gets a ref, which is balanced when the idle callback is called.
- fIdleContext->ref();
- tex->setIdleProc(IdleContext::IdleProc, fIdleContext.get());
+ this->addToIdleContext(tex.get());
static const GrUniqueKey::Domain kDomain = GrUniqueKey::GenerateDomain();
GrUniqueKey::Builder builder(&fLastFulfilledKey, kDomain, 2, "promise");
builder[0] = promiseTexture->uniqueID();
- builder[1] = fUniqueID;
+ builder[1] = fConfig;
builder.finish();
tex->resourcePriv().setUniqueKey(fLastFulfilledKey);
SkASSERT(fContextID == SK_InvalidUniqueID ||
@@ -498,39 +489,74 @@
// ownership of the IdleContext. Thus, the IdleContext is destroyed and calls the Done proc
// after the last fulfilled texture goes idle and calls the Release proc or the proxy's
// destructor destroys the lazy callback, whichever comes last.
- class IdleContext : public SkNVRefCnt<IdleContext> {
+ class IdleContext {
public:
- IdleContext() = delete;
- IdleContext(PromiseImageTextureReleaseProc releaseProc,
- PromiseImageTextureContext textureContext,
- sk_sp<GrReleaseProcHelper> doneHelper)
- : fReleaseProc(releaseProc)
- , fTextureContext(textureContext)
- , fDoneHelper(std::move(doneHelper)) {}
+ class PromiseImageReleaseContext;
- ~IdleContext() { SkASSERT(fWasReleased); }
+ IdleContext() = default;
- void reset() { fWasReleased = false; }
- bool wasReleased() const { return fWasReleased; }
+ ~IdleContext() = default;
- PromiseImageTextureContext textureContext() const { return fTextureContext; }
-
- static void IdleProc(void* context) {
- IdleContext* rc = static_cast<IdleContext*>(context);
- rc->fReleaseProc(rc->fTextureContext);
- rc->fWasReleased = true;
- // Drop the texture's implicit ref on the IdleContext.
- rc->unref();
+ void addImageReleaseContext(sk_sp<PromiseImageReleaseContext> context) {
+ fReleaseContexts.addToHead(std::move(context));
}
+ static void IdleProc(void* context) {
+ IdleContext* idleContext = static_cast<IdleContext*>(context);
+ for (ReleaseContextList::Iter iter = idleContext->fReleaseContexts.headIter();
+ iter.get();
+ iter.next()) {
+ (*iter.get())->release();
+ }
+ idleContext->fReleaseContexts.reset();
+ delete idleContext;
+ }
+
+ class PromiseImageReleaseContext : public SkNVRefCnt<PromiseImageReleaseContext> {
+ public:
+ PromiseImageReleaseContext(PromiseImageTextureReleaseProc releaseProc,
+ PromiseImageTextureContext textureContext,
+ sk_sp<GrReleaseProcHelper> doneHelper)
+ : fReleaseProc(releaseProc)
+ , fTextureContext(textureContext)
+ , fDoneHelper(std::move(doneHelper)) {}
+
+ ~PromiseImageReleaseContext() { SkASSERT(fIsReleased); }
+
+ void release() {
+ SkASSERT(!fIsReleased);
+ fReleaseProc(fTextureContext);
+ fIsReleased = true;
+ }
+
+ void notifyWasFulfilled() { fIsReleased = false; }
+ bool isReleased() const { return fIsReleased; }
+
+ PromiseImageTextureContext textureContext() const { return fTextureContext; }
+
+ private:
+ PromiseImageTextureReleaseProc fReleaseProc;
+ PromiseImageTextureContext fTextureContext;
+ sk_sp<GrReleaseProcHelper> fDoneHelper;
+ bool fIsReleased = true;
+ };
+
private:
- PromiseImageTextureReleaseProc fReleaseProc;
- PromiseImageTextureContext fTextureContext;
- sk_sp<GrReleaseProcHelper> fDoneHelper;
- bool fWasReleased = true;
+ using ReleaseContextList = SkTLList<sk_sp<PromiseImageReleaseContext>, 4>;
+ ReleaseContextList fReleaseContexts;
};
- sk_sp<IdleContext> fIdleContext;
+ void addToIdleContext(GrTexture* texture) {
+ SkASSERT(!fReleaseContext->isReleased());
+ IdleContext* idleContext = static_cast<IdleContext*>(texture->idleContext());
+ if (!idleContext) {
+ idleContext = new IdleContext();
+ texture->setIdleProc(IdleContext::IdleProc, idleContext);
+ }
+ idleContext->addImageReleaseContext(fReleaseContext);
+ }
+
+ sk_sp<IdleContext::PromiseImageReleaseContext> fReleaseContext;
PromiseImageTextureFulfillProc fFulfillProc;
GrPixelConfig fConfig;
@@ -538,8 +564,6 @@
uint32_t fLastFulfillID = 0;
// ID of the GrContext that we are interacting with.
uint32_t fContextID = SK_InvalidUniqueID;
- // Unique ID of this lazy instantiation callback.
- uint32_t fUniqueID;
GrUniqueKey fLastFulfilledKey;
} callback(fulfillProc, releaseProc, doneProc, textureContext, config);
diff --git a/tests/PromiseImageTest.cpp b/tests/PromiseImageTest.cpp
index bc9b26b..764fde4 100644
--- a/tests/PromiseImageTest.cpp
+++ b/tests/PromiseImageTest.cpp
@@ -10,6 +10,7 @@
#include "GrBackendSurface.h"
#include "GrContextPriv.h"
#include "GrGpu.h"
+#include "GrTexture.h"
#include "SkImage_Gpu.h"
#include "SkPromiseImageTexture.h"
@@ -267,11 +268,11 @@
GrGpu* gpu = ctx->contextPriv().getGpu();
GrBackendTexture backendTex1 = gpu->createTestingOnlyBackendTexture(
- nullptr, kWidth, kHeight, GrColorType::kRGBA_8888, true, GrMipMapped::kNo);
+ nullptr, kWidth, kHeight, GrColorType::kRGBA_8888, false, GrMipMapped::kNo);
GrBackendTexture backendTex2 = gpu->createTestingOnlyBackendTexture(
- nullptr, kWidth, kHeight, GrColorType::kRGBA_8888, true, GrMipMapped::kNo);
+ nullptr, kWidth, kHeight, GrColorType::kRGBA_8888, false, GrMipMapped::kNo);
GrBackendTexture backendTex3 = gpu->createTestingOnlyBackendTexture(
- nullptr, kWidth, kHeight, GrColorType::kRGBA_8888, true, GrMipMapped::kNo);
+ nullptr, kWidth, kHeight, GrColorType::kRGBA_8888, false, GrMipMapped::kNo);
REPORTER_ASSERT(reporter, backendTex1.isValid());
REPORTER_ASSERT(reporter, backendTex2.isValid());
REPORTER_ASSERT(reporter, backendTex3.isValid());
@@ -293,7 +294,8 @@
PromiseTextureChecker::Done,
&promiseChecker));
- SkImageInfo info = SkImageInfo::MakeN32Premul(kWidth, kHeight);
+ SkImageInfo info =
+ SkImageInfo::Make(kWidth, kHeight, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
sk_sp<SkSurface> surface = SkSurface::MakeRenderTarget(ctx, SkBudgeted::kNo, info);
SkCanvas* canvas = surface->getCanvas();
@@ -479,18 +481,14 @@
expectedDoneCnt,
reporter));
- // We currently expect each promise image to make and cache its own GrTexture. We will likely
- // try to make these share in the future.
+ // The two images should share a single GrTexture by using the same key. The key is only
+ // dependent on the pixel config and the PromiseImageTexture key.
keys = promiseChecker.uniqueKeys();
- REPORTER_ASSERT(reporter, keys.count() == 2);
- GrUniqueKey texKey4;
- if (keys.count() == 2) {
+ REPORTER_ASSERT(reporter, keys.count() == 1);
+ if (keys.count() > 0) {
REPORTER_ASSERT(reporter, texKey3 == keys[0]);
- texKey4 = keys[1];
}
ctx->contextPriv().getResourceCache()->purgeAsNeeded();
- REPORTER_ASSERT(reporter, ctx->contextPriv().resourceProvider()->findByUniqueKey<>(texKey3));
- REPORTER_ASSERT(reporter, ctx->contextPriv().resourceProvider()->findByUniqueKey<>(texKey4));
// If we delete the SkPromiseImageTexture we should trigger both key removals.
REPORTER_ASSERT(reporter,
@@ -499,7 +497,6 @@
ctx->contextPriv().getResourceCache()->purgeAsNeeded();
REPORTER_ASSERT(reporter, !ctx->contextPriv().resourceProvider()->findByUniqueKey<>(texKey3));
- REPORTER_ASSERT(reporter, !ctx->contextPriv().resourceProvider()->findByUniqueKey<>(texKey4));
gpu->deleteTestingOnlyBackendTexture(backendTex3);
// After deleting each image we should get a done call.
@@ -522,3 +519,92 @@
expectedDoneCnt,
reporter));
}
+
+DEF_GPUTEST_FOR_RENDERING_CONTEXTS(PromiseImageTextureReuseDifferentConfig, reporter, ctxInfo) {
+ // Try making two promise SkImages backed by the same texture but with different configs.
+ // This will only be testable on backends where a single texture format (8bit red unorm) can
+ // be used for alpha and gray image color types.
+
+ const int kWidth = 10;
+ const int kHeight = 10;
+
+ GrContext* ctx = ctxInfo.grContext();
+ GrGpu* gpu = ctx->contextPriv().getGpu();
+
+ GrBackendTexture backendTex1 = gpu->createTestingOnlyBackendTexture(
+ nullptr, kWidth, kHeight, GrColorType::kGray_8, false, GrMipMapped::kNo);
+ REPORTER_ASSERT(reporter, backendTex1.isValid());
+
+ GrBackendTexture backendTex2 = gpu->createTestingOnlyBackendTexture(
+ nullptr, kWidth, kHeight, GrColorType::kAlpha_8, false, GrMipMapped::kNo);
+ REPORTER_ASSERT(reporter, backendTex2.isValid());
+
+ SkImageInfo info =
+ SkImageInfo::Make(kWidth, kHeight, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+ sk_sp<SkSurface> surface = SkSurface::MakeRenderTarget(ctx, SkBudgeted::kNo, info);
+ SkCanvas* canvas = surface->getCanvas();
+
+ if (backendTex1.getBackendFormat() != backendTex2.getBackendFormat()) {
+ gpu->deleteTestingOnlyBackendTexture(backendTex1);
+ gpu->deleteTestingOnlyBackendTexture(backendTex2);
+ return;
+ }
+ PromiseTextureChecker promiseChecker(backendTex1);
+ sk_sp<SkImage> alphaImg(SkImage_Gpu::MakePromiseTexture(
+ ctx, backendTex1.getBackendFormat(), kWidth, kHeight, GrMipMapped::kNo,
+ kTopLeft_GrSurfaceOrigin, kAlpha_8_SkColorType, kPremul_SkAlphaType, nullptr,
+ PromiseTextureChecker::Fulfill, PromiseTextureChecker::Release,
+ PromiseTextureChecker::Done, &promiseChecker));
+ REPORTER_ASSERT(reporter, alphaImg);
+
+ sk_sp<SkImage> grayImg(SkImage_Gpu::MakePromiseTexture(
+ ctx, backendTex1.getBackendFormat(), kWidth, kHeight, GrMipMapped::kNo,
+ kBottomLeft_GrSurfaceOrigin, kGray_8_SkColorType, kOpaque_SkAlphaType, nullptr,
+ PromiseTextureChecker::Fulfill, PromiseTextureChecker::Release,
+ PromiseTextureChecker::Done, &promiseChecker));
+ REPORTER_ASSERT(reporter, grayImg);
+
+ canvas->drawImage(alphaImg, 0, 0);
+ canvas->drawImage(grayImg, 1, 1);
+ canvas->flush();
+ gpu->testingOnly_flushGpuAndSync();
+
+ int expectedFulfillCnt = 2;
+ int expectedReleaseCnt = 2;
+ int expectedDoneCnt = 0;
+ REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker,
+ true,
+ expectedFulfillCnt,
+ expectedReleaseCnt,
+ true,
+ expectedDoneCnt,
+ reporter));
+
+ // Because they use different configs, each image should have created a different GrTexture
+ // and they both should still be cached.
+ ctx->contextPriv().getResourceCache()->purgeAsNeeded();
+
+ auto keys = promiseChecker.uniqueKeys();
+ REPORTER_ASSERT(reporter, keys.count() == 2);
+ for (const auto& key : keys) {
+ auto surf = ctx->contextPriv().resourceProvider()->findByUniqueKey<GrSurface>(key);
+ REPORTER_ASSERT(reporter, surf && surf->asTexture());
+ if (surf && surf->asTexture()) {
+ REPORTER_ASSERT(reporter, !GrBackendTexture::TestingOnly_Equals(
+ backendTex1, surf->asTexture()->getBackendTexture()));
+ }
+ }
+
+ // Change the backing texture, this should invalidate the keys. The cached textures should
+ // get purged after purgeAsNeeded is called.
+ promiseChecker.replaceTexture(backendTex2);
+ ctx->contextPriv().getResourceCache()->purgeAsNeeded();
+
+ for (const auto& key : keys) {
+ auto surf = ctx->contextPriv().resourceProvider()->findByUniqueKey<GrSurface>(key);
+ REPORTER_ASSERT(reporter, !surf);
+ }
+
+ gpu->deleteTestingOnlyBackendTexture(backendTex1);
+ gpu->deleteTestingOnlyBackendTexture(backendTex2);
+}