Change the component type of STENCIL_INDEX8 to UNSIGNED_NORMALIZED

CONTRIBUTORS

@@ -40,6 +40,7 @@ Google Inc.
  thestig@chromium.org
  Justin Schuh
  Scott Graham
+ Corentin Wallez
 
 Adobe Systems Inc.
  Alexandru Chiculita

src/libGLESv2/formatutils.cpp

@@ -454,7 +454,7 @@ static InternalFormatInfoMap BuildInternalFormatInfoMap()
     map.insert(InternalFormatInfoPair(GL_DEPTH_COMPONENT32_OES, DepthStencilFormat(32, 0,  0, GL_DEPTH_COMPONENT, GL_UNSIGNED_INT,                   GL_UNSIGNED_NORMALIZED, RequireExt<&Extensions::depthTextures>,        RequireExt<&Extensions::depthTextures>,                                              AlwaysSupported                              )));
     map.insert(InternalFormatInfoPair(GL_DEPTH24_STENCIL8,      DepthStencilFormat(24, 8,  0, GL_DEPTH_STENCIL,   GL_UNSIGNED_INT_24_8,              GL_UNSIGNED_NORMALIZED, RequireESOrExt<3, &Extensions::depthTextures>, RequireESOrExtOrExt<3, &Extensions::depthTextures, &Extensions::packedDepthStencil>, AlwaysSupported                              )));
     map.insert(InternalFormatInfoPair(GL_DEPTH32F_STENCIL8,     DepthStencilFormat(32, 8, 24, GL_DEPTH_STENCIL,   GL_FLOAT_32_UNSIGNED_INT_24_8_REV, GL_FLOAT,               RequireES<3>,                                  RequireES<3>,                                                                        AlwaysSupported                              )));
-    map.insert(InternalFormatInfoPair(GL_STENCIL_INDEX8,        DepthStencilFormat( 0, 8,  0, GL_DEPTH_STENCIL,   GL_UNSIGNED_BYTE,                  GL_UNSIGNED_INT,        RequireES<2>,                                  RequireES<2>,                                                                        NeverSupported                               )));
+    // STENCIL_INDEX8 is special-cased, see around the bottom of the list.
 
     // Luminance alpha formats
     //                               | Internal format          |          | L | A | Format            | Type            | Component type        | Supported                                                                    | Renderable    | Filterable    |
@@ -511,6 +511,13 @@ static InternalFormatInfoMap BuildInternalFormatInfoMap()
     // From GL_ANGLE_texture_compression_dxt5
     map.insert(InternalFormatInfoPair(GL_COMPRESSED_RGBA_S3TC_DXT5_ANGLE, CompressedFormat(4, 4, 128, 4, GL_COMPRESSED_RGBA_S3TC_DXT5_ANGLE, GL_UNSIGNED_BYTE, false, RequireExt<&Extensions::textureCompressionDXT5>, NeverSupported, AlwaysSupported)));
 
+    // For STENCIL_INDEX8 we chose a normalized component type for the following reasons:
+    // - Multisampled buffer are disallowed for non-normalized integer component types and we want to support it for STENCIL_INDEX8
+    // - All other stencil formats (all depth-stencil) are either float or normalized
+    // - It affects only validation of internalformat in RenderbufferStorageMultisample.
+    //                               | Internal format  |                  |D |S |X | Format          | Type            | Component type        | Supported   | Renderable  | Filterable   |
+    map.insert(InternalFormatInfoPair(GL_STENCIL_INDEX8, DepthStencilFormat(0, 8, 0, GL_DEPTH_STENCIL, GL_UNSIGNED_BYTE, GL_UNSIGNED_NORMALIZED, RequireES<2>, RequireES<2>, NeverSupported)));
+
     return map;
 }
 

src/libGLESv2/libGLESv2.cpp

