cogl: Move the clip stack dirtiness to the context rather than the FB

Previously we tracked whether the clip stack needs flushing as part of
the CoglClipState which is part of the CoglFramebuffer state. This is
a bit odd because most of the clipping state (such as the clip planes
and the scissor) are part of the GL context's state rather than the
framebuffer. We were marking the clip state on the framebuffer dirty
every time we change the framebuffer anyway so it seems to make more
sense to have the dirtiness be part of the global context.

Instead of a just a single boolean to record whether the state needs
flushing, the CoglContext now holds a reference to the clip stack that
was flushed. That way we can flush arbitrary stack states and if it
happens to be the same as the state already flushed then Cogl will do
nothing. This will be useful if we log the clip stack in the journal
because then we will need to flush unrelated clip stack states for
each batch.

diff --git a/clutter/cogl/cogl/cogl-clip-stack.c b/clutter/cogl/cogl/cogl-clip-stack.c
index 0a2ccc8..ab117c9 100644 (file)
--- a/clutter/cogl/cogl/cogl-clip-stack.c
+++ b/clutter/cogl/cogl/cogl-clip-stack.c
@@ -582,8 +582,7 @@ _cogl_clip_stack_pop (CoglClipStack *stack)
 }
 
 void
-_cogl_clip_stack_flush (CoglClipStack *stack,
-                        gboolean *stencil_used_p)
+_cogl_clip_stack_flush (CoglClipStack *stack)
 {
   int has_clip_planes;
   gboolean using_clip_planes = FALSE;
@@ -592,11 +591,32 @@ _cogl_clip_stack_flush (CoglClipStack *stack,
   int scissor_y0 = 0;
   int scissor_x1 = G_MAXINT;
   int scissor_y1 = G_MAXINT;
-  CoglMatrixStack *modelview_stack =
-    _cogl_framebuffer_get_modelview_stack (_cogl_get_framebuffer ());
+  CoglMatrixStack *modelview_stack;
   CoglClipStack *entry;
   int scissor_y_start;
 
+  _COGL_GET_CONTEXT (ctx, NO_RETVAL);
+
+  /* If we have already flushed this state then we don't need to do
+     anything */
+  if (ctx->current_clip_stack_valid)
+    {
+      if (ctx->current_clip_stack == stack)
+        return;
+
+      _cogl_clip_stack_unref (ctx->current_clip_stack);
+    }
+
+  ctx->current_clip_stack_valid = TRUE;
+  ctx->current_clip_stack = _cogl_clip_stack_ref (stack);
+
+  /* The current primitive journal does not support tracking changes to the
+   * clip stack...  */
+  _cogl_journal_flush ();
+
+  modelview_stack =
+    _cogl_framebuffer_get_modelview_stack (_cogl_get_framebuffer ());
+
   has_clip_planes = cogl_features_available (COGL_FEATURE_FOUR_CLIP_PLANES);
 
   if (has_clip_planes)
@@ -606,7 +626,7 @@ _cogl_clip_stack_flush (CoglClipStack *stack,
   /* If the stack is empty then there's nothing else to do */
   if (stack == NULL)
     {
-      *stencil_used_p = FALSE;
+      ctx->current_clip_stack_uses_stencil = FALSE;
       GE (glDisable (GL_SCISSOR_TEST));
       return;
     }
@@ -718,5 +738,17 @@ _cogl_clip_stack_flush (CoglClipStack *stack,
   if (using_clip_planes)
     enable_clip_planes ();
 
-  *stencil_used_p = using_stencil_buffer;
+  ctx->current_clip_stack_uses_stencil = using_stencil_buffer;
+}
+
+void
+_cogl_clip_stack_dirty (void)
+{
+  _COGL_GET_CONTEXT (ctx, NO_RETVAL);
+
+  if (ctx->current_clip_stack_valid)
+    {
+      ctx->current_clip_stack_valid = FALSE;
+      _cogl_clip_stack_unref (ctx->current_clip_stack);
+    }
 }

diff --git a/clutter/cogl/cogl/cogl-clip-stack.h b/clutter/cogl/cogl/cogl-clip-stack.h
index a52512e..791cc7b 100644 (file)
--- a/clutter/cogl/cogl/cogl-clip-stack.h
+++ b/clutter/cogl/cogl/cogl-clip-stack.h
@@ -57,8 +57,7 @@ CoglClipStack *
 _cogl_clip_stack_pop (CoglClipStack *stack);
 
 void
-_cogl_clip_stack_flush (CoglClipStack *stack,
-                        gboolean *stencil_used_p);
+_cogl_clip_stack_flush (CoglClipStack *stack);
 
 CoglClipStack *
 _cogl_clip_stack_ref (CoglClipStack *stack);
@@ -66,4 +65,7 @@ _cogl_clip_stack_ref (CoglClipStack *stack);
 void
 _cogl_clip_stack_unref (CoglClipStack *stack);
 
+void
+_cogl_clip_stack_dirty (void);
+
 #endif /* __COGL_CLIP_STACK_H */

diff --git a/clutter/cogl/cogl/cogl-clip-state.c b/clutter/cogl/cogl/cogl-clip-state.c
index 18850e0..1b1033c 100644 (file)
--- a/clutter/cogl/cogl/cogl-clip-state.c
+++ b/clutter/cogl/cogl/cogl-clip-state.c
@@ -62,8 +62,6 @@ cogl_clip_push_window_rectangle (int x_offset,
     _cogl_clip_stack_push_window_rectangle (clip_state->stacks->data,
                                             x_offset, y_offset,
                                             width, height);
-
-  clip_state->stack_dirty = TRUE;
 }
 
 /* XXX: This is deprecated API */
@@ -156,8 +154,6 @@ cogl_clip_push_rectangle (float x_1,
     _cogl_clip_stack_push_rectangle (clip_state->stacks->data,
                                      x_1, y_1, x_2, y_2,
                                      &modelview_matrix);
-
-  clip_state->stack_dirty = TRUE;
 }
 
 /* XXX: Deprecated API */
@@ -194,8 +190,6 @@ cogl_clip_push_from_path_preserve (void)
   clip_state->stacks->data =
     _cogl_clip_stack_push_from_path (clip_state->stacks->data, cogl_get_path (),
                                      &modelview_matrix);
-
-  clip_state->stack_dirty = TRUE;
 }
 
 void
@@ -214,8 +208,6 @@ _cogl_clip_pop_real (CoglClipState *clip_state)
   _cogl_journal_flush ();
 
   clip_state->stacks->data = _cogl_clip_stack_pop (clip_state->stacks->data);
-
-  clip_state->stack_dirty = TRUE;
 }
 
 void
@@ -235,25 +227,9 @@ cogl_clip_pop (void)
 void
 _cogl_clip_state_flush (CoglClipState *clip_state)
 {
-  CoglClipStack *stack;
-
-  if (!clip_state->stack_dirty)
-    return;
-
-  /* The current primitive journal does not support tracking changes to the
-   * clip stack...  */
-  _cogl_journal_flush ();
-
-  /* XXX: the handling of clipping is quite complex. It may involve use of
-   * the Cogl Journal or other Cogl APIs which may end up recursively
-   * wanting to ensure the clip state is flushed. We need to ensure we
-   * don't recurse infinitely...
-   */
-  clip_state->stack_dirty = FALSE;
-
-  stack = clip_state->stacks->data;
-
-  _cogl_clip_stack_flush (stack, &clip_state->stencil_used);
+  /* Flush the topmost stack. The clip stack code will bail out early
+     if this is already flushed */
+  _cogl_clip_stack_flush (clip_state->stacks->data);
 }
 
 /* XXX: This should never have been made public API! */
@@ -274,7 +250,6 @@ _cogl_clip_stack_save_real (CoglClipState *clip_state)
   _cogl_journal_flush ();
 
   clip_state->stacks = g_slist_prepend (clip_state->stacks, NULL);
-  clip_state->stack_dirty = TRUE;
 }
 
 void
