Add internal _cogl_push_source to optionally disable legacy state

Some code in Cogl such as when flushing a stencil clip assumes that it
can push a temporary simple pipeline to reset to a known state for
internal drawing operations. However this breaks down if the
application has set any legacy state because that is set globally so
it will also get applied to the internal pipeline.

_cogl_draw_attributes already had an internal flag to disable applying
the legacy state but I think this is quite awkward to use because not
all places that push a pipeline draw the attribute buffers directly so
it is difficult to pass the flag down through the layers.

Conceptually the legacy state is meant to be like a layer on top of
the purely pipeline-based state API so I think ideally we should have
an internal function to push the source without the applying the
legacy state. The legacy state can't be applied as the pipeline is
pushed because the global state can be modified even after it is
pushed. This patch adds a _cogl_push_source() function which takes an
extra boolean flag to mark whether to enable the legacy state. The
value of this flag is stored alongside the pipeline in the pipeline
stack. Another new internal function called
_cogl_get_enable_legacy_state queries whether the top entry in the
pipeline stack has legacy state enabled. cogl-primitives and the
vertex array drawing code now use this to determine whether to apply
the legacy state when drawing. The COGL_DRAW_SKIP_LEGACY_STATE flag is
now removed.

Reviewed-by: Robert Bragg <robert@linux.intel.com>

cogl/cogl-attribute-private.h

@@ -62,17 +62,13 @@ typedef enum
   COGL_DRAW_SKIP_JOURNAL_FLUSH = 1 << 0,
   COGL_DRAW_SKIP_PIPELINE_VALIDATION = 1 << 1,
   COGL_DRAW_SKIP_FRAMEBUFFER_FLUSH = 1 << 2,
-  /* When flushing from the journal the logged pipeline will already
-     contain the legacy state overrides so we don't want to apply them
-     again when we flush the pipeline for drawing */
-  COGL_DRAW_SKIP_LEGACY_STATE = 1 << 3,
   /* By default the vertex attribute drawing code will assume that if
      there is a color attribute array enabled then we can't determine
      if the colors will be opaque so we need to enabling
      blending. However when drawing from the journal we know what the
      contents of the color array is so we can override this by passing
      this flag. */
-  COGL_DRAW_COLOR_ATTRIBUTE_IS_OPAQUE = 1 << 4
+  COGL_DRAW_COLOR_ATTRIBUTE_IS_OPAQUE = 1 << 3
 } CoglDrawFlags;
 
 CoglAttribute *

cogl/cogl-attribute.c

@@ -43,6 +43,7 @@
 #ifdef HAVE_COGL_GLES2
 #include "cogl-pipeline-progend-glsl-private.h"
 #endif
+#include "cogl-private.h"
 
 #include <string.h>
 #include <stdio.h>
@@ -535,7 +536,7 @@ enable_gl_state (CoglDrawFlags flags,
     }
 
   if (G_UNLIKELY (ctx->legacy_state_set) &&
-      (flags & COGL_DRAW_SKIP_LEGACY_STATE) == 0)
+      _cogl_get_enable_legacy_state ())
     {
       /* If we haven't already created a derived pipeline... */
       if (!copy)
@@ -1015,7 +1016,7 @@ draw_wireframe (CoglVerticesMode mode,
                                   0x00, 0xff, 0x00, 0xff);
     }
 
-  cogl_push_source (wire_pipeline);
+  _cogl_push_source (wire_pipeline, FALSE);
 
   /* temporarily disable the wireframe to avoid recursion! */
   COGL_DEBUG_CLEAR_FLAG (COGL_DEBUG_WIREFRAME);
@@ -1026,8 +1027,7 @@ draw_wireframe (CoglVerticesMode mode,
                          1,
                          COGL_DRAW_SKIP_JOURNAL_FLUSH |
                          COGL_DRAW_SKIP_PIPELINE_VALIDATION |
-                         COGL_DRAW_SKIP_FRAMEBUFFER_FLUSH |
+                         COGL_DRAW_SKIP_FRAMEBUFFER_FLUSH);
-                         COGL_DRAW_SKIP_LEGACY_STATE);
 
   COGL_DEBUG_SET_FLAG (COGL_DEBUG_WIREFRAME);
 

cogl/cogl-blit.c

@@ -35,6 +35,7 @@
 #include "cogl-context-private.h"
 #include "cogl-framebuffer-private.h"
 #include "cogl-texture-2d-private.h"
+#include "cogl-private.h"
 
 static const CoglBlitMode *_cogl_blit_default_mode = NULL;
 
@@ -90,7 +91,7 @@ _cogl_blit_texture_render_begin (CoglBlitData *data)
 
   cogl_pipeline_set_layer_texture (pipeline, 0, data->src_tex);
 
-  cogl_push_source (pipeline);
+  _cogl_push_source (pipeline, FALSE);
 
   return TRUE;
 }

cogl/cogl-clip-stack.c

@@ -41,6 +41,7 @@
 #include "cogl-path-private.h"
 #include "cogl-matrix-private.h"
 #include "cogl-primitives-private.h"
