OpenGL: Use non-filtering samplers for int/uint texture.

Using a sampler with filtering on int / uint textures makes them
incomplete, causing them to sample black on very conformant drivers.
Each opengl::Sampler is updated to create to GL sampler, a filtering one
and a non-filtering one.

PipelineGL and CommandBufferGL takes advantage of the new
BGLBinding::textureComponentType to know which of the two samplers to
use.

BUG=dawn:128

Change-Id: Idbf5668213bbe6a8639847d57e2be1244f97800c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10282
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn_native/opengl/CommandBufferGL.cpp

@@ -246,12 +246,18 @@ namespace dawn_native { namespace opengl {
                     } break;
 
                     case dawn::BindingType::Sampler: {
-                        GLuint sampler =
+                        Sampler* sampler = ToBackend(group->GetBindingAsSampler(bindingIndex));
-                            ToBackend(group->GetBindingAsSampler(bindingIndex))->GetHandle();
                         GLuint samplerIndex = indices[bindingIndex];
 
-                        for (auto unit : pipeline->GetTextureUnitsForSampler(samplerIndex)) {
+                        for (PipelineGL::SamplerUnit unit :
-                            gl.BindSampler(unit, sampler);
+                             pipeline->GetTextureUnitsForSampler(samplerIndex)) {
+                            // Only use filtering for certain texture units, because int and uint
+                            // texture are only complete without filtering
+                            if (unit.shouldUseFiltering) {
+                                gl.BindSampler(unit.unit, sampler->GetFilteringHandle());
+                            } else {
+                                gl.BindSampler(unit.unit, sampler->GetNonFilteringHandle());
+                            }
                         }
                     } break;
 

src/dawn_native/opengl/PipelineGL.cpp

@@ -173,20 +173,27 @@ namespace dawn_native { namespace opengl {
 
                 gl.Uniform1i(location, textureUnit);
 
-                GLuint samplerIndex =
-                    indices[combined.samplerLocation.group][combined.samplerLocation.binding];
-                mUnitsForSamplers[samplerIndex].push_back(textureUnit);
-
                 GLuint textureIndex =
                     indices[combined.textureLocation.group][combined.textureLocation.binding];
                 mUnitsForTextures[textureIndex].push_back(textureUnit);
 
+                dawn::TextureComponentType componentType =
+                    layout->GetBindGroupLayout(combined.textureLocation.group)
+                        ->GetBindingInfo()
+                        .textureComponentTypes[combined.textureLocation.binding];
+                bool shouldUseFiltering = componentType == dawn::TextureComponentType::Float;
+
+                GLuint samplerIndex =
+                    indices[combined.samplerLocation.group][combined.samplerLocation.binding];
+                mUnitsForSamplers[samplerIndex].push_back({textureUnit, shouldUseFiltering});
+
                 textureUnit++;
             }
         }
     }
 
-    const std::vector<GLuint>& PipelineGL::GetTextureUnitsForSampler(GLuint index) const {
+    const std::vector<PipelineGL::SamplerUnit>& PipelineGL::GetTextureUnitsForSampler(
+        GLuint index) const {
         ASSERT(index < mUnitsForSamplers.size());
         return mUnitsForSamplers[index];
     }

src/dawn_native/opengl/PipelineGL.h

@@ -39,7 +39,13 @@ namespace dawn_native { namespace opengl {
         using BindingLocations =
             std::array<std::array<GLint, kMaxBindingsPerGroup>, kMaxBindGroups>;
 
-        const std::vector<GLuint>& GetTextureUnitsForSampler(GLuint index) const;
+        // For each unit a sampler is bound to we need to know if we should use filtering or not
+        // because int and uint texture are only complete without filtering.
+        struct SamplerUnit {
+            GLuint unit;
+            bool shouldUseFiltering;
+        };
+        const std::vector<SamplerUnit>& GetTextureUnitsForSampler(GLuint index) const;
         const std::vector<GLuint>& GetTextureUnitsForTextureView(GLuint index) const;
         GLuint GetProgramHandle() const;
 
@@ -47,7 +53,7 @@ namespace dawn_native { namespace opengl {
 
       private:
         GLuint mProgram;
-        std::vector<std::vector<GLuint>> mUnitsForSamplers;
+        std::vector<std::vector<SamplerUnit>> mUnitsForSamplers;
         std::vector<std::vector<GLuint>> mUnitsForTextures;
     };
 

src/dawn_native/opengl/SamplerGL.cpp

@@ -76,26 +76,53 @@ namespace dawn_native { namespace opengl {
         : SamplerBase(device, descriptor) {
         const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
 
-        gl.GenSamplers(1, &mHandle);
+        gl.GenSamplers(1, &mFilteringHandle);
-        gl.SamplerParameteri(mHandle, GL_TEXTURE_MAG_FILTER, MagFilterMode(descriptor->magFilter));
+        SetupGLSampler(mFilteringHandle, descriptor, false);
-        gl.SamplerParameteri(mHandle, GL_TEXTURE_MIN_FILTER,
-                             MinFilterMode(descriptor->minFilter, descriptor->mipmapFilter));
-        gl.SamplerParameteri(mHandle, GL_TEXTURE_WRAP_R, WrapMode(descriptor->addressModeW));
-        gl.SamplerParameteri(mHandle, GL_TEXTURE_WRAP_S, WrapMode(descriptor->addressModeU));
-        gl.SamplerParameteri(mHandle, GL_TEXTURE_WRAP_T, WrapMode(descriptor->addressModeV));
 
-        gl.SamplerParameterf(mHandle, GL_TEXTURE_MIN_LOD, descriptor->lodMinClamp);
+        gl.GenSamplers(1, &mNonFilteringHandle);
-        gl.SamplerParameterf(mHandle, GL_TEXTURE_MAX_LOD, descriptor->lodMaxClamp);
+        SetupGLSampler(mNonFilteringHandle, descriptor, true);
+    }
+
+    Sampler::~Sampler() {
+        const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
+        gl.DeleteSamplers(1, &mFilteringHandle);
+        gl.DeleteSamplers(1, &mNonFilteringHandle);
+    }
+
+    void Sampler::SetupGLSampler(GLuint sampler,
+                                 const SamplerDescriptor* descriptor,
+                                 bool forceNearest) {
+        const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
+
+        if (forceNearest) {
+            gl.SamplerParameteri(sampler, GL_TEXTURE_MAG_FILTER, GL_NEAREST);
+            gl.SamplerParameteri(sampler, GL_TEXTURE_MIN_FILTER, GL_NEAREST_MIPMAP_NEAREST);
+        } else {
+            gl.SamplerParameteri(sampler, GL_TEXTURE_MAG_FILTER,
+                                 MagFilterMode(descriptor->magFilter));
+            gl.SamplerParameteri(sampler, GL_TEXTURE_MIN_FILTER,
+                                 MinFilterMode(descriptor->minFilter, descriptor->mipmapFilter));
+        }
+        gl.SamplerParameteri(sampler, GL_TEXTURE_WRAP_R, WrapMode(descriptor->addressModeW));
+        gl.SamplerParameteri(sampler, GL_TEXTURE_WRAP_S, WrapMode(descriptor->addressModeU));
+        gl.SamplerParameteri(sampler, GL_TEXTURE_WRAP_T, WrapMode(descriptor->addressModeV));
+
+        gl.SamplerParameterf(sampler, GL_TEXTURE_MIN_LOD, descriptor->lodMinClamp);
+        gl.SamplerParameterf(sampler, GL_TEXTURE_MAX_LOD, descriptor->lodMaxClamp);
 
         if (ToOpenGLCompareFunction(descriptor->compare) != GL_NEVER) {
-            gl.SamplerParameteri(mHandle, GL_TEXTURE_COMPARE_MODE, GL_COMPARE_REF_TO_TEXTURE);
+            gl.SamplerParameteri(sampler, GL_TEXTURE_COMPARE_MODE, GL_COMPARE_REF_TO_TEXTURE);
-            gl.SamplerParameteri(mHandle, GL_TEXTURE_COMPARE_FUNC,
+            gl.SamplerParameteri(sampler, GL_TEXTURE_COMPARE_FUNC,
                                  ToOpenGLCompareFunction(descriptor->compare));
         }
     }
 
-    GLuint Sampler::GetHandle() const {
+    GLuint Sampler::GetFilteringHandle() const {
-        return mHandle;
+        return mFilteringHandle;
+    }
+
+    GLuint Sampler::GetNonFilteringHandle() const {
+        return mNonFilteringHandle;
     }
 
 }}  // namespace dawn_native::opengl

src/dawn_native/opengl/SamplerGL.h

@@ -26,11 +26,19 @@ namespace dawn_native { namespace opengl {
     class Sampler : public SamplerBase {
       public:
         Sampler(Device* device, const SamplerDescriptor* descriptor);
+        ~Sampler();
 
-        GLuint GetHandle() const;
+        GLuint GetFilteringHandle() const;
+        GLuint GetNonFilteringHandle() const;
 
       private:
-        GLuint mHandle;
+        void SetupGLSampler(GLuint sampler, const SamplerDescriptor* descriptor, bool forceNearest);
+
+        GLuint mFilteringHandle;
+
+        // This is a sampler equivalent to mFilteringHandle except that it uses NEAREST filtering
+        // for everything, which is important to preserve texture completeness for u/int textures.
+        GLuint mNonFilteringHandle;
     };
 
 }}  // namespace dawn_native::opengl

src/tests/end2end/TextureFormatTests.cpp

@@ -498,146 +498,92 @@ TEST_P(TextureFormatTest, RGBA8Snorm) {
 
 // Test the R8Uint format
 TEST_P(TextureFormatTest, R8Uint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoUintTest<uint8_t>({dawn::TextureFormat::R8Uint, 1, dawn::TextureComponentType::Uint, 1});
 }
 
 // Test the RG8Uint format
 TEST_P(TextureFormatTest, RG8Uint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoUintTest<uint8_t>({dawn::TextureFormat::RG8Uint, 2, dawn::TextureComponentType::Uint, 2});
 }
 
 // Test the RGBA8Uint format
 TEST_P(TextureFormatTest, RGBA8Uint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoUintTest<uint8_t>({dawn::TextureFormat::RGBA8Uint, 4, dawn::TextureComponentType::Uint, 4});
 }
 
 // Test the R16Uint format
 TEST_P(TextureFormatTest, R16Uint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoUintTest<uint16_t>({dawn::TextureFormat::R16Uint, 2, dawn::TextureComponentType::Uint, 1});
 }
 
 // Test the RG16Uint format
 TEST_P(TextureFormatTest, RG16Uint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoUintTest<uint16_t>({dawn::TextureFormat::RG16Uint, 4, dawn::TextureComponentType::Uint, 2});
 }
 
 // Test the RGBA16Uint format
 TEST_P(TextureFormatTest, RGBA16Uint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoUintTest<uint16_t>({dawn::TextureFormat::RGBA16Uint, 8, dawn::TextureComponentType::Uint, 4});
 }
 
 // Test the R32Uint format
 TEST_P(TextureFormatTest, R32Uint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoUintTest<uint32_t>({dawn::TextureFormat::R32Uint, 4, dawn::TextureComponentType::Uint, 1});
 }
 
 // Test the RG32Uint format
 TEST_P(TextureFormatTest, RG32Uint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoUintTest<uint32_t>({dawn::TextureFormat::RG32Uint, 8, dawn::TextureComponentType::Uint, 2});
 }
 
 // Test the RGBA32Uint format
 TEST_P(TextureFormatTest, RGBA32Uint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoUintTest<uint32_t>(
         {dawn::TextureFormat::RGBA32Uint, 16, dawn::TextureComponentType::Uint, 4});
 }
 
 // Test the R8Sint format
 TEST_P(TextureFormatTest, R8Sint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoSintTest<int8_t>({dawn::TextureFormat::R8Sint, 1, dawn::TextureComponentType::Sint, 1});
 }
 
 // Test the RG8Sint format
 TEST_P(TextureFormatTest, RG8Sint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoSintTest<int8_t>({dawn::TextureFormat::RG8Sint, 2, dawn::TextureComponentType::Sint, 2});
 }
 
 // Test the RGBA8Sint format
 TEST_P(TextureFormatTest, RGBA8Sint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoSintTest<int8_t>({dawn::TextureFormat::RGBA8Sint, 4, dawn::TextureComponentType::Sint, 4});
 }
 
 // Test the R16Sint format
 TEST_P(TextureFormatTest, R16Sint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoSintTest<int16_t>({dawn::TextureFormat::R16Sint, 2, dawn::TextureComponentType::Sint, 1});
 }
 
 // Test the RG16Sint format
 TEST_P(TextureFormatTest, RG16Sint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoSintTest<int16_t>({dawn::TextureFormat::RG16Sint, 4, dawn::TextureComponentType::Sint, 2});
 }
 
 // Test the RGBA16Sint format
 TEST_P(TextureFormatTest, RGBA16Sint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoSintTest<int16_t>({dawn::TextureFormat::RGBA16Sint, 8, dawn::TextureComponentType::Sint, 4});
 }
 
 // Test the R32Sint format
 TEST_P(TextureFormatTest, R32Sint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoSintTest<int32_t>({dawn::TextureFormat::R32Sint, 4, dawn::TextureComponentType::Sint, 1});
 }
 
 // Test the RG32Sint format
 TEST_P(TextureFormatTest, RG32Sint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoSintTest<int32_t>({dawn::TextureFormat::RG32Sint, 8, dawn::TextureComponentType::Sint, 2});
 }
 
 // Test the RGBA32Sint format
 TEST_P(TextureFormatTest, RGBA32Sint) {
-    // TODO(cwallez@chromium.org): This fails on the Intel GL driver, understand why.
-    DAWN_SKIP_TEST_IF(IsOpenGL() && IsIntel());
-
     DoSintTest<int32_t>({dawn::TextureFormat::RGBA32Sint, 16, dawn::TextureComponentType::Sint, 4});
 }