From 372e3ed4bfc1008ad365918cf9e25c7ce7697799 Mon Sep 17 00:00:00 2001 From: Paul Bone Date: Wed, 24 Aug 2022 22:20:39 +0000 Subject: [PATCH] Bug 1781728 - Clamp WebGL mipmap levels r=jgilbert Differential Revision: https://phabricator.services.mozilla.com/D154524 --- dom/canvas/WebGLTexture.cpp | 80 +++++++++++++++++++++++-------------- dom/canvas/WebGLTexture.h | 10 +++++ 2 files changed, 59 insertions(+), 31 deletions(-) diff --git a/dom/canvas/WebGLTexture.cpp b/dom/canvas/WebGLTexture.cpp index e91dbd0bf879..9f2ac675b162 100644 --- a/dom/canvas/WebGLTexture.cpp +++ b/dom/canvas/WebGLTexture.cpp @@ -664,21 +664,12 @@ static bool ZeroTextureData(const WebGLContext* webgl, GLuint tex, } template -static R Clamp(const T val, const R min, const R max) { +static constexpr R Clamp(const T val, const R min, const R max) { if (val < min) return min; if (val > max) return max; return static_cast(val); } -template -static void ClampSelf(T* const out, const A min, const B max) { - if (*out < min) { - *out = T{min}; - } else if (*out > max) { - *out = T{max}; - } -} - void WebGLTexture::ClampLevelBaseAndMax() { if (!mImmutable) return; @@ -687,8 +678,20 @@ void WebGLTexture::ClampLevelBaseAndMax() { // `[0, levels-1]`, `level_max` is then clamped to the range ` // `[level_base, levels-1]`, where `levels` is the parameter passed to // TexStorage* for the texture object." - ClampSelf(&mBaseMipmapLevel, 0u, mImmutableLevelCount - 1u); - ClampSelf(&mMaxMipmapLevel, mBaseMipmapLevel, mImmutableLevelCount - 1u); + MOZ_ASSERT(mImmutableLevelCount > 0); + const auto oldBase = mBaseMipmapLevel; + const auto oldMax = mMaxMipmapLevel; + mBaseMipmapLevel = Clamp(mBaseMipmapLevel, 0u, mImmutableLevelCount - 1u); + mMaxMipmapLevel = + Clamp(mMaxMipmapLevel, mBaseMipmapLevel, mImmutableLevelCount - 1u); + if (oldBase != mBaseMipmapLevel && + mBaseMipmapLevelState != MIPMAP_LEVEL_DEFAULT) { + mBaseMipmapLevelState = MIPMAP_LEVEL_DIRTY; + } + if (oldMax != mMaxMipmapLevel && + mMaxMipmapLevelState != MIPMAP_LEVEL_DEFAULT) { + mMaxMipmapLevelState = MIPMAP_LEVEL_DIRTY; + } // Note: This means that immutable textures are *always* texture-complete! } @@ -729,6 +732,10 @@ bool WebGLTexture::BindTexture(TexTarget texTarget) { return true; } +static constexpr GLint ClampMipmapLevelForDriver(uint32_t level) { + return Clamp(level, uint8_t{0}, WebGLTexture::kMaxLevelCount); +} + void WebGLTexture::GenerateMipmap() { // GLES 3.0.4 p160: // "Mipmap generation replaces texel array levels level base + 1 through q @@ -806,21 +813,33 @@ void WebGLTexture::GenerateMipmap() { gl::GLContext* gl = mContext->gl; if (gl->WorkAroundDriverBugs()) { - // Don't generate mipmaps when the base level is out of range: - if (!(mImmutable && mBaseMipmapLevel >= mImmutableLevelCount)) { - // bug 696495 - to work around failures in the texture-mips.html test on - // various drivers, we set the minification filter before calling - // glGenerateMipmap. This should not carry a significant performance - // overhead so we do it unconditionally. - // - // note that the choice of GL_NEAREST_MIPMAP_NEAREST really matters. See - // Chromium bug 101105. - gl->fTexParameteri(mTarget.get(), LOCAL_GL_TEXTURE_MIN_FILTER, - LOCAL_GL_NEAREST_MIPMAP_NEAREST); - gl->fGenerateMipmap(mTarget.get()); - gl->fTexParameteri(mTarget.get(), LOCAL_GL_TEXTURE_MIN_FILTER, - mSamplingState.minFilter.get()); + // If we first set GL_TEXTURE_BASE_LEVEL to a number such as 20, then set + // MGL_TEXTURE_MAX_LEVEL to a smaller number like 8, our copy of the + // base level will be lowered, but we havn't yet updated the driver, we + // should do so now, before calling glGenerateMipmap(). + if (mBaseMipmapLevelState == MIPMAP_LEVEL_DIRTY) { + gl->fTexParameteri(mTarget.get(), LOCAL_GL_TEXTURE_BASE_LEVEL, + ClampMipmapLevelForDriver(mBaseMipmapLevel)); + mBaseMipmapLevelState = MIPMAP_LEVEL_CLEAN; } + if (mMaxMipmapLevelState == MIPMAP_LEVEL_DIRTY) { + gl->fTexParameteri(mTarget.get(), LOCAL_GL_TEXTURE_MAX_LEVEL, + ClampMipmapLevelForDriver(mMaxMipmapLevel)); + mMaxMipmapLevelState = MIPMAP_LEVEL_CLEAN; + } + + // bug 696495 - to work around failures in the texture-mips.html test on + // various drivers, we set the minification filter before calling + // glGenerateMipmap. This should not carry a significant performance + // overhead so we do it unconditionally. + // + // note that the choice of GL_NEAREST_MIPMAP_NEAREST really matters. See + // Chromium bug 101105. + gl->fTexParameteri(mTarget.get(), LOCAL_GL_TEXTURE_MIN_FILTER, + LOCAL_GL_NEAREST_MIPMAP_NEAREST); + gl->fGenerateMipmap(mTarget.get()); + gl->fTexParameteri(mTarget.get(), LOCAL_GL_TEXTURE_MIN_FILTER, + mSamplingState.minFilter.get()); } else { gl->fGenerateMipmap(mTarget.get()); } @@ -1031,18 +1050,17 @@ void WebGLTexture::TexParameter(TexTarget texTarget, GLenum pname, switch (pname) { case LOCAL_GL_TEXTURE_BASE_LEVEL: { mBaseMipmapLevel = clamped.i; + mBaseMipmapLevelState = MIPMAP_LEVEL_CLEAN; ClampLevelBaseAndMax(); - const auto forDriver = - Clamp(mBaseMipmapLevel, uint8_t{0}, kMaxLevelCount); - clamped = FloatOrInt(forDriver); + clamped = FloatOrInt(ClampMipmapLevelForDriver(mBaseMipmapLevel)); break; } case LOCAL_GL_TEXTURE_MAX_LEVEL: { mMaxMipmapLevel = clamped.i; + mMaxMipmapLevelState = MIPMAP_LEVEL_CLEAN; ClampLevelBaseAndMax(); - const auto forDriver = Clamp(mMaxMipmapLevel, uint8_t{0}, kMaxLevelCount); - clamped = FloatOrInt(forDriver); + clamped = FloatOrInt(ClampMipmapLevelForDriver(mMaxMipmapLevel)); break; } diff --git a/dom/canvas/WebGLTexture.h b/dom/canvas/WebGLTexture.h index fec5d5858438..8a451a927765 100644 --- a/dom/canvas/WebGLTexture.h +++ b/dom/canvas/WebGLTexture.h @@ -120,6 +120,16 @@ class WebGLTexture final : public WebGLContextBoundObject, // You almost certainly don't want to query mMaxMipmapLevel. // You almost certainly want MaxEffectiveMipmapLevel(). + // These "dirty" flags are set when the level is updated (eg indirectly by + // clamping) and cleared when we tell the driver. + enum MipmapLevelState : uint8_t { + MIPMAP_LEVEL_DEFAULT, + MIPMAP_LEVEL_CLEAN, + MIPMAP_LEVEL_DIRTY + }; + MipmapLevelState mBaseMipmapLevelState = MIPMAP_LEVEL_DEFAULT; + MipmapLevelState mMaxMipmapLevelState = MIPMAP_LEVEL_DEFAULT; + webgl::SamplingState mSamplingState; mutable const GLint* mCurSwizzle =