mesa: precompute draw time prim validation during state changes
authorMarek Olšák <marek.olsak@amd.com>
Sat, 16 Jan 2021 14:36:45 +0000 (09:36 -0500)
committerMarge Bot <eric+marge@anholt.net>
Mon, 15 Feb 2021 19:29:09 +0000 (19:29 +0000)
This moves the body of _mesa_valid_prim_mode into new function
_mesa_update_valid_to_render_state, which is called when the affected
states are changed and sets new variable gl_context::ValidPrimMask,
which determines errors reported by draw calls.

_mesa_valid_prim_mode only has to check ValidPrimMask and choose
between GL_INVALID_ENUM and GL_INVALID_OPERATION depending on whether
the primitive would be allowed by the GL version and extensions
(GL_INVALID_OPERATION) or not (GL_INVALID_ENUM).

Reviewed-by: Zoltán Böszörményi <zboszor@gmail.com>
Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/8798>

src/mesa/main/draw_validate.c
src/mesa/main/draw_validate.h
src/mesa/main/enable.c
src/mesa/main/mtypes.h
src/mesa/main/pipelineobj.c
src/mesa/main/polygon.c
src/mesa/main/shaderapi.c
src/mesa/main/transformfeedback.c
src/mesa/main/version.c

index 1e6df88..e519d20 100644 (file)
@@ -351,20 +351,121 @@ check_valid_to_render(struct gl_context *ctx, const char *function)
 }
 
 /**
- * Is 'mode' a valid value for glBegin(), glDrawArrays(), glDrawElements(),
- * etc?  Also, do additional checking related to transformation feedback.
- * Note: this function cannot be called during glNewList(GL_COMPILE) because
- * this code depends on current transform feedback state.
- * Also, do additional checking related to tessellation shaders.
+ * Compute the bitmask of allowed primitive types (ValidPrimMask) depending
+ * on shaders and current states. This is used by draw validation.
+ *
+ * If some combinations of shaders and states are invalid, ValidPrimMask is
+ * set to 0, which will always set GL_INVALID_OPERATION in draw calls
+ * except for invalid enums, which will set GL_INVALID_ENUM, minimizing
+ * the number of gl_context variables that have to be read by draw calls.
  */