@@ -3851,8 +3851,8 @@ void GL_APIENTRY glRenderbufferStorageMultisampleANGLE(GLenum target, GLsizei sa
     gl::Context *context = gl::getNonLostContext();
     if (context)
     {
-        if (!ValidateRenderbufferStorageParameters(context, target, samples, internalformat,
-                                                   width, height, true))
+        if (!ValidateRenderbufferStorageParametersANGLE(context, target, samples, internalformat,
+                                                        width, height))
         {
             return;
         }
@@ -5802,8 +5802,7 @@ void GL_APIENTRY glRenderbufferStorageMultisample(GLenum target, GLsizei samples
             return;
         }
 
-        if (!ValidateRenderbufferStorageParameters(context, target, samples, internalformat,
-                                                   width, height, false))
+        if (!ValidateES3RenderbufferStorageParameters(context, target, samples, internalformat, width, height))
         {
             return;
         }

src/libGLESv2/validationES.cpp

@@ -288,9 +288,8 @@ bool ValidateAttachmentTarget(gl::Context *context, GLenum attachment)
     return true;
 }
 
-bool ValidateRenderbufferStorageParameters(gl::Context *context, GLenum target, GLsizei samples,
-                                           GLenum internalformat, GLsizei width, GLsizei height,
-                                           bool angleExtension)
+bool ValidateRenderbufferStorageParametersBase(gl::Context *context, GLenum target, GLsizei samples,
+                                               GLenum internalformat, GLsizei width, GLsizei height)
 {
     switch (target)
     {
@@ -316,8 +315,7 @@ bool ValidateRenderbufferStorageParameters(gl::Context *context, GLenum target,
 
     // ANGLE_framebuffer_multisample does not explicitly state that the internal format must be
     // sized but it does state that the format must be in the ES2.0 spec table 4.5 which contains
-    // only sized internal formats. The ES3 spec (section 4.4.2) does, however, state that the
-    // internal format must be sized and not an integer format if samples is greater than zero.
+    // only sized internal formats.
     const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(internalformat);
     if (formatInfo.pixelBytes == 0)
     {
@@ -325,55 +323,47 @@ bool ValidateRenderbufferStorageParameters(gl::Context *context, GLenum target,
         return false;
     }
 
-    if ((formatInfo.componentType == GL_UNSIGNED_INT || formatInfo.componentType == GL_INT) && samples > 0)
+    if (static_cast<GLuint>(std::max(width, height)) > context->getCaps().maxRenderbufferSize)
     {
-        context->recordError(Error(GL_INVALID_OPERATION));
+        context->recordError(Error(GL_INVALID_VALUE));
         return false;
     }
 
-    if (static_cast<GLuint>(std::max(width, height)) > context->getCaps().maxRenderbufferSize)
+    GLuint handle = context->getState().getRenderbufferId();
+    if (handle == 0)
     {
-        context->recordError(Error(GL_INVALID_VALUE));
+        context->recordError(Error(GL_INVALID_OPERATION));
         return false;
     }
 
-    // ANGLE_framebuffer_multisample states that the value of samples must be less than or equal
-    // to MAX_SAMPLES_ANGLE (Context::getMaxSupportedSamples) while the ES3.0 spec (section 4.4.2)
-    // states that samples must be less than or equal to the maximum samples for the specified
-    // internal format.
-    if (angleExtension)
-    {
-        ASSERT(context->getExtensions().framebufferMultisample);
-        if (static_cast<GLuint>(samples) > context->getExtensions().maxSamples)
-        {
-            context->recordError(Error(GL_INVALID_VALUE));
-            return false;
-        }
+    return true;
+}
 
-        // Check if this specific format supports enough samples
-        if (static_cast<GLuint>(samples) > formatCaps.getMaxSamples())
-        {
-            context->recordError(Error(GL_OUT_OF_MEMORY));
-            return false;
-        }
-    }
-    else
+bool ValidateRenderbufferStorageParametersANGLE(gl::Context *context, GLenum target, GLsizei samples,
+                                                GLenum internalformat, GLsizei width, GLsizei height)
+{
+    ASSERT(context->getExtensions().framebufferMultisample);
+
+    // ANGLE_framebuffer_multisample states that the value of samples must be less than or equal
+    // to MAX_SAMPLES_ANGLE (Context::getExtensions().maxSamples) otherwise GL_INVALID_VALUE is
+    // generated.
+    if (static_cast<GLuint>(samples) > context->getExtensions().maxSamples)
     {
-        if (static_cast<GLuint>(samples) > formatCaps.getMaxSamples())
-        {
-            context->recordError(Error(GL_INVALID_VALUE));
-            return false;
-        }
+        context->recordError(Error(GL_INVALID_VALUE));
+        return false;
     }
 
-    GLuint handle = context->getState().getRenderbufferId();
-    if (handle == 0)
+    // ANGLE_framebuffer_multisample states GL_OUT_OF_MEMORY is generated on a failure to create
+    // the specified storage. This is different than ES 3.0 in which a sample number higher
+    // than the maximum sample number supported  by this format generates a GL_INVALID_VALUE.
+    const TextureCaps &formatCaps = context->getTextureCaps().get(internalformat);
+    if (static_cast<GLuint>(samples) > formatCaps.getMaxSamples())
     {
-        context->recordError(Error(GL_INVALID_OPERATION));
+        context->recordError(Error(GL_OUT_OF_MEMORY));
         return false;
     }
 
-    return true;
+    return ValidateRenderbufferStorageParametersBase(context, target, samples, internalformat, width, height);
 }
 
 bool ValidateFramebufferRenderbufferParameters(gl::Context *context, GLenum target, GLenum attachment,

src/libGLESv2/validationES.h

@@ -32,9 +32,11 @@ bool ValidQueryType(const Context *context, GLenum queryType);
 bool ValidProgram(Context *context, GLuint id);
 
 bool ValidateAttachmentTarget(Context *context, GLenum attachment);
-bool ValidateRenderbufferStorageParameters(Context *context, GLenum target, GLsizei samples,
-                                           GLenum internalformat, GLsizei width, GLsizei height,
-                                           bool angleExtension);
+bool ValidateRenderbufferStorageParametersBase(Context *context, GLenum target, GLsizei samples,
+                                               GLenum internalformat, GLsizei width, GLsizei height);
+bool ValidateRenderbufferStorageParametersANGLE(Context *context, GLenum target, GLsizei samples,
+                                                GLenum internalformat, GLsizei width, GLsizei height);
+
 bool ValidateFramebufferRenderbufferParameters(Context *context, GLenum target, GLenum attachment,
                                                GLenum renderbuffertarget, GLuint renderbuffer);
 

src/libGLESv2/validationES3.cpp

@@ -1187,6 +1187,33 @@ bool ValidES3ReadFormatType(Context *context, GLenum internalFormat, GLenum form
     return true;
 }
 
+bool ValidateES3RenderbufferStorageParameters(gl::Context *context, GLenum target, GLsizei samples,
+                                              GLenum internalformat, GLsizei width, GLsizei height)
+{
+    if (!ValidateRenderbufferStorageParametersBase(context, target, samples, internalformat, width, height))
+    {
+        return false;
+    }
+
+    //The ES3 spec(section 4.4.2) states that the internal format must be sized and not an integer format if samples is greater than zero.
+    const gl::InternalFormat &formatInfo = gl::GetInternalFormatInfo(internalformat);
+    if ((formatInfo.componentType == GL_UNSIGNED_INT || formatInfo.componentType == GL_INT) && samples > 0)
+    {
+        context->recordError(Error(GL_INVALID_OPERATION));
+        return false;
+    }
+
+    // The behavior is different than the ANGLE version, which would generate a GL_OUT_OF_MEMORY.
+    const TextureCaps &formatCaps = context->getTextureCaps().get(internalformat);
+    if (static_cast<GLuint>(samples) > formatCaps.getMaxSamples())
+    {
+        context->recordError(Error(GL_INVALID_VALUE));
+        return false;
+    }
+
+    return true;
+}
+
 bool ValidateInvalidateFramebufferParameters(Context *context, GLenum target, GLsizei numAttachments,
                                              const GLenum* attachments)
 {

src/libGLESv2/validationES3.h

@@ -32,6 +32,9 @@ bool ValidateFramebufferTextureLayer(Context *context, GLenum target, GLenum att
 
 bool ValidES3ReadFormatType(Context *context, GLenum internalFormat, GLenum format, GLenum type);
 
+bool ValidateES3RenderbufferStorageParameters(Context *context, GLenum target, GLsizei samples,
+                                              GLenum internalformat, GLsizei width, GLsizei height);
+
 bool ValidateInvalidateFramebufferParameters(Context *context, GLenum target, GLsizei numAttachments,
                                              const GLenum* attachments);
 

tests/angle_tests/FramebufferFormatsTest.cpp

 #include "ANGLETest.h"
 
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
-ANGLE_TYPED_TEST_CASE(FramebufferFormatsTest, ES2_D3D9, ES2_D3D11);
+ANGLE_TYPED_TEST_CASE(FramebufferFormatsTest, ES2_D3D9, ES2_D3D11, ES3_D3D11);
 
 template<typename T>
 class FramebufferFormatsTest : public ANGLETest
@@ -64,6 +64,56 @@ protected:
         glDeleteFramebuffers(1, &fbo);
     }
 
+    void testRenderbufferMultisampleFormat(int minESVersion, GLenum attachmentType, GLenum internalFormat)
+    {
+        if (T::GetGlesMajorVersion() < minESVersion)
+        {
+            return;
+        }
+
+        // Check that multisample is supported with at least two samples (minimum required is 1)
+        bool supports2Samples = false;
+
+        if (T::GetGlesMajorVersion() == 2)
+        {
+            if (extensionEnabled("ANGLE_framebuffer_multisample"))
+            {
+                int maxSamples;
+                glGetIntegerv(GL_MAX_SAMPLES_ANGLE, &maxSamples);
+                supports2Samples = maxSamples >= 2;
+            }
+        }
+        else
+        {
+            assert(T::GetGlesMajorVersion() >= 3);
+            int maxSamples;
+            glGetIntegerv(GL_MAX_SAMPLES, &maxSamples);
+            supports2Samples = maxSamples >= 2;
+        }
+
+        if (!supports2Samples)
+        {
+            return;
+        }
+
+        GLuint framebufferID;
+        glGenFramebuffers(1, &framebufferID);
+        glBindFramebuffer(GL_FRAMEBUFFER, framebufferID);
+
+        GLuint renderbufferID;
+        glGenRenderbuffers(1, &renderbufferID);
+        glBindRenderbuffer(GL_RENDERBUFFER, renderbufferID);
+
+        EXPECT_GL_NO_ERROR();
+        glRenderbufferStorageMultisampleANGLE(GL_RENDERBUFFER, 2, internalFormat, 128, 128);
+        EXPECT_GL_NO_ERROR();
+        glFramebufferRenderbuffer(GL_FRAMEBUFFER, attachmentType, GL_RENDERBUFFER, renderbufferID);
+        EXPECT_GL_NO_ERROR();
+
+        glDeleteRenderbuffers(1, &renderbufferID);
+        glDeleteFramebuffers(1, &framebufferID);
+    }
+
     virtual void SetUp()
     {
         ANGLETest::SetUp();
@@ -100,3 +150,32 @@ TYPED_TEST(FramebufferFormatsTest, RGBA8)
     testTextureFormat(GL_RGBA8_OES, 8, 8, 8, 8);
 }
 
+TYPED_TEST(FramebufferFormatsTest, RenderbufferMultisample_DEPTH16)
+{
+    testRenderbufferMultisampleFormat(2, GL_DEPTH_ATTACHMENT, GL_DEPTH_COMPONENT16);
+}
+
+TYPED_TEST(FramebufferFormatsTest, RenderbufferMultisample_DEPTH24)
+{
+    testRenderbufferMultisampleFormat(3, GL_DEPTH_ATTACHMENT, GL_DEPTH_COMPONENT24);
+}
+
+TYPED_TEST(FramebufferFormatsTest, RenderbufferMultisample_DEPTH32F)
+{
+    testRenderbufferMultisampleFormat(3, GL_DEPTH_ATTACHMENT, GL_DEPTH_COMPONENT32F);
+}
+
+TYPED_TEST(FramebufferFormatsTest, RenderbufferMultisample_DEPTH24_STENCIL8)
+{
+    testRenderbufferMultisampleFormat(3, GL_DEPTH_STENCIL_ATTACHMENT, GL_DEPTH24_STENCIL8);
+}
+
+TYPED_TEST(FramebufferFormatsTest, RenderbufferMultisample_DEPTH32F_STENCIL8)
+{
+    testRenderbufferMultisampleFormat(3, GL_DEPTH_STENCIL_ATTACHMENT, GL_DEPTH32F_STENCIL8);
+}
+
+TYPED_TEST(FramebufferFormatsTest, RenderbufferMultisample_STENCIL_INDEX8)
+{
+    testRenderbufferMultisampleFormat(2, GL_STENCIL_ATTACHMENT, GL_STENCIL_INDEX8);
+}