From 8e9b4c47575cd468d345e7b3a0e7ba17d21d42fc Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Fri, 20 Jul 2018 10:30:48 -0400 Subject: [PATCH] Reland "Don't require mips in GrTextureProducer if texture is 1x1." This reverts commit c861eee3a6f1f93ad3df27b8be94f44b245bd128. Reason for revert: Relanding with fix for gray8 not copyable Original change's description: > Revert "Don't require mips in GrTextureProducer if texture is 1x1." > > This reverts commit 5191fd7555d34225ef771ad4cac65bcbbb50a89c. > > Reason for revert: breaking angle > > Original change's description: > > Don't require mips in GrTextureProducer if texture is 1x1. > > > > Bug: chromium:862921 > > Change-Id: I5f3584ad36e160a5a09d0a37e31e147155076b4d > > Reviewed-on: https://skia-review.googlesource.com/142586 > > Reviewed-by: Brian Osman > > Reviewed-by: Brian Salomon > > Commit-Queue: Greg Daniel > > TBR=egdaniel@google.com,bsalomon@google.com,brianosman@google.com > > Change-Id: Iaef7a56b061cb41f4c75ec20d8df77d3e52b194d > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: chromium:862921 > Reviewed-on: https://skia-review.googlesource.com/142600 > Reviewed-by: Greg Daniel > Commit-Queue: Greg Daniel TBR=egdaniel@google.com,bsalomon@google.com,brianosman@google.com Change-Id: I52378fa43efe2fdf583335f5fa8aa5b04a68ae2f Bug: chromium:862921 Reviewed-on: https://skia-review.googlesource.com/142760 Commit-Queue: Greg Daniel Reviewed-by: Greg Daniel --- src/gpu/GrTextureAdjuster.cpp | 6 ++--- src/gpu/GrTextureAdjuster.h | 2 +- src/gpu/GrTextureMaker.cpp | 6 +---- src/gpu/GrTextureMaker.h | 6 ++--- src/gpu/GrTextureProducer.cpp | 17 +++++++++++-- src/gpu/GrTextureProducer.h | 8 +++++-- src/gpu/vk/GrVkGpuCommandBuffer.cpp | 3 ++- tests/GrMipMappedTest.cpp | 37 +++++++++++++++++++++++++++++ 8 files changed, 66 insertions(+), 19 deletions(-) diff --git a/src/gpu/GrTextureAdjuster.cpp b/src/gpu/GrTextureAdjuster.cpp index d016896c3f..b2fc0584ba 100644 --- a/src/gpu/GrTextureAdjuster.cpp +++ b/src/gpu/GrTextureAdjuster.cpp @@ -17,9 +17,8 @@ GrTextureAdjuster::GrTextureAdjuster(GrContext* context, sk_sp o SkAlphaType alphaType, uint32_t uniqueID, SkColorSpace* cs) - : INHERITED(original->width(), original->height(), + : INHERITED(context, original->width(), original->height(), GrPixelConfigIsAlphaOnly(original->config())) - , fContext(context) , fOriginal(std::move(original)) , fAlphaType(alphaType) , fColorSpace(cs) @@ -77,6 +76,7 @@ sk_sp GrTextureAdjuster::onRefTextureProxyForParams( const GrSamplerState& params, SkColorSpace* dstColorSpace, sk_sp* texColorSpace, + bool willBeMipped, SkScalar scaleAdjust[2]) { sk_sp proxy = this->originalProxyRef(); CopyParams copyParams; @@ -105,8 +105,6 @@ sk_sp GrTextureAdjuster::onRefTextureProxyForParams( } } - bool willBeMipped = GrSamplerState::Filter::kMipMap == params.filter() && - fContext->contextPriv().caps()->mipMapSupport(); sk_sp result = this->refTextureProxyCopy(copyParams, willBeMipped); if (!result && needsCopyForMipsOnly) { // If we were unable to make a copy and we only needed a copy for mips, then we will return diff --git a/src/gpu/GrTextureAdjuster.h b/src/gpu/GrTextureAdjuster.h index e43e87b1dc..ad02ae06c8 100644 --- a/src/gpu/GrTextureAdjuster.h +++ b/src/gpu/GrTextureAdjuster.h @@ -46,11 +46,11 @@ private: sk_sp onRefTextureProxyForParams(const GrSamplerState&, SkColorSpace* dstColorSpace, sk_sp* proxyColorSpace, + bool willBeMipped, SkScalar scaleAdjust[2]) override; sk_sp refTextureProxyCopy(const CopyParams& copyParams, bool willBeMipped); - GrContext* fContext; sk_sp fOriginal; SkAlphaType fAlphaType; SkColorSpace* fColorSpace; diff --git a/src/gpu/GrTextureMaker.cpp b/src/gpu/GrTextureMaker.cpp index 22870cc7c8..820e47ab7c 100644 --- a/src/gpu/GrTextureMaker.cpp +++ b/src/gpu/GrTextureMaker.cpp @@ -16,6 +16,7 @@ sk_sp GrTextureMaker::onRefTextureProxyForParams(const GrSamplerState& params, SkColorSpace* dstColorSpace, sk_sp* texColorSpace, + bool willBeMipped, SkScalar scaleAdjust[2]) { if (this->width() > fContext->contextPriv().caps()->maxTextureSize() || this->height() > fContext->contextPriv().caps()->maxTextureSize()) { @@ -23,11 +24,6 @@ sk_sp GrTextureMaker::onRefTextureProxyForParams(const GrSampler } CopyParams copyParams; - bool willBeMipped = params.filter() == GrSamplerState::Filter::kMipMap; - - if (!fContext->contextPriv().caps()->mipMapSupport()) { - willBeMipped = false; - } if (texColorSpace) { *texColorSpace = this->getColorSpace(dstColorSpace); diff --git a/src/gpu/GrTextureMaker.h b/src/gpu/GrTextureMaker.h index a365b50444..e2dd3c852a 100644 --- a/src/gpu/GrTextureMaker.h +++ b/src/gpu/GrTextureMaker.h @@ -28,8 +28,7 @@ public: protected: GrTextureMaker(GrContext* context, int width, int height, bool isAlphaOnly) - : INHERITED(width, height, isAlphaOnly) - , fContext(context) {} + : INHERITED(context, width, height, isAlphaOnly) {} /** * Return the maker's "original" texture. It is the responsibility of the maker to handle any @@ -54,10 +53,9 @@ private: sk_sp onRefTextureProxyForParams(const GrSamplerState&, SkColorSpace* dstColorSpace, sk_sp* proxyColorSpace, + bool willBeMipped, SkScalar scaleAdjust[2]) override; - GrContext* fContext; - typedef GrTextureProducer INHERITED; }; diff --git a/src/gpu/GrTextureProducer.cpp b/src/gpu/GrTextureProducer.cpp index 59962c55b1..8499b8abe5 100644 --- a/src/gpu/GrTextureProducer.cpp +++ b/src/gpu/GrTextureProducer.cpp @@ -11,6 +11,7 @@ #include "GrRenderTargetContext.h" #include "GrTextureProxy.h" #include "SkGr.h" +#include "SkMipMap.h" #include "SkRectPriv.h" #include "effects/GrBicubicEffect.h" #include "effects/GrSimpleTextureEffect.h" @@ -226,8 +227,14 @@ sk_sp GrTextureProducer::refTextureProxyForParams( SkDEBUGCODE(bool expectNoScale = (sampler.filter() != GrSamplerState::Filter::kMipMap && !sampler.isRepeated())); SkASSERT(scaleAdjust || expectNoScale); + + int mipCount = SkMipMap::ComputeLevelCount(this->width(), this->height()); + bool willBeMipped = GrSamplerState::Filter::kMipMap == sampler.filter() && mipCount && + fContext->contextPriv().caps()->mipMapSupport(); + auto result = - this->onRefTextureProxyForParams(sampler, dstColorSpace, proxyColorSpace, scaleAdjust); + this->onRefTextureProxyForParams(sampler, dstColorSpace, proxyColorSpace, willBeMipped, + scaleAdjust); // Check that the "no scaling expected" case always returns a proxy of the same size as the // producer. @@ -243,8 +250,14 @@ sk_sp GrTextureProducer::refTextureProxy(GrMipMapped willNeedMip GrMipMapped::kNo == willNeedMips ? GrSamplerState::Filter::kNearest : GrSamplerState::Filter::kMipMap; GrSamplerState sampler(GrSamplerState::WrapMode::kClamp, filter); + + int mipCount = SkMipMap::ComputeLevelCount(this->width(), this->height()); + bool willBeMipped = GrSamplerState::Filter::kMipMap == sampler.filter() && mipCount && + fContext->contextPriv().caps()->mipMapSupport(); + auto result = - this->onRefTextureProxyForParams(sampler, dstColorSpace, proxyColorSpace, nullptr); + this->onRefTextureProxyForParams(sampler, dstColorSpace, proxyColorSpace, + willBeMipped, nullptr); // Check that no scaling occured and we returned a proxy of the same size as the producer. SkASSERT(!result || (result->width() == this->width() && result->height() == this->height())); diff --git a/src/gpu/GrTextureProducer.h b/src/gpu/GrTextureProducer.h index b42bbcfc9a..66adb6fa4e 100644 --- a/src/gpu/GrTextureProducer.h +++ b/src/gpu/GrTextureProducer.h @@ -124,8 +124,9 @@ public: protected: friend class GrTextureProducer_TestAccess; - GrTextureProducer(int width, int height, bool isAlphaOnly) - : fWidth(width) + GrTextureProducer(GrContext* context, int width, int height, bool isAlphaOnly) + : fContext(context) + , fWidth(width) , fHeight(height) , fIsAlphaOnly(isAlphaOnly) {} @@ -185,10 +186,13 @@ protected: const SkRect& domain, const GrSamplerState::Filter* filterOrNullForBicubic); + GrContext* fContext; + private: virtual sk_sp onRefTextureProxyForParams(const GrSamplerState&, SkColorSpace* dstColorSpace, sk_sp* proxyColorSpace, + bool willBeMipped, SkScalar scaleAdjust[2]) = 0; const int fWidth; diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.cpp b/src/gpu/vk/GrVkGpuCommandBuffer.cpp index 91adb5a778..d58a8b5ff7 100644 --- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp +++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp @@ -622,7 +622,8 @@ static void prepare_sampled_images(const GrResourceIOProcessor& processor, } // Check if we need to regenerate any mip maps - if (GrSamplerState::Filter::kMipMap == sampler.samplerState().filter()) { + if (GrSamplerState::Filter::kMipMap == sampler.samplerState().filter() && + (vkTexture->width() != 1 || vkTexture->height() != 1)) { SkASSERT(vkTexture->texturePriv().mipMapped() == GrMipMapped::kYes); if (vkTexture->texturePriv().mipMapsAreDirty()) { gpu->regenerateMipMapLevels(vkTexture); diff --git a/tests/GrMipMappedTest.cpp b/tests/GrMipMappedTest.cpp index 3c47598e38..ee4838d2ce 100644 --- a/tests/GrMipMappedTest.cpp +++ b/tests/GrMipMappedTest.cpp @@ -289,3 +289,40 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(GrImageSnapshotMipMappedTest, reporter, ctxIn } } } + +// Test that we don't create a mip mapped texture if the size is 1x1 even if the filter mode is set +// to use mips. This test passes by not crashing or hitting asserts in code. +DEF_GPUTEST_FOR_RENDERING_CONTEXTS(Gr1x1TextureMipMappedTest, reporter, ctxInfo) { + GrContext* context = ctxInfo.grContext(); + if (!context->contextPriv().caps()->mipMapSupport()) { + return; + } + + // Make surface to draw into + SkImageInfo info = SkImageInfo::MakeN32(16, 16, kPremul_SkAlphaType); + sk_sp surface = SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, info); + + // Make 1x1 raster bitmap + SkBitmap bmp; + bmp.allocN32Pixels(1, 1); + SkPMColor* pixel = reinterpret_cast(bmp.getPixels()); + *pixel = 0; + + sk_sp bmpImage = SkImage::MakeFromBitmap(bmp); + + // Make sure we scale so we don't optimize out the use of mips. + surface->getCanvas()->scale(0.5f, 0.5f); + + SkPaint paint; + // This should upload the image to a non mipped GrTextureProxy. + surface->getCanvas()->drawImage(bmpImage, 0, 0, &paint); + surface->flush(); + + // Now set the filter quality to high so we use mip maps. We should find the non mipped texture + // in the cache for the SkImage. Since the texture is 1x1 we should just use that texture + // instead of trying to do a copy to a mipped texture. + paint.setFilterQuality(kHigh_SkFilterQuality); + surface->getCanvas()->drawImage(bmpImage, 0, 0, &paint); + surface->flush(); +} +