On 09/11/2015 10:55 AM, Ilia Mirkin wrote: > On Fri, Sep 11, 2015 at 12:36 PM, Marius Predut <marius.pre...@intel.com> > wrote: >> Comparison with a signed expression and unsigned value >> is converted to unsigned value, reason for minus value is interpreted >> as a big unsigned value. For this case the "for" loop >> is going into unexpected behavior. >> >> v1: Brian Paul: code style fix. >> v2: Ian Romanick: glDrawArrays(GL_QUADS, 0, (n * 4) + k) fail , k < 4. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38109 >> Signed-off-by: Marius Predut <marius.pre...@intel.com> >> --- >> src/mesa/tnl_dd/t_dd_dmatmp.h | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/src/mesa/tnl_dd/t_dd_dmatmp.h b/src/mesa/tnl_dd/t_dd_dmatmp.h >> index 7be3954..f99d977 100644 >> --- a/src/mesa/tnl_dd/t_dd_dmatmp.h >> +++ b/src/mesa/tnl_dd/t_dd_dmatmp.h >> @@ -627,6 +627,13 @@ static void TAG(render_quads_verts)( struct gl_context >> *ctx, >> LOCAL_VARS; >> GLuint j; >> >> + /* Page 18 (page 32 of the PDF) of the OpenGL 2.1 spec says: >> + * The total number of vertices between Begin and End is 4n + k, >> + * where 0 ≤ k ≤ 3; if k is not zero, the final k vertices are >> ignored. >> + */ >> + count = (count / 4) * 4; > > Might be just me, but I'd find > > count &= ~0x3 > > to be a lot clearer. Don't know if the compiler can make such an > optimization.
I think it can if count is unsigned. Of course, GLsizei is not unsigned. It is already invalid for count < 0, so your optimization is safe. > However this seems wrong... you're supposed to draw > start..count, so that's the value that has to be div-by-4. Further up, > when there's native quad support, the logic does: I don't think that's right. Count is the number of vertices, not the index of the last vertex. Calling glDrawArrays(GL_QUADS, 47000, 4); still draws one quad. Look at the pseudocode on page 28 (page 42 of the PDF) of the OpenGL 2.1 spec: "The command void DrawArrays(enum mode, int first, sizei count); constructs a sequence of geometric primitives using elements first through first + count − 1 of each enabled array. mode specifies what kind of primitives are constructed; it accepts the same token values as the mode parameter of the Begin command. The effect of DrawArrays(mode, first, count); is the same as the effect of the command sequence if (mode or count is invalid) generate appropriate error else { Begin(mode); for (int i = 0; i < count; i++) ArrayElement(first + i); End(); }" Combining that with the language previously quoted, I think this change is right. > /* Emit whole number of quads in total. dmasz is already a multiple > * of 4. > */ > count -= (count-start)%4; > > Which seems more accurate. > >> + if(count == 0) return; > > if (count == 0) > > note the space. That's the style used in all of mesa. > > $ git grep '\sif(' | wc -l > 1076 > $ git grep '\sif (' | wc -l > 58071 > > I guess a few 'if(' instances snuck through, mostly in src/gallium. > But the overwhelming majority are of the 'if (' style. > >> + >> INIT(GL_TRIANGLES); >> >> for (j = start; j < count-3; j += 4) { >> -- >> 1.9.1 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev