Fix mipmap generation on undefined cube texture

Cube textures must be cube complete to generate mipmaps, but when the
base level is undefined the glGenerateMipmap command must be silently
ignored. This was previously leading to a null dereference.

Bug chromium:924022
Bug https://gitlab.khronos.org/opengl/API/issues/72

Change-Id: I5d6e8533118e554efa12045fc376126c7b00f263
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/27491
Presubmit-Ready: Nicolas Capens <nicolascapens@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Kokoro-Presubmit: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>

diff --git a/src/OpenGL/libGLESv2/Context.cpp b/src/OpenGL/libGLESv2/Context.cpp
index 23f004d..310209f 100644 (file)
--- a/src/OpenGL/libGLESv2/Context.cpp
+++ b/src/OpenGL/libGLESv2/Context.cpp
@@ -1645,6 +1645,27 @@ Program *Context::getCurrentProgram() const
        return mResourceManager->getProgram(mState.currentProgram);
 }
 
+Texture *Context::getTargetTexture(GLenum target) const
+{
+       Texture *texture = nullptr;
+
+       switch(target)
+       {
+       case GL_TEXTURE_2D:            texture = getTexture2D();       break;
+       case GL_TEXTURE_2D_ARRAY:      texture = getTexture2DArray();  break;
+       case GL_TEXTURE_3D:            texture = getTexture3D();       break;
+       case GL_TEXTURE_CUBE_MAP:      texture = getTextureCubeMap();  break;
+       case GL_TEXTURE_EXTERNAL_OES:  texture = getTextureExternal(); break;
+       case GL_TEXTURE_RECTANGLE_ARB: texture = getTexture2DRect();   break;
+       default:
+               return error(GL_INVALID_ENUM, nullptr);
+       }
+
+       ASSERT(texture);  // Must always have a default texture to fall back to.
+
+       return texture;
+}
+
 Texture2D *Context::getTexture2D() const
 {
        return static_cast<Texture2D*>(getSamplerTexture(mState.activeSampler, TEXTURE_2D));

diff --git a/src/OpenGL/libGLESv2/Context.h b/src/OpenGL/libGLESv2/Context.h
index bb85011..b41fbef 100644 (file)
--- a/src/OpenGL/libGLESv2/Context.h
+++ b/src/OpenGL/libGLESv2/Context.h
@@ -640,6 +640,7 @@ public:
        GLenum getPixels(const GLvoid **data, GLenum type, GLsizei imageSize) const;
        bool getBuffer(GLenum target, es2::Buffer **buffer) const;
        Program *getCurrentProgram() const;
+       Texture *getTargetTexture(GLenum target) const;
        Texture2D *getTexture2D() const;
        Texture2D *getTexture2D(GLenum target) const;
        Texture3D *getTexture3D() const;

diff --git a/src/OpenGL/libGLESv2/Texture.cpp b/src/OpenGL/libGLESv2/Texture.cpp
index 0266158..a02a7a0 100644 (file)
--- a/src/OpenGL/libGLESv2/Texture.cpp
+++ b/src/OpenGL/libGLESv2/Texture.cpp
@@ -686,8 +686,7 @@ void Texture2D::setSharedImage(egl::Image *sharedImage)
        image[0] = sharedImage;
 }
 
