llvmpipe: fix a bug in opaque optimization

If there are queries active the opaque optimization reseting the bin needs to
be disabled.
(Not really tested since the bug was discovered by code inspection not
an actual test failure.)

Reviewed-by: Jose Fonseca <jfonseca@vmware.com>

diff --git a/src/gallium/drivers/llvmpipe/lp_scene.h b/src/gallium/drivers/llvmpipe/lp_scene.h
index 59cce7d18f44f2194ea8d8afe94a71db7980a4f2..5501d4079be87082d2bc016ba6f67c778d3c958f 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_scene.h
+++ b/src/gallium/drivers/llvmpipe/lp_scene.h
@@ -132,6 +132,8 @@ struct lp_scene {
    /* The queries still active at end of scene */
    struct llvmpipe_query *active_queries[LP_MAX_ACTIVE_BINNED_QUERIES];
    unsigned num_active_queries;
+   /* If queries were either active or there were begin/end query commands */
+   boolean had_queries;
 
    /* Framebuffer mappings - valid only between begin_rasterization()
     * and end_rasterization().

diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
index 49aead2c1fa57bd9eda66eb53181f4f06156247b..49b61c3709d25723d515ae69c92a5c215d1a1d2b 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_setup.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup.c
@@ -237,6 +237,8 @@ begin_binning( struct lp_setup_context *setup )
    setup->clear.zsmask = 0;
    setup->clear.zsvalue = 0;
 
+   scene->had_queries = !!setup->active_binned_queries;
+
    LP_DBG(DEBUG_SETUP, "%s done\n", __FUNCTION__);
    return TRUE;
 }
@@ -1237,6 +1239,7 @@ lp_setup_begin_query(struct lp_setup_context *setup,
             return;
          }
       }
+      setup->scene->had_queries |= TRUE;
    }
 }
 
@@ -1272,6 +1275,7 @@ lp_setup_end_query(struct lp_setup_context *setup, struct llvmpipe_query *pq)
                goto fail;
             }
          }
+         setup->scene->had_queries |= TRUE;
       }
    }
    else {

diff --git a/src/gallium/drivers/llvmpipe/lp_setup_tri.c b/src/gallium/drivers/llvmpipe/lp_setup_tri.c
index 6dc136c3aa30dcce6062346787af3daf2bf492ac..579f35126a543449aec33590eec06cf060a5aa54 100644 (file)
--- a/src/gallium/drivers/llvmpipe/lp_setup_tri.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup_tri.c
@@ -204,22 +204,22 @@ lp_setup_whole_tile(struct lp_setup_context *setup,
    LP_COUNT(nr_fully_covered_64);
 
    /* if variant is opaque and scissor doesn't effect the tile */
-   /*
-    * Need to disable this optimization for layered rendering and cannot use
-    * setup->layer_slot here to determine it, because it could incorrectly
-    * reset the tile if a previous shader used layer_slot but not this one
-    * (or maybe even "undo" clears). So determine this from presence of layers
-    * instead (in which case layer_slot will have no effect).
-    */
-   if (inputs->opaque && scene->fb_max_layer == 0) {
-      if (!scene->fb.zsbuf) {
+   if (inputs->opaque) {
+      /* Several things prevent this optimization from working:
+       * - For layered rendering we can't determine if this covers the same layer
+       * as previous rendering (or in case of clears those actually always cover
+       * all layers so optimization is impossible). Need to use fb_max_layer and
+       * not setup->layer_slot to determine this since even if there's currently
+       * no slot assigned previous rendering could have used one.
+       * - If there were any Begin/End query commands in the scene then those
+       * would get removed which would be very wrong. Furthermore, if queries
+       * were just active we also can't do the optimization since to get
+       * accurate query results we unfortunately need to execute the rendering
+       * commands.
+       */
+      if (!scene->fb.zsbuf && scene->fb_max_layer == 0 && !scene->had_queries) {
          /*
           * All previous rendering will be overwritten so reset the bin.
-          * XXX This is wrong wrt to all queries arriving here (timestamp,
-          * occlusion, ps invocations). Not counting stuff might be ok but it
-          * will kill the begin/end query commands too which is definitely
-          * wrong (and at this point we don't even know if there were any
-          * such commands here).
           */
          lp_scene_bin_reset( scene, tx, ty );
       }