Mesa (master): draw: fix user culling pipeline order. (v2)

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Wed Apr 15 04:49:32 UTC 2020


Module: Mesa
Branch: master
Commit: 381e9fe64a80d98144a4ad75044edd9b878c7de7
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=381e9fe64a80d98144a4ad75044edd9b878c7de7

Author: Dave Airlie <airlied at redhat.com>
Date:   Fri Mar 27 11:33:43 2020 +1000

draw: fix user culling pipeline order. (v2)

GL spec requires user culling, then clipping then face culling.
llvmpipe was doing clipping then user culling then face culling.

Fix the ordering by adding a new user_cull stage that does the user
culling

Fixes piglit clip_cull-4.shader_test

v2: simplify this a lot (Roland)

Reviewed-by: Roland Scheidegger <sroland at vmware.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4560>

---

 .gitlab-ci/piglit/quick_shader.txt               |   5 +-
 src/gallium/auxiliary/Makefile.sources           |   1 +
 src/gallium/auxiliary/draw/draw_pipe.c           |   4 +
 src/gallium/auxiliary/draw/draw_pipe.h           |   1 +
 src/gallium/auxiliary/draw/draw_pipe_cull.c      | 131 +--------------
 src/gallium/auxiliary/draw/draw_pipe_user_cull.c | 197 +++++++++++++++++++++++
 src/gallium/auxiliary/draw/draw_pipe_validate.c  |   8 +-
 src/gallium/auxiliary/draw/draw_private.h        |   1 +
 src/gallium/auxiliary/meson.build                |   1 +
 9 files changed, 216 insertions(+), 133 deletions(-)

diff --git a/.gitlab-ci/piglit/quick_shader.txt b/.gitlab-ci/piglit/quick_shader.txt
index ce33accbfde..a83073ceab5 100644
--- a/.gitlab-ci/piglit/quick_shader.txt
+++ b/.gitlab-ci/piglit/quick_shader.txt
@@ -86,7 +86,6 @@ spec/arb_compute_variable_group_size/execution/separate-global-id: skip
 spec/arb_compute_variable_group_size/execution/separate-global-id-2: skip
 spec/arb_compute_variable_group_size/linker/mixed_fixed_variable_local_work_size: skip
 spec/arb_compute_variable_group_size/linker/no_local_size_specified: skip
-spec/arb_cull_distance/clip-cull-4: fail
 spec/arb_geometry_shader4/execution/2darray-basic: skip
 spec/arb_geometry_shader4/execution/clip-distance-bulk-copy: skip
 spec/arb_geometry_shader4/execution/clip-distance-in-bulk-read: skip
@@ -4636,8 +4635,8 @@ spec/oes_viewport_array/viewport-gs-writes-out-of-range: skip
 summary:
        name:  results
        ----  --------
-       pass:    10711
-       fail:       58
+       pass:    10712
+       fail:       57
       crash:        0
        skip:     4577
     timeout:        0
diff --git a/src/gallium/auxiliary/Makefile.sources b/src/gallium/auxiliary/Makefile.sources
index b79dc1d5958..77ec768b5c3 100644
--- a/src/gallium/auxiliary/Makefile.sources
+++ b/src/gallium/auxiliary/Makefile.sources
@@ -26,6 +26,7 @@ C_SOURCES := \
 	draw/draw_pipe_stipple.c \
 	draw/draw_pipe_twoside.c \
 	draw/draw_pipe_unfilled.c \
+	draw/draw_pipe_user_cull.c \
 	draw/draw_pipe_util.c \
 	draw/draw_pipe_validate.c \
 	draw/draw_pipe_vbuf.c \
diff --git a/src/gallium/auxiliary/draw/draw_pipe.c b/src/gallium/auxiliary/draw/draw_pipe.c
index 27a37625263..339bc7f4df4 100644
--- a/src/gallium/auxiliary/draw/draw_pipe.c
+++ b/src/gallium/auxiliary/draw/draw_pipe.c
@@ -49,6 +49,7 @@ boolean draw_pipeline_init( struct draw_context *draw )
    draw->pipeline.clip      = draw_clip_stage( draw );
    draw->pipeline.flatshade = draw_flatshade_stage( draw );
    draw->pipeline.cull      = draw_cull_stage( draw );
+   draw->pipeline.user_cull = draw_user_cull_stage( draw );
    draw->pipeline.validate  = draw_validate_stage( draw );
    draw->pipeline.first     = draw->pipeline.validate;
 
