From ad8efda6e551364c61a19863564eea20a33bee1b Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Fri, 10 May 2019 09:22:49 -0400 Subject: [PATCH] Reland "Reland "Release YUVA planes in SkImage_GpuYUVA after flattenning to RGBA."" This reverts commit 8fbca482e5d9bd46ef6d97c0c3283cb108988ce4. Reason for revert: Original change's description: > Revert "Reland "Release YUVA planes in SkImage_GpuYUVA after flattenning to RGBA."" > > This reverts commit 622b6f5367e2d26dab276df8bd9df0451236808b. > > Reason for revert: ANGLE may not be fixed > > Original change's description: > > Reland "Release YUVA planes in SkImage_GpuYUVA after flattenning to RGBA." > > > > This reverts commit bc0c460349142140295593a3a32d507f7b45c622. > > > > Reason for revert: fixed isTextureBacked(), checked cc_unittests passes > > > > Original change's description: > > > Revert "Release YUVA planes in SkImage_GpuYUVA after flattenning to RGBA." > > > > > > This reverts commit a15f355be83136e7827ccfbc9bb9e9ea77f48518. > > > > > > Reason for revert: ANGLE failing new test > > > > > > Original change's description: > > > > Release YUVA planes in SkImage_GpuYUVA after flattenning to RGBA. > > > > > > > > Also removed unused virtual function from SkImage_GpuBase and override > > > > on SkImage_GpuYUVA. > > > > > > > > Change-Id: Ib47b46b529b16976181cb9453976133d66e0f952 > > > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212734 > > > > Commit-Queue: Brian Salomon > > > > Reviewed-by: Robert Phillips > > > > > > TBR=jvanverth@google.com,bsalomon@google.com,robertphillips@google.com > > > > > > Change-Id: Ib2566ff8aa99d3698d6044167e70a1d75542f2e9 > > > No-Presubmit: true > > > No-Tree-Checks: true > > > No-Try: true > > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212986 > > > Reviewed-by: Robert Phillips > > > Commit-Queue: Robert Phillips > > > > TBR=jvanverth@google.com,bsalomon@google.com,robertphillips@google.com > > > > Change-Id: I3c9454da33a764504e768d02b59a5cd99766aff1 > > No-Presubmit: true > > No-Tree-Checks: true > > No-Try: true > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212988 > > Commit-Queue: Brian Salomon > > Reviewed-by: Brian Salomon > > TBR=jvanverth@google.com,bsalomon@google.com,robertphillips@google.com > > Change-Id: I06632adc69844522c87cd72e8a6b0c2795974fe4 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/212989 > Reviewed-by: Brian Salomon > Commit-Queue: Brian Salomon TBR=jvanverth@google.com,bsalomon@google.com,robertphillips@google.com Change-Id: I15cad8a907dbf6faa892cf7ca15c0fa5da58f87f Reviewed-on: https://skia-review.googlesource.com/c/skia/+/213123 Reviewed-by: Brian Salomon Commit-Queue: Brian Salomon --- src/gpu/GrImageTextureMaker.cpp | 5 +-- src/image/SkImage_Base.h | 2 -- src/image/SkImage_GpuYUVA.cpp | 24 +++++++++---- src/image/SkImage_GpuYUVA.h | 20 +++++------ tests/ImageTest.cpp | 61 ++++++++++++++++++++++++++++----- 5 files changed, 82 insertions(+), 30 deletions(-) diff --git a/src/gpu/GrImageTextureMaker.cpp b/src/gpu/GrImageTextureMaker.cpp index 80b39d0e5e..0fb1cbee9c 100644 --- a/src/gpu/GrImageTextureMaker.cpp +++ b/src/gpu/GrImageTextureMaker.cpp @@ -91,8 +91,9 @@ std::unique_ptr GrYUVAImageTextureMaker::createFragmentProc bool coordsLimitedToConstraintRect, const GrSamplerState::Filter* filterOrNullForBicubic) { - // Check simple cases to see if we need to fall back to flattening the image - if (!filterOrNullForBicubic || this->domainNeedsDecal()) { + // Check simple cases to see if we need to fall back to flattening the image (or whether it's + // already been flattened.) + if (!filterOrNullForBicubic || this->domainNeedsDecal() || fImage->fRGBProxy) { return this->INHERITED::createFragmentProcessor(textureMatrix, constraintRect, filterConstraint, coordsLimitedToConstraintRect, diff --git a/src/image/SkImage_Base.h b/src/image/SkImage_Base.h index b06d7fc7f5..a3b86c5d94 100644 --- a/src/image/SkImage_Base.h +++ b/src/image/SkImage_Base.h @@ -64,8 +64,6 @@ public: return nullptr; } virtual bool isYUVA() const { return false; } - virtual bool asYUVATextureProxiesRef(sk_sp[4], SkYUVAIndex[4], - SkYUVColorSpace*) const { return false; } virtual GrTexture* onGetTexture() const { return nullptr; } #endif virtual GrBackendTexture onGetBackendTexture(bool flushPendingGrContextIO, diff --git a/src/image/SkImage_GpuYUVA.cpp b/src/image/SkImage_GpuYUVA.cpp index 915d0dea81..146e9f58d4 100644 --- a/src/image/SkImage_GpuYUVA.cpp +++ b/src/image/SkImage_GpuYUVA.cpp @@ -71,8 +71,12 @@ SkImage_GpuYUVA::SkImage_GpuYUVA(const SkImage_GpuYUVA* image, sk_spfYUVAIndices, &textureCount)); SkASSERT(textureCount == fNumProxies); - for (int i = 0; i < fNumProxies; ++i) { - fProxies[i] = image->fProxies[i]; // we ref in this case, not move + if (image->fRGBProxy) { + fRGBProxy = image->fRGBProxy; // we ref in this case, not move + } else { + for (int i = 0; i < fNumProxies; ++i) { + fProxies[i] = image->fProxies[i]; // we ref in this case, not move + } } memcpy(fYUVAIndices, image->fYUVAIndices, 4 * sizeof(SkYUVAIndex)); } @@ -80,6 +84,8 @@ SkImage_GpuYUVA::SkImage_GpuYUVA(const SkImage_GpuYUVA* image, sk_sppriv().matches(context)) { return false; } @@ -108,12 +114,15 @@ GrSemaphoresSubmitted SkImage_GpuYUVA::onFlush(GrContext* context, const GrFlush return GrSemaphoresSubmitted::kNo; } - GrSurfaceProxy* proxies[5] = {fProxies[0].get(), fProxies[1].get(), - fProxies[2].get(), fProxies[3].get(), nullptr}; + GrSurfaceProxy* proxies[4] = {fProxies[0].get(), fProxies[1].get(), + fProxies[2].get(), fProxies[3].get()}; int numProxies = fNumProxies; if (fRGBProxy) { - proxies[fNumProxies] = fRGBProxy.get(); - ++numProxies; + // Either we've already flushed the flattening draw or the flattening is unflushed. In the + // latter case it should still be ok to just pass fRGBProxy because it in turn depends on + // the planar proxies and will cause all of their work to flush as well. + proxies[0] = fRGBProxy.get(); + numProxies = 1; } return context->priv().flushSurfaces(proxies, numProxies, info); } @@ -155,6 +164,9 @@ sk_sp SkImage_GpuYUVA::asTextureProxyRef(GrRecordingContext* con } fRGBProxy = renderTargetContext->asTextureProxyRef(); + for (auto& p : fProxies) { + p.reset(); + } return fRGBProxy; } diff --git a/src/image/SkImage_GpuYUVA.h b/src/image/SkImage_GpuYUVA.h index daa9ee1be8..693a2501fc 100644 --- a/src/image/SkImage_GpuYUVA.h +++ b/src/image/SkImage_GpuYUVA.h @@ -36,29 +36,25 @@ public: GrTextureProxy* peekProxy() const override; sk_sp asTextureProxyRef(GrRecordingContext*) const override; - virtual bool onIsTextureBacked() const override { return SkToBool(fProxies[0].get()); } + virtual bool onIsTextureBacked() const override { return fProxies[0] || fRGBProxy; } sk_sp onMakeColorTypeAndColorSpace(GrRecordingContext*, SkColorType, sk_sp) const final; virtual bool isYUVA() const override { return true; } - virtual bool asYUVATextureProxiesRef(sk_sp proxies[4], - SkYUVAIndex yuvaIndices[4], - SkYUVColorSpace* yuvColorSpace) const override { - for (int i = 0; i < 4; ++i) { - proxies[i] = fProxies[i]; - yuvaIndices[i] = fYUVAIndices[i]; - } - *yuvColorSpace = fYUVColorSpace; - return true; - } bool setupMipmapsForPlanes(GrRecordingContext*) const; // Returns a ref-ed texture proxy with miplevels sk_sp asMippedTextureProxyRef(GrRecordingContext*) const; - bool testingOnly_IsFlattened() const { return SkToBool(fRGBProxy); } +#if GR_TEST_UTILS + bool testingOnly_IsFlattened() const { + // We should only have the flattened proxy or the planar proxies at one point in time. + SkASSERT(SkToBool(fRGBProxy) != SkToBool(fProxies[0])); + return SkToBool(fRGBProxy); + } +#endif /** * This is the implementation of SkDeferredDisplayListRecorder::makeYUVAPromiseTexture. diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp index bd4bad0f9f..e49766ad2a 100644 --- a/tests/ImageTest.cpp +++ b/tests/ImageTest.cpp @@ -1376,6 +1376,19 @@ DEF_TEST(Image_nonfinite_dst, reporter) { } } +static sk_sp make_yuva_image(GrContext* c) { + SkAutoPixmapStorage pm; + pm.alloc(SkImageInfo::Make(1, 1, kAlpha_8_SkColorType, kPremul_SkAlphaType)); + const SkPixmap pmaps[] = {pm, pm, pm, pm}; + SkYUVAIndex indices[] = {{0, SkColorChannel::kA}, + {1, SkColorChannel::kA}, + {2, SkColorChannel::kA}, + {3, SkColorChannel::kA}}; + + return SkImage::MakeFromYUVAPixmaps(c, kJPEG_SkYUVColorSpace, pmaps, indices, + SkISize::Make(1, 1), kTopLeft_GrSurfaceOrigin, false); +} + DEF_GPUTEST_FOR_ALL_CONTEXTS(ImageFlush, reporter, ctxInfo) { auto c = ctxInfo.grContext(); auto ii = SkImageInfo::Make(10, 10, kRGBA_8888_SkColorType, kPremul_SkAlphaType); @@ -1386,15 +1399,8 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(ImageFlush, reporter, ctxInfo) { s->getCanvas()->clear(SK_ColorBLUE); auto i1 = s->makeImageSnapshot(); s->getCanvas()->clear(SK_ColorGREEN); - // Make a YUVA image. - SkAutoPixmapStorage pm; - pm.alloc(SkImageInfo::Make(1, 1, kAlpha_8_SkColorType, kPremul_SkAlphaType)); - const SkPixmap pmaps[] = {pm, pm, pm, pm}; - SkYUVAIndex indices[] = {{0, SkColorChannel::kA}, {1, SkColorChannel::kA}, - {2, SkColorChannel::kA}, {3, SkColorChannel::kA}}; - auto i2 = SkImage::MakeFromYUVAPixmaps(c, kJPEG_SkYUVColorSpace, pmaps, indices, - SkISize::Make(1, 1), kTopLeft_GrSurfaceOrigin, false); + auto i2 = make_yuva_image(c); // Flush all the setup work we did above and then make little lambda that reports the flush // count delta since the last time it was called. @@ -1448,4 +1454,43 @@ DEF_GPUTEST_FOR_ALL_CONTEXTS(ImageFlush, reporter, ctxInfo) { // Since we just did a simple image draw it should not have been flattened. REPORTER_ASSERT(reporter, !static_cast(as_IB(i2.get()))->testingOnly_IsFlattened()); + REPORTER_ASSERT(reporter, static_cast(as_IB(i2.get()))->isTextureBacked()); + + // Flatten it and repeat. + as_IB(i2.get())->asTextureProxyRef(c); + REPORTER_ASSERT(reporter, + static_cast(as_IB(i2.get()))->testingOnly_IsFlattened()); + REPORTER_ASSERT(reporter, static_cast(as_IB(i2.get()))->isTextureBacked()); + s->getCanvas()->drawImage(i2, 0, 0); + // Flushing image 0 should do nothing. + i0->flush(c); + REPORTER_ASSERT(reporter, numFlushes() == 0); + // Flushing image 1 do nothing. + i1->flush(c); + REPORTER_ASSERT(reporter, numFlushes() == 0); + // Flushing image 2 should flush. + i2->flush(c); + REPORTER_ASSERT(reporter, numFlushes() == 1); + + // Test case where flatten happens before the first flush. + i2 = make_yuva_image(c); + // On some systems where preferVRAMUseOverFlushes is false (ANGLE on Windows) the above may + // actually flush in order to make textures for the YUV planes. TODO: Remove this when we + // make the YUVA planes from backend textures rather than pixmaps that GrContext must upload. + // Calling numFlushes rebases the flush count from here. + numFlushes(); + as_IB(i2.get())->asTextureProxyRef(c); + REPORTER_ASSERT(reporter, + static_cast(as_IB(i2.get()))->testingOnly_IsFlattened()); + REPORTER_ASSERT(reporter, static_cast(as_IB(i2.get()))->isTextureBacked()); + s->getCanvas()->drawImage(i2, 0, 0); + // Flushing image 0 should do nothing. + i0->flush(c); + REPORTER_ASSERT(reporter, numFlushes() == 0); + // Flushing image 1 do nothing. + i1->flush(c); + REPORTER_ASSERT(reporter, numFlushes() == 0); + // Flushing image 2 should flush. + i2->flush(c); + REPORTER_ASSERT(reporter, numFlushes() == 1); }