[Mesa-dev] [PATCH 15/19] i965: Change type of brw_context.primitive from GLenum to hardware primitive

[Chad Versace]

On 09/26/2011 12:05 PM, Eric Anholt wrote:
> On Fri, 23 Sep 2011 17:37:45 -0700, Chad Versace<chad at chad-versace.us>  wrote:
>> For example, GL_TRIANLES is converted to _3DPRIM_TRILIST.
>
> missing a "G"

Thanks.

>
>> -static const GLenum reduced_prim[GL_POLYGON+1] = {
>> -   GL_POINTS,
>> -   GL_LINES,
>> -   GL_LINES,
>> -   GL_LINES,
>> -   GL_TRIANGLES,
>> -   GL_TRIANGLES,
>> -   GL_TRIANGLES,
>> -   GL_TRIANGLES,
>> -   GL_TRIANGLES,
>> -   GL_TRIANGLES
>> +static const GLenum
>> +brw_get_reduced_prim(uint32_t hw_prim)
>> +{
>> +   switch (hw_prim) {
>> +   case _3DPRIM_POINTLIST:  return GL_POINTS;
>> +   case _3DPRIM_LINELIST:   return GL_LINES;
>> +   case _3DPRIM_LINELOOP:   return GL_LINES;
>> +   case _3DPRIM_LINESTRIP:  return GL_LINES;
>> +   case _3DPRIM_TRILIST:    return GL_TRIANGLES;
>> +   case _3DPRIM_TRISTRIP:   return GL_TRIANGLES;
>> +   case _3DPRIM_TRIFAN:     return GL_TRIANGLES;
>> +   case _3DPRIM_QUADLIST:   return GL_TRIANGLES;
>> +   case _3DPRIM_QUADSTRIP:  return GL_TRIANGLES;
>> +   case _3DPRIM_POLYGON:    return GL_TRIANGLES;
>> +   default:                 assert(0); return 0;
>> +   }
>>   };
>
> I always get scared by changes from tables to switch statements in hot
> paths because I don't trust gcc.  I haven't tested to see whether it
> does the wrong thing, though.

I investigated what gcc did here with -O3, and it did not do the obvious thing.
So I'll convert these back to lookup tables in v2.

>
>> @@ -79,41 +83,40 @@ static const GLenum reduced_prim[GL_POLYGON+1] = {
>>    * programs be immune to the active primitive (ie. cope with all
>>    * possibilities).  That may not be realistic however.
>>    */
>> -static GLuint brw_set_prim(struct brw_context *brw,
>> +static void brw_set_prim(struct brw_context *brw,
>>   			   const struct _mesa_prim *prim)
>>   {
>>      struct gl_context *ctx =&brw->intel.ctx;
>> -   GLenum mode = prim->mode;
>> +   uint32_t hw_prim;
>>
>>      DBG("PRIM: %s\n", _mesa_lookup_enum_by_nr(prim->mode));
>>
>>      /* Slight optimization to avoid the GS program when not needed:
>>       */
>> -   if (mode == GL_QUAD_STRIP&&
>> -       ctx->Light.ShadeModel != GL_FLAT&&
>> -       ctx->Polygon.FrontMode == GL_FILL&&
>> -       ctx->Polygon.BackMode == GL_FILL)
>> -      mode = GL_TRIANGLE_STRIP;
>> -
>> -   if (prim->mode == GL_QUADS&&  prim->count == 4&&
>> +   if (prim->mode == GL_QUAD_STRIP&&
>>          ctx->Light.ShadeModel != GL_FLAT&&
>>          ctx->Polygon.FrontMode == GL_FILL&&
>>          ctx->Polygon.BackMode == GL_FILL) {
>> -      mode = GL_TRIANGLE_FAN;
>> +      hw_prim = _3DPRIM_TRISTRIP;
>> +   } else if (prim->mode == GL_QUADS&&  prim->count == 4&&
>> +	      ctx->Light.ShadeModel != GL_FLAT&&
>> +	      ctx->Polygon.FrontMode == GL_FILL&&
>> +	      ctx->Polygon.BackMode == GL_FILL) {
>> +      hw_prim = _3DPRIM_TRIFAN;
>> +   } else {
>> +      hw_prim = prim_to_hw_prim[prim->mode];
>>      }
>
> Hmm, you know, this little optimization only matters to pre-snb (I would
> guess that it's a marginally bad thing on snb).  That doesn't leave much
> of this function shared between the two.

How about I precede this commit with
     i965: Split brw_set_prim into brw/gen6 variants
     
     The "slight optimization to avoid the GS program" in brw_set_prim() is not
     used by Gen 6, since Gen 6 doesn't use a GS program. Also, Gen 6 doesn't use
     reduced primitives.

>
>> -   if (mode != brw->primitive) {
>> -      brw->primitive = mode;
>> +   if (hw_prim != brw->primitive) {
>> +      brw->primitive = hw_prim;
>>         brw->state.dirty.brw |= BRW_NEW_PRIMITIVE;
>>
>>         if (brw->intel.gen<  6&&
>> -	  reduced_prim[mode] != brw->intel.reduced_primitive) {
>> -	 brw->intel.reduced_primitive = reduced_prim[mode];
>> +	  brw->intel.reduced_primitive != brw_get_reduced_prim(hw_prim)) {
>> +	 brw->intel.reduced_primitive = brw_get_reduced_prim(hw_prim);
>>   	 brw->state.dirty.brw |= BRW_NEW_REDUCED_PRIMITIVE;
>>         }
>>      }
>> -
>> -   return prim_to_hw_prim[mode];
>>   }

-- 
Chad Versace
chad at chad-versace.us