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=4352

Change-Id: I1f023d4f4133e7eb393367580c0558257e56c8db
Reviewed-on: https://skia-review.googlesource.com/4352
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Chris Dalton <csmartdalton@google.com>

diff --git a/include/gpu/GrTexture.h b/include/gpu/GrTexture.h
index 211f193..8fa0db9 100644 (file)
--- 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 3b0e54b..fa728ed 100644 (file)
--- 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 de1135a..6fc1580 100644 (file)
--- 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 675bc20..bddbd5a 100644 (file)
--- 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 c4e6538..c4c1855 100644 (file)
--- 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 8242362..fced736 100644 (file)
--- a/src/gpu/effects/GrSimpleTextureEffect.h
+++ b/src/gpu/effects/GrSimpleTextureEffect.h
@@ -31,8 +31,8 @@ public:
     /* clamp mode */
     static sk_sp<GrFragmentProcessor> Make(GrTexture* tex,
                                            sk_sp<GrColorSpaceXform> colorSpaceXform,
-                                            const SkMatrix& matrix,
-                                            GrTextureParams::FilterMode filterMode) {
+                                           const SkMatrix& matrix,
+                                           GrTextureParams::FilterMode filterMode) {
         return sk_sp<GrFragmentProcessor>(
             new GrSimpleTextureEffect(tex, std::move(colorSpaceXform), matrix, filterMode));
     }

diff --git a/src/gpu/gl/GrGLTexture.cpp b/src/gpu/gl/GrGLTexture.cpp
index ec0ad3b..c45d08f 100644 (file)
--- a/src/gpu/gl/GrGLTexture.cpp
+++ b/src/gpu/gl/GrGLTexture.cpp
@@ -12,7 +12,7 @@
 #define GPUGL static_cast<GrGLGpu*>(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 abfeafd..5b1fbe1 100644 (file)
--- 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 8c461f8..3b6108c 100644 (file)
--- 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 08311e4..b542986 100644 (file)
--- 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<uint32_t> 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<GrRenderTargetContext> 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<GrFragmentProcessor> 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());