[Mesa-dev] [PATCH] i965: Use the buffer object size for VERTEX_BUFFER_STATE's size field.

[Jason Ekstrand]

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

On Thu, May 26, 2016 at 12:31 AM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> commit 7c8dfa78b98a12c1c5 (i965/draw: Use the real size for vertex
> buffers) changed how we programmed the VERTEX_BUFFER_STATE size field.
>
> Previously, we programmed it to the size of the actual underlying BO,
> which is page-aligned, and potentially much larger than the GL buffer
> object.  This violated the ARB_robust_buffer_access spec.
>
> With that change, we started programming it based on the range of data
> we expect the draw call to actually access - which is based on the
> min_index and max_index information provided to glDrawRangeElements().
>
> Unfortunately, applications often provide inaccurate range information
> to glDrawRangeElements().  For example, all the Unreal demos appear to
> draw using a range of [0, 3] when the index buffer's actual index range
> is [0, 5].  Such results are undefined, and we are absolutely allowed
> to restrict access to the range they specified.  However, the failure
> mode is usually that nothing draws, or misrendering with wild geometry,
> which is kind of bad for a common mistake.  And people tend to assume
> the range information isn't that important when data is in VBOs.
>
> There's no real advantage, either.  ARB_robust_buffer_access only
> requires us to restrict access to the GL buffer object size, not
> the range of data we think they should access.  Doing that allows
> buggy applications to still function.  (Note that we still use this
> information for busy-tracking, so if they try to overwrite the data
> with glBufferSubData, they'll still hit a bug.)  This seems to be
> safer.
>
> We may want to provide the more strict range as a debug option,
> or scan the VBO and warn against bogus glDrawRangeElements in
> debug contexts.  That can be done as a later patch, though.
>
> Makes Unreal demos draw again.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_draw_upload.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> index f4d1b2c..bfe20c5 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw_upload.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw_upload.c
> @@ -543,6 +543,7 @@ brw_prepare_vertices(struct brw_context *brw)
>             buffer->offset = offset;
>             buffer->stride = glarray->StrideB;
>             buffer->step_rate = glarray->InstanceDivisor;
> +            buffer->size = glarray->BufferObj->Size - offset;
>
>              enabled_buffer[j] = intel_buffer;
>              buffer_range_start[j] = start;
> @@ -610,8 +611,6 @@ brw_prepare_vertices(struct brw_context *brw)
>
>        buffer->bo = intel_bufferobj_buffer(brw, enabled_buffer[i], start,
> range);
>        drm_intel_bo_reference(buffer->bo);
> -
> -      buffer->size = start + range;
>     }
>
>     /* If we need to upload all the arrays, then we can trim those arrays
> to
> --
> 2.8.2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160526/c27f7778/attachment.html>