@@ -309,8 +284,6 @@ _cogl_clip_stack_restore_real (CoglClipState *clip_state)
   /* Revert to an old stack */
   clip_state->stacks = g_slist_delete_link (clip_state->stacks,
                                             clip_state->stacks);
-
-  clip_state->stack_dirty = TRUE;
 }
 
 void
@@ -333,7 +306,6 @@ _cogl_clip_state_init (CoglClipState *clip_state)
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
   clip_state->stacks = NULL;
-  clip_state->stack_dirty = TRUE;
 
   /* Add an intial stack */
   _cogl_clip_stack_save_real (clip_state);
@@ -347,12 +319,6 @@ _cogl_clip_state_destroy (CoglClipState *clip_state)
     _cogl_clip_stack_restore_real (clip_state);
 }
 
-void
-_cogl_clip_state_dirty (CoglClipState *clip_state)
-{
-  clip_state->stack_dirty = TRUE;
-}
-
 CoglClipStack *
 _cogl_get_clip_stack (void)
 {

diff --git a/clutter/cogl/cogl/cogl-clip-state.h b/clutter/cogl/cogl/cogl-clip-state.h
index dd09449..6be552c 100644 (file)
--- a/clutter/cogl/cogl/cogl-clip-state.h
+++ b/clutter/cogl/cogl/cogl-clip-state.h
@@ -30,9 +30,6 @@ struct _CoglClipState
 {
   /* Stack of CoglClipStacks */
   GSList *stacks;
-
-  gboolean stack_dirty;
-  gboolean stencil_used;
 };
 
 void
@@ -42,9 +39,6 @@ void
 _cogl_clip_state_destroy (CoglClipState *state);
 
 void
-_cogl_clip_state_dirty (CoglClipState *state);
-
-void
 _cogl_clip_state_flush (CoglClipState *clip_state);
 
 /*

diff --git a/clutter/cogl/cogl/cogl-context.c b/clutter/cogl/cogl/cogl-context.c
index 7395b76..ad4f4f2 100644 (file)
--- a/clutter/cogl/cogl/cogl-context.c
+++ b/clutter/cogl/cogl/cogl-context.c
@@ -209,6 +209,8 @@ cogl_create_context (void)
 
   _context->texture_download_pipeline = COGL_INVALID_HANDLE;
 
+  _context->current_clip_stack_valid = FALSE;
+
   /* The default for GL_ALPHA_TEST is to always pass which is equivalent to
    * the test being disabled therefore we assume that for all drivers there
    * will be no performance impact if we always leave the test enabled which
@@ -310,6 +312,9 @@ _cogl_destroy_context (void)
   if (_context->default_layer_0)
     cogl_handle_unref (_context->default_layer_0);
 
+  if (_context->current_clip_stack_valid)
+    _cogl_clip_stack_unref (_context->current_clip_stack);
+
   if (_context->atlas)
     _cogl_atlas_free (_context->atlas);
 

diff --git a/clutter/cogl/cogl/cogl-context.h b/clutter/cogl/cogl/cogl-context.h
index 96157cb..e9a6775 100644 (file)
--- a/clutter/cogl/cogl/cogl-context.h
+++ b/clutter/cogl/cogl/cogl-context.h
@@ -187,6 +187,28 @@ typedef struct
      cogl_is_buffer */
   GSList           *buffer_types;
 
+  /* Clipping */
+  /* TRUE if we have a valid clipping stack flushed. In that case
+     current_clip_stack will describe what the current state is. If
+     this is FALSE then the current clip stack is completely unknown
+     so it will need to be reflushed. In that case current_clip_stack
+     doesn't need to be a valid pointer. We can't just use NULL in
+     current_clip_stack to mark a dirty state because NULL is a valid
+     stack (meaning no clipping) */
+  gboolean          current_clip_stack_valid;
+  /* The clip state that was flushed. This isn't intended to be used
+     as a stack to push and pop new entries. Instead the current stack
+     that the user wants is part of the framebuffer state. This is
+     just used to record the flush state so we can avoid flushing the
+     same state multiple times. When the clip state is flushed this
+     will hold a reference */
+  CoglClipStack    *current_clip_stack;
+  /* Whether the stencil buffer was used as part of the current clip
+     state. If TRUE then any further use of the stencil buffer (such
+     as for drawing paths) would need to be merged with the existing
+     stencil buffer */
+  gboolean          current_clip_stack_uses_stencil;
+
   CoglContextDriver drv;
   CoglContextWinsys winsys;
 } CoglContext;

