[Mesa-dev] i965: Make sure we always compute valid index bounds before drawing.

Submitted by Iago Toral Quiroga on March 27, 2014, 5:45 p.m.

Details

Message ID 1395942352-7265-2-git-send-email-itoral@igalia.com
State Superseded
Headers show

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga March 27, 2014, 5:45 p.m.
When doing software rendering (i.e. rendering to the selection buffer) we need
to make sure that we have valid index bounds before calling _tnl_draw_prims(),
otherwise we can crash.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59455
---
 src/mesa/drivers/dri/i965/brw_draw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
index d684c17..498cfe2 100644
--- a/src/mesa/drivers/dri/i965/brw_draw.c
+++ b/src/mesa/drivers/dri/i965/brw_draw.c
@@ -554,7 +554,8 @@  void brw_draw_prims( struct gl_context *ctx,
     * get the minimum and maximum of their index buffer so we know what range
     * to upload.
     */
-   if (!vbo_all_varyings_in_vbos(arrays) && !index_bounds_valid) {
+   if ((!vbo_all_varyings_in_vbos(arrays) || ctx->RenderMode != GL_RENDER)
+       && !index_bounds_valid) {
       perf_debug("Scanning index buffer to compute index buffer bounds.  "
                  "Use glDrawRangeElements() to avoid this.\n");
       vbo_get_minmax_indices(ctx, prims, ib, &min_index, &max_index, nr_prims);

Comments

On 03/27/2014 10:45 AM, Iago Toral Quiroga wrote:
> When doing software rendering (i.e. rendering to the selection buffer) we need
> to make sure that we have valid index bounds before calling _tnl_draw_prims(),
> otherwise we can crash.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59455
> ---
>  src/mesa/drivers/dri/i965/brw_draw.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
> index d684c17..498cfe2 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -554,7 +554,8 @@ void brw_draw_prims( struct gl_context *ctx,
>      * get the minimum and maximum of their index buffer so we know what range
>      * to upload.
>      */
> -   if (!vbo_all_varyings_in_vbos(arrays) && !index_bounds_valid) {
> +   if ((!vbo_all_varyings_in_vbos(arrays) || ctx->RenderMode != GL_RENDER)
> +       && !index_bounds_valid) {
>        perf_debug("Scanning index buffer to compute index buffer bounds.  "
>                   "Use glDrawRangeElements() to avoid this.\n");
>        vbo_get_minmax_indices(ctx, prims, ib, &min_index, &max_index, nr_prims);
> 

Thanks for the patch!  This looks good to me.

Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

My only suggestion would be to rearrange the order of things:

if (!index_bounds_valid &&
    (ctx->RenderMode != GL_RENDER || !vbo_all_varyings_in_vbos(array))) {
   ...
}

By checking index_bounds_valid first, we could avoid the cost of
vbo_all_varyings_in_vbos, which does a non-trivial amount of work.

Would you mind sending a patch which does that?  I'll gladly push it.

--Ken