[Cogl] [PATCH 1/2] Use a GList instead of a BSD list for CoglPipelineSnippetList

Robert Bragg robert at sixbynine.org
Wed Jun 12 03:12:49 PDT 2013


This looks good to land to me:

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

thanks,
- Robert

On Mon, Jun 10, 2013 at 2:15 PM, Neil Roberts <neil at linux.intel.com> wrote:
> Previously CoglPipelineSnippetList was using the BSD embedded list
> type with a mini struct to combine the list node with a pointer to the
> snippet. This is effectively equivalent to just using a GList so we
> might as well do that. This will help if we eventually want to get rid
> of cogl-queue.h
> ---
>  cogl/cogl-pipeline-layer.c                  |   4 +-
>  cogl/cogl-pipeline-snippet-private.h        |  13 +-
>  cogl/cogl-pipeline-snippet.c                | 289 +++++++++++++---------------
>  cogl/cogl-pipeline-state.c                  |   8 +-
>  cogl/driver/gl/cogl-pipeline-fragend-glsl.c |  12 +-
>  5 files changed, 154 insertions(+), 172 deletions(-)
>
> diff --git a/cogl/cogl-pipeline-layer.c b/cogl/cogl-pipeline-layer.c
> index b08d456..bb77f6b 100644
> --- a/cogl/cogl-pipeline-layer.c
> +++ b/cogl/cogl-pipeline-layer.c
> @@ -118,11 +118,11 @@ _cogl_pipeline_layer_has_alpha (CoglPipelineLayer *layer)
>    /* All bets are off if the layer contains any snippets */
>    snippets_authority = _cogl_pipeline_layer_get_authority
>      (layer, COGL_PIPELINE_LAYER_STATE_VERTEX_SNIPPETS);
> -  if (!COGL_LIST_EMPTY (&snippets_authority->big_state->vertex_snippets))
> +  if (snippets_authority->big_state->vertex_snippets.entries != NULL)
>      return TRUE;
>    snippets_authority = _cogl_pipeline_layer_get_authority
>      (layer, COGL_PIPELINE_LAYER_STATE_FRAGMENT_SNIPPETS);
> -  if (!COGL_LIST_EMPTY (&snippets_authority->big_state->fragment_snippets))
> +  if (snippets_authority->big_state->fragment_snippets.entries != NULL)
>      return TRUE;
>
>    return FALSE;
> diff --git a/cogl/cogl-pipeline-snippet-private.h b/cogl/cogl-pipeline-snippet-private.h
> index 02dee35..43cf9fa 100644
> --- a/cogl/cogl-pipeline-snippet-private.h
> +++ b/cogl/cogl-pipeline-snippet-private.h
> @@ -31,18 +31,11 @@
>  #include <glib.h>
>
>  #include "cogl-snippet.h"
> -#include "cogl-queue.h"
>
> -typedef struct _CoglPipelineSnippet CoglPipelineSnippet;
> -
> -COGL_LIST_HEAD (CoglPipelineSnippetList, CoglPipelineSnippet);
> -
> -struct _CoglPipelineSnippet
> +typedef struct
>  {
> -  COGL_LIST_ENTRY (CoglPipelineSnippet) list_node;
> -
> -  CoglSnippet *snippet;
> -};
> +  GList *entries;
> +} CoglPipelineSnippetList;
>
>  /* Arguments to pass to _cogl_pipeline_snippet_generate_code() */
>  typedef struct
> diff --git a/cogl/cogl-pipeline-snippet.c b/cogl/cogl-pipeline-snippet.c
> index 8bfa8be..8ce166b 100644
> --- a/cogl/cogl-pipeline-snippet.c
> +++ b/cogl/cogl-pipeline-snippet.c
> @@ -41,27 +41,32 @@
>  void
>  _cogl_pipeline_snippet_generate_code (const CoglPipelineSnippetData *data)
>  {
> -  CoglPipelineSnippet *first_snippet, *snippet;
> +  GList *first_snippet, *l;
> +  CoglSnippet *snippet;
>    int snippet_num = 0;
>    int n_snippets = 0;
>
> -  first_snippet = COGL_LIST_FIRST (data->snippets);
> +  first_snippet = data->snippets->entries;
>
>    /* First count the number of snippets so we can easily tell when
>       we're at the last one */
> -  COGL_LIST_FOREACH (snippet, data->snippets, list_node)
> -    if (snippet->snippet->hook == data->hook)
> -      {
> -        /* Don't bother processing any previous snippets if we reach
> -           one that has a replacement */
> -        if (snippet->snippet->replace)
> -          {
> -            n_snippets = 1;
> -            first_snippet = snippet;
> -          }
> -        else
> -          n_snippets++;
> -      }
> +  for (l = data->snippets->entries; l; l = l->next)
> +    {
> +      snippet = l->data;
> +
> +      if (snippet->hook == data->hook)
> +        {
> +          /* Don't bother processing any previous snippets if we reach
> +             one that has a replacement */
> +          if (snippet->replace)
> +            {
> +              n_snippets = 1;
> +              first_snippet = l;
> +            }
> +          else
> +            n_snippets++;
> +        }
> +    }
>
>    /* If there weren't any snippets then generate a stub function with
>       the final name */
> @@ -98,90 +103,92 @@ _cogl_pipeline_snippet_generate_code (const CoglPipelineSnippetData *data)
>        return;
>      }
>
> -  for (snippet = first_snippet, snippet_num = 0;
> -       snippet_num < n_snippets;
> -       snippet = COGL_LIST_NEXT (snippet, list_node))
> -    if (snippet->snippet->hook == data->hook)
> -      {
> -        const char *source;
> -
> -        if ((source = cogl_snippet_get_declarations (snippet->snippet)))
> -          g_string_append (data->source_buf, source);
> -
> -        g_string_append_printf (data->source_buf,
> -                                "\n"
> -                                "%s\n",
> -                                data->return_type ?
> -                                data->return_type :
> -                                "void");
> -
> -        if (snippet_num + 1 < n_snippets)
> -          g_string_append_printf (data->source_buf,
> -                                  "%s_%i",
> -                                  data->function_prefix,
> -                                  snippet_num);
> -        else
> -          g_string_append (data->source_buf, data->final_name);
> -
> -        g_string_append (data->source_buf, " (");
> +  for (l = first_snippet; snippet_num < n_snippets; l = l->next)
> +    {
> +      snippet = l->data;
>
> -        if (data->argument_declarations)
> -          g_string_append (data->source_buf, data->argument_declarations);
> +      if (snippet->hook == data->hook)
> +        {
> +          const char *source;
>
> -        g_string_append (data->source_buf,
> -                         ")\n"
> -                         "{\n");
> +          if ((source = cogl_snippet_get_declarations (snippet)))
> +            g_string_append (data->source_buf, source);
>
> -        if (data->return_type && !data->return_variable_is_argument)
> -          g_string_append_printf (data->source_buf,
> -                                  "  %s %s;\n"
> -                                  "\n",
> -                                  data->return_type,
> -                                  data->return_variable);
> -
> -        if ((source = cogl_snippet_get_pre (snippet->snippet)))
> -          g_string_append (data->source_buf, source);
> -
> -        /* Chain on to the next function, or bypass it if there is
> -           a replace string */
> -        if ((source = cogl_snippet_get_replace (snippet->snippet)))
> -          g_string_append (data->source_buf, source);
> -        else
> -          {
> -            g_string_append (data->source_buf, "  ");
> -
> -            if (data->return_type)
> -              g_string_append_printf (data->source_buf,
> -                                      "%s = ",
> -                                      data->return_variable);
> -
> -            if (snippet_num > 0)
> -              g_string_append_printf (data->source_buf,
> -                                      "%s_%i",
> -                                      data->function_prefix,
> -                                      snippet_num - 1);
> -            else
> -              g_string_append (data->source_buf, data->chain_function);
> -
> -            g_string_append (data->source_buf, " (");
> -
> -            if (data->arguments)
> -              g_string_append (data->source_buf, data->arguments);
> -
> -            g_string_append (data->source_buf, ");\n");
> -          }
> -
> -        if ((source = cogl_snippet_get_post (snippet->snippet)))
> -          g_string_append (data->source_buf, source);
> -
> -        if (data->return_type)
>            g_string_append_printf (data->source_buf,
> -                                  "  return %s;\n",
> -                                  data->return_variable);
> -
> -        g_string_append (data->source_buf, "}\n");
> -        snippet_num++;
> -      }
> +                                  "\n"
> +                                  "%s\n",
> +                                  data->return_type ?
> +                                  data->return_type :
> +                                  "void");
> +
> +          if (snippet_num + 1 < n_snippets)
> +            g_string_append_printf (data->source_buf,
> +                                    "%s_%i",
> +                                    data->function_prefix,
> +                                    snippet_num);
> +          else
> +            g_string_append (data->source_buf, data->final_name);
> +
> +          g_string_append (data->source_buf, " (");
> +
> +          if (data->argument_declarations)
> +            g_string_append (data->source_buf, data->argument_declarations);
> +
> +          g_string_append (data->source_buf,
> +                           ")\n"
> +                           "{\n");
> +
> +          if (data->return_type && !data->return_variable_is_argument)
> +            g_string_append_printf (data->source_buf,
> +                                    "  %s %s;\n"
> +                                    "\n",
> +                                    data->return_type,
> +                                    data->return_variable);
> +
> +          if ((source = cogl_snippet_get_pre (snippet)))
> +            g_string_append (data->source_buf, source);
> +
> +          /* Chain on to the next function, or bypass it if there is
> +             a replace string */
> +          if ((source = cogl_snippet_get_replace (snippet)))
> +            g_string_append (data->source_buf, source);
> +          else
> +            {
> +              g_string_append (data->source_buf, "  ");
> +
> +              if (data->return_type)
> +                g_string_append_printf (data->source_buf,
> +                                        "%s = ",
> +                                        data->return_variable);
> +
> +              if (snippet_num > 0)
> +                g_string_append_printf (data->source_buf,
> +                                        "%s_%i",
> +                                        data->function_prefix,
> +                                        snippet_num - 1);
> +              else
> +                g_string_append (data->source_buf, data->chain_function);
> +
> +              g_string_append (data->source_buf, " (");
> +
> +              if (data->arguments)
> +                g_string_append (data->source_buf, data->arguments);
> +
> +              g_string_append (data->source_buf, ");\n");
> +            }
> +
> +          if ((source = cogl_snippet_get_post (snippet)))
> +            g_string_append (data->source_buf, source);
> +
> +          if (data->return_type)
> +            g_string_append_printf (data->source_buf,
> +                                    "  return %s;\n",
> +                                    data->return_variable);
> +
> +          g_string_append (data->source_buf, "}\n");
> +          snippet_num++;
> +        }
> +    }
>  }
>
>  void
> @@ -189,92 +196,70 @@ _cogl_pipeline_snippet_generate_declarations (GString *declarations_buf,
>                                                CoglSnippetHook hook,
>                                                CoglPipelineSnippetList *snippets)
>  {
> -  CoglPipelineSnippet *snippet;
> +  GList *l;
>
> -  COGL_LIST_FOREACH (snippet, snippets, list_node)
> -    if (snippet->snippet->hook == hook)
> -      {
> -        const char *source;
> +  for (l = snippets->entries; l; l = l->next)
> +    {
> +      CoglSnippet *snippet = l->data;
>
> -        if ((source = cogl_snippet_get_declarations (snippet->snippet)))
> -          g_string_append (declarations_buf, source);
> -      }
> -}
> +      if (snippet->hook == hook)
> +        {
> +          const char *source;
>
> -static void
> -_cogl_pipeline_snippet_free (CoglPipelineSnippet *pipeline_snippet)
> -{
> -  cogl_object_unref (pipeline_snippet->snippet);
> -  g_slice_free (CoglPipelineSnippet, pipeline_snippet);
> +          if ((source = cogl_snippet_get_declarations (snippet)))
> +            g_string_append (declarations_buf, source);
> +        }
> +    }
>  }
>
>  void
>  _cogl_pipeline_snippet_list_free (CoglPipelineSnippetList *list)
>  {
> -  CoglPipelineSnippet *pipeline_snippet, *tmp;
> +  GList *l, *tmp;
>
> -  COGL_LIST_FOREACH_SAFE (pipeline_snippet, list, list_node, tmp)
> -    _cogl_pipeline_snippet_free (pipeline_snippet);
> +  for (l = list->entries; l; l = tmp)
> +    {
> +      tmp = l->next;
> +
> +      cogl_object_unref (l->data);
> +      g_list_free_1 (l);
> +    }
>  }
>
>  void
>  _cogl_pipeline_snippet_list_add (CoglPipelineSnippetList *list,
>                                   CoglSnippet *snippet)
>  {
> -  CoglPipelineSnippet *pipeline_snippet = g_slice_new (CoglPipelineSnippet);
> -
> -  pipeline_snippet->snippet = cogl_object_ref (snippet);
> -
> -  _cogl_snippet_make_immutable (pipeline_snippet->snippet);
> -
> -  if (COGL_LIST_EMPTY (list))
> -    COGL_LIST_INSERT_HEAD (list, pipeline_snippet, list_node);
> -  else
> -    {
> -      CoglPipelineSnippet *tail;
> -
> -      for (tail = COGL_LIST_FIRST (list);
> -           COGL_LIST_NEXT (tail, list_node);
> -           tail = COGL_LIST_NEXT (tail, list_node));
> +  list->entries = g_list_append (list->entries, cogl_object_ref (snippet));
>
> -      COGL_LIST_INSERT_AFTER (tail, pipeline_snippet, list_node);
> -    }
> +  _cogl_snippet_make_immutable (snippet);
>  }
>
>  void
>  _cogl_pipeline_snippet_list_copy (CoglPipelineSnippetList *dst,
>                                    const CoglPipelineSnippetList *src)
>  {
> -  CoglPipelineSnippet *tail = NULL;
> -  const CoglPipelineSnippet *l;
> -
> -  COGL_LIST_INIT (dst);
> -
> -  COGL_LIST_FOREACH (l, src, list_node)
> -    {
> -      CoglPipelineSnippet *copy = g_slice_dup (CoglPipelineSnippet, l);
> +  GQueue queue = G_QUEUE_INIT;
> +  const GList *l;
>
> -      cogl_object_ref (copy->snippet);
> +  for (l = src->entries; l; l = l->next)
> +    g_queue_push_tail (&queue, cogl_object_ref (l->data));
>
> -      if (tail)
> -        COGL_LIST_INSERT_AFTER (tail, copy, list_node);
> -      else
> -        COGL_LIST_INSERT_HEAD (dst, copy, list_node);
> -
> -      tail = copy;
> -    }
> +  dst->entries = queue.head;
>  }
>
>  void
>  _cogl_pipeline_snippet_list_hash (CoglPipelineSnippetList *list,
>                                    unsigned int *hash)
>  {
> -  CoglPipelineSnippet *l;
> +  GList *l;
>
> -  COGL_LIST_FOREACH (l, list, list_node)
> +  for (l = list->entries; l; l = l->next)
>      {
> +      CoglSnippet *snippet = l->data;
> +
>        *hash = _cogl_util_one_at_a_time_hash (*hash,
> -                                             &l->snippet,
> +                                             &snippet,
>                                               sizeof (CoglSnippet *));
>      }
>  }
> @@ -283,12 +268,12 @@ CoglBool
>  _cogl_pipeline_snippet_list_equal (CoglPipelineSnippetList *list0,
>                                     CoglPipelineSnippetList *list1)
>  {
> -  CoglPipelineSnippet *l0, *l1;
> +  GList *l0, *l1;
>
> -  for (l0 = COGL_LIST_FIRST (list0), l1 = COGL_LIST_FIRST (list1);
> +  for (l0 = list0->entries, l1 = list1->entries;
>         l0 && l1;
> -       l0 = COGL_LIST_NEXT (l0, list_node), l1 = COGL_LIST_NEXT (l1, list_node))
> -    if (l0->snippet != l1->snippet)
> +       l0 = l0->next, l1 = l1->next)
> +    if (l0->data != l1->data)
>        return FALSE;
>
>    return l0 == NULL && l1 == NULL;
> diff --git a/cogl/cogl-pipeline-state.c b/cogl/cogl-pipeline-state.c
> index d71974e..3f4eb43 100644
> --- a/cogl/cogl-pipeline-state.c
> +++ b/cogl/cogl-pipeline-state.c
> @@ -1222,7 +1222,7 @@ _cogl_pipeline_has_non_layer_vertex_snippets (CoglPipeline *pipeline)
>      _cogl_pipeline_get_authority (pipeline,
>                                    COGL_PIPELINE_STATE_VERTEX_SNIPPETS);
>
> -  return !COGL_LIST_EMPTY (&authority->big_state->vertex_snippets);
> +  return authority->big_state->vertex_snippets.entries != NULL;
>  }
>
>  static CoglBool
> @@ -1234,7 +1234,7 @@ check_layer_has_vertex_snippet (CoglPipelineLayer *layer,
>      _cogl_pipeline_layer_get_authority (layer, state);
>    CoglBool *found_vertex_snippet = user_data;
>
> -  if (!COGL_LIST_EMPTY (&authority->big_state->vertex_snippets))
> +  if (authority->big_state->vertex_snippets.entries)
>      {
>        *found_vertex_snippet = TRUE;
>        return FALSE;
> @@ -1265,7 +1265,7 @@ _cogl_pipeline_has_non_layer_fragment_snippets (CoglPipeline *pipeline)
>      _cogl_pipeline_get_authority (pipeline,
>                                    COGL_PIPELINE_STATE_FRAGMENT_SNIPPETS);
>
> -  return !COGL_LIST_EMPTY (&authority->big_state->fragment_snippets);
> +  return authority->big_state->fragment_snippets.entries != NULL;
>  }
>
>  static CoglBool
> @@ -1277,7 +1277,7 @@ check_layer_has_fragment_snippet (CoglPipelineLayer *layer,
>      _cogl_pipeline_layer_get_authority (layer, state);
>    CoglBool *found_fragment_snippet = user_data;
>
> -  if (!COGL_LIST_EMPTY (&authority->big_state->fragment_snippets))
> +  if (authority->big_state->fragment_snippets.entries)
>      {
>        *found_fragment_snippet = TRUE;
>        return FALSE;
> diff --git a/cogl/driver/gl/cogl-pipeline-fragend-glsl.c b/cogl/driver/gl/cogl-pipeline-fragend-glsl.c
> index f432a0c..0b06dd4 100644
> --- a/cogl/driver/gl/cogl-pipeline-fragend-glsl.c
> +++ b/cogl/driver/gl/cogl-pipeline-fragend-glsl.c
> @@ -195,11 +195,15 @@ static CoglBool
>  has_replace_hook (CoglPipelineLayer *layer,
>                    CoglSnippetHook hook)
>  {
> -  CoglPipelineSnippet *snippet;
> +  GList *l;
>
> -  COGL_LIST_FOREACH (snippet, get_layer_fragment_snippets (layer), list_node)
> -    if (snippet->snippet->hook == hook && snippet->snippet->replace)
> -      return TRUE;
> +  for (l = get_layer_fragment_snippets (layer)->entries; l; l = l->next)
> +    {
> +      CoglSnippet *snippet = l->data;
> +
> +      if (snippet->hook == hook && snippet->replace)
> +        return TRUE;
> +    }
>
>    return FALSE;
>  }
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl


More information about the Cogl mailing list