-GLboolean
-_mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode, const char *name)
+void
+_mesa_update_valid_to_render_state(struct gl_context *ctx)
 {
-   bool valid_enum = _mesa_is_valid_prim_mode(ctx, mode);
+   struct gl_pipeline_object *shader = ctx->_Shader;
+   unsigned mask = ctx->SupportedPrimMask;
 
-   if (!valid_enum) {
-      _mesa_error(ctx, GL_INVALID_ENUM, "%s(mode=%x)", name, mode);
-      return GL_FALSE;
+   if (_mesa_is_no_error_enabled(ctx)) {
+      ctx->ValidPrimMask = mask;
+      return;
+   }
+
+   /* Start with an empty mask and set this to the trimmed mask at the end. */
+   ctx->ValidPrimMask = 0;
+
+   /* From GL_INTEL_conservative_rasterization spec:
+    *
+    * The conservative rasterization option applies only to polygons with
+    * PolygonMode state set to FILL. Draw requests for polygons with different
+    * PolygonMode setting or for other primitive types (points/lines) generate
+    * INVALID_OPERATION error.
+    */
+   if (ctx->IntelConservativeRasterization) {
+      if (ctx->Polygon.FrontMode != GL_FILL ||
+          ctx->Polygon.BackMode != GL_FILL) {
+         return;
+      } else {
+         mask &= (1 << GL_TRIANGLES) |
+                 (1 << GL_TRIANGLE_STRIP) |
+                 (1 << GL_TRIANGLE_FAN) |
+                 (1 << GL_QUADS) |
+                 (1 << GL_QUAD_STRIP) |
+                 (1 << GL_POLYGON) |
+                 (1 << GL_TRIANGLES_ADJACENCY) |
+                 (1 << GL_TRIANGLE_STRIP_ADJACENCY);
+      }
+   }
+
+   /* From the GL_EXT_transform_feedback spec:
+    *
+    *     "The error INVALID_OPERATION is generated if Begin, or any command
+    *      that performs an explicit Begin, is called when:
+    *
+    *      * a geometry shader is not active and <mode> does not match the
+    *        allowed begin modes for the current transform feedback state as
+    *        given by table X.1.
+    *
+    *      * a geometry shader is active and the output primitive type of the
+    *        geometry shader does not match the allowed begin modes for the
+    *        current transform feedback state as given by table X.1.
+    *
+    */
+   if (_mesa_is_xfb_active_and_unpaused(ctx)) {
+      if(shader->CurrentProgram[MESA_SHADER_GEOMETRY]) {
+         switch (shader->CurrentProgram[MESA_SHADER_GEOMETRY]->
+                    info.gs.output_primitive) {
+         case GL_POINTS:
+            if (ctx->TransformFeedback.Mode != GL_POINTS)
+               mask = 0;
+            break;
+         case GL_LINE_STRIP:
+            if (ctx->TransformFeedback.Mode != GL_LINES)
+               mask = 0;
+            break;
+         case GL_TRIANGLE_STRIP:
+            if (ctx->TransformFeedback.Mode != GL_TRIANGLES)
+               mask = 0;
+            break;
+         default:
+            mask = 0;
+         }
+      }
+      else if (shader->CurrentProgram[MESA_SHADER_TESS_EVAL]) {
+         struct gl_program *tes =
+            shader->CurrentProgram[MESA_SHADER_TESS_EVAL];
+         if (tes->info.tess.point_mode) {
+            if (ctx->TransformFeedback.Mode != GL_POINTS)
+               mask = 0;
+         } else if (tes->info.tess.primitive_mode == GL_ISOLINES) {
+            if (ctx->TransformFeedback.Mode != GL_LINES)
+               mask = 0;
+         } else {
+            if (ctx->TransformFeedback.Mode != GL_TRIANGLES)
+               mask = 0;
+         }
+      }
+      else {
+         switch (ctx->TransformFeedback.Mode) {
+         case GL_POINTS:
+            mask &= 1 << GL_POINTS;
+            break;
+         case GL_LINES:
+            mask &= (1 << GL_LINES) |
+                    (1 << GL_LINE_LOOP) |
+                    (1 << GL_LINE_STRIP);
+            break;
+         case GL_TRIANGLES:
+            /* TODO: This doesn't look right, but it matches the original code. */
+            mask &= ~((1 << GL_POINTS) |
+                      (1 << GL_LINES) |
+                      (1 << GL_LINE_LOOP) |
+                      (1 << GL_LINE_STRIP));
+            break;
+         }
+      }
+
+      if (!mask)
+         return;
    }
 
    /* From the OpenGL 4.5 specification, section 11.3.1:
@@ -395,62 +496,51 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode, const char *name)
     * the draw primitive mode, the tessellation evaluation shader primitive
     * mode should be used for the checking.
    */
-   if (ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]) {
+   if (shader->CurrentProgram[MESA_SHADER_GEOMETRY]) {
       const GLenum geom_mode =
-         ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]->
+         shader->CurrentProgram[MESA_SHADER_GEOMETRY]->
             info.gs.input_primitive;
       struct gl_program *tes =
-         ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL];
-      GLenum mode_before_gs = mode;
+         shader->CurrentProgram[MESA_SHADER_TESS_EVAL];
 
       if (tes) {
+         bool valid;
+
          if (tes->info.tess.point_mode)
-            mode_before_gs = GL_POINTS;
+            valid = geom_mode == GL_POINTS;
          else if (tes->info.tess.primitive_mode == GL_ISOLINES)
-            mode_before_gs = GL_LINES;
+            valid = geom_mode == GL_LINES;
          else
             /* the GL_QUADS mode generates triangles too */
-            mode_before_gs = GL_TRIANGLES;
-      }
+            valid = geom_mode == GL_TRIANGLES;
 
-      switch (mode_before_gs) {
-      case GL_POINTS:
-         valid_enum = (geom_mode == GL_POINTS);
-         break;
-      case GL_LINES:
-      case GL_LINE_LOOP:
-      case GL_LINE_STRIP:
-         valid_enum = (geom_mode == GL_LINES);
-         break;
-      case GL_TRIANGLES:
-      case GL_TRIANGLE_STRIP:
-      case GL_TRIANGLE_FAN:
-         valid_enum = (geom_mode == GL_TRIANGLES);
-         break;
-      case GL_QUADS:
-      case GL_QUAD_STRIP:
-      case GL_POLYGON:
-         valid_enum = false;
-         break;
-      case GL_LINES_ADJACENCY:
-      case GL_LINE_STRIP_ADJACENCY:
-         valid_enum = (geom_mode == GL_LINES_ADJACENCY);
-         break;
-      case GL_TRIANGLES_ADJACENCY:
-      case GL_TRIANGLE_STRIP_ADJACENCY:
-         valid_enum = (geom_mode == GL_TRIANGLES_ADJACENCY);
-         break;
-      default:
-         valid_enum = false;
-         break;
-      }
-      if (!valid_enum) {
-         _mesa_error(ctx, GL_INVALID_OPERATION,
-                     "%s(mode=%s vs geometry shader input %s)",
-                     name,
-                     _mesa_lookup_prim_by_nr(mode_before_gs),
-                     _mesa_lookup_prim_by_nr(geom_mode));
-         return GL_FALSE;
+         /* TES and GS use incompatible primitive types. Discard all draws. */
+         if (!valid)
+            return;
+      } else {
+         switch (geom_mode) {
+         case GL_POINTS:
+            mask &= 1 << GL_POINTS;
+            break;
+         case GL_LINES:
+            mask &= (1 << GL_LINES) |
+                    (1 << GL_LINE_LOOP) |
+                    (1 << GL_LINE_STRIP);
+            break;
+         case GL_TRIANGLES:
+            mask &= (1 << GL_TRIANGLES) |
+                    (1 << GL_TRIANGLE_STRIP) |
+                    (1 << GL_TRIANGLE_FAN);
+            break;
+         case GL_LINES_ADJACENCY:
+            mask &= (1 << GL_LINES_ADJACENCY) |
+                    (1 << GL_LINE_STRIP_ADJACENCY);
+            break;
+         case GL_TRIANGLES_ADJACENCY:
+            mask &= (1 << GL_TRIANGLES_ADJACENCY) |
+                    (1 << GL_TRIANGLE_STRIP_ADJACENCY);
+            break;
+         }
       }
    }
 
