[Cogl] [PATCH 2/2] Fix removing layers when the pipeline is not the owner

Neil Roberts neil at linux.intel.com
Thu May 24 04:54:44 PDT 2012


If cogl_pipeline_remove_layer is called on a copied pipeline to remove
a parent layer then it will still end up calling
_cogl_pipeline_remove_layer_difference on the layer. This function
was directly trying to remove the layer from the pipeline's list of
layer differences. However in the child pipeline the layer isn't in
the list because it is unchanged from its parent. The function had an
assertion to verify that this situation wasn't hit so in a debug build
it would just bail out.

This patch removes the assertion and changes it to only remove the
layer if it is owned by the pipeline. Otherwise it just sets the
COGL_PIPELINE_STATE_LAYERS difference as normal and decrements the
number of layers. This will cause it to successfully remove the layer
because either it is the last layer in which case it will be ignored
after n_layers is decreased or if it is in the middle of the list then
the subsequent layers will all be shifted down so there will be a
replacement layer difference.
---
 cogl/cogl-pipeline.c              |   22 +++++++++++++++-------
 tests/conform/test-conform-main.c |    2 +-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/cogl/cogl-pipeline.c b/cogl/cogl-pipeline.c
index ef38df5..0c2dc9d 100644
--- a/cogl/cogl-pipeline.c
+++ b/cogl/cogl-pipeline.c
@@ -1377,8 +1377,6 @@ _cogl_pipeline_remove_layer_difference (CoglPipeline *pipeline,
                                         CoglPipelineLayer *layer,
                                         CoglBool dec_n_layers)
 {
-  _COGL_RETURN_IF_FAIL (layer->owner == pipeline);
-
   /* - Flush journal primitives referencing the current state.
    * - Make sure the pipeline has no dependants so it may be modified.
    * - If the pipeline isn't currently an authority for the state being
@@ -1394,13 +1392,23 @@ _cogl_pipeline_remove_layer_difference (CoglPipeline *pipeline,
                                     NULL,
                                     !dec_n_layers);
 
-  layer->owner = NULL;
-  cogl_object_unref (layer);
+  /* We only need to remove the layer difference if the pipeline is
+   * currently the owner. If it is not the owner then one of two
+   * things will happen to make sure this layer is replaced. If it is
+   * the last layer being removed then decrementing n_layers will
+   * ensure that the last layer is skipped. If it is any other layer
+   * then the subsequent layers will have been shifted down and cause
+   * it be replaced */
+  if (layer->owner == pipeline)
+    {
+      layer->owner = NULL;
+      cogl_object_unref (layer);
 
-  pipeline->differences |= COGL_PIPELINE_STATE_LAYERS;
+      pipeline->layer_differences =
+        g_list_remove (pipeline->layer_differences, layer);
+    }
 
-  pipeline->layer_differences =
-    g_list_remove (pipeline->layer_differences, layer);
+  pipeline->differences |= COGL_PIPELINE_STATE_LAYERS;
 
   if (dec_n_layers)
     pipeline->n_layers--;
diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c
index 02f428f..aa2f746 100644
--- a/tests/conform/test-conform-main.c
+++ b/tests/conform/test-conform-main.c
@@ -59,7 +59,7 @@ main (int argc, char **argv)
   ADD_TEST (test_depth_test, 0);
   ADD_TEST (test_color_mask, 0);
   ADD_TEST (test_backface_culling, TEST_REQUIREMENT_NPOT);
-  ADD_TEST (test_layer_remove, TEST_KNOWN_FAILURE);
+  ADD_TEST (test_layer_remove, 0);
 
   ADD_TEST (test_sparse_pipeline, 0);
 
-- 
1.7.3.16.g9464b



More information about the Cogl mailing list