On 09/11/2015 03:41 AM, Predut, Marius wrote:
>
>
>> -----Original Message-----
>> From: Ian Romanick [mailto:i...@freedesktop.org]
>> Sent: Wednesday, September 09, 2015 8:54 PM
>> To: Predut, Marius; mesa-dev@lists.freedesktop.org
>> Subject: Re: [Mesa-dev] [PATCH v2] i915: fixing driver crashes if too few
>> vertices are submitted
>>
>> On 09/09/2015 11:16 AM, Marius Predut 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.
>>
>> I don't think you really did...
>>
>>> 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 | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/mesa/tnl_dd/t_dd_dmatmp.h b/src/mesa/tnl_dd/t_dd_dmatmp.h
>>> index 7be3954..79de224 100644
>>> --- a/src/mesa/tnl_dd/t_dd_dmatmp.h
>>> +++ b/src/mesa/tnl_dd/t_dd_dmatmp.h
>>> @@ -627,6 +627,9 @@ static void TAG(render_quads_verts)( struct gl_context
>> *ctx,
>>> LOCAL_VARS;
>>> GLuint j;
>>>
>>> + if(count % 4 != 0)
>> ^
>> ...because I'm quite sure Brian's code had a space here, per Mesa's
>> coding standards.
>
> So code style is: if( count % 4 != 0 )?
> I think Brian used: if(count % 4 != 0)
I will copy and paste directly from Brian's e-mail. I don't understand
why this is difficult.
if (count % 4 != 0)
return
Space between "if" and the opening parenthesis.
Spaces around binary operators.
>> Also, this change is incorrect. If an application does
>> glDrawArrays(GL_QUADS, 0, (n * 4) + 3), this change will cause zero
>> quads to be drawn when n quads should be drawn.
>>
>> 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."
>>
>> We probably don't have a piglit test for this scenario, so one should be
>> added. You can CC me on that patch. :)
>
>
> Ok
>
>>
>> I think the correct change is to trim count such that (count % 4) == 0.
>> If the modified value of count is zero, bail out. With that change,
>> the other hunk (below) is unnecessary.
>>
>>> + return;
>>> +
>>> INIT(GL_TRIANGLES);
>>>
>>> for (j = start; j < count-3; j += 4) {
>>> @@ -1248,7 +1251,7 @@ static GLboolean TAG(validate_render)( struct
>> gl_context *ctx,
>>> ok = (GLint) count < GET_SUBSEQUENT_VB_MAX_ELTS();
>>> }
>>> else {
>>> - ok = HAVE_TRIANGLES; /* flatshading is ok. */
>>> + ok = HAVE_TRIANGLES && (count % 4 == 0); /* flatshading is ok. */
>>> }
>>> break;
>>> default:
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