@@ -468,133 +558,40 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode, const char *name)
     *      PATCHES."
     *
     */
-   if (ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL] ||
-       ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_CTRL]) {
-      if (mode != GL_PATCHES) {
-         _mesa_error(ctx, GL_INVALID_OPERATION,
-                     "only GL_PATCHES valid with tessellation");
-         return GL_FALSE;
-      }
+   if (shader->CurrentProgram[MESA_SHADER_TESS_EVAL] ||
+       shader->CurrentProgram[MESA_SHADER_TESS_CTRL]) {
+      mask &= 1 << GL_PATCHES;
    }
    else {
-      if (mode == GL_PATCHES) {
-         _mesa_error(ctx, GL_INVALID_OPERATION,
-                     "GL_PATCHES only valid with tessellation");
-         return GL_FALSE;
-      }
+      mask &= ~(1 << GL_PATCHES);
    }
 
-   /* From the GL_EXT_transform_feedback spec:
-    *
-    *     "The error INVALID_OPERATION is generated if Begin, or any command
-    *      that performs an explicit Begin, is called when:
-    *
-    *      * a geometry shader is not active and <mode> does not match the
-    *        allowed begin modes for the current transform feedback state as
-    *        given by table X.1.
-    *
-    *      * a geometry shader is active and the output primitive type of the
-    *        geometry shader does not match the allowed begin modes for the
-    *        current transform feedback state as given by table X.1.
-    *
-    */
-   if (_mesa_is_xfb_active_and_unpaused(ctx)) {
-      GLboolean pass = GL_TRUE;
-
-      if(ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]) {
-         switch (ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]->
-                    info.gs.output_primitive) {
-         case GL_POINTS:
-            pass = ctx->TransformFeedback.Mode == GL_POINTS;
-            break;
-         case GL_LINE_STRIP:
-            pass = ctx->TransformFeedback.Mode == GL_LINES;
-            break;
-         case GL_TRIANGLE_STRIP:
-            pass = ctx->TransformFeedback.Mode == GL_TRIANGLES;
-            break;
-         default:
-            pass = GL_FALSE;
-         }
-      }
-      else if (ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL]) {
-         struct gl_program *tes =
-            ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL];
-         if (tes->info.tess.point_mode)
-            pass = ctx->TransformFeedback.Mode == GL_POINTS;
-         else if (tes->info.tess.primitive_mode == GL_ISOLINES)
-            pass = ctx->TransformFeedback.Mode == GL_LINES;
-         else
-            pass = ctx->TransformFeedback.Mode == GL_TRIANGLES;
-      }
-      else {
-         switch (mode) {
-         case GL_POINTS:
-            pass = ctx->TransformFeedback.Mode == GL_POINTS;
-            break;
-         case GL_LINES:
-         case GL_LINE_STRIP:
-         case GL_LINE_LOOP:
-            pass = ctx->TransformFeedback.Mode == GL_LINES;
-            break;
-         default:
-            pass = ctx->TransformFeedback.Mode == GL_TRIANGLES;
-            break;
-         }
-      }
-      if (!pass) {
-         _mesa_error(ctx, GL_INVALID_OPERATION,
-                         "%s(mode=%s vs transform feedback %s)",
-                         name,
-                         _mesa_lookup_prim_by_nr(mode),
-                         _mesa_lookup_prim_by_nr(ctx->TransformFeedback.Mode));
-         return GL_FALSE;
-      }
-   }
+   ctx->ValidPrimMask = mask;
+}
 
