From fc5060df5394117dbf803aa0deed864b5b4abcd8 Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Wed, 4 Oct 2017 18:36:15 +0000 Subject: [PATCH] Revert "Revert "Update lockTextureProxy to return mipped proxys if mipping is requested."" This reverts commit 87c76edd2c2247a6d0194a0e511a7bdc259aedf4. Reason for revert: attempt to reland Original change's description: > Revert "Update lockTextureProxy to return mipped proxys if mipping is requested." > > This reverts commit 97abf014b40cc17c4100768299ef8cccd335aff7. > > Reason for revert: REALLY Really really really big perf regressions > > Original change's description: > > Update lockTextureProxy to return mipped proxys if mipping is requested. > > > > We will either create a new mipped surface from scratch, or just create > > the base layer and copy that into the mipped surface. We then defer the > > creation of the rest of the mips to the GPU. > > > > Bug: skia: > > Change-Id: Ida3000ad5e666153c61b05e714f033138593b09b > > Reviewed-on: https://skia-review.googlesource.com/52743 > > Commit-Queue: Greg Daniel > > Reviewed-by: Robert Phillips > > TBR=egdaniel@google.com,robertphillips@google.com,brianosman@google.com > > Change-Id: If3b1ff555ef310b75493412a7533175195994684 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: skia: > Reviewed-on: https://skia-review.googlesource.com/54320 > Reviewed-by: Greg Daniel > Commit-Queue: Greg Daniel TBR=egdaniel@google.com,robertphillips@google.com,brianosman@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: skia: Change-Id: I176ff7279f29eff536ab13aff8fda882b6ef7a2f Reviewed-on: https://skia-review.googlesource.com/55081 Reviewed-by: Greg Daniel Commit-Queue: Greg Daniel --- src/gpu/GrBitmapTextureMaker.cpp | 83 ++++++++++++++++++++------------ src/image/SkImage_Lazy.cpp | 73 +++++++++++++++++++++------- 2 files changed, 106 insertions(+), 50 deletions(-) diff --git a/src/gpu/GrBitmapTextureMaker.cpp b/src/gpu/GrBitmapTextureMaker.cpp index ab8ee249a9..efb9e20291 100644 --- a/src/gpu/GrBitmapTextureMaker.cpp +++ b/src/gpu/GrBitmapTextureMaker.cpp @@ -37,51 +37,70 @@ sk_sp GrBitmapTextureMaker::refOriginalTextureProxy(bool willBeM return nullptr; } - sk_sp originalProxy; + sk_sp proxy; if (fOriginalKey.isValid()) { - originalProxy = this->context()->resourceProvider()->findOrCreateProxyByUniqueKey( + proxy = this->context()->resourceProvider()->findOrCreateProxyByUniqueKey( fOriginalKey, kTopLeft_GrSurfaceOrigin); - if (originalProxy && (!willBeMipped || originalProxy->isMipMapped())) { - return originalProxy; + if (proxy && (!willBeMipped || proxy->isMipMapped())) { + return proxy; } } - sk_sp proxy; - if (willBeMipped) { - if (!originalProxy) { + if (!proxy) { + if (willBeMipped) { proxy = GrGenerateMipMapsAndUploadToTextureProxy(this->context(), fBitmap, dstColorSpace); - } else { - proxy = GrCopyBaseMipMapToTextureProxy(this->context(), originalProxy.get(), - dstColorSpace); - if (!proxy) { - // We failed to make a mipped proxy with the base copied into it. This could have - // been from failure to make the proxy or failure to do the copy. Thus we will fall - // back to just using the non mipped proxy; See skbug.com/7094. - return originalProxy; + } + if (!proxy) { + proxy = GrUploadBitmapToTextureProxy(this->context()->resourceProvider(), fBitmap, + dstColorSpace); + } + if (proxy) { + if (fOriginalKey.isValid()) { + this->context()->resourceProvider()->assignUniqueKeyToProxy(fOriginalKey, + proxy.get()); + } + if (!willBeMipped || proxy->isMipMapped()) { + SkASSERT(proxy->origin() == kTopLeft_GrSurfaceOrigin); + if (fOriginalKey.isValid()) { + GrInstallBitmapUniqueKeyInvalidator(fOriginalKey, fBitmap.pixelRef()); + } + return proxy; } } } - if (!proxy) { - proxy = GrUploadBitmapToTextureProxy(this->context()->resourceProvider(), fBitmap, - dstColorSpace); - } - if (proxy && fOriginalKey.isValid()) { - SkASSERT(proxy->origin() == kTopLeft_GrSurfaceOrigin); - if (originalProxy) { - // In this case we are stealing the key from the original proxy which should only happen - // when we have just generated mipmaps for an originally unmipped proxy/texture. This - // means that all future uses of the key will access the mipmapped version. The texture - // backing the unmipped version will remain in the resource cache until the last texture - // proxy referencing it is deleted at which time it too will be deleted or recycled. - this->context()->resourceProvider()->removeUniqueKeyFromProxy(fOriginalKey, - originalProxy.get()); + + if (proxy) { + SkASSERT(willBeMipped); + SkASSERT(!proxy->isMipMapped()); + // We need a mipped proxy, but we either found a proxy earlier that wasn't mipped or + // generated a non mipped proxy. Thus we generate a new mipped surface and copy the original + // proxy into the base layer. We will then let the gpu generate the rest of the mips. + if (auto mippedProxy = GrCopyBaseMipMapToTextureProxy(this->context(), proxy.get(), + dstColorSpace)) { + SkASSERT(mippedProxy->origin() == kTopLeft_GrSurfaceOrigin); + if (fOriginalKey.isValid()) { + // In this case we are stealing the key from the original proxy which should only + // happen when we have just generated mipmaps for an originally unmipped + // proxy/texture. This means that all future uses of the key will access the + // mipmapped version. The texture backing the unmipped version will remain in the + // resource cache until the last texture proxy referencing it is deleted at which + // time it too will be deleted or recycled. + this->context()->resourceProvider()->removeUniqueKeyFromProxy(fOriginalKey, + proxy.get()); + this->context()->resourceProvider()->assignUniqueKeyToProxy(fOriginalKey, + mippedProxy.get()); + GrInstallBitmapUniqueKeyInvalidator(fOriginalKey, fBitmap.pixelRef()); + } + return mippedProxy; } - this->context()->resourceProvider()->assignUniqueKeyToProxy(fOriginalKey, proxy.get()); - GrInstallBitmapUniqueKeyInvalidator(fOriginalKey, fBitmap.pixelRef()); + // We failed to make a mipped proxy with the base copied into it. This could have + // been from failure to make the proxy or failure to do the copy. Thus we will fall + // back to just using the non mipped proxy; See skbug.com/7094. + return proxy; } - return proxy; + return nullptr; } void GrBitmapTextureMaker::makeCopyKey(const CopyParams& copyParams, GrUniqueKey* copyKey, diff --git a/src/image/SkImage_Lazy.cpp b/src/image/SkImage_Lazy.cpp index 003b28ba98..c67d4df356 100644 --- a/src/image/SkImage_Lazy.cpp +++ b/src/image/SkImage_Lazy.cpp @@ -696,9 +696,17 @@ public: }; static void set_key_on_proxy(GrResourceProvider* resourceProvider, - GrTextureProxy* proxy, const GrUniqueKey& key) { + GrTextureProxy* proxy, GrTextureProxy* originalProxy, + const GrUniqueKey& key) { if (key.isValid()) { SkASSERT(proxy->origin() == kTopLeft_GrSurfaceOrigin); + if (originalProxy) { + SkASSERT(proxy->isMipMapped() && !originalProxy->isMipMapped()); + // If we had an originalProxy, that means there already is a proxy in the cache which + // matches the key, but it does not have mip levels and we require them. Thus we must + // remove the unique key from that proxy. + resourceProvider->removeUniqueKeyFromProxy(key, originalProxy); + } resourceProvider->assignUniqueKeyToProxy(key, proxy); } } @@ -751,13 +759,18 @@ sk_sp SkImage_Lazy::lockTextureProxy(GrContext* ctx, GrUniqueKey key; this->makeCacheKeyFromOrigKey(origKey, format, &key); + sk_sp proxy; + // 1. Check the cache for a pre-existing one if (key.isValid()) { - if (sk_sp proxy = ctx->resourceProvider()->findOrCreateProxyByUniqueKey( - key, kTopLeft_GrSurfaceOrigin)) { + proxy = ctx->resourceProvider()->findOrCreateProxyByUniqueKey(key, + kTopLeft_GrSurfaceOrigin); + if (proxy) { SK_HISTOGRAM_ENUMERATION("LockTexturePath", kPreExisting_LockTexturePath, kLockTexturePathCount); - return proxy; + if (!willBeMipped || proxy->isMipMapped()) { + return proxy; + } } } @@ -769,23 +782,27 @@ sk_sp SkImage_Lazy::lockTextureProxy(GrContext* ctx, SkTransferFunctionBehavior behavior = getGeneratorBehaviorAndInfo(&genPixelsInfo); // 2. Ask the generator to natively create one - { + if (!proxy) { ScopedGenerator generator(fSharedGenerator); if (GrTextureMaker::AllowedTexGenType::kCheap == genType && SkImageGenerator::TexGenType::kCheap != generator->onCanGenerateTexture()) { return nullptr; } - if (sk_sp proxy = - generator->generateTexture(ctx, genPixelsInfo, fOrigin, behavior)) { + // TODO: Pass a flag into generateTexture which says we want to be mipped. If the generator + // can handle creating a mipped surface, then it can either generate the base layer or all + // the layers directly. Otherwise it just returns a non mipped surface as it currently does. + if ((proxy = generator->generateTexture(ctx, genPixelsInfo, fOrigin, behavior))) { SK_HISTOGRAM_ENUMERATION("LockTexturePath", kNative_LockTexturePath, kLockTexturePathCount); - set_key_on_proxy(ctx->resourceProvider(), proxy.get(), key); - return proxy; + set_key_on_proxy(ctx->resourceProvider(), proxy.get(), nullptr, key); + if (!willBeMipped || proxy->isMipMapped()) { + return proxy; + } } } // 3. Ask the generator to return YUV planes, which the GPU can convert - if (!ctx->contextPriv().disableGpuYUVConversion()) { + if (!proxy && !ctx->contextPriv().disableGpuYUVConversion()) { const GrSurfaceDesc desc = GrImageInfoToSurfaceDesc(cacheInfo, *ctx->caps()); ScopedGenerator generator(fSharedGenerator); Generator_GrYUVProvider provider(generator); @@ -797,33 +814,53 @@ sk_sp SkImage_Lazy::lockTextureProxy(GrContext* ctx, fSharedGenerator->fGenerator->getInfo().colorSpace(); const SkColorSpace* thisColorSpace = fInfo.colorSpace(); - sk_sp proxy = - provider.refAsTextureProxy(ctx, desc, true, generatorColorSpace, thisColorSpace); + // TODO: Update to create the mipped surface in the YUV generator and draw the base layer + // directly into the mipped surface. + proxy = provider.refAsTextureProxy(ctx, desc, true, generatorColorSpace, thisColorSpace); if (proxy) { SK_HISTOGRAM_ENUMERATION("LockTexturePath", kYUV_LockTexturePath, kLockTexturePathCount); - set_key_on_proxy(ctx->resourceProvider(), proxy.get(), key); - return proxy; + set_key_on_proxy(ctx->resourceProvider(), proxy.get(), nullptr, key); + if (!willBeMipped || proxy->isMipMapped()) { + return proxy; + } } } // 4. Ask the generator to return RGB(A) data, which the GPU can convert SkBitmap bitmap; - if (this->lockAsBitmap(&bitmap, chint, format, genPixelsInfo, behavior)) { - sk_sp proxy; + if (!proxy && this->lockAsBitmap(&bitmap, chint, format, genPixelsInfo, behavior)) { if (willBeMipped) { proxy = GrGenerateMipMapsAndUploadToTextureProxy(ctx, bitmap, dstColorSpace); } if (!proxy) { proxy = GrUploadBitmapToTextureProxy(ctx->resourceProvider(), bitmap, dstColorSpace); } - if (proxy) { + if (proxy && (!willBeMipped || proxy->isMipMapped())) { SK_HISTOGRAM_ENUMERATION("LockTexturePath", kRGBA_LockTexturePath, kLockTexturePathCount); - set_key_on_proxy(ctx->resourceProvider(), proxy.get(), key); + set_key_on_proxy(ctx->resourceProvider(), proxy.get(), nullptr, key); return proxy; } } + + if (proxy) { + // We need a mipped proxy, but we either found a proxy earlier that wasn't mipped, generated + // a native non mipped proxy, or generated a non-mipped yuv proxy. Thus we generate a new + // mipped surface and copy the original proxy into the base layer. We will then let the gpu + // generate the rest of the mips. + SkASSERT(willBeMipped); + SkASSERT(!proxy->isMipMapped()); + if (auto mippedProxy = GrCopyBaseMipMapToTextureProxy(ctx, proxy.get(), dstColorSpace)) { + set_key_on_proxy(ctx->resourceProvider(), mippedProxy.get(), proxy.get(), key); + return mippedProxy; + } + // We failed to make a mipped proxy with the base copied into it. This could have + // been from failure to make the proxy or failure to do the copy. Thus we will fall + // back to just using the non mipped proxy; See skbug.com/7094. + return proxy; + } + SK_HISTOGRAM_ENUMERATION("LockTexturePath", kFailure_LockTexturePath, kLockTexturePathCount); return nullptr;