@@ -61,6 +62,7 @@ boolean draw_pipeline_init( struct draw_context *draw )
        !draw->pipeline.clip ||
        !draw->pipeline.flatshade ||
        !draw->pipeline.cull ||
+       !draw->pipeline.user_cull ||
        !draw->pipeline.validate)
       return FALSE;
 
@@ -95,6 +97,8 @@ void draw_pipeline_destroy( struct draw_context *draw )
       draw->pipeline.flatshade->destroy( draw->pipeline.flatshade );
    if (draw->pipeline.cull)
       draw->pipeline.cull->destroy( draw->pipeline.cull );
+   if (draw->pipeline.user_cull)
+      draw->pipeline.user_cull->destroy( draw->pipeline.user_cull );
    if (draw->pipeline.validate)
       draw->pipeline.validate->destroy( draw->pipeline.validate );
    if (draw->pipeline.aaline)
diff --git a/src/gallium/auxiliary/draw/draw_pipe.h b/src/gallium/auxiliary/draw/draw_pipe.h
index e69dcbded0e..4ae8883f096 100644
--- a/src/gallium/auxiliary/draw/draw_pipe.h
+++ b/src/gallium/auxiliary/draw/draw_pipe.h
@@ -87,6 +87,7 @@ extern struct draw_stage *draw_offset_stage( struct draw_context *context );
 extern struct draw_stage *draw_clip_stage( struct draw_context *context );
 extern struct draw_stage *draw_flatshade_stage( struct draw_context *context );
 extern struct draw_stage *draw_cull_stage( struct draw_context *context );
+extern struct draw_stage *draw_user_cull_stage( struct draw_context *draw );
 extern struct draw_stage *draw_stipple_stage( struct draw_context *context );
 extern struct draw_stage *draw_wide_line_stage( struct draw_context *context );
 extern struct draw_stage *draw_wide_point_stage( struct draw_context *context );
diff --git a/src/gallium/auxiliary/draw/draw_pipe_cull.c b/src/gallium/auxiliary/draw/draw_pipe_cull.c
index 318d743dbbf..a873edbe76c 100644
--- a/src/gallium/auxiliary/draw/draw_pipe_cull.c
+++ b/src/gallium/auxiliary/draw/draw_pipe_cull.c
@@ -51,105 +51,12 @@ static inline struct cull_stage *cull_stage( struct draw_stage *stage )
    return (struct cull_stage *)stage;
 }
 
