Re: [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<c...@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
c...@chad-versace.us
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev