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>
diff --git a/src/dawn_native/opengl/CommandBufferGL.cpp b/src/dawn_native/opengl/CommandBufferGL.cpp
index 9fafb6f..2dccfe5 100644
--- a/src/dawn_native/opengl/CommandBufferGL.cpp
+++ b/src/dawn_native/opengl/CommandBufferGL.cpp
@@ -246,12 +246,18 @@
} break;
case dawn::BindingType::Sampler: {
- GLuint sampler =
- ToBackend(group->GetBindingAsSampler(bindingIndex))->GetHandle();
+ Sampler* sampler = ToBackend(group->GetBindingAsSampler(bindingIndex));
GLuint samplerIndex = indices[bindingIndex];
- for (auto unit : pipeline->GetTextureUnitsForSampler(samplerIndex)) {
- gl.BindSampler(unit, sampler);
+ for (PipelineGL::SamplerUnit unit :
+ 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;
diff --git a/src/dawn_native/opengl/PipelineGL.cpp b/src/dawn_native/opengl/PipelineGL.cpp
index e04dbc7..f7adf95 100644
--- a/src/dawn_native/opengl/PipelineGL.cpp
+++ b/src/dawn_native/opengl/PipelineGL.cpp
@@ -173,20 +173,27 @@
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];
}
diff --git a/src/dawn_native/opengl/PipelineGL.h b/src/dawn_native/opengl/PipelineGL.h
index b574589..6a081c1 100644
--- a/src/dawn_native/opengl/PipelineGL.h
+++ b/src/dawn_native/opengl/PipelineGL.h
@@ -39,7 +39,13 @@
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 @@
private:
GLuint mProgram;
- std::vector<std::vector<GLuint>> mUnitsForSamplers;
+ std::vector<std::vector<SamplerUnit>> mUnitsForSamplers;
std::vector<std::vector<GLuint>> mUnitsForTextures;
};
diff --git a/src/dawn_native/opengl/SamplerGL.cpp b/src/dawn_native/opengl/SamplerGL.cpp
index 832a4ff..17bf353 100644
--- a/src/dawn_native/opengl/SamplerGL.cpp
+++ b/src/dawn_native/opengl/SamplerGL.cpp
@@ -76,26 +76,53 @@
: SamplerBase(device, descriptor) {
const OpenGLFunctions& gl = ToBackend(GetDevice())->gl;
- gl.GenSamplers(1, &mHandle);
- gl.SamplerParameteri(mHandle, GL_TEXTURE_MAG_FILTER, MagFilterMode(descriptor->magFilter));
- 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.GenSamplers(1, &mFilteringHandle);
+ SetupGLSampler(mFilteringHandle, descriptor, false);
- gl.SamplerParameterf(mHandle, GL_TEXTURE_MIN_LOD, descriptor->lodMinClamp);
- gl.SamplerParameterf(mHandle, GL_TEXTURE_MAX_LOD, descriptor->lodMaxClamp);
+ gl.GenSamplers(1, &mNonFilteringHandle);
+ 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(mHandle, GL_TEXTURE_COMPARE_FUNC,
+ gl.SamplerParameteri(sampler, GL_TEXTURE_COMPARE_MODE, GL_COMPARE_REF_TO_TEXTURE);
+ gl.SamplerParameteri(sampler, GL_TEXTURE_COMPARE_FUNC,
ToOpenGLCompareFunction(descriptor->compare));
}
}
- GLuint Sampler::GetHandle() const {
- return mHandle;
+ GLuint Sampler::GetFilteringHandle() const {
+ return mFilteringHandle;
+ }
+
+ GLuint Sampler::GetNonFilteringHandle() const {
+ return mNonFilteringHandle;
}
}} // namespace dawn_native::opengl
diff --git a/src/dawn_native/opengl/SamplerGL.h b/src/dawn_native/opengl/SamplerGL.h
index 3c4911b..19b185b 100644
--- a/src/dawn_native/opengl/SamplerGL.h
+++ b/src/dawn_native/opengl/SamplerGL.h
@@ -26,11 +26,19 @@
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
diff --git a/src/tests/end2end/TextureFormatTests.cpp b/src/tests/end2end/TextureFormatTests.cpp
index b530c696..79029cf 100644
--- a/src/tests/end2end/TextureFormatTests.cpp
+++ b/src/tests/end2end/TextureFormatTests.cpp
@@ -498,146 +498,92 @@
// 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});
}
