<div dir="ltr">On 2 September 2013 16:47, Ian Romanick <span dir="ltr"><<a href="mailto:idr@freedesktop.org" target="_blank">idr@freedesktop.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="im">On 09/01/2013 07:05 PM, Kenneth Graunke wrote:<br>
> can_cut_index_handle_prims() was passed an array of _mesa_prim objects<br>
> and a count, and ran a loop for that many iterations. However, it<br>
> treated the array like a pointer, repeatedly checking the first element.<br>
<br>
</div>Blarg. How would an application be able to observe the old bug<br>
behavior? Can we come up with a test case that might tickle it?<br></blockquote><div><br></div><div>We already had some discussion about this (see <a href="http://lists.freedesktop.org/archives/mesa-dev/2013-August/044129.html">http://lists.freedesktop.org/archives/mesa-dev/2013-August/044129.html</a>). Briefly, I wasn't able to come up with a test case that produced incorrect rendering, but I was able to produce a test case where prim[i] != prim[j]. Considering how difficult the VBO module is to understand, I think it's wise to err on the safe side and do what Ken has done in this patch.</div>
<div><br></div><div>Patch is:</div><div><br></div><div>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im"><br>
> This patch makes it actually check every primitive.<br>
><br>
> Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
<br>
</div>Either way, the patch is obviously correct.<br>
<br>
Reviewed-by: Ian Romanick <<a href="mailto:ian.d.romanick@intel.com">ian.d.romanick@intel.com</a>><br>
<div class="im"><br>
> ---<br>
> src/mesa/drivers/dri/i965/brw_primitive_restart.c | 4 ++--<br>
> 1 file changed, 2 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_primitive_restart.c b/src/mesa/drivers/dri/i965/brw_primitive_restart.c<br>
> index 0dbc48f..b305dca 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_primitive_restart.c<br>
> +++ b/src/mesa/drivers/dri/i965/brw_primitive_restart.c<br>
> @@ -92,8 +92,8 @@ can_cut_index_handle_prims(struct gl_context *ctx,<br>
> return false;<br>
> }<br>
><br>
> - for ( ; nr_prims > 0; nr_prims--) {<br>
> - switch(prim->mode) {<br>
> + for (int i = 0; i < nr_prims; i++) {<br>
> + switch (prim[i].mode) {<br>
> case GL_POINTS:<br>
> case GL_LINES:<br>
> case GL_LINE_STRIP:<br>
><br>
<br>
</div><div class=""><div class="h5">_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>