On Mon, Jun 27, 2016 at 4:17 PM, Gurchetan Singh
<gurchetansi...@chromium.org> wrote:
> There 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
> ---
> src/mesa/main/buffers.c | 14 ++++++++++++--
> src/mesa/main/clear.c | 8 ++++++++
> src/mesa/main/get.c | 9 +++++++++
> 3 files changed, 29 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/get.c b/src/mesa/main/get.c
> index 6ffa99c..ec6bc3e 100644
> --- a/src/mesa/main/get.c
> +++ b/src/mesa/main/get.c
> @@ -627,6 +627,15 @@ find_custom_value(struct gl_context *ctx, const struct
> value_desc *d, union valu
> break;
>
> case GL_READ_BUFFER:
> + /* 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(ctx))
> + if (ctx->ReadBuffer->ColorReadBuffer == GL_FRONT)
> + ctx->ReadBuffer->ColorReadBuffer = GL_BACK;
Why is this OK to do? Shouldn't these just get not get set that way in
the first place? Writing values when doing a random glGet() seems like
a recipe for confusion and hard-to-reproduce bugs.
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
