From 5f4b09d523a761a3a5c622bb01eeba47905da5f0 Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Tue, 12 Jun 2018 16:39:59 -0400 Subject: Allow caller to specify if the want mip maps in makeTextureImage call. Since Ganesh no longer will allocate mips late, this gives the clients a way to tell skia that they want the texture they will be using to have mips. It also supports allowing a client to take a non mipped texture backed image and turn it into a new image which is mipped and texture backed. Bug: chromium:834837 Change-Id: I1781ce618c22023b6309f248e7ee49e69bd3c6df Reviewed-on: https://skia-review.googlesource.com/134323 Commit-Queue: Greg Daniel Reviewed-by: Eric Karl Reviewed-by: Cary Clark Reviewed-by: Brian Salomon --- docs/SkImage_Reference.bmh | 10 ++++-- include/core/SkImage.h | 11 ++++-- src/gpu/GrTextureProducer.cpp | 15 ++++++++ src/gpu/GrTextureProducer.h | 15 ++++++++ src/image/SkImage.cpp | 3 +- src/image/SkImage_Gpu.cpp | 35 +++++++++++++------ tests/ImageTest.cpp | 80 +++++++++++++++++++++++++------------------ 7 files changed, 118 insertions(+), 51 deletions(-) diff --git a/docs/SkImage_Reference.bmh b/docs/SkImage_Reference.bmh index 687447aca7..c63be58f72 100644 --- a/docs/SkImage_Reference.bmh +++ b/docs/SkImage_Reference.bmh @@ -1774,18 +1774,22 @@ pixels in Image could not be read or copied. # ------------------------------------------------------------------------------ -#Method sk_sp makeTextureImage(GrContext* context, SkColorSpace* dstColorSpace) const +#Method sk_sp makeTextureImage(GrContext* context, SkColorSpace* dstColorSpace, + GrMipMapped mipMapped = GrMipMapped::kNo) const #In Constructor #Line # creates Image matching Color_Space if possible ## Returns Image backed by GPU_Texture associated with context. Returned Image is -compatible with Surface created with dstColorSpace. Returns original -Image if context and dstColorSpace match. +compatible with Surface created with dstColorSpace. The returned Image will also +support the request GrMipMapped status. In other words if mipMapped is GrMipMapped::kYes, +then the backing texture will have Mip_Map levels allocated. Returns original Image if context +and dstColorSpace match and mipMapped is compatible with the backing GPU_Texture. Returns nullptr if context is nullptr, or if Image was created with another GrContext. #Param context GPU_Context ## #Param dstColorSpace range of colors of matching Surface on GPU ## +#Param mipMapped whether the returned SkImage's texture must have allocated Mip_Map levels ## #Return created Image, or nullptr ## diff --git a/include/core/SkImage.h b/include/core/SkImage.h index 5aba2433ad..788abe69c8 100644 --- a/include/core/SkImage.h +++ b/include/core/SkImage.h @@ -740,17 +740,22 @@ public: sk_sp makeSubset(const SkIRect& subset) const; /** Returns SkImage backed by GPU texture associated with context. Returned SkImage is - compatible with SkSurface created with dstColorSpace. Returns original - SkImage if context and dstColorSpace match. + compatible with SkSurface created with dstColorSpace. The returned SkImage will also + support the request GrMipMapped status. In other words if mipMapped is GrMipMapped::kYes, + then the backing texture will have mip map levles allocated. Returns original SkImage if + context and dstColorSpace match and mipMapped is compatible with the backing GPU_Texture. Returns nullptr if context is nullptr, or if SkImage was created with another GrContext. @param context GPU context @param dstColorSpace range of colors of matching SkSurface on GPU + @param mipMapped whether the returned SkImage's texture must have allocated Mip_Map + levels. @return created SkImage, or nullptr */ - sk_sp makeTextureImage(GrContext* context, SkColorSpace* dstColorSpace) const; + sk_sp makeTextureImage(GrContext* context, SkColorSpace* dstColorSpace, + GrMipMapped mipMapped = GrMipMapped::kNo) const; /** Returns raster image or lazy image. Copies SkImage backed by GPU texture into CPU memory if needed. Returns original SkImage if decoded in raster bitmap, diff --git a/src/gpu/GrTextureProducer.cpp b/src/gpu/GrTextureProducer.cpp index 1e9c3acd16..59962c55b1 100644 --- a/src/gpu/GrTextureProducer.cpp +++ b/src/gpu/GrTextureProducer.cpp @@ -235,3 +235,18 @@ sk_sp GrTextureProducer::refTextureProxyForParams( (result->width() == this->width() && result->height() == this->height())); return result; } + +sk_sp GrTextureProducer::refTextureProxy(GrMipMapped willNeedMips, + SkColorSpace* dstColorSpace, + sk_sp* proxyColorSpace) { + GrSamplerState::Filter filter = + GrMipMapped::kNo == willNeedMips ? GrSamplerState::Filter::kNearest + : GrSamplerState::Filter::kMipMap; + GrSamplerState sampler(GrSamplerState::WrapMode::kClamp, filter); + auto result = + this->onRefTextureProxyForParams(sampler, dstColorSpace, proxyColorSpace, 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())); + return result; +} diff --git a/src/gpu/GrTextureProducer.h b/src/gpu/GrTextureProducer.h index 467948dfa6..c458d4fbe6 100644 --- a/src/gpu/GrTextureProducer.h +++ b/src/gpu/GrTextureProducer.h @@ -98,6 +98,21 @@ public: proxyColorSpace, scaleAdjust); } + /** + * Returns a texture that is safe for use with the dstColorSpace. If willNeedMips is true then + * the returned texture is guaranteed to have allocated mip map levels. This can be a + * performance win if future draws with the texture require mip maps. + * + * Places the color space of the texture in (*proxyColorSpace). + */ + // TODO: Once we remove support for npot textures, we should add a flag for must support repeat + // wrap mode. To support that flag now would require us to support scaleAdjust array like in + // refTextureProxyForParams, however the current public API that uses this call does not expose + // that array. + sk_sp refTextureProxy(GrMipMapped willNeedMips, + SkColorSpace* dstColorSpace, + sk_sp* proxyColorSpace); + virtual ~GrTextureProducer() {} int width() const { return fWidth; } diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp index 1d5e2dbf27..ee0eff08a9 100644 --- a/src/image/SkImage.cpp +++ b/src/image/SkImage.cpp @@ -385,7 +385,8 @@ sk_sp SkImage::MakeFromNV12TexturesCopy(GrContext* ctx, SkYUVColorSpace return nullptr; } -sk_sp SkImage::makeTextureImage(GrContext*, SkColorSpace* dstColorSpace) const { +sk_sp SkImage::makeTextureImage(GrContext*, SkColorSpace* dstColorSpace, + GrMipMapped mipMapped) const { return nullptr; } diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp index 01f0c1ff8a..faba37383c 100644 --- a/src/image/SkImage_Gpu.cpp +++ b/src/image/SkImage_Gpu.cpp @@ -498,12 +498,13 @@ sk_sp SkImage::MakeFromNV12TexturesCopy(GrContext* ctx, SkYUVColorSpace size, origin, std::move(imageColorSpace)); } -static sk_sp create_image_from_maker(GrContext* context, GrTextureMaker* maker, - SkAlphaType at, uint32_t id, - SkColorSpace* dstColorSpace) { +static sk_sp create_image_from_producer(GrContext* context, GrTextureProducer* producer, + SkAlphaType at, uint32_t id, + SkColorSpace* dstColorSpace, + GrMipMapped mipMapped) { sk_sp texColorSpace; - sk_sp proxy(maker->refTextureProxyForParams( - GrSamplerState::ClampNearest(), dstColorSpace, &texColorSpace, nullptr)); + sk_sp proxy(producer->refTextureProxy(mipMapped, dstColorSpace, + &texColorSpace)); if (!proxy) { return nullptr; } @@ -511,24 +512,36 @@ static sk_sp create_image_from_maker(GrContext* context, GrTextureMaker std::move(texColorSpace), SkBudgeted::kNo); } -sk_sp SkImage::makeTextureImage(GrContext* context, SkColorSpace* dstColorSpace) const { +sk_sp SkImage::makeTextureImage(GrContext* context, SkColorSpace* dstColorSpace, + GrMipMapped mipMapped) const { if (!context) { return nullptr; } if (GrContext* incumbent = as_IB(this)->context()) { - return incumbent == context ? sk_ref_sp(const_cast(this)) : nullptr; + if (incumbent != context) { + return nullptr; + } + sk_sp proxy = as_IB(this)->asTextureProxyRef(); + SkASSERT(proxy); + if (GrMipMapped::kNo == mipMapped || proxy->mipMapped() == mipMapped) { + return sk_ref_sp(const_cast(this)); + } + GrTextureAdjuster adjuster(context, std::move(proxy), this->alphaType(), + this->uniqueID(), this->colorSpace()); + return create_image_from_producer(context, &adjuster, this->alphaType(), + this->uniqueID(), dstColorSpace, mipMapped); } if (this->isLazyGenerated()) { GrImageTextureMaker maker(context, this, kDisallow_CachingHint); - return create_image_from_maker(context, &maker, this->alphaType(), - this->uniqueID(), dstColorSpace); + return create_image_from_producer(context, &maker, this->alphaType(), + this->uniqueID(), dstColorSpace, mipMapped); } if (const SkBitmap* bmp = as_IB(this)->onPeekBitmap()) { GrBitmapTextureMaker maker(context, *bmp); - return create_image_from_maker(context, &maker, this->alphaType(), - this->uniqueID(), dstColorSpace); + return create_image_from_producer(context, &maker, this->alphaType(), + this->uniqueID(), dstColorSpace, mipMapped); } return nullptr; } diff --git a/tests/ImageTest.cpp b/tests/ImageTest.cpp index 930d7c0c86..dc7576acad 100644 --- a/tests/ImageTest.cpp +++ b/tests/ImageTest.cpp @@ -147,9 +147,10 @@ static sk_sp create_codec_image() { sk_sp src(sk_tool_utils::EncodeImageToData(bitmap, SkEncodedImageFormat::kPNG, 100)); return SkImage::MakeFromEncoded(std::move(src)); } -static sk_sp create_gpu_image(GrContext* context) { +static sk_sp create_gpu_image(GrContext* context, bool withMips = false) { const SkImageInfo info = SkImageInfo::MakeN32(20, 20, kOpaque_SkAlphaType); - auto surface(SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, info)); + auto surface(SkSurface::MakeRenderTarget(context, SkBudgeted::kNo, info, 0, + kBottomLeft_GrSurfaceOrigin, nullptr, withMips)); draw_image_test_pattern(surface->getCanvas()); return surface->makeImageSnapshot(); } @@ -385,6 +386,8 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkImage_makeTextureImage, reporter, contextIn create_picture_image, // Create a texture image. [context] { return create_gpu_image(context); }, + // Create a texture image with mips + //[context] { return create_gpu_image(context, true); }, // Create a texture image in a another GrContext. [otherContextInfo] { auto restore = otherContextInfo.testContext()->makeCurrentAndAutoRestore(); @@ -400,43 +403,54 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SkImage_makeTextureImage, reporter, contextIn }; for (auto& dstColorSpace : dstColorSpaces) { - for (auto factory : imageFactories) { - sk_sp image(factory()); - if (!image) { - ERRORF(reporter, "Error creating image."); - continue; - } + for (auto mipMapped : {GrMipMapped::kNo, GrMipMapped::kYes}) { + for (auto factory : imageFactories) { + sk_sp image(factory()); + if (!image) { + ERRORF(reporter, "Error creating image."); + continue; + } - sk_sp texImage(image->makeTextureImage(context, dstColorSpace.get())); - if (!texImage) { - GrContext* imageContext = as_IB(image)->context(); + sk_sp texImage(image->makeTextureImage(context, dstColorSpace.get(), + mipMapped)); + if (!texImage) { + GrContext* imageContext = as_IB(image)->context(); - // We expect to fail if image comes from a different GrContext. - if (!image->isTextureBacked() || imageContext == context) { - ERRORF(reporter, "makeTextureImage failed."); + // We expect to fail if image comes from a different GrContext. + if (!image->isTextureBacked() || imageContext == context) { + ERRORF(reporter, "makeTextureImage failed."); + } + continue; } - continue; - } - if (!texImage->isTextureBacked()) { - ERRORF(reporter, "makeTextureImage returned non-texture image."); - continue; - } - if (image->isTextureBacked()) { - GrSurfaceProxy* origProxy = as_IB(image)->peekProxy(); - GrSurfaceProxy* copyProxy = as_IB(texImage)->peekProxy(); - - if (origProxy->underlyingUniqueID() != copyProxy->underlyingUniqueID()) { - ERRORF(reporter, "makeTextureImage made unnecessary texture copy."); + if (!texImage->isTextureBacked()) { + ERRORF(reporter, "makeTextureImage returned non-texture image."); + continue; + } + if (GrMipMapped::kYes == mipMapped && + as_IB(texImage)->peekProxy()->mipMapped() != mipMapped) { + ERRORF(reporter, "makeTextureImage returned non-mipmapped texture."); + continue; + } + if (image->isTextureBacked()) { + GrSurfaceProxy* origProxy = as_IB(image)->peekProxy(); + GrSurfaceProxy* copyProxy = as_IB(texImage)->peekProxy(); + + if (origProxy->underlyingUniqueID() != copyProxy->underlyingUniqueID()) { + SkASSERT(origProxy->asTextureProxy()); + if (GrMipMapped::kNo == mipMapped || + GrMipMapped::kYes == origProxy->asTextureProxy()->mipMapped()) { + ERRORF(reporter, "makeTextureImage made unnecessary texture copy."); + } + } + } + if (image->width() != texImage->width() || image->height() != texImage->height()) { + ERRORF(reporter, "makeTextureImage changed the image size."); + } + if (image->alphaType() != texImage->alphaType()) { + ERRORF(reporter, "makeTextureImage changed image alpha type."); } - } - if (image->width() != texImage->width() || image->height() != texImage->height()) { - ERRORF(reporter, "makeTextureImage changed the image size."); - } - if (image->alphaType() != texImage->alphaType()) { - ERRORF(reporter, "makeTextureImage changed image alpha type."); } } - context->flush(); } } -- cgit v1.2.3