From f00b16233d5641df848602b4b1356a4788cf201b Mon Sep 17 00:00:00 2001 From: Jim Van Verth Date: Wed, 10 Oct 2018 13:03:23 -0400 Subject: [PATCH] Fix up SkImage_GpuYUVA MakePromiseYUVATexture Bug: skia:7901 Change-Id: I8722373148f99cb16e2aa9c825832bb83fa4e979 Reviewed-on: https://skia-review.googlesource.com/c/161043 Reviewed-by: Brian Salomon Reviewed-by: Robert Phillips Commit-Queue: Jim Van Verth --- include/core/SkYUVAIndex.h | 31 ++++++++ src/gpu/effects/GrYUVtoRGBEffect.cpp | 31 +------- src/image/SkImage_GpuYUVA.cpp | 115 +++++++++++++++------------ 3 files changed, 95 insertions(+), 82 deletions(-) diff --git a/include/core/SkYUVAIndex.h b/include/core/SkYUVAIndex.h index 8f2c39deaa..7cb8cf14ce 100644 --- a/include/core/SkYUVAIndex.h +++ b/include/core/SkYUVAIndex.h @@ -52,6 +52,37 @@ struct SK_API SkYUVAIndex { /** The channel describes from which channel to read the info from. Currently we only deal with * YUV and NV12 and channel info is ignored. */ SkColorChannel fChannel; + + static bool AreValidIndices(const SkYUVAIndex yuvaIndices[4], int* numPlanes) { + // Note that 'numPlanes' is always filled in even if the indices are not valid. + // This means it can always be used to process the backing resources (but be careful + // of empty intervening slots). + int maxSlotUsed = -1; + bool used[4] = { false, false, false, false }; + bool valid = true; + for (int i = 0; i < 4; ++i) { + if (yuvaIndices[i].fIndex < 0) { + if (SkYUVAIndex::kA_Index != i) { + valid = false; // only the 'A' plane can be omitted + } + } else if (yuvaIndices[i].fIndex > 3) { + valid = false; // A maximum of four input textures is allowed + } else { + maxSlotUsed = SkTMax(yuvaIndices[i].fIndex, maxSlotUsed); + used[i] = true; + } + } + + // All the used slots should be packed starting at 0 with no gaps + for (int i = 0; i <= maxSlotUsed; ++i) { + if (!used[i]) { + valid = false; + } + } + + *numPlanes = maxSlotUsed + 1; + return valid; + } }; #endif diff --git a/src/gpu/effects/GrYUVtoRGBEffect.cpp b/src/gpu/effects/GrYUVtoRGBEffect.cpp index 1862cfb3c4..c4d58a70a4 100644 --- a/src/gpu/effects/GrYUVtoRGBEffect.cpp +++ b/src/gpu/effects/GrYUVtoRGBEffect.cpp @@ -28,40 +28,11 @@ static const float kRec709ConversionMatrix[16] = { 0.0f, 0.0f, 0.0f, 1.0f }; -static bool is_valid_yuv(const SkYUVAIndex yuvaIndices[4], int* numPlanes) { - - int maxSlotUsed = -1; - bool used[4] = { false, false, false, false }; - for (int i = 0; i < 4; ++i) { - - if (yuvaIndices[i].fIndex < 0) { - if (SkYUVAIndex::kA_Index != i) { - return false; // only the 'A' plane can be omitted - } - } else if (yuvaIndices[i].fIndex > 3) { - return false; // A maximum of four input textures is allowed - } else { - maxSlotUsed = SkTMax(yuvaIndices[i].fIndex, maxSlotUsed); - used[i] = true; - } - } - - // All the used slots should be packed starting at 0 with no gaps - for (int i = 0; i <= maxSlotUsed; ++i) { - if (!used[i]) { - return false; - } - } - - *numPlanes = maxSlotUsed+1; - return true; -} - std::unique_ptr GrYUVtoRGBEffect::Make(const sk_sp proxies[], const SkYUVAIndex yuvaIndices[4], SkYUVColorSpace yuvColorSpace) { int numPlanes; - SkAssertResult(is_valid_yuv(yuvaIndices, &numPlanes)); + SkAssertResult(SkYUVAIndex::AreValidIndices(yuvaIndices, &numPlanes)); const SkISize YSize = proxies[yuvaIndices[SkYUVAIndex::kY_Index].fIndex]->isize(); diff --git a/src/image/SkImage_GpuYUVA.cpp b/src/image/SkImage_GpuYUVA.cpp index 70c449e7ac..278ebd92ab 100644 --- a/src/image/SkImage_GpuYUVA.cpp +++ b/src/image/SkImage_GpuYUVA.cpp @@ -168,13 +168,11 @@ sk_sp SkImage_GpuYUVA::MakeFromYUVATextures(GrContext* ctx, SkASSERT(yuvaTexturesCopy[textureIndex].isValid()); tempTextureProxies[textureIndex] = proxyProvider->wrapBackendTexture(yuvaTexturesCopy[textureIndex], origin); + if (!tempTextureProxies[textureIndex]) { + return nullptr; + } } } - if (!tempTextureProxies[yuvaIndices[SkYUVAIndex::kY_Index].fIndex] || - !tempTextureProxies[yuvaIndices[SkYUVAIndex::kU_Index].fIndex] || - !tempTextureProxies[yuvaIndices[SkYUVAIndex::kV_Index].fIndex]) { - return nullptr; - } return sk_make_sp(sk_ref_sp(ctx), kNeedNewImageUniqueID, colorSpace, tempTextureProxies, yuvaIndices, size, origin, @@ -193,11 +191,25 @@ sk_sp SkImage_GpuYUVA::MakePromiseYUVATexture(GrContext* context, TextureReleaseProc textureReleaseProc, PromiseDoneProc promiseDoneProc, TextureContext textureContexts[]) { + // The contract here is that if 'promiseDoneProc' is passed in it should always be called, + // even if creation of the SkImage fails. if (!promiseDoneProc) { return nullptr; } - // TODO: fill in the promise image helpers here so promiseDoneProc will always be called + int numTextures; + bool valid = SkYUVAIndex::AreValidIndices(yuvaIndices, &numTextures); + + // Set up promise helpers + SkPromiseImageHelper promiseHelpers[4]; + for (int texIdx = 0; texIdx < numTextures; ++texIdx) { + promiseHelpers[texIdx].set(textureFulfillProc, textureReleaseProc, promiseDoneProc, + textureContexts[texIdx]); + } + + if (!valid) { + return nullptr; + } if (!context) { return nullptr; @@ -219,68 +231,67 @@ sk_sp SkImage_GpuYUVA::MakePromiseYUVATexture(GrContext* context, return nullptr; } - GrProxyProvider* proxyProvider = context->contextPriv().proxyProvider(); - - SkColorType colorTypes[4] = { kUnknown_SkColorType, kUnknown_SkColorType, - kUnknown_SkColorType, kUnknown_SkColorType }; - for (int i = 0; i < 4; ++i) { - if (yuvaIndices[i].fIndex < 0) { - SkASSERT(SkYUVAIndex::kA_Index == i); // We had better have YUV channels + // Set up color types + SkColorType texColorTypes[4] = { kUnknown_SkColorType, kUnknown_SkColorType, + kUnknown_SkColorType, kUnknown_SkColorType }; + for (int yuvIndex = 0; yuvIndex < 4; ++yuvIndex) { + int texIdx = yuvaIndices[yuvIndex].fIndex; + if (texIdx < 0) { + SkASSERT(SkYUVAIndex::kA_Index); continue; } - - SkASSERT(yuvaIndices[i].fIndex < 4); - if (kUnknown_SkColorType == colorTypes[i]) { - colorTypes[i] = kAlpha_8_SkColorType; + if (kUnknown_SkColorType == texColorTypes[texIdx]) { + texColorTypes[texIdx] = kAlpha_8_SkColorType; } else { - colorTypes[i] = kRGBA_8888_SkColorType; + texColorTypes[texIdx] = kRGBA_8888_SkColorType; } } + // If UV is interleaved, then Y will have RGBA color type + if (kRGBA_8888_SkColorType == texColorTypes[yuvaIndices[SkYUVAIndex::kU_Index].fIndex]) { + texColorTypes[yuvaIndices[SkYUVAIndex::kY_Index].fIndex] = kRGBA_8888_SkColorType; + } // Get lazy proxies - struct { - GrPixelConfig fConfig; - SkPromiseImageHelper fPromiseHelper; - } params; - GrProxyProvider::LazyInstantiateCallback lazyInstCallback = - [params](GrResourceProvider* resourceProvider) mutable { - if (!resourceProvider || !params.fPromiseHelper.isValid()) { - if (params.fPromiseHelper.isValid()) { - params.fPromiseHelper.reset(); - } - return sk_sp(); - } - - return params.fPromiseHelper.getTexture(resourceProvider, params.fConfig); - }; - + GrProxyProvider* proxyProvider = context->contextPriv().proxyProvider(); GrSurfaceDesc desc; desc.fFlags = kNone_GrSurfaceFlags; desc.fWidth = size.width(); desc.fHeight = size.height(); - // For now, we'll replace this for each proxy - desc.fConfig = kUnknown_GrPixelConfig; + desc.fConfig = kUnknown_GrPixelConfig; // We'll replace this for each proxy. desc.fSampleCnt = 1; sk_sp proxies[4]; - for (int i = 0; i < 4; ++i) { + for (int texIdx = 0; texIdx < numTextures; ++texIdx) { // for each proxy we need to fill in - if (kUnknown_SkColorType != colorTypes[i]) { - GrPixelConfig pixelConfig; - if (!context->contextPriv().caps()->getConfigFromBackendFormat(yuvaFormats[i], - colorTypes[i], - &pixelConfig)) { - return nullptr; + struct { + GrPixelConfig fConfig; + SkPromiseImageHelper fPromiseHelper; + } params; + if (!context->contextPriv().caps()->getConfigFromBackendFormat(yuvaFormats[texIdx], + texColorTypes[texIdx], + ¶ms.fConfig)) { + return nullptr; + } + params.fPromiseHelper = promiseHelpers[texIdx]; + + GrProxyProvider::LazyInstantiateCallback lazyInstCallback = + [params](GrResourceProvider* resourceProvider) mutable { + if (!resourceProvider || !params.fPromiseHelper.isValid()) { + if (params.fPromiseHelper.isValid()) { + params.fPromiseHelper.reset(); + } + return sk_sp(); } - desc.fConfig = pixelConfig; - proxies[i] = proxyProvider->createLazyProxy( - std::move(lazyInstCallback), desc, imageOrigin, GrMipMapped::kNo, - GrTextureType::k2D, GrInternalSurfaceFlags::kNone, - SkBackingFit::kExact, SkBudgeted::kNo, - GrSurfaceProxy::LazyInstantiationType::kUninstantiate); - if (!proxies[i]) { - return nullptr; - } + return params.fPromiseHelper.getTexture(resourceProvider, params.fConfig); + }; + desc.fConfig = params.fConfig; + proxies[texIdx] = proxyProvider->createLazyProxy( + std::move(lazyInstCallback), desc, imageOrigin, GrMipMapped::kNo, + GrTextureType::k2D, GrInternalSurfaceFlags::kNone, + SkBackingFit::kExact, SkBudgeted::kNo, + GrSurfaceProxy::LazyInstantiationType::kUninstantiate); + if (!proxies[texIdx]) { + return nullptr; } }