-   /* From GL_INTEL_conservative_rasterization spec:
-    *
-    * The conservative rasterization option applies only to polygons with
-    * PolygonMode state set to FILL. Draw requests for polygons with different
-    * PolygonMode setting or for other primitive types (points/lines) generate
-    * INVALID_OPERATION error.
-    */
-   if (ctx->IntelConservativeRasterization) {
-      GLboolean pass = GL_TRUE;
-
-      switch (mode) {
-      case GL_POINTS:
-      case GL_LINES:
-      case GL_LINE_LOOP:
-      case GL_LINE_STRIP:
-      case GL_LINES_ADJACENCY:
-      case GL_LINE_STRIP_ADJACENCY:
-         pass = GL_FALSE;
-         break;
-      case GL_TRIANGLES:
-      case GL_TRIANGLE_STRIP:
-      case GL_TRIANGLE_FAN:
-      case GL_QUADS:
-      case GL_QUAD_STRIP:
-      case GL_POLYGON:
-      case GL_TRIANGLES_ADJACENCY:
-      case GL_TRIANGLE_STRIP_ADJACENCY:
-         if (ctx->Polygon.FrontMode != GL_FILL ||
-             ctx->Polygon.BackMode != GL_FILL)
-            pass = GL_FALSE;
-         break;
-      default:
-         pass = GL_FALSE;
-      }
-      if (!pass) {
-         _mesa_error(ctx, GL_INVALID_OPERATION,
-                     "mode=%s invalid with GL_INTEL_conservative_rasterization",
-                     _mesa_lookup_prim_by_nr(mode));
-         return GL_FALSE;
-      }
+/**
+ * Is 'mode' a valid value for glBegin(), glDrawArrays(), glDrawElements(),
+ * etc?  Also, do additional checking related to transformation feedback.
+ * Note: this function cannot be called during glNewList(GL_COMPILE) because
+ * this code depends on current transform feedback state.
+ * Also, do additional checking related to tessellation shaders.
+ */
+GLboolean
+_mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode, const char *name)
+{
+   /* All primitive type enums are less than 32, so we can use the shift. */
+   if (mode >= 32 || !((1u << mode) & ctx->ValidPrimMask)) {
+      /* If the primitive type is not in SupportedPrimMask, set GL_INVALID_ENUM,
+       * else set GL_INVALID_OPERATION.
+       */
+      _mesa_error(ctx,
+                  mode >= 32 || !((1u << mode) & ctx->SupportedPrimMask) ?
+                     GL_INVALID_ENUM : GL_INVALID_OPERATION,
+                  "%s(mode=%x)", name, mode);
+      return false;
    }
 
-   return GL_TRUE;
+   return true;
 }
 
 /**
index 1fcf0a4..b9129e9 100644 (file)
@@ -127,6 +127,9 @@ _mesa_validate_MultiDrawElementsIndirectCount(struct gl_context *ctx,
                                               GLsizei maxdrawcount,
                                               GLsizei stride);
 
+extern void
+_mesa_update_valid_to_render_state(struct gl_context *ctx);
+
 /**
  * Is 'mode' a valid value for glBegin(), glDrawArrays(), glDrawElements(),
  * etc?  The set of legal values depends on whether geometry shaders/programs
index 67a112d..fa0c86a 100644 (file)
@@ -34,6 +34,7 @@
 #include "clip.h"
 #include "context.h"
 #include "debug_output.h"
+#include "draw_validate.h"
 #include "enable.h"
 #include "errors.h"
 #include "light.h"
@@ -607,6 +608,7 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, GLboolean state)
          ctx->NewDriverState |=
             ctx->DriverFlags.NewIntelConservativeRasterization;
          ctx->IntelConservativeRasterization = state;
+         _mesa_update_valid_to_render_state(ctx);
          break;
       case GL_CONSERVATIVE_RASTERIZATION_NV:
          if (!_mesa_has_NV_conservative_raster(ctx))
index 1301bfb..8017271 100644 (file)
@@ -5227,6 +5227,13 @@ struct gl_context
     */
    GLbitfield SupportedPrimMask;
 