-static inline boolean
-cull_distance_is_out(float dist)
-{
-   return (dist < 0.0f) || util_is_inf_or_nan(dist);
-}
-
 /*
- * If the shader writes the culldistance then we can
- * perform distance based culling. Distance based
- * culling doesn't require a face and can be performed
- * on primitives without faces (e.g. points and lines)
- */
-static void cull_point( struct draw_stage *stage,
-                        struct prim_header *header )
-{
-   const unsigned num_written_culldistances =
-      draw_current_shader_num_written_culldistances(stage->draw);
-   const unsigned num_written_clipdistances =
-      draw_current_shader_num_written_clipdistances(stage->draw);
-   unsigned i;
-
-   debug_assert(num_written_culldistances);
-
-   for (i = 0; i < num_written_culldistances; ++i) {
-      unsigned cull_idx = (num_written_clipdistances + i) / 4;
-      unsigned out_idx =
-         draw_current_shader_ccdistance_output(stage->draw, cull_idx);
-      unsigned idx = (num_written_clipdistances + i) % 4;
-      float cull1 = header->v[0]->data[out_idx][idx];
-      boolean vert1_out = cull_distance_is_out(cull1);
-      if (vert1_out)
-         return;
-   }
-   stage->next->point( stage->next, header );
-}
-
-/*
- * If the shader writes the culldistance then we can
- * perform distance based culling. Distance based
- * culling doesn't require a face and can be performed
- * on primitives without faces (e.g. points and lines)
- */
-static void cull_line( struct draw_stage *stage,
-                       struct prim_header *header )
-{
-   const unsigned num_written_culldistances =
-      draw_current_shader_num_written_culldistances(stage->draw);
-   const unsigned num_written_clipdistances =
-      draw_current_shader_num_written_clipdistances(stage->draw);
-   unsigned i;
-
-   debug_assert(num_written_culldistances);
-
-   for (i = 0; i < num_written_culldistances; ++i) {
-      unsigned cull_idx = (num_written_clipdistances + i) / 4;
-      unsigned out_idx =
-         draw_current_shader_ccdistance_output(stage->draw, cull_idx);
-      unsigned idx = (num_written_clipdistances + i) % 4;
-      float cull1 = header->v[0]->data[out_idx][idx];
-      float cull2 = header->v[1]->data[out_idx][idx];
-      boolean vert1_out = cull_distance_is_out(cull1);
-      boolean vert2_out = cull_distance_is_out(cull2);
-      if (vert1_out && vert2_out)
-         return;
-   }
-   stage->next->line( stage->next, header );
-}
-
-/*
- * Triangles can be culled either using the cull distance
- * shader outputs or the regular face culling. If required
- * this function performs both, starting with distance culling.
+ * Triangles can be culled using regular face cull.
  */
 static void cull_tri( struct draw_stage *stage,
 		      struct prim_header *header )
 {
-   const unsigned num_written_culldistances =
-      draw_current_shader_num_written_culldistances(stage->draw);
-   const unsigned num_written_clipdistances =
-      draw_current_shader_num_written_clipdistances(stage->draw);
-   /* Do the distance culling */
-   if (num_written_culldistances) {
-      unsigned i;
-      for (i = 0; i < num_written_culldistances; ++i) {
-         unsigned cull_idx = (num_written_clipdistances + i) / 4;
-         unsigned out_idx =
-            draw_current_shader_ccdistance_output(stage->draw, cull_idx);
-         unsigned idx = (num_written_clipdistances + i) % 4;
-         float cull1 = header->v[0]->data[out_idx][idx];
-         float cull2 = header->v[1]->data[out_idx][idx];
-         float cull3 = header->v[2]->data[out_idx][idx];
-         boolean vert1_out = cull_distance_is_out(cull1);
-         boolean vert2_out = cull_distance_is_out(cull2);
-         boolean vert3_out = cull_distance_is_out(cull3);
-         if (vert1_out && vert2_out && vert3_out)
-            return;
-      }
-   }
-
    /* Do the regular face culling */
    {
       const unsigned pos = draw_current_shader_position_output(stage->draw);
@@ -195,36 +102,6 @@ static void cull_tri( struct draw_stage *stage,
    }
 }
 
-static void cull_first_point( struct draw_stage *stage,
-                              struct prim_header *header )
-{
-   const unsigned num_written_culldistances =
-      draw_current_shader_num_written_culldistances(stage->draw);
-
-   if (num_written_culldistances) {
-      stage->point = cull_point;
-      stage->point( stage, header );
-   } else {
-      stage->point = draw_pipe_passthrough_point;
-      stage->point( stage, header );
-   }
-}
-
-static void cull_first_line( struct draw_stage *stage,
-			    struct prim_header *header )
-{
-   const unsigned num_written_culldistances =
-      draw_current_shader_num_written_culldistances(stage->draw);
-
-   if (num_written_culldistances) {
-      stage->line = cull_line;
-      stage->line( stage, header );
-   } else {
-      stage->line = draw_pipe_passthrough_line;
-      stage->line( stage, header );
-   }
-}
-
 static void cull_first_tri( struct draw_stage *stage,
 			    struct prim_header *header )
 {
@@ -240,8 +117,6 @@ static void cull_first_tri( struct draw_stage *stage,
 
 static void cull_flush( struct draw_stage *stage, unsigned flags )
 {
-   stage->point = cull_first_point;
-   stage->line = cull_first_line;
    stage->tri = cull_first_tri;
    stage->next->flush( stage->next, flags );
 }
@@ -272,8 +147,8 @@ struct draw_stage *draw_cull_stage( struct draw_context *draw )
    cull->stage.draw = draw;
    cull->stage.name = "cull";
    cull->stage.next = NULL;
-   cull->stage.point = cull_first_point;
-   cull->stage.line = cull_first_line;
+   cull->stage.point = draw_pipe_passthrough_point;
+   cull->stage.line = draw_pipe_passthrough_line;
    cull->stage.tri = cull_first_tri;
    cull->stage.flush = cull_flush;
    cull->stage.reset_stipple_counter = cull_reset_stipple_counter;
diff --git a/src/gallium/auxiliary/draw/draw_pipe_user_cull.c b/src/gallium/auxiliary/draw/draw_pipe_user_cull.c
new file mode 100644
index 00000000000..fcc177e0449
--- /dev/null
+++ b/src/gallium/auxiliary/draw/draw_pipe_user_cull.c
@@ -0,0 +1,197 @@
+/**************************************************************************
+ *
+ * Copyright 2007 VMware, Inc.
+ * All Rights Reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sub license, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the
+ * next paragraph) shall be included in all copies or substantial portions
+ * of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
+ * IN NO EVENT SHALL VMWARE AND/OR ITS SUPPLIERS BE LIABLE FOR
+ * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
+ * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
+ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ *
+ **************************************************************************/
+
+/**
+ * \brief  Drawing stage for user culling
+ */
+
+#include "util/u_math.h"
+#include "util/u_memory.h"
+#include "pipe/p_defines.h"
+#include "draw_pipe.h"
+
+struct user_cull_stage {
+   struct draw_stage stage;
+};
+
+static inline struct user_cull_stage *user_cull_stage( struct draw_stage *stage )
+{
+   return (struct user_cull_stage *)stage;
+}
+
+static inline boolean
+cull_distance_is_out(float dist)
+{
+   return (dist < 0.0f) || util_is_inf_or_nan(dist);
+}
+
+/*
+ * If the shader writes the culldistance then we can
+ * perform distance based culling. Distance based
+ * culling doesn't require a face and can be performed
+ * on primitives without faces (e.g. points and lines)
+ */
+static void user_cull_point( struct draw_stage *stage,
+                        struct prim_header *header )
+{
+   const unsigned num_written_culldistances =
+      draw_current_shader_num_written_culldistances(stage->draw);
+   const unsigned num_written_clipdistances =
+      draw_current_shader_num_written_clipdistances(stage->draw);
+   unsigned i;
+
+   debug_assert(num_written_culldistances);
+
+   for (i = 0; i < num_written_culldistances; ++i) {
+      unsigned cull_idx = (num_written_clipdistances + i) / 4;
+      unsigned out_idx =
+         draw_current_shader_ccdistance_output(stage->draw, cull_idx);
+      unsigned idx = (num_written_clipdistances + i) % 4;
+      float cull1 = header->v[0]->data[out_idx][idx];
+      boolean vert1_out = cull_distance_is_out(cull1);
+      if (vert1_out)
+         return;
+   }
+   stage->next->point( stage->next, header );
+}
+
+/*
+ * If the shader writes the culldistance then we can
+ * perform distance based culling. Distance based
+ * culling doesn't require a face and can be performed
+ * on primitives without faces (e.g. points and lines)
+ */
+static void user_cull_line( struct draw_stage *stage,
+                       struct prim_header *header )
+{
+   const unsigned num_written_culldistances =
+      draw_current_shader_num_written_culldistances(stage->draw);
+   const unsigned num_written_clipdistances =
+      draw_current_shader_num_written_clipdistances(stage->draw);
+   unsigned i;
+
+   debug_assert(num_written_culldistances);
+
+   for (i = 0; i < num_written_culldistances; ++i) {
+      unsigned cull_idx = (num_written_clipdistances + i) / 4;
+      unsigned out_idx =
+         draw_current_shader_ccdistance_output(stage->draw, cull_idx);
+      unsigned idx = (num_written_clipdistances + i) % 4;
+      float cull1 = header->v[0]->data[out_idx][idx];
+      float cull2 = header->v[1]->data[out_idx][idx];
+      boolean vert1_out = cull_distance_is_out(cull1);
+      boolean vert2_out = cull_distance_is_out(cull2);
+      if (vert1_out && vert2_out)
+         return;
+   }
+   stage->next->line( stage->next, header );
+}
+
+/*
+ * Triangles can be culled either using the cull distance
+ * shader outputs or the regular face culling. If required
+ * this function performs both, starting with distance culling.
+ */
+static void user_cull_tri( struct draw_stage *stage,
+                           struct prim_header *header )
+{
+   const unsigned num_written_culldistances =
+      draw_current_shader_num_written_culldistances(stage->draw);
+   const unsigned num_written_clipdistances =
+      draw_current_shader_num_written_clipdistances(stage->draw);
+   unsigned i;
+
+   debug_assert(num_written_culldistances);
+
+   /* Do the distance culling */
+   for (i = 0; i < num_written_culldistances; ++i) {
+      unsigned cull_idx = (num_written_clipdistances + i) / 4;
+      unsigned out_idx =
+         draw_current_shader_ccdistance_output(stage->draw, cull_idx);
+      unsigned idx = (num_written_clipdistances + i) % 4;
+      float cull1 = header->v[0]->data[out_idx][idx];
+      float cull2 = header->v[1]->data[out_idx][idx];
+      float cull3 = header->v[2]->data[out_idx][idx];
+      boolean vert1_out = cull_distance_is_out(cull1);
+      boolean vert2_out = cull_distance_is_out(cull2);
+      boolean vert3_out = cull_distance_is_out(cull3);
+      if (vert1_out && vert2_out && vert3_out) {
+         return;
+      }
+   }
+   stage->next->tri( stage->next, header );
+}
+
+static void user_cull_flush( struct draw_stage *stage, unsigned flags )
+{
+   stage->point = user_cull_point;
+   stage->line = user_cull_line;
+   stage->tri = user_cull_tri;
+   stage->next->flush( stage->next, flags );
+}
+
+static void user_cull_reset_stipple_counter( struct draw_stage *stage )
+{
+   stage->next->reset_stipple_counter( stage->next );
+}
+
+static void user_cull_destroy( struct draw_stage *stage )
+{
+   draw_free_temp_verts( stage );
+   FREE( stage );
+}
+
+/**
+ * Create a new polygon culling stage.
+ */
+struct draw_stage *draw_user_cull_stage( struct draw_context *draw )
+{
+   struct user_cull_stage *user_cull = CALLOC_STRUCT(user_cull_stage);
+   if (!user_cull)
+      goto fail;
+
+   user_cull->stage.draw = draw;
+   user_cull->stage.name = "user_cull";
+   user_cull->stage.next = NULL;
+   user_cull->stage.point = user_cull_point;
+   user_cull->stage.line = user_cull_line;
+   user_cull->stage.tri = user_cull_tri;
+   user_cull->stage.flush = user_cull_flush;
+   user_cull->stage.reset_stipple_counter = user_cull_reset_stipple_counter;
+   user_cull->stage.destroy = user_cull_destroy;
+
+   if (!draw_alloc_temp_verts( &user_cull->stage, 0 ))
+      goto fail;
+
+   return &user_cull->stage;
+
+fail:
+   if (user_cull)
+      user_cull->stage.destroy( &user_cull->stage );
+
+   return NULL;
+}
diff --git a/src/gallium/auxiliary/draw/draw_pipe_validate.c b/src/gallium/auxiliary/draw/draw_pipe_validate.c
index a013c2ef640..940ca5644de 100644
--- a/src/gallium/auxiliary/draw/draw_pipe_validate.c
+++ b/src/gallium/auxiliary/draw/draw_pipe_validate.c
@@ -255,8 +255,7 @@ static struct draw_stage *validate_pipeline( struct draw_stage *stage )
     * to less work emitting vertices, smaller vertex buffers, etc.
     * It's difficult to say whether this will be true in general.
     */
-   if (need_det || rast->cull_face != PIPE_FACE_NONE ||
-       draw_current_shader_num_written_culldistances(draw)) {
+   if (need_det || rast->cull_face != PIPE_FACE_NONE) {
       draw->pipeline.cull->next = next;
       next = draw->pipeline.cull;
    }
@@ -269,6 +268,11 @@ static struct draw_stage *validate_pipeline( struct draw_stage *stage )
       next = draw->pipeline.clip;
    }
 
+   if (draw_current_shader_num_written_culldistances(draw)) {
+      draw->pipeline.user_cull->next = next;
+      next = draw->pipeline.user_cull;
+   }
+
    draw->pipeline.first = next;
 
    if (0) {
diff --git a/src/gallium/auxiliary/draw/draw_private.h b/src/gallium/auxiliary/draw/draw_private.h
index e0195f455e1..a5a1b4e9b22 100644
--- a/src/gallium/auxiliary/draw/draw_private.h
+++ b/src/gallium/auxiliary/draw/draw_private.h
@@ -122,6 +122,7 @@ struct draw_context
       struct draw_stage *flatshade;
       struct draw_stage *clip;
       struct draw_stage *cull;
+      struct draw_stage *user_cull;
       struct draw_stage *twoside;
       struct draw_stage *offset;
       struct draw_stage *unfilled;
diff --git a/src/gallium/auxiliary/meson.build b/src/gallium/auxiliary/meson.build
index 7db7614ebb5..8b02084c0d0 100644
--- a/src/gallium/auxiliary/meson.build
+++ b/src/gallium/auxiliary/meson.build
@@ -46,6 +46,7 @@ files_libgallium = files(
   'draw/draw_pipe_stipple.c',
   'draw/draw_pipe_twoside.c',
   'draw/draw_pipe_unfilled.c',
+  'draw/draw_pipe_user_cull.c',
   'draw/draw_pipe_util.c',
   'draw/draw_pipe_validate.c',
   'draw/draw_pipe_vbuf.c',



More information about the mesa-commit mailing list