[Cogl] [PATCH 1/2] Preserve the CoglDrawFlags when drawing a wireframe

[Robert Bragg]

This looks good to land to me:

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

thanks,
- Robert

On Sat, May 19, 2012 at 11:17 PM, Neil Roberts <neil at linux.intel.com> wrote:
> Previously the CoglDrawFlags passed to
> _cogl_framebuffer_draw_indexed_attributes when drawing is redirected
> to draw a wireframe are overriden to avoid validating the pipeline,
> flushing the framebuffer state and flushing the journal. This ends up
> breaking scenes that only contain models drawn from attributes in the
> application because nothing will flush the matrices. It seems to make
> more sense to just use whatever draw flags were passed from the
> original draw command so that it will flush the matrices if the caller
> was expecting it.
>
> One problem with this is that if the wireframe causes the journal to
> be flushed then it will already have temporarily disabled the
> wireframe debug flag so the journal will not be drawn with wireframes.
> To fix this the patch adds a CoglDrawFlag to disable the wireframe and
> uses that instead of disabling the debug flag.
> ---
>  cogl/cogl-attribute-private.h |    5 ++++-
>  cogl/cogl-framebuffer.c       |   22 ++++++++++++----------
>  2 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/cogl/cogl-attribute-private.h b/cogl/cogl-attribute-private.h
> index bde6fe2..09b41f4 100644
> --- a/cogl/cogl-attribute-private.h
> +++ b/cogl/cogl-attribute-private.h
> @@ -76,7 +76,10 @@ typedef enum
>      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 << 3
> +  COGL_DRAW_COLOR_ATTRIBUTE_IS_OPAQUE = 1 << 3,
> +  /* This forcibly disables the debug option to divert all drawing to
> +   * wireframes */
> +  COGL_DRAW_SKIP_DEBUG_WIREFRAME = 1 << 4
>  } CoglDrawFlags;
>
>  /* During CoglContext initialization we register the "cogl_color_in"
> diff --git a/cogl/cogl-framebuffer.c b/cogl/cogl-framebuffer.c
> index 8ea41fb..fc0bf0b 100644
> --- a/cogl/cogl-framebuffer.c
> +++ b/cogl/cogl-framebuffer.c
> @@ -3229,7 +3229,8 @@ draw_wireframe (CoglContext *ctx,
>                 int n_vertices,
>                 CoglAttribute **attributes,
>                 int n_attributes,
> -                CoglIndices *indices)
> +                CoglIndices *indices,
> +                CoglDrawFlags flags)
>  {
>   CoglIndices *wire_indices;
>   CoglPipeline *wire_pipeline;
> @@ -3278,7 +3279,7 @@ draw_wireframe (CoglContext *ctx,
>     }
>
>   /* temporarily disable the wireframe to avoid recursion! */
> -  COGL_DEBUG_CLEAR_FLAG (COGL_DEBUG_WIREFRAME);
> +  flags |= COGL_DRAW_SKIP_DEBUG_WIREFRAME;
>   _cogl_framebuffer_draw_indexed_attributes (
>                                            framebuffer,
>                                            wire_pipeline,
> @@ -3288,10 +3289,7 @@ draw_wireframe (CoglContext *ctx,
>                                            wire_indices,
>                                            attributes,
>                                            n_attributes,
> -                                           COGL_DRAW_SKIP_JOURNAL_FLUSH |
> -                                           COGL_DRAW_SKIP_PIPELINE_VALIDATION |
> -                                           COGL_DRAW_SKIP_FRAMEBUFFER_FLUSH);
> -  COGL_DEBUG_SET_FLAG (COGL_DEBUG_WIREFRAME);
> +                                           flags);
>
>   cogl_object_unref (wire_indices);
>  }
> @@ -3311,11 +3309,13 @@ _cogl_framebuffer_draw_attributes (CoglFramebuffer *framebuffer,
>                                    CoglDrawFlags flags)
>  {
>  #ifdef COGL_ENABLE_DEBUG
> -  if (G_UNLIKELY (COGL_DEBUG_ENABLED (COGL_DEBUG_WIREFRAME)))
> +  if (G_UNLIKELY (COGL_DEBUG_ENABLED (COGL_DEBUG_WIREFRAME) &&
> +                  (flags & COGL_DRAW_SKIP_DEBUG_WIREFRAME) == 0))
>     draw_wireframe (framebuffer->context,
>                     framebuffer, pipeline,
>                     mode, first_vertex, n_vertices,
> -                    attributes, n_attributes, NULL);
> +                    attributes, n_attributes, NULL,
> +                    flags);
>   else
>  #endif
>     {
> @@ -3405,11 +3405,13 @@ _cogl_framebuffer_draw_indexed_attributes (CoglFramebuffer *framebuffer,
>                                            CoglDrawFlags flags)
>  {
>  #ifdef COGL_ENABLE_DEBUG
> -  if (G_UNLIKELY (COGL_DEBUG_ENABLED (COGL_DEBUG_WIREFRAME)))
> +  if (G_UNLIKELY (COGL_DEBUG_ENABLED (COGL_DEBUG_WIREFRAME) &&
> +                  (flags & COGL_DRAW_SKIP_DEBUG_WIREFRAME) == 0))
>     draw_wireframe (framebuffer->context,
>                     framebuffer, pipeline,
>                     mode, first_vertex, n_vertices,
> -                    attributes, n_attributes, indices);
> +                    attributes, n_attributes, indices,
> +                    flags);
>   else
>  #endif
>     {
> --
> 1.7.3.16.g9464b
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl