From 739c5bf111baf977fe418a24fa00ce260989ee9a Mon Sep 17 00:00:00 2001 From: Brian Salomon Date: Mon, 7 Nov 2016 09:53:44 -0500 Subject: [PATCH] Revert "Revert "Limit GL_TEXTURE_RECTANGLE filtering to bilinear."" This reverts commit ce4d04ae8eace6ba53ff8b8c8d8f4a2e6af4e59f. BUG=skia:5932 Original CL description: > >Limit GL_TEXTURE_RECTANGLE filtering to bilinear. > >Adds a clamp for GrTexture filtering that can be set by a subclass at construction. The clamping is performed by GrTextureParams. GrGLTexture limits filtering to bilinear for rectangle and external textures. > >Also moves samplerType() to GrTexturePriv from GrTexture. > >BUG=skia:5932 GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4455 Change-Id: I4a9f4abac44979cb900f5b04fe741524eade66b1 Reviewed-on: https://skia-review.googlesource.com/4455 Reviewed-by: Brian Salomon Commit-Queue: Brian Salomon --- include/gpu/GrTexture.h | 16 +++++------ src/gpu/GrProgramDesc.cpp | 4 ++- src/gpu/GrTexture.cpp | 3 +- src/gpu/GrTextureAccess.cpp | 3 ++ src/gpu/GrTexturePriv.h | 5 ++++ src/gpu/effects/GrSimpleTextureEffect.h | 4 +-- src/gpu/gl/GrGLTexture.cpp | 20 +++++++++---- src/gpu/glsl/GrGLSLProgramBuilder.cpp | 3 +- src/gpu/vk/GrVkTexture.cpp | 9 ++++-- tests/RectangleTextureTest.cpp | 38 ++++++++++++++++++++----- 10 files changed, 77 insertions(+), 28 deletions(-) diff --git a/include/gpu/GrTexture.h b/include/gpu/GrTexture.h index 211f1937da..8fa0db9587 100644 --- a/include/gpu/GrTexture.h +++ b/include/gpu/GrTexture.h @@ -10,17 +10,16 @@ #define GrTexture_DEFINED #include "GrSurface.h" +#include "GrTextureParams.h" #include "SkPoint.h" #include "SkRefCnt.h" -class GrTextureParams; class GrTexturePriv; class GrTexture : virtual public GrSurface { public: GrTexture* asTexture() override { return this; } const GrTexture* asTexture() const override { return this; } - GrSLType samplerType() const { return fSamplerType; } /** * Return the native ID or handle to the texture, depending on the @@ -46,7 +45,8 @@ public: inline const GrTexturePriv texturePriv() const; protected: - GrTexture(GrGpu*, const GrSurfaceDesc&, GrSLType, bool wasMipMapDataProvided); + GrTexture(GrGpu*, const GrSurfaceDesc&, GrSLType samplerType, + GrTextureParams::FilterMode highestFilterMode, bool wasMipMapDataProvided); void validateDesc() const; @@ -61,11 +61,11 @@ private: kValid_MipMapsStatus }; - GrSLType fSamplerType; - MipMapsStatus fMipMapsStatus; - int fMaxMipMapLevel; - SkSourceGammaTreatment fGammaTreatment; - + GrSLType fSamplerType; + GrTextureParams::FilterMode fHighestFilterMode; + MipMapsStatus fMipMapsStatus; + int fMaxMipMapLevel; + SkSourceGammaTreatment fGammaTreatment; friend class GrTexturePriv; typedef GrSurface INHERITED; diff --git a/src/gpu/GrProgramDesc.cpp b/src/gpu/GrProgramDesc.cpp index 3b0e54bcd2..fa728ed682 100644 --- a/src/gpu/GrProgramDesc.cpp +++ b/src/gpu/GrProgramDesc.cpp @@ -9,6 +9,7 @@ #include "GrProcessor.h" #include "GrPipeline.h" #include "GrRenderTargetPriv.h" +#include "GrTexturePriv.h" #include "SkChecksum.h" #include "glsl/GrGLSLFragmentProcessor.h" #include "glsl/GrGLSLFragmentShaderBuilder.h" @@ -45,7 +46,8 @@ static void add_sampler_keys(GrProcessorKeyBuilder* b, const GrProcessor& proc, for (; i < numTextures; ++i) { const GrTextureAccess& access = proc.textureAccess(i); const GrTexture* tex = access.getTexture(); - k16[i] = sampler_key(tex->samplerType(), tex->config(), access.getVisibility(), caps); + k16[i] = sampler_key(tex->texturePriv().samplerType(), tex->config(), + access.getVisibility(), caps); } for (; i < numSamplers; ++i) { const GrBufferAccess& access = proc.bufferAccess(i - numTextures); diff --git a/src/gpu/GrTexture.cpp b/src/gpu/GrTexture.cpp index de1135a1eb..6fc1580e89 100644 --- a/src/gpu/GrTexture.cpp +++ b/src/gpu/GrTexture.cpp @@ -70,9 +70,10 @@ GrSurfaceOrigin resolve_origin(const GrSurfaceDesc& desc) { ////////////////////////////////////////////////////////////////////////////// GrTexture::GrTexture(GrGpu* gpu, const GrSurfaceDesc& desc, GrSLType samplerType, - bool wasMipMapDataProvided) + GrTextureParams::FilterMode highestFilterMode, bool wasMipMapDataProvided) : INHERITED(gpu, desc) , fSamplerType(samplerType) + , fHighestFilterMode(highestFilterMode) // Gamma treatment is explicitly set after creation via GrTexturePriv , fGammaTreatment(SkSourceGammaTreatment::kIgnore) { if (wasMipMapDataProvided) { diff --git a/src/gpu/GrTextureAccess.cpp b/src/gpu/GrTextureAccess.cpp index 675bc20777..bddbd5a2d7 100644 --- a/src/gpu/GrTextureAccess.cpp +++ b/src/gpu/GrTextureAccess.cpp @@ -8,6 +8,7 @@ #include "GrTextureAccess.h" #include "GrColor.h" #include "GrTexture.h" +#include "GrTexturePriv.h" GrTextureAccess::GrTextureAccess() {} @@ -28,6 +29,7 @@ void GrTextureAccess::reset(GrTexture* texture, SkASSERT(texture); fTexture.set(SkRef(texture), kRead_GrIOType); fParams = params; + fParams.setFilterMode(SkTMin(params.filterMode(), texture->texturePriv().highestFilterMode())); fVisibility = visibility; } @@ -37,6 +39,7 @@ void GrTextureAccess::reset(GrTexture* texture, GrShaderFlags visibility) { SkASSERT(texture); fTexture.set(SkRef(texture), kRead_GrIOType); + filterMode = SkTMin(filterMode, texture->texturePriv().highestFilterMode()); fParams.reset(tileXAndY, filterMode); fVisibility = visibility; } diff --git a/src/gpu/GrTexturePriv.h b/src/gpu/GrTexturePriv.h index c4e6538d16..c4c185560e 100644 --- a/src/gpu/GrTexturePriv.h +++ b/src/gpu/GrTexturePriv.h @@ -49,6 +49,11 @@ public: return fTexture->fMaxMipMapLevel; } + GrSLType samplerType() const { return fTexture->fSamplerType; } + + /** The filter used is clamped to this value in GrTextureAccess. */ + GrTextureParams::FilterMode highestFilterMode() const { return fTexture->fHighestFilterMode; } + void setGammaTreatment(SkSourceGammaTreatment gammaTreatment) const { fTexture->fGammaTreatment = gammaTreatment; } diff --git a/src/gpu/effects/GrSimpleTextureEffect.h b/src/gpu/effects/GrSimpleTextureEffect.h index 8242362a9c..fced736183 100644 --- a/src/gpu/effects/GrSimpleTextureEffect.h +++ b/src/gpu/effects/GrSimpleTextureEffect.h @@ -31,8 +31,8 @@ public: /* clamp mode */ static sk_sp Make(GrTexture* tex, sk_sp colorSpaceXform, - const SkMatrix& matrix, - GrTextureParams::FilterMode filterMode) { + const SkMatrix& matrix, + GrTextureParams::FilterMode filterMode) { return sk_sp( new GrSimpleTextureEffect(tex, std::move(colorSpaceXform), matrix, filterMode)); } diff --git a/src/gpu/gl/GrGLTexture.cpp b/src/gpu/gl/GrGLTexture.cpp index ec0ad3b7f3..c45d08f93c 100644 --- a/src/gpu/gl/GrGLTexture.cpp +++ b/src/gpu/gl/GrGLTexture.cpp @@ -12,7 +12,7 @@ #define GPUGL static_cast(this->getGpu()) #define GL_CALL(X) GR_GL_CALL(GPUGL->glInterface(), X) -inline static GrSLType sampler_type(const GrGLTexture::IDDesc& idDesc, const GrGLGpu* gpu) { +static inline GrSLType sampler_type(const GrGLTexture::IDDesc& idDesc, const GrGLGpu* gpu) { if (idDesc.fInfo.fTarget == GR_GL_TEXTURE_EXTERNAL) { SkASSERT(gpu->glCaps().glslCaps()->externalTextureSupport()); return kTextureExternalSampler_GrSLType; @@ -25,11 +25,19 @@ inline static GrSLType sampler_type(const GrGLTexture::IDDesc& idDesc, const GrG } } +static inline GrTextureParams::FilterMode highest_filter_mode(const GrGLTexture::IDDesc& idDesc) { + if (idDesc.fInfo.fTarget == GR_GL_TEXTURE_RECTANGLE || + idDesc.fInfo.fTarget == GR_GL_TEXTURE_EXTERNAL) { + return GrTextureParams::kBilerp_FilterMode; + } + return GrTextureParams::kMipMap_FilterMode; +} + // Because this class is virtually derived from GrSurface we must explicitly call its constructor. GrGLTexture::GrGLTexture(GrGLGpu* gpu, SkBudgeted budgeted, const GrSurfaceDesc& desc, const IDDesc& idDesc) : GrSurface(gpu, desc) - , INHERITED(gpu, desc, sampler_type(idDesc, gpu), false) { + , INHERITED(gpu, desc, sampler_type(idDesc, gpu), highest_filter_mode(idDesc), false) { this->init(desc, idDesc); this->registerWithCache(budgeted); } @@ -38,14 +46,15 @@ GrGLTexture::GrGLTexture(GrGLGpu* gpu, SkBudgeted budgeted, const GrSurfaceDesc& const IDDesc& idDesc, bool wasMipMapDataProvided) : GrSurface(gpu, desc) - , INHERITED(gpu, desc, sampler_type(idDesc, gpu), wasMipMapDataProvided) { + , INHERITED(gpu, desc, sampler_type(idDesc, gpu), highest_filter_mode(idDesc), + wasMipMapDataProvided) { this->init(desc, idDesc); this->registerWithCache(budgeted); } GrGLTexture::GrGLTexture(GrGLGpu* gpu, Wrapped, const GrSurfaceDesc& desc, const IDDesc& idDesc) : GrSurface(gpu, desc) - , INHERITED(gpu, desc, sampler_type(idDesc, gpu), false) { + , INHERITED(gpu, desc, sampler_type(idDesc, gpu), highest_filter_mode(idDesc), false) { this->init(desc, idDesc); this->registerWithCacheWrapped(); } @@ -53,7 +62,8 @@ GrGLTexture::GrGLTexture(GrGLGpu* gpu, Wrapped, const GrSurfaceDesc& desc, const GrGLTexture::GrGLTexture(GrGLGpu* gpu, const GrSurfaceDesc& desc, const IDDesc& idDesc, bool wasMipMapDataProvided) : GrSurface(gpu, desc) - , INHERITED(gpu, desc, sampler_type(idDesc, gpu), wasMipMapDataProvided) { + , INHERITED(gpu, desc, sampler_type(idDesc, gpu), highest_filter_mode(idDesc), + wasMipMapDataProvided) { this->init(desc, idDesc); } diff --git a/src/gpu/glsl/GrGLSLProgramBuilder.cpp b/src/gpu/glsl/GrGLSLProgramBuilder.cpp index abfeafda0c..5b1fbe1c79 100644 --- a/src/gpu/glsl/GrGLSLProgramBuilder.cpp +++ b/src/gpu/glsl/GrGLSLProgramBuilder.cpp @@ -8,6 +8,7 @@ #include "glsl/GrGLSLProgramBuilder.h" #include "GrPipeline.h" +#include "GrTexturePriv.h" #include "glsl/GrGLSLFragmentProcessor.h" #include "glsl/GrGLSLGeometryProcessor.h" #include "glsl/GrGLSLVarying.h" @@ -244,7 +245,7 @@ void GrGLSLProgramBuilder::emitSamplers(const GrProcessor& processor, int numTextures = processor.numTextures(); for (int t = 0; t < numTextures; ++t) { const GrTextureAccess& access = processor.textureAccess(t); - GrSLType samplerType = access.getTexture()->samplerType(); + GrSLType samplerType = access.getTexture()->texturePriv().samplerType(); if (kTextureExternalSampler_GrSLType == samplerType) { const char* externalFeatureString = this->glslCaps()->externalTextureExtensionString(); // We shouldn't ever create a GrGLTexture that requires external sampler type diff --git a/src/gpu/vk/GrVkTexture.cpp b/src/gpu/vk/GrVkTexture.cpp index 8c461f8308..3b6108cd52 100644 --- a/src/gpu/vk/GrVkTexture.cpp +++ b/src/gpu/vk/GrVkTexture.cpp @@ -24,7 +24,8 @@ GrVkTexture::GrVkTexture(GrVkGpu* gpu, const GrVkImageView* view) : GrSurface(gpu, desc) , GrVkImage(info, GrVkImage::kNot_Wrapped) - , INHERITED(gpu, desc, kTexture2DSampler_GrSLType, desc.fIsMipMapped) + , INHERITED(gpu, desc, kTexture2DSampler_GrSLType, GrTextureParams::kMipMap_FilterMode, + desc.fIsMipMapped) , fTextureView(view) , fLinearTextureView(nullptr) { this->registerWithCache(budgeted); @@ -38,7 +39,8 @@ GrVkTexture::GrVkTexture(GrVkGpu* gpu, GrVkImage::Wrapped wrapped) : GrSurface(gpu, desc) , GrVkImage(info, wrapped) - , INHERITED(gpu, desc, kTexture2DSampler_GrSLType, desc.fIsMipMapped) + , INHERITED(gpu, desc, kTexture2DSampler_GrSLType, GrTextureParams::kMipMap_FilterMode, + desc.fIsMipMapped) , fTextureView(view) , fLinearTextureView(nullptr) { this->registerWithCacheWrapped(); @@ -52,7 +54,8 @@ GrVkTexture::GrVkTexture(GrVkGpu* gpu, GrVkImage::Wrapped wrapped) : GrSurface(gpu, desc) , GrVkImage(info, wrapped) - , INHERITED(gpu, desc, kTexture2DSampler_GrSLType, desc.fIsMipMapped) + , INHERITED(gpu, desc, kTexture2DSampler_GrSLType, GrTextureParams::kMipMap_FilterMode, + desc.fIsMipMapped) , fTextureView(view) , fLinearTextureView(nullptr) { } diff --git a/tests/RectangleTextureTest.cpp b/tests/RectangleTextureTest.cpp index 10c392ddb0..21540d99c0 100644 --- a/tests/RectangleTextureTest.cpp +++ b/tests/RectangleTextureTest.cpp @@ -15,20 +15,19 @@ #include "gl/GLTestContext.h" static void test_read_pixels(skiatest::Reporter* reporter, GrContext* context, - GrTexture* rectangleTexture, uint32_t expectedPixelValues[]) { - int pixelCnt = rectangleTexture->width() * rectangleTexture->height(); + GrTexture* texture, uint32_t expectedPixelValues[]) { + int pixelCnt = texture->width() * texture->height(); SkAutoTMalloc pixels(pixelCnt); memset(pixels.get(), 0, sizeof(uint32_t)*pixelCnt); - bool read = rectangleTexture->readPixels(0, 0, rectangleTexture->width(), - rectangleTexture->height(), kRGBA_8888_GrPixelConfig, - pixels.get()); + bool read = texture->readPixels(0, 0, texture->width(), texture->height(), + kRGBA_8888_GrPixelConfig, pixels.get()); if (!read) { ERRORF(reporter, "Error reading rectangle texture."); } for (int i = 0; i < pixelCnt; ++i) { if (pixels.get()[i] != expectedPixelValues[i]) { - ERRORF(reporter, "Error, rectangle texture pixel value %d should be 0x%08x," - " got 0x%08x.", i, expectedPixelValues[i], pixels.get()[i]); + ERRORF(reporter, "Error, pixel value %d should be 0x%08x, got 0x%08x.", i, + expectedPixelValues[i], pixels.get()[i]); break; } } @@ -90,6 +89,29 @@ static void test_copy_surface_dst(skiatest::Reporter* reporter, GrContext* conte } } +// skbug.com/5932 +static void test_basic_draw(skiatest::Reporter* reporter, GrContext* context, + GrTexture* rectangleTexture, uint32_t expectedPixelValues[]) { + sk_sp rtContext( + context->makeRenderTargetContext(SkBackingFit::kExact, rectangleTexture->width(), + rectangleTexture->height(), rectangleTexture->config(), + nullptr)); + SkMatrix m; + m.setIDiv(rectangleTexture->width(), rectangleTexture->height()); + for (auto filter : {GrTextureParams::kNone_FilterMode, + GrTextureParams::kBilerp_FilterMode, + GrTextureParams::kMipMap_FilterMode}) { + rtContext->clear(nullptr, 0xDDCCBBAA, true); + sk_sp fp(GrSimpleTextureEffect::Make(rectangleTexture, + nullptr, m, filter)); + GrPaint paint; + paint.setPorterDuffXPFactory(SkBlendMode::kSrc); + paint.addColorFragmentProcessor(std::move(fp)); + rtContext->drawPaint(GrNoClip(), paint, SkMatrix::I()); + test_read_pixels(reporter, context, rtContext->asTexture().get(), expectedPixelValues); + } +} + static void test_clear(skiatest::Reporter* reporter, GrContext* context, GrTexture* rectangleTexture) { if (rectangleTexture->asRenderTarget()) { @@ -200,6 +222,8 @@ DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(RectangleTexture, reporter, ctxInfo) { test_read_pixels(reporter, context, rectangleTexture.get(), refPixels); + test_basic_draw(reporter, context, rectangleTexture.get(), refPixels); + test_copy_surface_src(reporter, context, rectangleTexture.get(), refPixels); test_copy_surface_dst(reporter, context, rectangleTexture.get());