[Mesa-dev] [PATCH] Make single-buffered GLES representation internally consistent

[Stéphane Marchesin]

On Thu, Jun 30, 2016 at 3:20 PM, Gurchetan Singh
<gurchetansingh at chromium.org> wrote:
> There are a few places in the code where clearing and reading are done on
> incorrect buffers for GLES contexts.  See comments for details.  This
> fixes 75 GLES3 dEQP tests on the surfaceless platform with no regressions.
>
> v2: Corrected unclear comment
> v3: Make the change in context.c instead of get.c
> v4: Removed whitespace

I looked for a better way than initializing from makecurrent, but
there doesn't seem to be one, so...

Reviewed-by: <Stéphane Marchesin marcheu at chromium.org>

> ---
>  src/mesa/main/buffers.c | 14 ++++++++++++--
>  src/mesa/main/clear.c   |  8 ++++++++
>  src/mesa/main/context.c | 10 ++++++++++
>  3 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
> index e8aedde..86696b8 100644
> --- a/src/mesa/main/buffers.c
> +++ b/src/mesa/main/buffers.c
> @@ -173,12 +173,22 @@ draw_buffer_enum_to_bitmask(const struct gl_context *ctx, GLenum buffer)
>   * return -1 for an invalid buffer.
>   */
>  static gl_buffer_index
> -read_buffer_enum_to_index(GLenum buffer)
> +read_buffer_enum_to_index(const struct gl_context *ctx, GLenum buffer)
>  {
>     switch (buffer) {
>        case GL_FRONT:
>           return BUFFER_FRONT_LEFT;
>        case GL_BACK:
> +         if (_mesa_is_gles(ctx)) {
> +            /* In draw_buffer_enum_to_bitmask, when GLES contexts draw to
> +             * GL_BACK with a single-buffered configuration, we actually end
> +             * up drawing to the sole front buffer in our internal
> +             * representation.  For consistency, we must read from that
> +             * front left buffer too.
> +             */
> +            if (!ctx->DrawBuffer->Visual.doubleBufferMode)
> +               return BUFFER_FRONT_LEFT;
> +         }
>           return BUFFER_BACK_LEFT;
>        case GL_RIGHT:
>           return BUFFER_FRONT_RIGHT;
> @@ -724,7 +734,7 @@ read_buffer(struct gl_context *ctx, struct gl_framebuffer *fb,
>        if (_mesa_is_gles3(ctx) && !is_legal_es3_readbuffer_enum(buffer))
>           srcBuffer = -1;
>        else
> -         srcBuffer = read_buffer_enum_to_index(buffer);
> +         srcBuffer = read_buffer_enum_to_index(ctx, buffer);
>
>        if (srcBuffer == -1) {
>           _mesa_error(ctx, GL_INVALID_ENUM,
> diff --git a/src/mesa/main/clear.c b/src/mesa/main/clear.c
> index 35b912c..a1bb36e 100644
> --- a/src/mesa/main/clear.c
> +++ b/src/mesa/main/clear.c
> @@ -267,6 +267,14 @@ make_color_buffer_mask(struct gl_context *ctx, GLint drawbuffer)
>           mask |= BUFFER_BIT_FRONT_RIGHT;
>        break;
>     case GL_BACK:
> +      /* For GLES contexts with a single buffered configuration, we actually
> +       * only have a front renderbuffer, so any clear calls to GL_BACK should
> +       * affect that buffer. See draw_buffer_enum_to_bitmask for details.
> +       */
> +      if (_mesa_is_gles(ctx))
> +         if (!ctx->DrawBuffer->Visual.doubleBufferMode)
> +            if (att[BUFFER_FRONT_LEFT].Renderbuffer)
> +               mask |= BUFFER_BIT_FRONT_LEFT;
>        if (att[BUFFER_BACK_LEFT].Renderbuffer)
>           mask |= BUFFER_BIT_BACK_LEFT;
>        if (att[BUFFER_BACK_RIGHT].Renderbuffer)
> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
> index 85cd779..8925626 100644
> --- a/src/mesa/main/context.c
> +++ b/src/mesa/main/context.c
> @@ -1676,6 +1676,16 @@ _mesa_make_current( struct gl_context *newCtx,
>           }
>           if (!newCtx->ReadBuffer || _mesa_is_winsys_fbo(newCtx->ReadBuffer)) {
>              _mesa_reference_framebuffer(&newCtx->ReadBuffer, readBuffer);
> +            /* In _mesa_initialize_window_framebuffer, for single-buffered
> +             * visuals, the ColorReadBuffer is set to be GL_FRONT, even with
> +             * GLES contexts. When calling read_buffer, we verify we are reading
> +             * from GL_BACK in is_legal_es3_readbuffer_enum.  But the default is
> +             * incorrect, and certain dEQP tests check this.  So fix it here.
> +             */
> +            if (_mesa_is_gles(newCtx) &&
> +               !newCtx->ReadBuffer->Visual.doubleBufferMode)
> +               if (newCtx->ReadBuffer->ColorReadBuffer == GL_FRONT)
> +                  newCtx->ReadBuffer->ColorReadBuffer = GL_BACK;
>           }
>
>           /* XXX only set this flag if we're really changing the draw/read
> --
> 2.1.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev