[Mesa-dev] [PATCH 1/6] mesa: move check for no-op glFrontFace call earlier

[Brian Paul]

On 07/20/2015 12:27 PM, Ian Romanick wrote:
> Patches 1, 2, and 3 are
>
> Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
>
> The other patches look correct too, but the whole series is
> optimizations, so do you have any before / after performance data?

No, I don't.  I happened across these while looking at apitraces of 
Google Earth.  It emits a surprising number of no-change/redundant GL 
commands.  There are recurring sequences of drawing commands with 
needless glLoadIdentity() and glLoadMatrixf(identy) calls.  Avoiding the 
state validation for that seems like a good thing.

I did some quick and dirty counting of the number of times matrix 
validation was done and this reduces it, but I don't have a good way to 
measure an overall performance change.

As for Eric's concern: "I'm pretty skeptical of adding the memcmp in 
6/6, though."  I don't think the memcmp() will be a big deal in 
practice.  I think the only case where the whole 16-element comparison 
is needed is when the matrix is a pure translation (or identity).  For 
other types of matrices, the m[0] element will usually not be 1.0 and 
the memcmp() would stop early.  In any case, glLoadMatrix is not super 
common and I'd expect the 64-byte memcmp() to be cheaper than the 
matrix-analyze code in m_matrix.c

I'm not super attached to this change so if you're unconvinced I won't 
mind dropping it.

-Brian



>
> On 07/17/2015 05:48 PM, Brian Paul wrote:
>> If the new mode matches the current mode, there can be no error.
>> ---
>>   src/mesa/main/polygon.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/mesa/main/polygon.c b/src/mesa/main/polygon.c
>> index a1f0aa0..6be6f33 100644
>> --- a/src/mesa/main/polygon.c
>> +++ b/src/mesa/main/polygon.c
>> @@ -93,14 +93,14 @@ _mesa_FrontFace( GLenum mode )
>>      if (MESA_VERBOSE&VERBOSE_API)
>>         _mesa_debug(ctx, "glFrontFace %s\n", _mesa_lookup_enum_by_nr(mode));
>>
>> +   if (ctx->Polygon.FrontFace == mode)
>> +      return;
>> +
>>      if (mode!=GL_CW && mode!=GL_CCW) {
>>         _mesa_error( ctx, GL_INVALID_ENUM, "glFrontFace" );
>>         return;
>>      }
>>
>> -   if (ctx->Polygon.FrontFace == mode)
>> -      return;
>> -
>>      FLUSH_VERTICES(ctx, _NEW_POLYGON);
>>      ctx->Polygon.FrontFace = mode;
>>
>>
>