[Mesa-dev] [PATCH] mesa/api: validate buffers are unmapped

[Dave Airlie]

On 2 September 2015 at 09:42, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Tue, Sep 1, 2015 at 7:39 PM, Dave Airlie <airlied at gmail.com> wrote:
>> From: Dave Airlie <airlied at redhat.com>
>>
>> GL 3.2 introduced new language in this area, and CTS enforces it,
>> this patches checks all the vertex buffers and the index buffer
>> are mapped in the core profile case. I'm not sure what GLES
>> expects here.
>>
>> This fixes
>> GL33-CTS.draw_elements_base_vertex_tests.invalid_mapped_bos
>>
>> Signed-off-by: Dave Airlie <airlied at redhat.com>
>> ---
>>  src/mesa/main/api_validate.c | 33 +++++++++++++++++++++++++++++++++
>>  1 file changed, 33 insertions(+)
>>
>> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
>> index 53c8fb8..8b79d5c 100644
>> --- a/src/mesa/main/api_validate.c
>> +++ b/src/mesa/main/api_validate.c
>> @@ -34,7 +34,21 @@
>>  #include "transformfeedback.h"
>>  #include <stdbool.h>
>>
>> +/* GL3.2 introduces checks for buffer mappings */
>> +static bool
>> +check_buffers_are_unmapped(struct gl_vertex_buffer_binding *inputs)
>> +{
>> +   GLuint i;
>>
>> +   for (i = 0; i < VERT_ATTRIB_MAX; i++) {
>> +      if (inputs[i].BufferObj) {
>> +         struct gl_buffer_object *obj = inputs[i].BufferObj;
>> +         if (_mesa_check_disallowed_mapping(obj))
>> +            return false;
>> +      }
>> +   }
>> +   return true;
>> +}
>>  /**
>>   * Check if OK to draw arrays/elements.
>>   */
>> @@ -58,6 +72,25 @@ check_valid_to_render(struct gl_context *ctx, const char *function)
>>        break;
>>
>>     case API_OPENGL_CORE:
>> +
>> +      /* Section 2.9.3 (Mapping and Unmapping Buffer Data" of the OpenGL 4.2
>> +       * Core Profile says:
>> +       *    "Most, but not all GL commands will detect attempts to read data
>> +       *     from a mapped buffer object. When such an attempt is detected, an
>> +       *     INVALID_OPERATION error will be generated.
>> +       */
>> +      if (!check_buffers_are_unmapped(ctx->Array.VAO->VertexBinding)) {
>> +         _mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)", function);
>> +         return false;
>> +      }
>> +
>> +      if (ctx->Array.VAO->IndexBufferObj) {
>> +         if (_mesa_check_disallowed_mapping(ctx->Array.VAO->IndexBufferObj)) {
>> +            _mesa_error(ctx, GL_INVALID_OPERATION, "%s(buffers mapped)", function);
>
> May I suggest improving the messages to indicate that a vertex or
> index buffer was being mapped? This should reduce confusion,
> hopefully.

you mean someone is going to see this error message in the wild? I
admire your optimism :-)

But yes I spotted that myself, so I can do so.

Dave.