Re: [Mesa-dev] gl_render_buffer->_BaseFormat

[Brian Paul]

On 02/17/2016 06:20 PM, Ilia Mirkin wrote:

Hi Brian,

In commit

commit 45e76d2665b38ba3787548310efc59e969124c01
Author: Brian Paul <bri...@vmware.com>
Date:   Thu Oct 8 20:27:27 2009 -0600

     mesa: remove a bunch of gl_renderbuffer fields

     _ActualFormat is replaced by Format (MESA_FORMAT_x).
     ColorEncoding, ComponentType, RedBits, GreenBits, BlueBits, etc. are
     all replaced by MESA_FORMAT_x queries.

Some short 6.5 years ago, you replaced a bunch of fields with a mesa
format field.  In this change were these hunks:

@@ -431,6 +430,9 @@ test_attachment_completeness(const GLcontext *ctx,
GLenum format,
        }
     }
     else if (att->Type == GL_RENDERBUFFER_EXT) {
+      const GLenum baseFormat =
+         _mesa_get_format_base_format(att->Renderbuffer->Format);
+
        ASSERT(att->Renderbuffer);
        if (!att->Renderbuffer->InternalFormat ||
            att->Renderbuffer->Width < 1 ||
@@ -440,24 +442,19 @@ test_attachment_completeness(const GLcontext
*ctx, GLenum format,
           return;
        }
        if (format == GL_COLOR) {
-         if (att->Renderbuffer->_BaseFormat != GL_RGB &&
-             att->Renderbuffer->_BaseFormat != GL_RGBA) {
+         if (baseFormat != GL_RGB &&
+             baseFormat != GL_RGBA) {
              att_incomplete("bad renderbuffer color format");
              att->Complete = GL_FALSE;
              return;
           }
-         ASSERT(att->Renderbuffer->RedBits);
-         ASSERT(att->Renderbuffer->GreenBits);
-         ASSERT(att->Renderbuffer->BlueBits);
        }
        else if (format == GL_DEPTH) {
-         if (att->Renderbuffer->_BaseFormat == GL_DEPTH_COMPONENT) {
-            ASSERT(att->Renderbuffer->DepthBits);
+         if (baseFormat == GL_DEPTH_COMPONENT) {
              /* OK */
           }
           else if (ctx->Extensions.EXT_packed_depth_stencil &&
-                  att->Renderbuffer->_BaseFormat == GL_DEPTH_STENCIL_EXT) {
-            ASSERT(att->Renderbuffer->DepthBits);
+                  baseFormat == GL_DEPTH_STENCIL_EXT) {
              /* OK */
           }
           else {
@@ -468,13 +465,11 @@ test_attachment_completeness(const GLcontext
*ctx, GLenum format,
        }
        else {
           assert(format == GL_STENCIL);
-         if (att->Renderbuffer->_BaseFormat == GL_STENCIL_INDEX) {
-            ASSERT(att->Renderbuffer->StencilBits);
+         if (baseFormat == GL_STENCIL_INDEX) {
              /* OK */
           }
           else if (ctx->Extensions.EXT_packed_depth_stencil &&
-                  att->Renderbuffer->_BaseFormat == GL_DEPTH_STENCIL_EXT) {
-            ASSERT(att->Renderbuffer->StencilBits);
+                  baseFormat == GL_DEPTH_STENCIL_EXT) {
              /* OK */
           }
           else {

Which seemingly prefer using the driver format for checking attachment
completeness to the actual baseformat that the RB was created with.
The issue I'm running into is that we might create a STENCIL_INDEX
renderbuffer, but it will be backed by a DEPTH_STENCIL format. However
if you put that renderbuffer as GL_DEPTH_ATTACHMENT, the fbo should be
incomplete. This does not happen, since the underlying format is a
DEPTH_STENCIL one.

This problem, in turn, is causing some dEQP GL_OES_texture_stencil8
tests to fail.

Any objections to flipping these back to looking at the Renderbuffer's
_BaseFormat field?

Makes sense.  I'll assume you did a piglit run to check things over.

-Brian


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev