On 03/24/2016 07:54 AM, Kenneth Graunke wrote:
 From the ES 3.2 spec, section 16.1.1 (Selecting Buffers for Reading):

    "An INVALID_ENUM error is generated if src is not BACK or one of
     the values from table 15.5."

Table 15.5 contains NONE and COLOR_ATTACHMENTi.

Mesa properly returned INVALID_ENUM for unknown enums, but it decided
what was known by using read_buffer_enum_to_index, which handles all
enums in every API.  So enums that were valid in GL were making it
past the "valid enum" check.  Such targets would then be classified
as unsupported, and we'd raise INVALID_OPERATION, but that's technically
the wrong error code.

Fixes dEQP-GLES31's
functional.debug.negative_coverage.get_error.buffer.read_buffer

Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
---
  src/mesa/main/buffers.c | 10 ++++++++++
  1 file changed, 10 insertions(+)

diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
index 26dafd1..da90e00 100644
--- a/src/mesa/main/buffers.c
+++ b/src/mesa/main/buffers.c
@@ -222,6 +222,12 @@ read_buffer_enum_to_index(GLenum buffer)
     }
  }

+static bool
+is_legal_es3_readbuffer_enum(GLenum buf)
+{
+   return buf == GL_BACK || buf == GL_NONE ||
+          (buf >= GL_COLOR_ATTACHMENT0 && buf <= GL_COLOR_ATTACHMENT31);
+}


Technically, reading past what is reported by GL_MAX_COLOR_ATTACHMENTS is not legal, so here you should probably use ctx->Const.MaxColorAttachments.

From GLES 3.1, (in various sections):

    "i in COLOR_ATTACHMENTi may range from zero to the value of
     MAX_COLOR_ATTACHMENTS minus one".

  /**
   * Called by glDrawBuffer() and glNamedFramebufferDrawBuffer().
@@ -716,6 +722,10 @@ read_buffer(struct gl_context *ctx, struct gl_framebuffer 
*fb,
     else {
        /* general case / window-system framebuffer */
        srcBuffer = read_buffer_enum_to_index(buffer);
+
+      if (_mesa_is_gles3(ctx) && !is_legal_es3_readbuffer_enum(buffer))
+         srcBuffer = -1;
+

I think you can move this block above the previous one that sets srcBuffer, so that if buffer is not legal, you don't need to call read_buffer_enum_to_index().

        if (srcBuffer == -1) {
           _mesa_error(ctx, GL_INVALID_ENUM,
                       "%s(invalid buffer %s)", caller,


With those two things, patch is:

Reviewed-by: Eduardo Lima Mitev <el...@igalia.com>

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

Reply via email to