+   /**
+    * Bitmask of valid primitive types depending on current states (such as
+    * shaders). This is 0 if the current states should result in
+    * GL_INVALID_OPERATION in draw calls.
+    */
+   GLbitfield ValidPrimMask;
+
    /** \name The various 4x4 matrix stacks */
    /*@{*/
    struct gl_matrix_stack ModelviewMatrixStack;
index 0ff3e5c..5dedebd 100644 (file)
@@ -34,6 +34,7 @@
 #include <stdbool.h>
 #include "main/glheader.h"
 #include "main/context.h"
+#include "main/draw_validate.h"
 #include "main/enums.h"
 #include "main/hash.h"
 #include "main/mtypes.h"
@@ -537,6 +538,7 @@ _mesa_bind_pipeline(struct gl_context *ctx,
       _mesa_update_vertex_processing_mode(ctx);
       _mesa_update_allow_draw_out_of_order(ctx);
       _mesa_update_primitive_id_is_unused(ctx);
+      _mesa_update_valid_to_render_state(ctx);
    }
 }
 
index dac56e4..3bb416d 100644 (file)
@@ -31,6 +31,7 @@
 #include "glheader.h"
 
 #include "context.h"
+#include "draw_validate.h"
 #include "image.h"
 #include "enums.h"
 #include "pack.h"
@@ -222,6 +223,9 @@ polygon_mode(struct gl_context *ctx, GLenum face, GLenum mode, bool no_error)
 
    if (ctx->Driver.PolygonMode)
       ctx->Driver.PolygonMode(ctx, face, mode);
+
+   if (ctx->Extensions.INTEL_conservative_rasterization)
+      _mesa_update_valid_to_render_state(ctx);
 }
 
 
index 941d8fb..6909ac2 100644 (file)
@@ -38,6 +38,7 @@
 
 #include "main/glheader.h"
 #include "main/context.h"
+#include "draw_validate.h"
 #include "main/enums.h"
 #include "main/glspirv.h"
 #include "main/hash.h"
@@ -2582,6 +2583,7 @@ _mesa_use_program(struct gl_context *ctx, gl_shader_stage stage,
       _mesa_reference_program(ctx, target, prog);
       _mesa_update_allow_draw_out_of_order(ctx);
       _mesa_update_primitive_id_is_unused(ctx);
+      _mesa_update_valid_to_render_state(ctx);
       if (stage == MESA_SHADER_VERTEX)
          _mesa_update_vertex_processing_mode(ctx);
       return;
index 04ba74e..e24a388 100644 (file)
@@ -33,6 +33,7 @@
 
 #include "buffers.h"
 #include "context.h"
+#include "draw_validate.h"
 #include "hash.h"
 #include "macros.h"
 #include "mtypes.h"
@@ -483,6 +484,7 @@ begin_transform_feedback(struct gl_context *ctx, GLenum mode, bool no_error)
 
    assert(ctx->Driver.BeginTransformFeedback);
    ctx->Driver.BeginTransformFeedback(ctx, mode, obj);
+   _mesa_update_valid_to_render_state(ctx);
 }
 
 
@@ -516,6 +518,7 @@ end_transform_feedback(struct gl_context *ctx,
    ctx->TransformFeedback.CurrentObject->Active = GL_FALSE;
    ctx->TransformFeedback.CurrentObject->Paused = GL_FALSE;
    ctx->TransformFeedback.CurrentObject->EndedAnytime = GL_TRUE;
+   _mesa_update_valid_to_render_state(ctx);
 }
 
 
@@ -1256,6 +1259,7 @@ pause_transform_feedback(struct gl_context *ctx,
    ctx->Driver.PauseTransformFeedback(ctx, obj);
 
    obj->Paused = GL_TRUE;
+   _mesa_update_valid_to_render_state(ctx);
 }
 
 
@@ -1300,6 +1304,7 @@ resume_transform_feedback(struct gl_context *ctx,
 
    assert(ctx->Driver.ResumeTransformFeedback);
    ctx->Driver.ResumeTransformFeedback(ctx, obj);
+   _mesa_update_valid_to_render_state(ctx);
 }
 
 
index 95113c7..083a310 100644 (file)
@@ -25,6 +25,7 @@
 
 #include <stdio.h>
 #include "context.h"
+#include "draw_validate.h"
 
 #include "util/os_misc.h"
 #include "util/simple_mtx.h"
@@ -702,6 +703,9 @@ done:
 
    if (_mesa_has_tessellation(ctx))
       ctx->SupportedPrimMask |= 1 << GL_PATCHES;
+
+   /* First time initialization. */
+   _mesa_update_valid_to_render_state(ctx);
 }