+#include "cogl-private.h"
 
 #ifndef GL_CLIP_PLANE0
 #define GL_CLIP_PLANE0 0x3000
@@ -210,7 +211,7 @@ add_stencil_clip_rectangle (CoglFramebuffer *framebuffer,
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
   /* temporarily swap in our special stenciling pipeline */
-  cogl_push_source (ctx->stencil_pipeline);
+  _cogl_push_source (ctx->stencil_pipeline, FALSE);
 
   /* This can be called from the journal code which doesn't flush
      the matrix stacks between calls so we need to ensure they're

cogl/cogl-journal.c

@@ -38,6 +38,7 @@
 #include "cogl-profile.h"
 #include "cogl-attribute-private.h"
 #include "cogl-point-in-poly-private.h"
+#include "cogl-private.h"
 
 #include <string.h>
 #include <gmodule.h>
@@ -272,8 +273,7 @@ _cogl_journal_flush_modelview_and_entries (CoglJournalEntry *batch_start,
   CoglAttribute **attributes;
   CoglDrawFlags draw_flags = (COGL_DRAW_SKIP_JOURNAL_FLUSH |
                               COGL_DRAW_SKIP_PIPELINE_VALIDATION |
-                              COGL_DRAW_SKIP_FRAMEBUFFER_FLUSH |
+                              COGL_DRAW_SKIP_FRAMEBUFFER_FLUSH);
-                              COGL_DRAW_SKIP_LEGACY_STATE);
 
   COGL_STATIC_TIMER (time_flush_modelview_and_entries,
                      "flush: pipeline+entries", /* parent */
@@ -297,7 +297,7 @@ _cogl_journal_flush_modelview_and_entries (CoglJournalEntry *batch_start,
     }
 
   attributes = (CoglAttribute **)state->attributes->data;
-  cogl_push_source (state->source);
+  _cogl_push_source (state->source, FALSE);
 
   if (!_cogl_pipeline_get_real_blend_enabled (state->source))
     draw_flags |= COGL_DRAW_COLOR_ATTRIBUTE_IS_OPAQUE;
@@ -1545,12 +1545,6 @@ _cogl_journal_log_quad (CoglJournal  *journal,
 
   source = pipeline;
 
-  if (G_UNLIKELY (ctx->legacy_state_set))
-    {
-      source = cogl_pipeline_copy (pipeline);
-      _cogl_pipeline_apply_legacy_state (source);
-    }
-
   flush_options.flags = 0;
   if (G_UNLIKELY (cogl_pipeline_get_n_layers (pipeline) != n_layers))
     {
@@ -1567,8 +1561,6 @@ _cogl_journal_log_quad (CoglJournal  *journal,
 
   if (G_UNLIKELY (flush_options.flags))
     {
-      /* If we haven't already created a derived pipeline... */
-      if (source == pipeline)
       source = cogl_pipeline_copy (pipeline);
       _cogl_pipeline_apply_overrides (source, &flush_options);
     }

cogl/cogl-primitives.c

@@ -36,6 +36,7 @@
 #include "cogl-vertex-buffer-private.h"
 #include "cogl-framebuffer-private.h"
 #include "cogl-attribute-private.h"
+#include "cogl-private.h"
 
 #include <string.h>
 #include <math.h>
@@ -711,13 +712,13 @@ _cogl_rectangles_with_multitexture_coords (
                                         struct _CoglMutiTexturedRect *rects,
                                         int                           n_rects)
 {
-  CoglPipeline *pipeline;
+  CoglPipeline *original_pipeline, *pipeline;
   ValidateLayerState state;
   int i;
 
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
-  pipeline = cogl_get_source ();
+  pipeline = original_pipeline = cogl_get_source ();
 
   /*
    * Validate all the layers of the current source pipeline...
@@ -733,6 +734,15 @@ _cogl_rectangles_with_multitexture_coords (
   if (state.override_source)
     pipeline = state.override_source;
 
+  if (G_UNLIKELY (ctx->legacy_state_set) &&
+      _cogl_get_enable_legacy_state ())
+    {
+      /* If we haven't already made a pipeline copy */
+      if (pipeline == original_pipeline)
+        pipeline = cogl_pipeline_copy (pipeline);
+      _cogl_pipeline_apply_legacy_state (pipeline);
+    }
+
   /*
    * Emit geometry for each of the rectangles...
    */
@@ -789,7 +799,7 @@ _cogl_rectangles_with_multitexture_coords (
                                               tex_coords[3]);
     }
 
-  if (state.override_source)
+  if (pipeline != original_pipeline)
     cogl_object_unref (pipeline);
 }
 

cogl/cogl-private.h

@@ -24,6 +24,8 @@
 #ifndef __COGL_PRIVATE_H__
 #define __COGL_PRIVATE_H__
 
+#include <cogl/cogl-pipeline.h>
+
 G_BEGIN_DECLS
 
 gboolean
@@ -53,6 +55,12 @@ _cogl_read_pixels_with_rowstride (int x,
 void
 _cogl_init (void);
 
+void
+_cogl_push_source (CoglPipeline *pipeline, gboolean enable_legacy);
+
+gboolean
+_cogl_get_enable_legacy_state (void);
+
 G_END_DECLS
 
 #endif /* __COGL_PRIVATE_H__ */

cogl/cogl-texture.c

@@ -1099,7 +1099,7 @@ _cogl_texture_draw_and_read (CoglHandle   handle,
                                NULL);
     }
 
-  cogl_push_source (ctx->texture_download_pipeline);
+  _cogl_push_source (ctx->texture_download_pipeline, FALSE);
 
   cogl_pipeline_set_layer_texture (ctx->texture_download_pipeline, 0, handle);
 

cogl/cogl.c

@@ -49,6 +49,7 @@
 #include "cogl-framebuffer-private.h"
 #include "cogl-renderer-private.h"
 #include "cogl-config-private.h"
+#include "cogl-private.h"
 
 #ifndef GL_PACK_INVERT_MESA
 #define GL_PACK_INVERT_MESA 0x8758
@@ -927,15 +928,22 @@ typedef struct _CoglSourceState
 {
   CoglPipeline *pipeline;
   int push_count;
+  /* If this is TRUE then the pipeline will be copied and the legacy
+     state will be applied whenever the pipeline is used. This is
+     necessary because some internal Cogl code expects to be able to
+     push a temporary pipeline to put GL into a known state. For that
+     to work it also needs to prevent applying the legacy state */
+  gboolean enable_legacy;
 } CoglSourceState;
 
 static void
-_push_source_real (CoglPipeline *pipeline)
+_push_source_real (CoglPipeline *pipeline, gboolean enable_legacy)
 {
   CoglSourceState *top = g_slice_new (CoglSourceState);
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
   top->pipeline = cogl_object_ref (pipeline);
+  top->enable_legacy = enable_legacy;
   top->push_count = 1;
 
   ctx->source_stack = g_list_prepend (ctx->source_stack, top);
@@ -947,9 +955,21 @@ _push_source_real (CoglPipeline *pipeline)
 void
 cogl_push_source (void *material_or_pipeline)
 {
-  CoglSourceState *top;
   CoglPipeline *pipeline = COGL_PIPELINE (material_or_pipeline);
 
+  g_return_if_fail (cogl_is_pipeline (pipeline));
+
+  _cogl_push_source (pipeline, TRUE);
+}
+
+/* This internal version of cogl_push_source is the same except it
+   never applies the legacy state. Some parts of Cogl use this
+   internally to set a temporary pipeline with a known state */
+void
+_cogl_push_source (CoglPipeline *pipeline, gboolean enable_legacy)
+{
+  CoglSourceState *top;
+
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
   g_return_if_fail (cogl_is_pipeline (pipeline));
@@ -957,16 +977,16 @@ cogl_push_source (void *material_or_pipeline)
   if (ctx->source_stack)
     {
       top = ctx->source_stack->data;
-      if (top->pipeline == pipeline)
+      if (top->pipeline == pipeline && top->enable_legacy == enable_legacy)
         {
           top->push_count++;
           return;
         }
       else
-        _push_source_real (pipeline);
+        _push_source_real (pipeline, enable_legacy);
     }
   else
-    _push_source_real (pipeline);
+    _push_source_real (pipeline, enable_legacy);
 }
 
 /* FIXME: This needs to take a context pointer for Cogl 2.0 */
@@ -1004,6 +1024,19 @@ cogl_get_source (void)
   return top->pipeline;
 }
 
+gboolean
+_cogl_get_enable_legacy_state (void)
+{
+  CoglSourceState *top;
+
+  _COGL_GET_CONTEXT (ctx, FALSE);
+
+  g_return_val_if_fail (ctx->source_stack, FALSE);
+
+  top = ctx->source_stack->data;
+  return top->enable_legacy;
+}
+
 void
 cogl_set_source (void *material_or_pipeline)
 {
@@ -1016,7 +1049,7 @@ cogl_set_source (void *material_or_pipeline)
   g_return_if_fail (ctx->source_stack);
 
   top = ctx->source_stack->data;
-  if (top->pipeline == pipeline)
+  if (top->pipeline == pipeline && top->enable_legacy)
     return;
 
   if (top->push_count == 1)
@@ -1026,6 +1059,7 @@ cogl_set_source (void *material_or_pipeline)
       cogl_object_ref (pipeline);
       cogl_object_unref (top->pipeline);
       top->pipeline = pipeline;
+      top->enable_legacy = TRUE;
     }
   else
     {

cogl/cogl2-path.c

@@ -368,7 +368,7 @@ _cogl_add_path_to_stencil_buffer (CoglPath *path,
                                   COGL_MATRIX_PROJECTION);
 
   /* Just setup a simple pipeline that doesn't use texturing... */
-  cogl_push_source (ctx->stencil_pipeline);
+  _cogl_push_source (ctx->stencil_pipeline, FALSE);
 
   _cogl_pipeline_flush_gl_state (ctx->stencil_pipeline, FALSE, 0);