Revert "Change promise image contract to for when Release and Done are called." This reverts commit c5e8e150c86d817834b4e9ac13c3c60eedbc6f4e. Reason for revert: Reverting to revert 9ac040700659cefff3c9a7ec6d4ada98948c307c https://skia-review.googlesource.com/c/skia/+/197163 Original change's description: > Change promise image contract to for when Release and Done are called. > > Now Release is called when all work related to the SkImage is flushed > and Done is called when it is finished on the GPU in Vulkan. In GL they > are still both called back-to-back after flushing. > > Bug: skia:8800 > Change-Id: I990be7b9ebef8411590afd860ef40511fb5fee32 > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/198165 > Commit-Queue: Brian Salomon <bsalomon@google.com> > Reviewed-by: Robert Phillips <robertphillips@google.com> TBR=egdaniel@google.com,bsalomon@google.com,robertphillips@google.com Change-Id: I13e8211d89ed1b7694c2d7734eeaaf4ba9ad410a No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: skia:8800 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/198602 Reviewed-by: Brian Salomon <bsalomon@google.com> Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/include/core/SkDeferredDisplayListRecorder.h b/include/core/SkDeferredDisplayListRecorder.h index c0446476..7516001 100644 --- a/include/core/SkDeferredDisplayListRecorder.h +++ b/include/core/SkDeferredDisplayListRecorder.h
@@ -58,45 +58,31 @@ using PromiseImageTextureReleaseProc = void (*)(PromiseImageTextureContext); using PromiseImageTextureDoneProc = void (*)(PromiseImageTextureContext); - enum class PromiseImageApiVersion { kLegacy, kNew }; - /** - Create a new SkImage that is very similar to an SkImage created by MakeFromTexture. The - difference is that the caller need not have created the texture nor populated it with the - image pixel data. Moreover, the SkImage may be created on a thread as the creation of the - image does not require access to the backend API or GrContext. Instead of passing a - GrBackendTexture the client supplies a description of the texture consisting of - GrBackendFormat, width, height, and GrMipMapped state. The resulting SkImage can be drawn - to a SkDeferredDisplayListRecorder or directly to a GPU-backed SkSurface. + Create a new SkImage that is very similar to an SkImage created by MakeFromTexture. The main + difference is that the client doesn't have the backend texture on the gpu yet but they know + all the properties of the texture. So instead of passing in a GrBackendTexture the client + supplies a GrBackendFormat, width, height, and GrMipMapped state. - When the actual texture is required to perform a backend API draw, textureFulfillProc will - be called to receive a GrBackendTexture. The properties of the GrBackendTexture must match - those set during the SkImage creation, and it must refer to a valid existing texture in the - backend API context/device, and be populated with the image pixel data. The texture contents - cannot be modified until textureReleaseProc is called. The texture cannot be deleted until - textureDoneProc is called. + When we actually send the draw calls to the GPU, we will call the textureFulfillProc and + the client will return a GrBackendTexture to us. The properties of the GrBackendTexture must + match those set during the SkImage creation, and it must have a valid backend gpu texture. + The gpu texture supplied by the client must stay valid until we call the textureReleaseProc. When all the following are true: - * the promise SkImage is deleted, + * the promise image is deleted, * any SkDeferredDisplayLists that recorded draws referencing the image are deleted, - * and all draws referencing the texture have been flushed (via GrContext::flush or - SkSurface::flush) - the textureReleaseProc is called. When the following additional constraint is met - * the texture is safe to delete in the underlying API - the textureDoneProc is called. For some APIs (e.g. GL) the two states are equivalent. - However, for others (e.g. Vulkan) they are not as it is not legal to delete a texture until - the GPU work referencing it has completed. + * and the texture is safe to delete in the underlying API with respect to drawn + SkDeferredDisplayLists that reference the image + the textureReleaseProc and then textureDoneProc are called. The texture can be deleted + by the client as soon as textureReleaseProc is called. There is at most one call to each of + textureFulfillProc, textureReleaseProc, and textureDoneProc. textureDoneProc is always + called even if image creation fails or if the image is never fulfilled (e.g. it is never + drawn). If textureFulfillProc is called then textureReleaseProc will always be called even + if textureFulfillProc fails. - There is at most one call to each of textureFulfillProc, textureReleaseProc, and - textureDoneProc. textureDoneProc is always called even if image creation fails or if the - image is never fulfilled (e.g. it is never drawn or all draws are clipped out). If - textureFulfillProc is called then textureReleaseProc will always be called even if - textureFulfillProc failed. - If 'version' is set to kLegacy then the textureReleaseProc call is delayed until the - conditions for textureDoneProc are met and then they are both called. - - This call is only valid if the SkDeferredDisplayListRecorder is backed by a GPU context. + This call is only valid if the SkDeferredDisplayListRecorder is backed by a gpu context. @param backendFormat format of promised gpu texture @param width width of promised gpu texture @@ -112,51 +98,42 @@ @param colorSpace range of colors; may be nullptr @param textureFulfillProc function called to get actual gpu texture @param textureReleaseProc function called when texture can be released - @param textureDoneProc function called when we will no longer call textureFulfillProc + @param promiseDoneProc function called when we will no longer call textureFulfillProc @param textureContext state passed to textureFulfillProc and textureReleaseProc - @param version controls when textureReleaseProc is called @return created SkImage, or nullptr */ - sk_sp<SkImage> makePromiseTexture( - const GrBackendFormat& backendFormat, - int width, - int height, - GrMipMapped mipMapped, - GrSurfaceOrigin origin, - SkColorType colorType, - SkAlphaType alphaType, - sk_sp<SkColorSpace> colorSpace, - PromiseImageTextureFulfillProc textureFulfillProc, - PromiseImageTextureReleaseProc textureReleaseProc, - PromiseImageTextureDoneProc textureDoneProc, - PromiseImageTextureContext textureContext, - PromiseImageApiVersion version = PromiseImageApiVersion::kLegacy); + sk_sp<SkImage> makePromiseTexture(const GrBackendFormat& backendFormat, + int width, + int height, + GrMipMapped mipMapped, + GrSurfaceOrigin origin, + SkColorType colorType, + SkAlphaType alphaType, + sk_sp<SkColorSpace> colorSpace, + PromiseImageTextureFulfillProc textureFulfillProc, + PromiseImageTextureReleaseProc textureReleaseProc, + PromiseImageTextureDoneProc textureDoneProc, + PromiseImageTextureContext textureContext); /** - This entry point operates like 'makePromiseTexture' but it is used to construct a SkImage - from YUV[A] data. The source data may be planar (i.e. spread across multiple textures). In - the extreme Y, U, V, and A are all in different planes and thus the image is specified by - four textures. 'yuvaIndices' specifies the mapping from texture color channels to Y, U, V, - and possibly A components. It therefore indicates how many unique textures compose the full - image. Separate textureFulfillProc, textureReleaseProc, and textureDoneProc calls are made - for each texture and each texture has its own PromiseImageTextureContext. 'yuvFormats', - 'yuvaSizes', and 'textureContexts' have one entry for each of the up to four textures, as - indicated by 'yuvaIndices'. + This entry point operates the same as 'makePromiseTexture' except that its + textureFulfillProc can be called up to four times to fetch the required YUVA + planes (passing a different textureContext to each call). So, if the 'yuvaIndices' + indicate that only the first two backend textures are used, 'textureFulfillProc' will + be called with the first two 'textureContexts'. */ - sk_sp<SkImage> makeYUVAPromiseTexture( - SkYUVColorSpace yuvColorSpace, - const GrBackendFormat yuvaFormats[], - const SkISize yuvaSizes[], - const SkYUVAIndex yuvaIndices[4], - int imageWidth, - int imageHeight, - GrSurfaceOrigin imageOrigin, - sk_sp<SkColorSpace> imageColorSpace, - PromiseImageTextureFulfillProc textureFulfillProc, - PromiseImageTextureReleaseProc textureReleaseProc, - PromiseImageTextureDoneProc textureDoneProc, - PromiseImageTextureContext textureContexts[], - PromiseImageApiVersion version = PromiseImageApiVersion::kLegacy); + sk_sp<SkImage> makeYUVAPromiseTexture(SkYUVColorSpace yuvColorSpace, + const GrBackendFormat yuvaFormats[], + const SkISize yuvaSizes[], + const SkYUVAIndex yuvaIndices[4], + int imageWidth, + int imageHeight, + GrSurfaceOrigin imageOrigin, + sk_sp<SkColorSpace> imageColorSpace, + PromiseImageTextureFulfillProc textureFulfillProc, + PromiseImageTextureReleaseProc textureReleaseProc, + PromiseImageTextureDoneProc textureDoneProc, + PromiseImageTextureContext textureContexts[]); private: bool init();
diff --git a/include/core/SkYUVAIndex.h b/include/core/SkYUVAIndex.h index b9bc396..f52786c 100644 --- a/include/core/SkYUVAIndex.h +++ b/include/core/SkYUVAIndex.h
@@ -49,7 +49,7 @@ }; static constexpr int kIndexCount = kLast_Index + 1; - /** The index is a number between -1..3 which defines which image source to read from, where -1 + /** The index is a number between -1..3 which definies which image source to read from, where -1 * means the image source doesn't exist. The assumption is we will always have image sources for * each of YUV planes, but optionally have image source for A plane. */ int fIndex;
diff --git a/src/core/SkDeferredDisplayListRecorder.cpp b/src/core/SkDeferredDisplayListRecorder.cpp index e81dbba..8063349 100644 --- a/src/core/SkDeferredDisplayListRecorder.cpp +++ b/src/core/SkDeferredDisplayListRecorder.cpp
@@ -34,8 +34,7 @@ PromiseImageTextureFulfillProc textureFulfillProc, PromiseImageTextureReleaseProc textureReleaseProc, PromiseImageTextureDoneProc textureDoneProc, - PromiseImageTextureContext textureContext, - PromiseImageApiVersion) { + PromiseImageTextureContext textureContext) { return nullptr; } @@ -51,8 +50,7 @@ PromiseImageTextureFulfillProc textureFulfillProc, PromiseImageTextureReleaseProc textureReleaseProc, PromiseImageTextureDoneProc textureDoneProc, - PromiseImageTextureContext textureContexts[], - PromiseImageApiVersion) { + PromiseImageTextureContext textureContexts[]) { return nullptr; } @@ -235,8 +233,7 @@ PromiseImageTextureFulfillProc textureFulfillProc, PromiseImageTextureReleaseProc textureReleaseProc, PromiseImageTextureDoneProc textureDoneProc, - PromiseImageTextureContext textureContext, - PromiseImageApiVersion version) { + PromiseImageTextureContext textureContext) { if (!fContext) { return nullptr; } @@ -253,8 +250,7 @@ textureFulfillProc, textureReleaseProc, textureDoneProc, - textureContext, - version); + textureContext); } sk_sp<SkImage> SkDeferredDisplayListRecorder::makeYUVAPromiseTexture( @@ -269,8 +265,7 @@ PromiseImageTextureFulfillProc textureFulfillProc, PromiseImageTextureReleaseProc textureReleaseProc, PromiseImageTextureDoneProc textureDoneProc, - PromiseImageTextureContext textureContexts[], - PromiseImageApiVersion version) { + PromiseImageTextureContext textureContexts[]) { if (!fContext) { return nullptr; } @@ -287,8 +282,7 @@ textureFulfillProc, textureReleaseProc, textureDoneProc, - textureContexts, - version); + textureContexts); } #endif
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index 4fec2ea..33f77cf 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp
@@ -405,8 +405,7 @@ PromiseImageTextureFulfillProc textureFulfillProc, PromiseImageTextureReleaseProc textureReleaseProc, PromiseImageTextureDoneProc textureDoneProc, - PromiseImageTextureContext textureContext, - PromiseImageApiVersion version) { + PromiseImageTextureContext textureContext) { // The contract here is that if 'promiseDoneProc' is passed in it should always be called, // even if creation of the SkImage fails. Once we call MakePromiseImageLazyProxy it takes // responsibility for calling the done proc. @@ -437,7 +436,7 @@ callDone.clear(); auto proxy = MakePromiseImageLazyProxy(context, width, height, origin, config, backendFormat, mipMapped, textureFulfillProc, textureReleaseProc, - textureDoneProc, textureContext, version); + textureDoneProc, textureContext); if (!proxy) { return nullptr; }
diff --git a/src/image/SkImage_Gpu.h b/src/image/SkImage_Gpu.h index 39ba49e..e05ec6c 100644 --- a/src/image/SkImage_Gpu.h +++ b/src/image/SkImage_Gpu.h
@@ -55,8 +55,7 @@ PromiseImageTextureFulfillProc textureFulfillProc, PromiseImageTextureReleaseProc textureReleaseProc, PromiseImageTextureDoneProc textureDoneProc, - PromiseImageTextureContext textureContext, - PromiseImageApiVersion); + PromiseImageTextureContext textureContext); static sk_sp<SkImage> ConvertYUVATexturesToRGB(GrContext*, SkYUVColorSpace yuvColorSpace, const GrBackendTexture yuvaTextures[],
diff --git a/src/image/SkImage_GpuBase.cpp b/src/image/SkImage_GpuBase.cpp index 8d64cd0..2618dd4 100644 --- a/src/image/SkImage_GpuBase.cpp +++ b/src/image/SkImage_GpuBase.cpp
@@ -386,8 +386,7 @@ PromiseImageTextureFulfillProc fulfillProc, PromiseImageTextureReleaseProc releaseProc, PromiseImageTextureDoneProc doneProc, - PromiseImageTextureContext textureContext, - PromiseImageApiVersion version) { + PromiseImageTextureContext textureContext) { SkASSERT(context); SkASSERT(width > 0 && height > 0); SkASSERT(doneProc); @@ -426,12 +425,8 @@ PromiseImageTextureReleaseProc releaseProc, PromiseImageTextureDoneProc doneProc, PromiseImageTextureContext context, - GrPixelConfig config, - PromiseImageApiVersion version) - : fFulfillProc(fulfillProc) - , fReleaseProc(releaseProc) - , fConfig(config) - , fVersion(version) { + GrPixelConfig config) + : fFulfillProc(fulfillProc), fReleaseProc(releaseProc), fConfig(config) { fDoneCallback = sk_make_sp<GrRefCntedCallback>(doneProc, context); } PromiseLazyInstantiateCallback(PromiseLazyInstantiateCallback&&) = default; @@ -485,10 +480,7 @@ return sk_sp<GrTexture>(); } } - auto releaseIdleState = fVersion == PromiseImageApiVersion::kLegacy - ? GrTexture::IdleState::kFinished - : GrTexture::IdleState::kFlushed; - tex->addIdleProc(std::move(releaseCallback), releaseIdleState); + tex->addIdleProc(std::move(releaseCallback), GrTexture::IdleState::kFinished); tex->addIdleProc(std::move(fDoneCallback), GrTexture::IdleState::kFinished); promiseTexture->addKeyToInvalidate(tex->getContext()->priv().contextID(), key); return std::move(tex); @@ -499,8 +491,7 @@ PromiseImageTextureReleaseProc fReleaseProc; sk_sp<GrRefCntedCallback> fDoneCallback; GrPixelConfig fConfig; - PromiseImageApiVersion fVersion; - } callback(fulfillProc, releaseProc, doneProc, textureContext, config, version); + } callback(fulfillProc, releaseProc, doneProc, textureContext, config); GrProxyProvider* proxyProvider = context->priv().proxyProvider();
diff --git a/src/image/SkImage_GpuBase.h b/src/image/SkImage_GpuBase.h index ad5fc1d..0058711 100644 --- a/src/image/SkImage_GpuBase.h +++ b/src/image/SkImage_GpuBase.h
@@ -78,7 +78,6 @@ using PromiseImageTextureDoneProc = SkDeferredDisplayListRecorder::PromiseImageTextureDoneProc; protected: - using PromiseImageApiVersion = SkDeferredDisplayListRecorder::PromiseImageApiVersion; // Helper for making a lazy proxy for a promise image. The PromiseDoneProc we be called, // if not null, immediately if this function fails. Othwerwise, it is installed in the // proxy along with the TextureFulfillProc and TextureReleaseProc. PromiseDoneProc must not @@ -86,7 +85,7 @@ static sk_sp<GrTextureProxy> MakePromiseImageLazyProxy( GrContext*, int width, int height, GrSurfaceOrigin, GrPixelConfig, GrBackendFormat, GrMipMapped, PromiseImageTextureFulfillProc, PromiseImageTextureReleaseProc, - PromiseImageTextureDoneProc, PromiseImageTextureContext, PromiseImageApiVersion); + PromiseImageTextureDoneProc, PromiseImageTextureContext); static bool RenderYUVAToRGBA(GrContext* ctx, GrRenderTargetContext* renderTargetContext, const SkRect& rect, SkYUVColorSpace yuvColorSpace,
diff --git a/src/image/SkImage_GpuYUVA.cpp b/src/image/SkImage_GpuYUVA.cpp index ffa801c..fe3eb92 100644 --- a/src/image/SkImage_GpuYUVA.cpp +++ b/src/image/SkImage_GpuYUVA.cpp
@@ -282,8 +282,7 @@ PromiseImageTextureFulfillProc textureFulfillProc, PromiseImageTextureReleaseProc textureReleaseProc, PromiseImageTextureDoneProc promiseDoneProc, - PromiseImageTextureContext textureContexts[], - PromiseImageApiVersion version) { + PromiseImageTextureContext textureContexts[]) { int numTextures; bool valid = SkYUVAIndex::AreValidIndices(yuvaIndices, &numTextures); @@ -343,7 +342,7 @@ proxies[texIdx] = MakePromiseImageLazyProxy( context, yuvaSizes[texIdx].width(), yuvaSizes[texIdx].height(), imageOrigin, config, yuvaFormats[texIdx], GrMipMapped::kNo, textureFulfillProc, textureReleaseProc, - promiseDoneProc, textureContexts[texIdx], version); + promiseDoneProc, textureContexts[texIdx]); ++proxiesCreated; if (!proxies[texIdx]) { return nullptr;
diff --git a/src/image/SkImage_GpuYUVA.h b/src/image/SkImage_GpuYUVA.h index 4f01735..33bea53 100644 --- a/src/image/SkImage_GpuYUVA.h +++ b/src/image/SkImage_GpuYUVA.h
@@ -73,8 +73,7 @@ PromiseImageTextureFulfillProc textureFulfillProc, PromiseImageTextureReleaseProc textureReleaseProc, PromiseImageTextureDoneProc textureDoneProc, - PromiseImageTextureContext textureContexts[], - PromiseImageApiVersion); + PromiseImageTextureContext textureContexts[]); private: SkImage_GpuYUVA(const SkImage_GpuYUVA* image, sk_sp<SkColorSpace>);
diff --git a/tests/DeferredDisplayListTest.cpp b/tests/DeferredDisplayListTest.cpp index c7584f6..8adc90b 100644 --- a/tests/DeferredDisplayListTest.cpp +++ b/tests/DeferredDisplayListTest.cpp
@@ -675,8 +675,7 @@ dummy_fulfill_proc, dummy_release_proc, dummy_done_proc, - nullptr, - SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew); + nullptr); REPORTER_ASSERT(reporter, !image); } @@ -780,8 +779,7 @@ dummy_fulfill_proc, dummy_release_proc, dummy_done_proc, - nullptr, - SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew); + nullptr); if (GR_GL_TEXTURE_2D != target && mipMapped == GrMipMapped::kYes) { REPORTER_ASSERT(reporter, !image); continue;
diff --git a/tests/PromiseImageTest.cpp b/tests/PromiseImageTest.cpp index 40b68ed..93a09ca 100644 --- a/tests/PromiseImageTest.cpp +++ b/tests/PromiseImageTest.cpp
@@ -63,100 +63,66 @@ } }; -enum class ReleaseBalanceExpectation { +enum class ReleaseBalanceExpecation { kBalanced, - kAllUnbalanced, - kUnbalancedByOne, + kBalancedOrPlusOne, + kAny }; -enum class DoneBalanceExpectation { - kBalanced, - kAllUnbalanced, - kUnknown, - kUnbalancedByOne, - kBalancedOrOffByOne, -}; - -static void check_fulfill_and_release_cnts(skiatest::Reporter* reporter, - const PromiseTextureChecker& promiseChecker, +static bool check_fulfill_and_release_cnts(const PromiseTextureChecker& promiseChecker, + ReleaseBalanceExpecation balanceExpecation, int expectedFulfillCnt, - ReleaseBalanceExpectation releaseBalanceExpecation, - DoneBalanceExpectation doneBalanceExpecation) { - REPORTER_ASSERT(reporter, promiseChecker.fFulfillCount == expectedFulfillCnt); - if (!expectedFulfillCnt) { - // Release and Done should only ever be called after Fulfill. - REPORTER_ASSERT(reporter, !promiseChecker.fReleaseCount); - REPORTER_ASSERT(reporter, !promiseChecker.fDoneCount); - return; + int expectedReleaseCnt, + bool expectedRequired, + int expectedDoneCnt, + skiatest::Reporter* reporter) { + bool result = true; + int countDiff = promiseChecker.fFulfillCount - promiseChecker.fReleaseCount; + // FulfillCount should always equal ReleaseCount or be at most one higher + if (countDiff != 0) { + if (balanceExpecation == ReleaseBalanceExpecation::kBalanced) { + result = false; + REPORTER_ASSERT(reporter, 0 == countDiff); + } else if (countDiff != 1 && + balanceExpecation == ReleaseBalanceExpecation::kBalancedOrPlusOne) { + result = false; + REPORTER_ASSERT(reporter, 0 == countDiff || 1 == countDiff); + } else if (countDiff < 0) { + result = false; + REPORTER_ASSERT(reporter, countDiff >= 0); + } } - int releaseDiff = promiseChecker.fFulfillCount - promiseChecker.fReleaseCount; - switch (releaseBalanceExpecation) { - case ReleaseBalanceExpectation::kBalanced: - REPORTER_ASSERT(reporter, !releaseDiff); - break; - case ReleaseBalanceExpectation::kAllUnbalanced: - REPORTER_ASSERT(reporter, releaseDiff == promiseChecker.fFulfillCount); - break; - case ReleaseBalanceExpectation::kUnbalancedByOne: - REPORTER_ASSERT(reporter, releaseDiff == 1); - break; + + int fulfillDiff = expectedFulfillCnt - promiseChecker.fFulfillCount; + REPORTER_ASSERT(reporter, fulfillDiff >= 0); + if (fulfillDiff != 0) { + if (expectedRequired) { + result = false; + REPORTER_ASSERT(reporter, expectedFulfillCnt == promiseChecker.fFulfillCount); + } else if (fulfillDiff > 1) { + result = false; + REPORTER_ASSERT(reporter, fulfillDiff <= 1); + } } - int doneDiff = promiseChecker.fFulfillCount - promiseChecker.fDoneCount; - switch (doneBalanceExpecation) { - case DoneBalanceExpectation::kBalanced: - REPORTER_ASSERT(reporter, !doneDiff); - break; - case DoneBalanceExpectation::kAllUnbalanced: - REPORTER_ASSERT(reporter, doneDiff == promiseChecker.fFulfillCount); - break; - case DoneBalanceExpectation::kUnknown: - REPORTER_ASSERT(reporter, doneDiff >= 0 && doneDiff <= promiseChecker.fFulfillCount); - break; - case DoneBalanceExpectation::kUnbalancedByOne: - REPORTER_ASSERT(reporter, doneDiff == 1); - break; - case DoneBalanceExpectation::kBalancedOrOffByOne: - REPORTER_ASSERT(reporter, doneDiff == 0 || doneDiff == 1); - break; + + int releaseDiff = expectedReleaseCnt - promiseChecker.fReleaseCount; + REPORTER_ASSERT(reporter, releaseDiff >= 0); + if (releaseDiff != 0) { + if (expectedRequired) { + result = false; + REPORTER_ASSERT(reporter, expectedReleaseCnt == promiseChecker.fReleaseCount); + } else if (releaseDiff > 1) { + result = false; + REPORTER_ASSERT(reporter, releaseDiff <= 1); + } } -} -static void check_unfulfilled(const PromiseTextureChecker& promiseChecker, - skiatest::Reporter* reporter) { - check_fulfill_and_release_cnts(reporter, promiseChecker, 0, - ReleaseBalanceExpectation::kBalanced, - DoneBalanceExpectation::kBalanced); -} - -static void check_only_fulfilled(skiatest::Reporter* reporter, - const PromiseTextureChecker& promiseChecker, - int expectedFulfillCnt = 1) { - check_fulfill_and_release_cnts(reporter, promiseChecker, expectedFulfillCnt, - ReleaseBalanceExpectation::kAllUnbalanced, - DoneBalanceExpectation::kAllUnbalanced); -} - -static void check_all_flushed_but_not_synced(skiatest::Reporter* reporter, - const PromiseTextureChecker& promiseChecker, - GrBackendApi api, - int expectedFulfillCnt = 1) { - DoneBalanceExpectation doneBalanceExpectation = DoneBalanceExpectation::kBalanced; - // On Vulkan Done isn't guaranteed to be called until a sync has occurred. - if (api == GrBackendApi::kVulkan) { - doneBalanceExpectation = expectedFulfillCnt == 1 - ? DoneBalanceExpectation::kBalancedOrOffByOne - : DoneBalanceExpectation::kUnknown; + if (expectedDoneCnt != promiseChecker.fDoneCount) { + result = false; + REPORTER_ASSERT(reporter, expectedDoneCnt == promiseChecker.fDoneCount); } - check_fulfill_and_release_cnts(reporter, promiseChecker, expectedFulfillCnt, - ReleaseBalanceExpectation::kBalanced, doneBalanceExpectation); -} -static void check_all_done(skiatest::Reporter* reporter, - const PromiseTextureChecker& promiseChecker, - int expectedFulfillCnt = 1) { - check_fulfill_and_release_cnts(reporter, promiseChecker, expectedFulfillCnt, - ReleaseBalanceExpectation::kBalanced, - DoneBalanceExpectation::kBalanced); + return result; } DEF_GPUTEST_FOR_RENDERING_CONTEXTS(PromiseImageTest, reporter, ctxInfo) { @@ -184,22 +150,47 @@ PromiseTextureChecker::Fulfill, PromiseTextureChecker::Release, PromiseTextureChecker::Done, - &promiseChecker, - SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew)); + &promiseChecker)); SkImageInfo info = SkImageInfo::MakeN32Premul(kWidth, kHeight); sk_sp<SkSurface> surface = SkSurface::MakeRenderTarget(ctx, SkBudgeted::kNo, info); SkCanvas* canvas = surface->getCanvas(); - canvas->drawImage(refImg, 0, 0); - check_unfulfilled(promiseChecker, reporter); + int expectedFulfillCnt = 0; + int expectedReleaseCnt = 0; + int expectedDoneCnt = 0; + ReleaseBalanceExpecation balanceExpecation = ReleaseBalanceExpecation::kBalanced; + canvas->drawImage(refImg, 0, 0); + REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker, + balanceExpecation, + expectedFulfillCnt, + expectedReleaseCnt, + true, + expectedDoneCnt, + reporter)); + + bool isVulkan = GrBackendApi::kVulkan == ctx->backend(); surface->flush(); - // We still own the image so we should not have called Release or Done. - check_only_fulfilled(reporter, promiseChecker); + expectedFulfillCnt++; + // Because we've delayed release, we expect a +1 balance. + balanceExpecation = ReleaseBalanceExpecation::kBalancedOrPlusOne; + REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker, + balanceExpecation, + expectedFulfillCnt, + expectedReleaseCnt, + !isVulkan, + expectedDoneCnt, + reporter)); gpu->testingOnly_flushGpuAndSync(); - check_only_fulfilled(reporter, promiseChecker); + REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker, + balanceExpecation, + expectedFulfillCnt, + expectedReleaseCnt, + true, + expectedDoneCnt, + reporter)); canvas->drawImage(refImg, 0, 0); canvas->drawImage(refImg, 0, 0); @@ -207,25 +198,49 @@ surface->flush(); gpu->testingOnly_flushGpuAndSync(); - // Image should still be fulfilled from the first time we drew/flushed it. - check_only_fulfilled(reporter, promiseChecker); + REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker, + balanceExpecation, + expectedFulfillCnt, + expectedReleaseCnt, + true, + expectedDoneCnt, + reporter)); canvas->drawImage(refImg, 0, 0); surface->flush(); - check_only_fulfilled(reporter, promiseChecker); + REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker, + balanceExpecation, + expectedFulfillCnt, + expectedReleaseCnt, + !isVulkan, + expectedDoneCnt, + reporter)); canvas->drawImage(refImg, 0, 0); + refImg.reset(); - // We no longer own the image but the last draw is still unflushed. - check_only_fulfilled(reporter, promiseChecker); + + REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker, + balanceExpecation, + expectedFulfillCnt, + expectedReleaseCnt, + !isVulkan, + expectedDoneCnt, + reporter)); surface->flush(); - // Flushing should have called Release. Depending on the backend and timing it may have called - // done. - check_all_flushed_but_not_synced(reporter, promiseChecker, ctx->backend()); gpu->testingOnly_flushGpuAndSync(); - // Now Done should definitely have been called. - check_all_done(reporter, promiseChecker); + // We released the image already and we flushed and synced. + balanceExpecation = ReleaseBalanceExpecation::kBalanced; + expectedReleaseCnt++; + expectedDoneCnt++; + REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker, + balanceExpecation, + expectedFulfillCnt, + expectedReleaseCnt, + !isVulkan, + expectedDoneCnt, + reporter)); gpu->deleteTestingOnlyBackendTexture(backendTex); } @@ -265,23 +280,32 @@ ctx, backendTex1.getBackendFormat(), kWidth, kHeight, GrMipMapped::kNo, kTopLeft_GrSurfaceOrigin, kAlpha_8_SkColorType, kPremul_SkAlphaType, nullptr, PromiseTextureChecker::Fulfill, PromiseTextureChecker::Release, - PromiseTextureChecker::Done, &promiseChecker, - SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew)); + 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, - SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew)); + PromiseTextureChecker::Done, &promiseChecker)); REPORTER_ASSERT(reporter, grayImg); canvas->drawImage(alphaImg, 0, 0); canvas->drawImage(grayImg, 1, 1); surface->flush(); gpu->testingOnly_flushGpuAndSync(); - check_only_fulfilled(reporter, promiseChecker, 2); + + int expectedFulfillCnt = 2; + int expectedReleaseCnt = 0; + int expectedDoneCnt = 0; + ReleaseBalanceExpecation balanceExpecation = ReleaseBalanceExpecation::kAny; + REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker, + balanceExpecation, + 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. @@ -299,7 +323,7 @@ } } - // Invalidate the backing texture, this should invalidate the keys. + // Change the backing texture, this should invalidate the keys. promiseChecker.releaseTexture(); ctx->priv().getResourceCache()->purgeAsNeeded(); @@ -307,12 +331,6 @@ auto surf = ctx->priv().resourceProvider()->findByUniqueKey<GrSurface>(key); REPORTER_ASSERT(reporter, !surf); } - alphaImg.reset(); - check_fulfill_and_release_cnts(reporter, promiseChecker, 2, - ReleaseBalanceExpectation::kUnbalancedByOne, - DoneBalanceExpectation::kUnbalancedByOne); - grayImg.reset(); - check_all_done(reporter, promiseChecker, 2); gpu->deleteTestingOnlyBackendTexture(backendTex1); } @@ -364,8 +382,7 @@ ctx, backendTex.getBackendFormat(), kWidth, kHeight, GrMipMapped::kNo, kTopLeft_GrSurfaceOrigin, kAlpha_8_SkColorType, kPremul_SkAlphaType, nullptr, PromiseTextureChecker::Fulfill, PromiseTextureChecker::Release, - PromiseTextureChecker::Done, &promiseChecker, - SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew)); + PromiseTextureChecker::Done, &promiseChecker)); REPORTER_ASSERT(reporter, image); canvas->drawImage(image, 0, 0); @@ -377,7 +394,17 @@ ctx->flush(); contextDeath(&factory, ctx); - check_all_done(reporter, promiseChecker); + int expectedFulfillCnt = 1; + int expectedReleaseCnt = 1; + int expectedDoneCnt = 1; + ReleaseBalanceExpecation balanceExpecation = ReleaseBalanceExpecation::kBalanced; + REPORTER_ASSERT(reporter, check_fulfill_and_release_cnts(promiseChecker, + balanceExpecation, + expectedFulfillCnt, + expectedReleaseCnt, + true, + expectedDoneCnt, + reporter)); } } } @@ -403,8 +430,7 @@ ctx, backendTex.getBackendFormat(), kWidth, kHeight, GrMipMapped::kNo, kTopLeft_GrSurfaceOrigin, kAlpha_8_SkColorType, kPremul_SkAlphaType, nullptr, PromiseTextureChecker::Fulfill, PromiseTextureChecker::Release, - PromiseTextureChecker::Done, &promiseChecker, - SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew)); + PromiseTextureChecker::Done, &promiseChecker)); REPORTER_ASSERT(reporter, image); // Make the cache full. This tests that we don't preemptively purge cached textures for
diff --git a/tools/DDLPromiseImageHelper.cpp b/tools/DDLPromiseImageHelper.cpp index 5e56952..bb922c4 100644 --- a/tools/DDLPromiseImageHelper.cpp +++ b/tools/DDLPromiseImageHelper.cpp
@@ -202,20 +202,18 @@ sizes[i] = SkISize::MakeEmpty(); } - image = recorder->makeYUVAPromiseTexture( - curImage.yuvColorSpace(), - backendFormats, - sizes, - curImage.yuvaIndices(), - curImage.overallWidth(), - curImage.overallHeight(), - GrSurfaceOrigin::kTopLeft_GrSurfaceOrigin, - curImage.refOverallColorSpace(), - DDLPromiseImageHelper::PromiseImageFulfillProc, - DDLPromiseImageHelper::PromiseImageReleaseProc, - DDLPromiseImageHelper::PromiseImageDoneProc, - contexts, - SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew); + image = recorder->makeYUVAPromiseTexture(curImage.yuvColorSpace(), + backendFormats, + sizes, + curImage.yuvaIndices(), + curImage.overallWidth(), + curImage.overallHeight(), + GrSurfaceOrigin::kTopLeft_GrSurfaceOrigin, + curImage.refOverallColorSpace(), + DDLPromiseImageHelper::PromiseImageFulfillProc, + DDLPromiseImageHelper::PromiseImageReleaseProc, + DDLPromiseImageHelper::PromiseImageDoneProc, + contexts); for (int i = 0; i < textureCount; ++i) { curImage.callbackContext(i)->wasAddedToImage(); } @@ -237,20 +235,18 @@ // Each DDL recorder gets its own ref on the promise callback context for the // promise images it creates. // DDL TODO: sort out mipmapping - image = recorder->makePromiseTexture( - backendFormat, - curImage.overallWidth(), - curImage.overallHeight(), - GrMipMapped::kNo, - GrSurfaceOrigin::kTopLeft_GrSurfaceOrigin, - curImage.overallColorType(), - curImage.overallAlphaType(), - curImage.refOverallColorSpace(), - DDLPromiseImageHelper::PromiseImageFulfillProc, - DDLPromiseImageHelper::PromiseImageReleaseProc, - DDLPromiseImageHelper::PromiseImageDoneProc, - (void*)curImage.refCallbackContext(0).release(), - SkDeferredDisplayListRecorder::PromiseImageApiVersion::kNew); + image = recorder->makePromiseTexture(backendFormat, + curImage.overallWidth(), + curImage.overallHeight(), + GrMipMapped::kNo, + GrSurfaceOrigin::kTopLeft_GrSurfaceOrigin, + curImage.overallColorType(), + curImage.overallAlphaType(), + curImage.refOverallColorSpace(), + DDLPromiseImageHelper::PromiseImageFulfillProc, + DDLPromiseImageHelper::PromiseImageReleaseProc, + DDLPromiseImageHelper::PromiseImageDoneProc, + (void*)curImage.refCallbackContext(0).release()); curImage.callbackContext(0)->wasAddedToImage(); } perRecorderContext->fPromiseImages->push_back(image);