[Mesa-dev] [PATCH 2/2] util/draw: replace assertions with conditionals in util_draw_max_index()

[Brian Paul]

New patch posted.  Let me know if that's what you have in mind.

-Brian

On 11/14/2011 04:00 AM, Jose Fonseca wrote:
> Looks good, but some warnings on util_draw_max_index would be useful, as this is not something apps normally do, so are much more often a symptom of a state tracker bug than apps bugs.
>
> Jose
>
> ----- Original Message -----
>> Don't assert/die if a VBO is too small.  Return zero instead.
>>
>> Note that util_draw_max_index() now returns max_index+1 instead of
>> max_index.  This lets us return zero to indicate that one of the VBOs
>> is too small to draw anything.
>>
>> Fixes a failure with the new piglit vbo-too-small test.
>> ---
>>   src/gallium/auxiliary/draw/draw_pt.c         |   19 ++++++++++++----
>>   src/gallium/auxiliary/util/u_draw.c          |   30
>>   +++++++++++++++++++------
>>   src/mesa/state_tracker/st_cb_bufferobjects.c |    1 +
>>   3 files changed, 38 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/draw/draw_pt.c
>> b/src/gallium/auxiliary/draw/draw_pt.c
>> index e0eda67..737dd79 100644
>> --- a/src/gallium/auxiliary/draw/draw_pt.c
>> +++ b/src/gallium/auxiliary/draw/draw_pt.c
>> @@ -422,6 +422,7 @@ draw_vbo(struct draw_context *draw,
>>   {
>>      unsigned reduced_prim = u_reduced_prim(info->mode);
>>      unsigned instance;
>> +   unsigned index_limit;
>>
>>      assert(info->instance_count>  0);
>>      if (info->indexed)
>> @@ -470,11 +471,19 @@ draw_vbo(struct draw_context *draw,
>>      if (0)
>>         draw_print_arrays(draw, info->mode, info->start,
>>         MIN2(info->count, 20));
>>
>> -   draw->pt.max_index = util_draw_max_index(draw->pt.vertex_buffer,
>> -
>>                                             draw->pt.nr_vertex_buffers,
>> -                                            draw->pt.vertex_element,
>> -
>>                                             draw->pt.nr_vertex_elements,
>> -                                            info);
>> +   index_limit = util_draw_max_index(draw->pt.vertex_buffer,
>> +                                     draw->pt.nr_vertex_buffers,
>> +                                     draw->pt.vertex_element,
>> +                                     draw->pt.nr_vertex_elements,
>> +                                     info);
>> +
>> +   if (index_limit == 0) {
>> +      /* one of the buffers is too small to do any valid drawing */
>> +      return;
>> +   }
>> +
>> +   draw->pt.max_index = index_limit - 1;
>> +
>>
>>      /*
>>       * TODO: We could use draw->pt.max_index to further narrow
>> diff --git a/src/gallium/auxiliary/util/u_draw.c
>> b/src/gallium/auxiliary/util/u_draw.c
>> index 20837be..2da1adb 100644
>> --- a/src/gallium/auxiliary/util/u_draw.c
>> +++ b/src/gallium/auxiliary/util/u_draw.c
>> @@ -33,9 +33,13 @@
>>
>>
>>   /**
>> - * Returns the largest legal index value for the current set of
>> bound vertex
>> - * buffers.  Regardless of any other consideration, all vertex
>> lookups need to
>> - * be clamped to 0..max_index to prevent an out-of-bound access.
>> + * Returns the largest legal index value plus one for the current
>> set
>> + * of bound vertex buffers.  Regardless of any other consideration,
>> + * all vertex lookups need to be clamped to 0..max_index-1 to
>> prevent
>> + * an out-of-bound access.
>> + *
>> + * Note that if zero is returned it means that one or more buffers
>> is
>> + * too small to contain any valid vertex data.
>>    */
>>   unsigned
>>   util_draw_max_index(
>> @@ -68,13 +72,25 @@ util_draw_max_index(
>>         assert(format_desc->block.bits % 8 == 0);
>>         format_size = format_desc->block.bits/8;
>>
>> -      assert(buffer_size - buffer->buffer_offset<= buffer_size);
>> +      if (buffer->buffer_offset>= buffer_size) {
>> +         /* buffer is too small */
>> +         return 0;
>> +      }
>> +
>>         buffer_size -= buffer->buffer_offset;
>>
>> -      assert(buffer_size - element->src_offset<= buffer_size);
>> +      if (element->src_offset>= buffer_size) {
>> +         /* buffer is too small */
>> +         return 0;
>> +      }
>> +
>>         buffer_size -= element->src_offset;
>>
>> -      assert(buffer_size - format_size<= buffer_size);
>> +      if (format_size>  buffer_size) {
>> +         /* buffer is too small */
>> +         return 0;
>> +      }
>> +
>>         buffer_size -= format_size;
>>
>>         if (buffer->stride != 0) {
>> @@ -95,5 +111,5 @@ util_draw_max_index(
>>         }
>>      }
>>
>> -   return max_index;
>> +   return max_index + 1;
>>   }
>> diff --git a/src/mesa/state_tracker/st_cb_bufferobjects.c
>> b/src/mesa/state_tracker/st_cb_bufferobjects.c
>> index 3abf029..286fa02 100644
>> --- a/src/mesa/state_tracker/st_cb_bufferobjects.c
>> +++ b/src/mesa/state_tracker/st_cb_bufferobjects.c
>> @@ -221,6 +221,7 @@ st_bufferobj_data(struct gl_context *ctx,
>>
>>      pipe_resource_reference(&st_obj->buffer, NULL );
>>
>> +   /* note that size=0 is OK and we should get a non-null value */
>>      st_obj->buffer = pipe_buffer_create(pipe->screen, bind,
>>      pipe_usage, size);
>>
>>      if (!st_obj->buffer) {
>> --
>> 1.7.3.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>