Or the best way to fix it ... outside branches add : " count -= (count-start) & 3; if (cont == 0) return; " and inside branches remove: " count -= (count-start)%4; " and " count -= (count-start) & 3; "
? > -----Original Message----- > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf Of > Predut, Marius > Sent: Monday, September 14, 2015 11:25 AM > To: Ian Romanick; Ilia Mirkin > Cc: mesa-dev@lists.freedesktop.org; Romanick, Ian D > Subject: Re: [Mesa-dev] [PATCH v3] i915: fixing driver crashes if too few > vertices are submitted > > Ok , > So I propose this code to be included into the patch: > > count -= (count - start) % 4; > if (cont == 0) > return; > > What do you think? > > > > > -----Original Message----- > > From: Ian Romanick [mailto:i...@freedesktop.org] > > Sent: Saturday, September 12, 2015 4:46 AM > > To: Ilia Mirkin; Predut, Marius > > Cc: mesa-dev@lists.freedesktop.org; Romanick, Ian D > > Subject: Re: [Mesa-dev] [PATCH v3] i915: fixing driver crashes if too > > few vertices are submitted > > > > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev