draw: fix user culling pipeline order. (v2)
authorDave Airlie <airlied@redhat.com>
Fri, 27 Mar 2020 01:33:43 +0000 (11:33 +1000)
committerDave Airlie <airlied@redhat.com>
Wed, 15 Apr 2020 04:26:29 +0000 (14:26 +1000)
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@vmware.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4560>

.gitlab-ci/piglit/quick_shader.txt
src/gallium/auxiliary/Makefile.sources
src/gallium/auxiliary/draw/draw_pipe.c
src/gallium/auxiliary/draw/draw_pipe.h
src/gallium/auxiliary/draw/draw_pipe_cull.c
src/gallium/auxiliary/draw/draw_pipe_user_cull.c [new file with mode: 0644]
src/gallium/auxiliary/draw/draw_pipe_validate.c
src/gallium/auxiliary/draw/draw_private.h
src/gallium/auxiliary/meson.build

index ce33accbfdeed00c434a6b8bb0c5031d42d06488..a83073ceab51af1fd16a0b7cf9597394728fa60b 100644 (file)
@@ -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
index b79dc1d5958e466cb5dcb5537fe7cabdb190188e..77ec768b5c3add31c2ac3cffcc90aa80467e629c 100644 (file)
@@ -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 \
index 27a37625263965b95b696bffa4a5b232dc6999a6..339bc7f4df4336e24f6d807d6a11a46fb373b8a8 100644 (file)
@@ -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)
index e69dcbded0eaf550c1a4e35286b99a5b0c372be4..4ae8883f096fb37948cdda4fd92e3b55c8838640 100644 (file)
@@ -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 );
index 318d743dbbf2b753c19233721149a23c54c150a9..a873edbe76c109fec647bfa1584f09e755faf357 100644 (file)
@@ -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 (file)
index 0000000..fcc177e
--- /dev/null
@@ -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;
+}
index a013c2ef6404836b6d7e3875409a992a3b06cf3f..940ca5644deb3dd401353fde25a9f70e6975f0fc 100644 (file)
@@ -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) {
index e0195f455e1224f14ce1896067ac1e1443751ffc..a5a1b4e9b22f1dc95478ce7c6a72cfc463c7e28c 100644 (file)
@@ -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;
index 7db7614ebb505ba9162968658dda2e80285fee68..8b02084c0d06f129ff32b811152dbc551fde9ba5 100644 (file)
@@ -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',