-// Tests for 2D texture sampling completeness. [OpenGL ES 3.0.5] section 3.8.13 page 160.
-bool Texture2D::isSamplerComplete(Sampler *sampler) const
+bool Texture2D::isBaseLevelDefined() const
 {
        if(!image[mBaseLevel])
        {
@@ -702,6 +701,17 @@ bool Texture2D::isSamplerComplete(Sampler *sampler) const
                return false;
        }
 
+       return true;
+}
+
+// Tests for 2D texture sampling completeness. [OpenGL ES 3.0.5] section 3.8.13 page 160.
+bool Texture2D::isSamplerComplete(Sampler *sampler) const
+{
+       if(!isBaseLevelDefined())
+       {
+               return false;
+       }
+
        if(isMipmapFiltered(sampler))
        {
                if(!isMipmapComplete())
@@ -1042,8 +1052,7 @@ void TextureCubeMap::subImageCompressed(GLenum target, GLint level, GLint xoffse
        Texture::subImageCompressed(xoffset, yoffset, 0, width, height, 1, format, imageSize, pixels, image[CubeFaceIndex(target)][level]);
 }
 
-// Tests for cube map sampling completeness. [OpenGL ES 3.0.5] section 3.8.13 page 161.
-bool TextureCubeMap::isSamplerComplete(Sampler *sampler) const
+bool TextureCubeMap::isBaseLevelDefined() const
 {
        for(int face = 0; face < 6; face++)
        {
@@ -1060,6 +1069,17 @@ bool TextureCubeMap::isSamplerComplete(Sampler *sampler) const
                return false;
        }
 
+       return true;
+}
+
+// Tests for cube map sampling completeness. [OpenGL ES 3.0.5] section 3.8.13 page 161.
+bool TextureCubeMap::isSamplerComplete(Sampler *sampler) const
+{
+       if(!isBaseLevelDefined())
+       {
+               return false;
+       }
+
        if(!isMipmapFiltered(sampler))
        {
                if(!isCubeComplete())
@@ -1081,6 +1101,11 @@ bool TextureCubeMap::isSamplerComplete(Sampler *sampler) const
 // Tests for cube texture completeness. [OpenGL ES 3.0.5] section 3.8.13 page 160.
 bool TextureCubeMap::isCubeComplete() const
 {
+       if(!isBaseLevelDefined())
+       {
+               return false;
+       }
+
        if(image[0][mBaseLevel]->getWidth() <= 0 || image[0][mBaseLevel]->getHeight() != image[0][mBaseLevel]->getWidth())
        {
                return false;
@@ -1649,8 +1674,7 @@ void Texture3D::setSharedImage(egl::Image *sharedImage)
        image[0] = sharedImage;
 }
 
-// Tests for 3D texture sampling completeness. [OpenGL ES 3.0.5] section 3.8.13 page 160.
-bool Texture3D::isSamplerComplete(Sampler *sampler) const
+bool Texture3D::isBaseLevelDefined() const
 {
        if(!image[mBaseLevel])
        {
@@ -1666,6 +1690,17 @@ bool Texture3D::isSamplerComplete(Sampler *sampler) const
                return false;
        }
 
+       return true;
+}
+
+// Tests for 3D texture sampling completeness. [OpenGL ES 3.0.5] section 3.8.13 page 160.
+bool Texture3D::isSamplerComplete(Sampler *sampler) const
+{
+       if(!isBaseLevelDefined())
+       {
+               return false;
+       }
+
        if(isMipmapFiltered(sampler))
        {
                if(!isMipmapComplete())

diff --git a/src/OpenGL/libGLESv2/Texture.h b/src/OpenGL/libGLESv2/Texture.h
index f27879d..495403a 100644 (file)
--- a/src/OpenGL/libGLESv2/Texture.h
+++ b/src/OpenGL/libGLESv2/Texture.h
@@ -148,6 +148,7 @@ public:
        virtual int getTopLevel() const = 0;
        virtual bool requiresSync() const = 0;
 
+       virtual bool isBaseLevelDefined() const = 0;
        virtual bool isSamplerComplete(Sampler *sampler) const = 0;
        virtual bool isCompressed(GLenum target, GLint level) const = 0;
        virtual bool isDepth(GLenum target, GLint level) const = 0;
@@ -220,6 +221,7 @@ public:
 
        void setSharedImage(egl::Image *image);
 
+       bool isBaseLevelDefined() const override;
        bool isSamplerComplete(Sampler *sampler) const override;
        bool isCompressed(GLenum target, GLint level) const override;
        bool isDepth(GLenum target, GLint level) const override;
@@ -287,6 +289,7 @@ public:
        void copyImage(GLenum target, GLint level, GLenum internalformat, GLint x, GLint y, GLsizei width, GLsizei height, Renderbuffer *source);
        void copySubImage(GLenum target, GLint level, GLint xoffset, GLint yoffset, GLint zoffset, GLint x, GLint y, GLsizei width, GLsizei height, Renderbuffer *source) override;
 
+       bool isBaseLevelDefined() const override;
        bool isSamplerComplete(Sampler *sampler) const override;
        bool isCompressed(GLenum target, GLint level) const override;
        bool isDepth(GLenum target, GLint level) const override;
@@ -350,6 +353,7 @@ public:
 
        void setSharedImage(egl::Image *image);
 
+       bool isBaseLevelDefined() const override;
        bool isSamplerComplete(Sampler *sampler) const override;
        bool isCompressed(GLenum target, GLint level) const override;
        bool isDepth(GLenum target, GLint level) const override;

diff --git a/src/OpenGL/libGLESv2/libGLESv2.cpp b/src/OpenGL/libGLESv2/libGLESv2.cpp
index a58563e..86ebabb 100644 (file)
--- a/src/OpenGL/libGLESv2/libGLESv2.cpp
+++ b/src/OpenGL/libGLESv2/libGLESv2.cpp
@@ -2108,35 +2108,11 @@ void GenerateMipmap(GLenum target)
 
        if(context)
        {
-               es2::Texture *texture = nullptr;
+               es2::Texture *texture = context->getTargetTexture(target);
 
-               switch(target)
+               if(!texture)
                {
-               case GL_TEXTURE_2D:
-                       texture = context->getTexture2D();
-                       break;
-               case GL_TEXTURE_CUBE_MAP:
-                       {
-                               TextureCubeMap *cube = context->getTextureCubeMap();
-                               texture = cube;
-
-                               if(!cube->isCubeComplete())
-                               {
-                                       return error(GL_INVALID_OPERATION);
-                               }
-                       }
-                       break;
-               case GL_TEXTURE_2D_ARRAY:
-                       texture = context->getTexture2DArray();
-                       break;
-               case GL_TEXTURE_3D:
-                       texture = context->getTexture3D();
-                       break;
-               case GL_TEXTURE_RECTANGLE_ARB:
-                       texture = context->getTexture2DRect();
-                       break;
-               default:
-                       return error(GL_INVALID_ENUM);
+                       return;
                }
 
                if(!IsMipmappable(texture->getFormat(target, texture->getBaseLevel())))
@@ -2144,6 +2120,23 @@ void GenerateMipmap(GLenum target)
                        return error(GL_INVALID_OPERATION);
                }
 
+               if(target == GL_TEXTURE_CUBE_MAP)
+               {
+                       TextureCubeMap *cube = context->getTextureCubeMap();
+
+                       if(!cube->isCubeComplete())
+                       {
+                               return error(GL_INVALID_OPERATION);
+                       }
+               }
+
+               // [OpenGL ES 3.2]: "Otherwise, if levelbase is not defined, or if any dimension
+               // is zero, all mipmap levels are left unchanged. This is not an error."
+               if(!texture->isBaseLevelDefined())
+               {
+                       return;
+               }
+
                texture->generateMipmaps();
        }
 }
@@ -3296,18 +3289,11 @@ void GetTexParameterfv(GLenum target, GLenum pname, GLfloat* params)
 
        if(context)
        {
-               es2::Texture *texture;
+               es2::Texture *texture = context->getTargetTexture(target);
 
-               switch(target)
+               if(!texture)
                {
-               case GL_TEXTURE_2D:            texture = context->getTexture2D();       break;
-               case GL_TEXTURE_2D_ARRAY:      texture = context->getTexture2DArray();  break;
-               case GL_TEXTURE_3D:            texture = context->getTexture3D();       break;
-               case GL_TEXTURE_CUBE_MAP:      texture = context->getTextureCubeMap();  break;
-               case GL_TEXTURE_EXTERNAL_OES:  texture = context->getTextureExternal(); break;
-               case GL_TEXTURE_RECTANGLE_ARB: texture = context->getTexture2DRect();   break;
-               default:
-                       return error(GL_INVALID_ENUM);
+                       return;
                }
 
                switch(pname)
@@ -3383,18 +3369,11 @@ void GetTexParameteriv(GLenum target, GLenum pname, GLint* params)
 
        if(context)
        {
-               es2::Texture *texture;
+               es2::Texture *texture = context->getTargetTexture(target);
 
-               switch(target)
+               if(!texture)
                {
-               case GL_TEXTURE_2D:            texture = context->getTexture2D();       break;
-               case GL_TEXTURE_2D_ARRAY:      texture = context->getTexture2DArray();  break;
-               case GL_TEXTURE_3D:            texture = context->getTexture3D();       break;
-               case GL_TEXTURE_CUBE_MAP:      texture = context->getTextureCubeMap();  break;
-               case GL_TEXTURE_EXTERNAL_OES:  texture = context->getTextureExternal(); break;
-               case GL_TEXTURE_RECTANGLE_ARB: texture = context->getTexture2DRect();   break;
-               default:
-                       return error(GL_INVALID_ENUM);
+                       return;
                }
 
                switch(pname)
@@ -4655,18 +4634,11 @@ void TexParameterf(GLenum target, GLenum pname, GLfloat param)
 
        if(context)
        {
-               es2::Texture *texture;
+               es2::Texture *texture = context->getTargetTexture(target);
 
-               switch(target)
+               if(!texture)
                {
-               case GL_TEXTURE_2D:            texture = context->getTexture2D();       break;
-               case GL_TEXTURE_2D_ARRAY:      texture = context->getTexture2DArray();  break;
-               case GL_TEXTURE_3D:            texture = context->getTexture3D();       break;
-               case GL_TEXTURE_CUBE_MAP:      texture = context->getTextureCubeMap();  break;
-               case GL_TEXTURE_EXTERNAL_OES:  texture = context->getTextureExternal(); break;
-               case GL_TEXTURE_RECTANGLE_ARB: texture = context->getTexture2DRect();   break;
-               default:
-                       return error(GL_INVALID_ENUM);
+                       return;
                }
 
                switch(pname)
@@ -4786,18 +4758,11 @@ void TexParameteri(GLenum target, GLenum pname, GLint param)
 
        if(context)
        {
-               es2::Texture *texture;
+               es2::Texture *texture = context->getTargetTexture(target);
 
-               switch(target)
+               if(!texture)
                {
-               case GL_TEXTURE_2D:            texture = context->getTexture2D();       break;
-               case GL_TEXTURE_2D_ARRAY:      texture = context->getTexture2DArray();  break;
-               case GL_TEXTURE_3D:            texture = context->getTexture3D();       break;
-               case GL_TEXTURE_CUBE_MAP:      texture = context->getTextureCubeMap();  break;
-               case GL_TEXTURE_EXTERNAL_OES:  texture = context->getTextureExternal(); break;
-               case GL_TEXTURE_RECTANGLE_ARB: texture = context->getTexture2DRect();   break;
-               default:
-                       return error(GL_INVALID_ENUM);
+                       return;
                }
 
                switch(pname)