mesa: flatten STATE_MATERIAL and STATE_LIGHTPROD tokens
authorMarek Olšák <marek.olsak@amd.com>
Sat, 5 Dec 2020 01:22:15 +0000 (20:22 -0500)
committerMarge Bot <eric+marge@anholt.net>
Thu, 21 Jan 2021 21:59:29 +0000 (21:59 +0000)
Flattening continue to get optimal code in fetch_state.

This merges the "face" field with the "attrib" field using the combined
MAT_ATTRIB_* enums. The outcome is that the inner switch statements can
be flatten because we can use MAT_ATTRIB_* to index into the attrib array
directly.

With LightSource attributes that don't have two sides, more math is
involved to get the correct index but it works out nicely too.

Reviewed-by: Eric Anholt <eric@anholt.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8183>

src/compiler/glsl/builtin_variables.cpp
src/mesa/main/ffvertex_prog.c
src/mesa/program/prog_statevars.c
src/mesa/program/program_parse.y

index 4201790..4687bc2 100644 (file)
@@ -74,19 +74,19 @@ static const struct gl_builtin_uniform_element gl_Point_elements[] = {
 };
 
 static const struct gl_builtin_uniform_element gl_FrontMaterial_elements[] = {
-   {"emission", {STATE_MATERIAL, 0, STATE_EMISSION}, SWIZZLE_XYZW},
-   {"ambient", {STATE_MATERIAL, 0, STATE_AMBIENT}, SWIZZLE_XYZW},
-   {"diffuse", {STATE_MATERIAL, 0, STATE_DIFFUSE}, SWIZZLE_XYZW},
-   {"specular", {STATE_MATERIAL, 0, STATE_SPECULAR}, SWIZZLE_XYZW},
-   {"shininess", {STATE_MATERIAL, 0, STATE_SHININESS}, SWIZZLE_XXXX},
+   {"emission", {STATE_MATERIAL, MAT_ATTRIB_FRONT_EMISSION}, SWIZZLE_XYZW},
+   {"ambient", {STATE_MATERIAL, MAT_ATTRIB_FRONT_AMBIENT}, SWIZZLE_XYZW},
+   {"diffuse", {STATE_MATERIAL, MAT_ATTRIB_FRONT_DIFFUSE}, SWIZZLE_XYZW},
+   {"specular", {STATE_MATERIAL, MAT_ATTRIB_FRONT_SPECULAR}, SWIZZLE_XYZW},
+   {"shininess", {STATE_MATERIAL, MAT_ATTRIB_FRONT_SHININESS}, SWIZZLE_XXXX},
 };
 
 static const struct gl_builtin_uniform_element gl_BackMaterial_elements[] = {
-   {"emission", {STATE_MATERIAL, 1, STATE_EMISSION}, SWIZZLE_XYZW},
-   {"ambient", {STATE_MATERIAL, 1, STATE_AMBIENT}, SWIZZLE_XYZW},
-   {"diffuse", {STATE_MATERIAL, 1, STATE_DIFFUSE}, SWIZZLE_XYZW},
-   {"specular", {STATE_MATERIAL, 1, STATE_SPECULAR}, SWIZZLE_XYZW},
-   {"shininess", {STATE_MATERIAL, 1, STATE_SHININESS}, SWIZZLE_XXXX},
+   {"emission", {STATE_MATERIAL, MAT_ATTRIB_BACK_EMISSION}, SWIZZLE_XYZW},
+   {"ambient", {STATE_MATERIAL, MAT_ATTRIB_BACK_AMBIENT}, SWIZZLE_XYZW},
+   {"diffuse", {STATE_MATERIAL, MAT_ATTRIB_BACK_DIFFUSE}, SWIZZLE_XYZW},
+   {"specular", {STATE_MATERIAL, MAT_ATTRIB_BACK_SPECULAR}, SWIZZLE_XYZW},
+   {"shininess", {STATE_MATERIAL, MAT_ATTRIB_BACK_SHININESS}, SWIZZLE_XXXX},
 };
 
 static const struct gl_builtin_uniform_element gl_LightSource_elements[] = {
@@ -121,15 +121,15 @@ static const struct gl_builtin_uniform_element gl_BackLightModelProduct_elements
 };
 
 static const struct gl_builtin_uniform_element gl_FrontLightProduct_elements[] = {
-   {"ambient", {STATE_LIGHTPROD, 0, 0, STATE_AMBIENT}, SWIZZLE_XYZW},
-   {"diffuse", {STATE_LIGHTPROD, 0, 0, STATE_DIFFUSE}, SWIZZLE_XYZW},
-   {"specular", {STATE_LIGHTPROD, 0, 0, STATE_SPECULAR}, SWIZZLE_XYZW},
+   {"ambient", {STATE_LIGHTPROD, 0, MAT_ATTRIB_FRONT_AMBIENT}, SWIZZLE_XYZW},
+   {"diffuse", {STATE_LIGHTPROD, 0, MAT_ATTRIB_FRONT_DIFFUSE}, SWIZZLE_XYZW},
+   {"specular", {STATE_LIGHTPROD, 0, MAT_ATTRIB_FRONT_SPECULAR}, SWIZZLE_XYZW},
 };
 
 static const struct gl_builtin_uniform_element gl_BackLightProduct_elements[] = {
-   {"ambient", {STATE_LIGHTPROD, 0, 1, STATE_AMBIENT}, SWIZZLE_XYZW},
-   {"diffuse", {STATE_LIGHTPROD, 0, 1, STATE_DIFFUSE}, SWIZZLE_XYZW},
-   {"specular", {STATE_LIGHTPROD, 0, 1, STATE_SPECULAR}, SWIZZLE_XYZW},
+   {"ambient", {STATE_LIGHTPROD, 0, MAT_ATTRIB_BACK_AMBIENT}, SWIZZLE_XYZW},
+   {"diffuse", {STATE_LIGHTPROD, 0, MAT_ATTRIB_BACK_DIFFUSE}, SWIZZLE_XYZW},
+   {"specular", {STATE_LIGHTPROD, 0, MAT_ATTRIB_BACK_SPECULAR}, SWIZZLE_XYZW},
 };
 
 static const struct gl_builtin_uniform_element gl_TextureEnvColor_elements[] = {
index 9158a35..dd04c2f 100644 (file)
@@ -881,7 +881,7 @@ static struct ureg get_material( struct tnl_program *p, GLuint side,
       return register_input( p, VERT_ATTRIB_MAT(attrib) );
    }
    else
-      return register_param3( p, STATE_MATERIAL, side, property );
+      return register_param2(p, STATE_MATERIAL, attrib);
 }
 
 #define SCENE_COLOR_BITS(side) (( MAT_BIT_FRONT_EMISSION | \
