[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 <brianp at 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