[cogl] Fix removing layers when the pipeline is not the owner



commit 88e73dd93fa09a158064a946ab229591a5888b97
Author: Neil Roberts <neil linux intel com>
Date:   Thu May 24 12:54:44 2012 +0100

    Fix removing layers when the pipeline is not the owner
    
    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.
    
    Reviewed-by: Robert Bragg <robert linux intel com>

 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);
 



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]