From ba7ebf5ea6b65af907e43ea9bffa24cef67ec155 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Thu, 8 Apr 2021 17:27:39 -0700 Subject: [PATCH] better handle anisotropic filtering with various drivers Some drivers claim to support anisotropic filtering but don't allow setting it via glSamplerParameter. Until now we were disabling anisotropic filtering entirely on these drivers at compile time. With this change, we should detect at runtime which drivers have this issue and use a workaround -- instead of disabling anisotropic filtering entirely. --- filament/backend/src/opengl/OpenGLContext.cpp | 19 +++++++++++--- filament/backend/src/opengl/OpenGLContext.h | 2 +- filament/backend/src/opengl/OpenGLDriver.cpp | 13 +++++++--- filament/backend/src/opengl/OpenGLDriver.h | 1 + filament/backend/src/opengl/OpenGLProgram.cpp | 25 ++++++++++++++----- filament/backend/src/opengl/OpenGLProgram.h | 2 +- 6 files changed, 47 insertions(+), 15 deletions(-) diff --git a/filament/backend/src/opengl/OpenGLContext.cpp b/filament/backend/src/opengl/OpenGLContext.cpp index f5ce85266c..71c63516d7 100644 --- a/filament/backend/src/opengl/OpenGLContext.cpp +++ b/filament/backend/src/opengl/OpenGLContext.cpp @@ -70,7 +70,6 @@ OpenGLContext::OpenGLContext() noexcept { bugs.dont_use_timer_query = true; } if (strstr(renderer, "Mali-G")) { - bugs.disable_texture_filter_anisotropic = true; // note: We have verified that timer queries work well at least on some Mali-G. } } else if (strstr(renderer, "Intel")) { @@ -83,9 +82,9 @@ OpenGLContext::OpenGLContext() noexcept { bugs.disable_invalidate_framebuffer = true; } +#if defined(__EMSCRIPTEN__) // Chrome does not support feedback loops in WebGL 2.0. See also: // https://bugs.chromium.org/p/chromium/issues/detail?id=1066201 -#if defined(__EMSCRIPTEN__) bugs.disable_feedback_loops = true; #endif @@ -147,8 +146,22 @@ OpenGLContext::OpenGLContext() noexcept { #endif #ifdef GL_EXT_texture_filter_anisotropic - if (ext.EXT_texture_filter_anisotropic && !bugs.disable_texture_filter_anisotropic) { + if (ext.EXT_texture_filter_anisotropic) { + // make sure we don't have any error flag + while (glGetError() != GL_NO_ERROR) { } + + // query max anisotropy glGetFloatv(GL_MAX_TEXTURE_MAX_ANISOTROPY_EXT, &gets.maxAnisotropy); + + // check that we can actually set the anysotropy on the sampler + GLuint s; + glGenSamplers(1, &s); + glSamplerParameterf(s, GL_TEXTURE_MAX_ANISOTROPY_EXT, gets.maxAnisotropy); + if (glGetError() != GL_NO_ERROR) { + // some drivers only allow to set the anisotropy on the texture itself + bugs.texture_filter_anisotropic_broken_on_sampler = true; + } + glDeleteSamplers(1, &s); } #endif diff --git a/filament/backend/src/opengl/OpenGLContext.h b/filament/backend/src/opengl/OpenGLContext.h index ded3e831ad..0ed40d1c41 100644 --- a/filament/backend/src/opengl/OpenGLContext.h +++ b/filament/backend/src/opengl/OpenGLContext.h @@ -169,7 +169,7 @@ public: // Some drivers declare GL_EXT_texture_filter_anisotropic but don't support // calling glSamplerParameter() with GL_TEXTURE_MAX_ANISOTROPY_EXT - bool disable_texture_filter_anisotropic = false; + bool texture_filter_anisotropic_broken_on_sampler = false; // Some drivers have issues when reading from a mip while writing to a different mip. // In the OpenGL ES 3.0 specification this is covered in section 4.4.3, diff --git a/filament/backend/src/opengl/OpenGLDriver.cpp b/filament/backend/src/opengl/OpenGLDriver.cpp index 6adc3875c8..b6d166021d 100644 --- a/filament/backend/src/opengl/OpenGLDriver.cpp +++ b/filament/backend/src/opengl/OpenGLDriver.cpp @@ -211,6 +211,10 @@ ShaderModel OpenGLDriver::getShaderModel() const noexcept { // Change and track GL state // ------------------------------------------------------------------------------------------------ +void OpenGLDriver::bindSampler(GLuint unit, backend::SamplerParams params) noexcept { + mContext.bindSampler(unit, getSampler(params)); +} + void OpenGLDriver::bindTexture(GLuint unit, GLTexture const* t) noexcept { assert_invariant(t != nullptr); mContext.bindTexture(unit, t->gl.target, t->gl.id, t->gl.targetIndex); @@ -2813,12 +2817,13 @@ GLuint OpenGLDriver::getSamplerSlow(SamplerParams params) const noexcept { glSamplerParameteri(s, GL_TEXTURE_COMPARE_MODE, getTextureCompareMode(params.compareMode)); glSamplerParameteri(s, GL_TEXTURE_COMPARE_FUNC, getTextureCompareFunc(params.compareFunc)); -// TODO: Why does this fail with WebGL 2.0? The run-time check should suffice. -#if defined(GL_EXT_texture_filter_anisotropic) && !defined(__EMSCRIPTEN__) +#if defined(GL_EXT_texture_filter_anisotropic) auto& gl = mContext; - if (gl.ext.EXT_texture_filter_anisotropic && !gl.bugs.disable_texture_filter_anisotropic) { + if (gl.ext.EXT_texture_filter_anisotropic && + !gl.bugs.texture_filter_anisotropic_broken_on_sampler) { GLfloat anisotropy = float(1u << params.anisotropyLog2); - glSamplerParameterf(s, GL_TEXTURE_MAX_ANISOTROPY_EXT, std::min(gl.gets.maxAnisotropy, anisotropy)); + glSamplerParameterf(s, GL_TEXTURE_MAX_ANISOTROPY_EXT, + std::min(gl.gets.maxAnisotropy, anisotropy)); } #endif CHECK_GL_ERROR(utils::slog.e) diff --git a/filament/backend/src/opengl/OpenGLDriver.h b/filament/backend/src/opengl/OpenGLDriver.h index 318d249ac8..7a8c7347fb 100644 --- a/filament/backend/src/opengl/OpenGLDriver.h +++ b/filament/backend/src/opengl/OpenGLDriver.h @@ -357,6 +357,7 @@ private: /* State tracking GL wrappers... */ void bindTexture(GLuint unit, GLTexture const* t) noexcept; + void bindSampler(GLuint unit, backend::SamplerParams params) noexcept; inline void useProgram(OpenGLProgram* p) noexcept; enum class ResolveAction { LOAD, STORE }; diff --git a/filament/backend/src/opengl/OpenGLProgram.cpp b/filament/backend/src/opengl/OpenGLProgram.cpp index f36da93286..f6a6656b6e 100644 --- a/filament/backend/src/opengl/OpenGLProgram.cpp +++ b/filament/backend/src/opengl/OpenGLProgram.cpp @@ -195,11 +195,14 @@ OpenGLProgram::~OpenGLProgram() noexcept { } } -void OpenGLProgram::updateSamplers(OpenGLDriver* gl) noexcept { +void OpenGLProgram::updateSamplers(OpenGLDriver* gld) noexcept { using GLTexture = OpenGLDriver::GLTexture; // cache a few member variable locally, outside of the loop - auto const& UTILS_RESTRICT samplerBindings = gl->getSamplerBindings(); + OpenGLContext& glc = gld->getContext(); + const bool anisotropyWorkaround = glc.ext.EXT_texture_filter_anisotropic && + glc.bugs.texture_filter_anisotropic_broken_on_sampler; + auto const& UTILS_RESTRICT samplerBindings = gld->getSamplerBindings(); auto const& UTILS_RESTRICT indicesRun = mIndicesRuns; auto const& UTILS_RESTRICT blockInfos = mBlockInfos; @@ -222,17 +225,27 @@ void OpenGLProgram::updateSamplers(OpenGLDriver* gl) noexcept { continue; } - const GLTexture* const UTILS_RESTRICT t = gl->handle_cast(th); + const GLTexture* const UTILS_RESTRICT t = gld->handle_cast(th); if (UTILS_UNLIKELY(t->gl.fence)) { glWaitSync(t->gl.fence, 0, GL_TIMEOUT_IGNORED); glDeleteSync(t->gl.fence); t->gl.fence = nullptr; } - gl->bindTexture(tmu, t); + gld->bindTexture(tmu, t); + gld->bindSampler(tmu, samplers[index].s); - GLuint sampler = gl->getSampler(samplers[index].s); - gl->getContext().bindSampler(tmu, sampler); +#if defined(GL_EXT_texture_filter_anisotropic) + if (UTILS_UNLIKELY(anisotropyWorkaround)) { + // Driver claims to support anisotropic filtering, but it fails when set on + // the sampler, we have to set it on the texture instead. + // The texture is already bound here. + SamplerParams params = samplers[index].s; + GLfloat anisotropy = float(1u << params.anisotropyLog2); + glTexParameterf(t->gl.target, GL_TEXTURE_MAX_ANISOTROPY_EXT, + std::min(glc.gets.maxAnisotropy, anisotropy)); + } +#endif } } CHECK_GL_ERROR(utils::slog.e) diff --git a/filament/backend/src/opengl/OpenGLProgram.h b/filament/backend/src/opengl/OpenGLProgram.h index fbe782335e..5fa16f88a7 100644 --- a/filament/backend/src/opengl/OpenGLProgram.h +++ b/filament/backend/src/opengl/OpenGLProgram.h @@ -95,7 +95,7 @@ private: // runs of indices into SamplerGroup -- run start index and size given by BlockInfo std::array mIndicesRuns; // 16 bytes - void updateSamplers(OpenGLDriver* gl) noexcept; + void updateSamplers(OpenGLDriver* gld) noexcept; };