@@ -930,7 +930,7 @@ static struct ureg get_lightprod( struct tnl_program *p, GLuint light,
       return tmp;
    }
    else
-      return register_param4(p, STATE_LIGHTPROD, light, side, property);
+      return register_param3(p, STATE_LIGHTPROD, light, attrib);
 }
 
 
index 7ee39ba..3dbdb22 100644 (file)
@@ -30,6 +30,7 @@
 
 
 #include <stdio.h>
+#include <stddef.h>
 #include "main/glheader.h"
 #include "main/context.h"
 #include "main/blend.h"
@@ -87,39 +88,20 @@ fetch_state(struct gl_context *ctx, const gl_state_index16 state[],
    switch (state[0]) {
    case STATE_MATERIAL:
       {
-         /* state[1] is either 0=front or 1=back side */
-         const GLuint face = (GLuint) state[1];
+         /* state[1] is MAT_ATTRIB_FRONT_* */
+         const GLuint index = (GLuint) state[1];
          const struct gl_material *mat = &ctx->Light.Material;
-         assert(face == 0 || face == 1);
-         /* we rely on tokens numbered so that _BACK_ == _FRONT_+ 1 */
-         assert(MAT_ATTRIB_FRONT_AMBIENT + 1 == MAT_ATTRIB_BACK_AMBIENT);
-         /* XXX we could get rid of this switch entirely with a little
-          * work in arbprogparse.c's parse_state_single_item().
-          */
-         /* state[2] is the material attribute */
-         switch (state[2]) {
-         case STATE_AMBIENT:
-            COPY_4V(value, mat->Attrib[MAT_ATTRIB_FRONT_AMBIENT + face]);
-            return;
-         case STATE_DIFFUSE:
-            COPY_4V(value, mat->Attrib[MAT_ATTRIB_FRONT_DIFFUSE + face]);
-            return;
-         case STATE_SPECULAR:
-            COPY_4V(value, mat->Attrib[MAT_ATTRIB_FRONT_SPECULAR + face]);
-            return;
-         case STATE_EMISSION:
-            COPY_4V(value, mat->Attrib[MAT_ATTRIB_FRONT_EMISSION + face]);
-            return;
-         case STATE_SHININESS:
-            value[0] = mat->Attrib[MAT_ATTRIB_FRONT_SHININESS + face][0];
+         assert(index >= MAT_ATTRIB_FRONT_AMBIENT &&
+                index <= MAT_ATTRIB_BACK_SHININESS);
+         if (index >= MAT_ATTRIB_FRONT_SHININESS) {
+            value[0] = mat->Attrib[index][0];
             value[1] = 0.0F;
             value[2] = 0.0F;
             value[3] = 1.0F;
-            return;
-         default:
-            unreachable("Invalid material state in fetch_state");
-            return;
+         } else {
+            COPY_4V(value, mat->Attrib[index]);
          }
+         return;
       }
    case STATE_LIGHT:
       {
@@ -175,38 +157,26 @@ fetch_state(struct gl_context *ctx, const gl_state_index16 state[],
    case STATE_LIGHTPROD:
       {
          const GLuint ln = (GLuint) state[1];
-         const GLuint face = (GLuint) state[2];
-         GLint i;
-         assert(face == 0 || face == 1);
-         switch (state[3]) {
-            case STATE_AMBIENT:
-               for (i = 0; i < 3; i++) {
-                  value[i] = ctx->Light.LightSource[ln].Ambient[i] *
-                     ctx->Light.Material.Attrib[MAT_ATTRIB_FRONT_AMBIENT+face][i];
-               }
-               /* [3] = material alpha */
-               value[3] = ctx->Light.Material.Attrib[MAT_ATTRIB_FRONT_AMBIENT+face][3];
-               return;
-            case STATE_DIFFUSE:
-               for (i = 0; i < 3; i++) {
-                  value[i] = ctx->Light.LightSource[ln].Diffuse[i] *
-                     ctx->Light.Material.Attrib[MAT_ATTRIB_FRONT_DIFFUSE+face][i];
-               }
-               /* [3] = material alpha */
-               value[3] = ctx->Light.Material.Attrib[MAT_ATTRIB_FRONT_DIFFUSE+face][3];
-               return;
-            case STATE_SPECULAR:
-               for (i = 0; i < 3; i++) {
-                  value[i] = ctx->Light.LightSource[ln].Specular[i] *
-                     ctx->Light.Material.Attrib[MAT_ATTRIB_FRONT_SPECULAR+face][i];
-               }
-               /* [3] = material alpha */
-               value[3] = ctx->Light.Material.Attrib[MAT_ATTRIB_FRONT_SPECULAR+face][3];
-               return;
-            default:
-               unreachable("Invalid lightprod state in fetch_state");
-               return;
+         const GLuint index = (GLuint) state[2];
+         const GLuint attr = (index / 2) * 4;
+         assert(index >= MAT_ATTRIB_FRONT_AMBIENT &&
+                index <= MAT_ATTRIB_BACK_SPECULAR);
+         for (int i = 0; i < 3; i++) {
+            /* We want attr to access out of bounds into the following Diffuse
+             * and Specular fields. This is guaranteed to work because
+             * STATE_LIGHT and STATE_LIGHT_ATTRIBS also rely on this memory
+             * layout.
+             */
+            STATIC_ASSERT(offsetof(struct gl_light_uniforms, Ambient) + 16 ==
+                          offsetof(struct gl_light_uniforms, Diffuse));
+            STATIC_ASSERT(offsetof(struct gl_light_uniforms, Diffuse) + 16 ==
+                          offsetof(struct gl_light_uniforms, Specular));
+            value[i] = ctx->Light.LightSource[ln].Ambient[attr + i] *
+                       ctx->Light.Material.Attrib[index][i];
          }
+         /* [3] = material alpha */
+         value[3] = ctx->Light.Material.Attrib[index][3];
+         return;
       }
    case STATE_TEXGEN:
       {
@@ -835,7 +805,7 @@ append_token(char *dst, gl_state_index k)
 {
    switch (k) {
    case STATE_MATERIAL:
-      append(dst, "material.");
+      append(dst, "material");
       break;
    case STATE_LIGHT:
       append(dst, "light");
@@ -1062,15 +1032,6 @@ append_token(char *dst, gl_state_index k)
 }
 
 static void
-append_face(char *dst, GLint face)
-{
-   if (face == 0)
-      append(dst, "front.");
-   else
-      append(dst, "back.");
-}
-
-static void
 append_index(char *dst, GLint index, bool structure)
 {
    char s[20];
@@ -1094,8 +1055,7 @@ _mesa_program_state_string(const gl_state_index16 state[STATE_LENGTH])
 
    switch (state[0]) {
    case STATE_MATERIAL:
-      append_face(str, state[1]);
-      append_token(str, state[2]);
+      append_index(str, state[1], false);
       break;
    case STATE_LIGHT:
       append_index(str, state[1], true); /* light number [i]. */
@@ -1116,9 +1076,8 @@ _mesa_program_state_string(const gl_state_index16 state[STATE_LENGTH])
       }
       break;
    case STATE_LIGHTPROD:
-      append_index(str, state[1], true); /* light number [i]. */
-      append_face(str, state[2]);
-      append_token(str, state[3]);
+      append_index(str, state[1], false); /* light number [i] */
+      append_index(str, state[2], false);
       break;
    case STATE_TEXGEN:
       append_index(str, state[1], true); /* tex unit [i] */
index 8d46461..a42a988 100644 (file)
@@ -260,7 +260,8 @@ static struct asm_instruction *asm_instruction_copy_ctor(
 %type <integer> stateOptModMatNum stateModMatNum statePaletteMatNum
 %type <integer> stateProgramMatNum
 
-%type <integer> ambDiffSpecProperty
+%type <integer> ambDiffSpecPropertyMaterial
+%type <integer> ambDiffSpecPropertyLight
 
 %type <state> programSingleItem progEnvParam progLocalParam
 %type <state> programMultipleItem progEnvParams progLocalParams
@@ -1240,22 +1241,22 @@ stateMaterialItem: MATERIAL optFaceType stateMatProperty
        {
           memset($$, 0, sizeof($$));
           $$[0] = STATE_MATERIAL;
-          $$[1] = $2;
-          $$[2] = $3;
+          $$[1] = $3 + $2;
+          $$[2] = 0;
        }
        ;
 
-stateMatProperty: ambDiffSpecProperty
+stateMatProperty: ambDiffSpecPropertyMaterial
        {
           $$ = $1;
        }
        | EMISSION
        {
-          $$ = STATE_EMISSION;
+          $$ = MAT_ATTRIB_FRONT_EMISSION;
        }
        | SHININESS
        {
-          $$ = STATE_SHININESS;
+          $$ = MAT_ATTRIB_FRONT_SHININESS;
        }
        ;
 
@@ -1268,7 +1269,7 @@ stateLightItem: LIGHT '[' stateLightNumber ']' stateLightProperty
        }
        ;
 
-stateLightProperty: ambDiffSpecProperty
+stateLightProperty: ambDiffSpecPropertyLight
        {
           $$ = $1;
        }
@@ -1326,12 +1327,12 @@ stateLightProdItem: LIGHTPROD '[' stateLightNumber ']' optFaceType stateLProdPro
           memset($$, 0, sizeof($$));
           $$[0] = STATE_LIGHTPROD;
           $$[1] = $3;
-          $$[2] = $5;
-          $$[3] = $6;
+          $$[2] = $6 + $5;
+          $$[3] = 0;
        }
        ;
 
-stateLProdProperty: ambDiffSpecProperty;
+stateLProdProperty: ambDiffSpecPropertyMaterial;
 
 stateTexEnvItem: TEXENV optLegacyTexUnitNum stateTexEnvProperty
        {
@@ -1347,20 +1348,34 @@ stateTexEnvProperty: COLOR
        }
        ;
 
-ambDiffSpecProperty: AMBIENT
+ambDiffSpecPropertyMaterial: AMBIENT
        {
-          $$ = STATE_AMBIENT;
+          $$ = MAT_ATTRIB_FRONT_AMBIENT;
        }
        | DIFFUSE
        {
-          $$ = STATE_DIFFUSE;
+          $$ = MAT_ATTRIB_FRONT_DIFFUSE;
        }
        | SPECULAR
        {
-          $$ = STATE_SPECULAR;
+          $$ = MAT_ATTRIB_FRONT_SPECULAR;
        }
        ;
 
+ambDiffSpecPropertyLight: AMBIENT
+        {
+           $$ = STATE_AMBIENT;
+        }
+        | DIFFUSE
+        {
+           $$ = STATE_DIFFUSE;
+        }
+        | SPECULAR
+        {
+           $$ = STATE_SPECULAR;
+        }
+        ;
+
 stateLightNumber: INTEGER
        {
           if ((unsigned) $1 >= state->MaxLights) {