[Mesa-dev] [PATCH 1/3] mesa: add gl_framebuffer::_Color0IsInteger

[Brian Paul]

On 09/28/2016 07:44 AM, Marek Olšák wrote:
> From: Marek Olšák <marek.olsak at amd.com>
>
> The other flag seems to have a slightly different meaning.

I don't recall what the GL spec says w.r.t. mixing floating point and 
integer color buffers.  I suspect it may be implementation dependent.

I wonder if we should replace _IntegerColor and _Color0IsInteger with a 
bitmask indicating which color buffers are integer-valued.  That is:

struct gl_framebuffer {
    ...
    GLbitfield _IntegerColorBuffers;
    ...
};


_mesa_test_framebuffer_completeness()
...
if (_mesa_is_format_integer_color(attFormat))
     fb->_IntegerColorBuffers |= (1 << i);


What do you think?

We could probably do the same thing for the _AllColorBuffersFixedPoint 
and _HasSNormOrFloatColorBuffer fields later.

-Brian



> ---
>   src/mesa/main/fbobject.c    | 4 ++++
>   src/mesa/main/framebuffer.c | 1 +
>   src/mesa/main/mtypes.h      | 6 ++++++
>   3 files changed, 11 insertions(+)
>
> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
> index 09da6b7..488adf7 100644
> --- a/src/mesa/main/fbobject.c
> +++ b/src/mesa/main/fbobject.c
> @@ -960,20 +960,21 @@ _mesa_test_framebuffer_completeness(struct gl_context *ctx,
>      bool has_stencil_attachment = false;
>
>      assert(_mesa_is_user_fbo(fb));
>
>      /* we're changing framebuffer fields here */
>      FLUSH_VERTICES(ctx, _NEW_BUFFERS);
>
>      numImages = 0;
>      fb->Width = 0;
>      fb->Height = 0;
> +   fb->_Color0IsInteger = false;
>      fb->_AllColorBuffersFixedPoint = GL_TRUE;
>      fb->_HasSNormOrFloatColorBuffer = GL_FALSE;
>      fb->_HasAttachments = true;
>
>      /* Start at -2 to more easily loop over all attachment points.
>       *  -2: depth buffer
>       *  -1: stencil buffer
>       * >=0: color buffer
>       */
>      for (i = -2; i < (GLint) ctx->Const.MaxColorAttachments; i++) {
> @@ -1084,20 +1085,23 @@ _mesa_test_framebuffer_completeness(struct gl_context *ctx,
>               fbo_incomplete(ctx, "inconsistent fixed sample locations", -1);
>               return;
>            }
>         }
>         else {
>            assert(att->Type == GL_NONE);
>            continue;
>         }
>
>         /* check if integer color */
> +      if (i == 0)
> +         fb->_Color0IsInteger = _mesa_is_format_integer_color(attFormat);
> +
>         fb->_IntegerColor = _mesa_is_format_integer_color(attFormat);
>
>         /* Update _AllColorBuffersFixedPoint and _HasSNormOrFloatColorBuffer. */
>         if (i >= 0) {
>            GLenum type = _mesa_get_format_datatype(attFormat);
>
>            fb->_AllColorBuffersFixedPoint =
>               fb->_AllColorBuffersFixedPoint &&
>               (type == GL_UNSIGNED_NORMALIZED || type == GL_SIGNED_NORMALIZED);
>
> diff --git a/src/mesa/main/framebuffer.c b/src/mesa/main/framebuffer.c
> index 46a6f64..4f4d5fd 100644
> --- a/src/mesa/main/framebuffer.c
> +++ b/src/mesa/main/framebuffer.c
> @@ -148,20 +148,21 @@ _mesa_initialize_window_framebuffer(struct gl_framebuffer *fb,
>      else {
>         fb->_NumColorDrawBuffers = 1;
>         fb->ColorDrawBuffer[0] = GL_FRONT;
>         fb->_ColorDrawBufferIndexes[0] = BUFFER_FRONT_LEFT;
>         fb->ColorReadBuffer = GL_FRONT;
>         fb->_ColorReadBufferIndex = BUFFER_FRONT_LEFT;
>      }
>
>      fb->Delete = _mesa_destroy_framebuffer;
>      fb->_Status = GL_FRAMEBUFFER_COMPLETE_EXT;
> +   fb->_Color0IsInteger = false;
>      fb->_AllColorBuffersFixedPoint = !visual->floatMode;
>      fb->_HasSNormOrFloatColorBuffer = visual->floatMode;
>      fb->_HasAttachments = true;
>
>      compute_depth_max(fb);
>   }
>
>
>   /**
>    * Initialize a user-created gl_framebuffer object.
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index ee811ff..873003b 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -3311,20 +3311,26 @@ struct gl_framebuffer
>       * _Ymax do NOT take into account _HasAttachments being false). To get the
>       * geometry of the framebuffer, the  helper functions
>       *   _mesa_geometric_width(),
>       *   _mesa_geometric_height(),
>       *   _mesa_geometric_samples() and
>       *   _mesa_geometric_layers()
>       * are available that check _HasAttachments.
>       */
>      bool _HasAttachments;
>
> +   /**
> +    * Whether color attachment 0 is integer.
> +    * Used to disable alpha test, alpha to coverage, and alpha to one.
> +    */
> +   bool _Color0IsInteger;
> +
>      /** Integer color values */
>      GLboolean _IntegerColor;
>
>      /* ARB_color_buffer_float */
>      GLboolean _AllColorBuffersFixedPoint; /* no integer, no float */
>      GLboolean _HasSNormOrFloatColorBuffer;
>
>      /**
>       * The maximum number of layers in the framebuffer, or 0 if the framebuffer
>       * is not layered.  For cube maps and cube map arrays, each cube face
>