diff --git a/clutter/cogl/cogl/cogl-framebuffer.c b/clutter/cogl/cogl/cogl-framebuffer.c
index 0a5388a..47b1664 100644 (file)
--- a/clutter/cogl/cogl/cogl-framebuffer.c
+++ b/clutter/cogl/cogl/cogl-framebuffer.c
@@ -636,7 +636,7 @@ _cogl_set_framebuffer_real (CoglFramebuffer *framebuffer)
    * we flush */
   _cogl_matrix_stack_dirty (framebuffer->modelview_stack);
   _cogl_matrix_stack_dirty (framebuffer->projection_stack);
-  _cogl_clip_state_dirty (&framebuffer->clip_state);
+  _cogl_clip_stack_dirty ();
 }
 
 void

diff --git a/clutter/cogl/cogl/cogl-path.c b/clutter/cogl/cogl/cogl-path.c
index 34f6b79..02d32ef 100644 (file)
--- a/clutter/cogl/cogl/cogl-path.c
+++ b/clutter/cogl/cogl/cogl-path.c
@@ -266,18 +266,14 @@ _cogl_path_get_bounds (CoglPath *path,
 static void
 _cogl_path_fill_nodes_with_stencil_buffer (CoglPath *path)
 {
-  CoglFramebuffer *framebuffer;
-  CoglClipState *clip_state;
-
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
   _cogl_journal_flush ();
 
-  framebuffer = _cogl_get_framebuffer ();
-  clip_state = _cogl_framebuffer_get_clip_state (framebuffer);
+  g_assert (ctx->current_clip_stack_valid);
 
   _cogl_add_path_to_stencil_buffer (path,
-                                    clip_state->stencil_used,
+                                    ctx->current_clip_stack_uses_stencil,
                                     FALSE);
 
   cogl_rectangle (path->data->path_nodes_min.x,
@@ -294,7 +290,7 @@ _cogl_path_fill_nodes_with_stencil_buffer (CoglPath *path)
    * we call cogl_flush() to emtpy the journal.
    */
   cogl_flush ();
-  _cogl_clip_state_dirty (clip_state);
+  _cogl_clip_stack_dirty ();
 }
 
 static void