[Cogl] [PATCH] pipeline: Don't notify the undefined progend of layer changes

[Robert Bragg]

This looks good to land to me:

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

thanks,
- Robert

On Fri, Sep 28, 2012 at 6:38 PM, Neil Roberts <neil at linux.intel.com> wrote:
> When a layer changes before the pipeline has decided which progend to
> use it doesn't need to notify the progend of the change. This was
> causing it to crash. This patch makes that change and also simplifies
> the notification a bit by just making the calls directly instead of
> having three separate functions.
> ---
>  cogl/cogl-pipeline-layer.c   | 25 ++++++++++++++++++---
>  cogl/cogl-pipeline-private.h | 15 -------------
>  cogl/cogl-pipeline.c         | 53 --------------------------------------------
>  3 files changed, 22 insertions(+), 71 deletions(-)
>
> diff --git a/cogl/cogl-pipeline-layer.c b/cogl/cogl-pipeline-layer.c
> index 18f46f9..d9590c8 100644
> --- a/cogl/cogl-pipeline-layer.c
> +++ b/cogl/cogl-pipeline-layer.c
> @@ -282,9 +282,28 @@ _cogl_pipeline_layer_pre_change_notify (CoglPipeline *required_owner,
>     * this layer (required_owner), and there are no other layers
>     * dependant on this layer so it's ok to modify it. */
>
> -  _cogl_pipeline_fragend_layer_change_notify (required_owner, layer, change);
> -  _cogl_pipeline_vertend_layer_change_notify (required_owner, layer, change);
> -  _cogl_pipeline_progend_layer_change_notify (required_owner, layer, change);
> +  /* NB: Although layers can have private state associated with them
> +   * by multiple backends we know that a layer can't be *changed* if
> +   * it has multiple dependants so if we reach here we know we only
> +   * have a single owner and can only be associated with a single
> +   * backend that needs to be notified of the layer change...
> +   */
> +  if (required_owner->progend != COGL_PIPELINE_PROGEND_UNDEFINED)
> +    {
> +      const CoglPipelineProgend *progend =
> +        _cogl_pipeline_progends[required_owner->progend];
> +      const CoglPipelineFragend *fragend =
> +        _cogl_pipeline_fragends[progend->fragend];
> +      const CoglPipelineVertend *vertend =
> +        _cogl_pipeline_vertends[progend->vertend];
> +
> +      if (fragend->layer_pre_change_notify)
> +        fragend->layer_pre_change_notify (required_owner, layer, change);
> +      if (vertend->layer_pre_change_notify)
> +        vertend->layer_pre_change_notify (required_owner, layer, change);
> +      if (progend->layer_pre_change_notify)
> +        progend->layer_pre_change_notify (required_owner, layer, change);
> +    }
>
>    /* If the layer being changed is the same as the last layer we
>     * flushed to the corresponding texture unit then we keep a track of
> diff --git a/cogl/cogl-pipeline-private.h b/cogl/cogl-pipeline-private.h
> index 71e43e0..1a3d049 100644
> --- a/cogl/cogl-pipeline-private.h
> +++ b/cogl/cogl-pipeline-private.h
> @@ -872,26 +872,11 @@ _cogl_pipeline_init_state_hash_functions (void);
>  void
>  _cogl_pipeline_init_layer_state_hash_functions (void);
>
> -void
> -_cogl_pipeline_fragend_layer_change_notify (CoglPipeline *owner,
> -                                            CoglPipelineLayer *layer,
> -                                            CoglPipelineLayerState change);
> -
>  CoglPipelineLayerState
>  _cogl_pipeline_get_layer_state_for_fragment_codegen (CoglContext *context);
>
>  CoglPipelineState
>  _cogl_pipeline_get_state_for_fragment_codegen (CoglContext *context);
>
> -void
> -_cogl_pipeline_vertend_layer_change_notify (CoglPipeline *owner,
> -                                            CoglPipelineLayer *layer,
> -                                            CoglPipelineLayerState change);
> -
> -void
> -_cogl_pipeline_progend_layer_change_notify (CoglPipeline *owner,
> -                                            CoglPipelineLayer *layer,
> -                                            CoglPipelineLayerState change);
> -
>  #endif /* __COGL_PIPELINE_PRIVATE_H */
>
> diff --git a/cogl/cogl-pipeline.c b/cogl/cogl-pipeline.c
> index ad573eb..ac7b1cf 100644
> --- a/cogl/cogl-pipeline.c
> +++ b/cogl/cogl-pipeline.c
> @@ -1449,59 +1449,6 @@ _cogl_pipeline_prune_to_n_layers (CoglPipeline *pipeline, int n)
>    pipeline->differences |= COGL_PIPELINE_STATE_LAYERS;
>  }
>
> -void
> -_cogl_pipeline_fragend_layer_change_notify (CoglPipeline *owner,
> -                                            CoglPipelineLayer *layer,
> -                                            CoglPipelineLayerState change)
> -{
> -  /* NB: Although layers can have private state associated with them
> -   * by multiple backends we know that a layer can't be *changed* if
> -   * it has multiple dependants so if we reach here we know we only
> -   * have a single owner and can only be associated with a single
> -   * backend that needs to be notified of the layer change...
> -   */
> -  if (owner->progend != COGL_PIPELINE_PROGEND_UNDEFINED)
> -    {
> -      const CoglPipelineProgend *progend =
> -        _cogl_pipeline_progends[owner->progend];
> -      const CoglPipelineFragend *fragend =
> -        _cogl_pipeline_fragends[progend->fragend];
> -
> -      if (fragend->layer_pre_change_notify)
> -        fragend->layer_pre_change_notify (owner, layer, change);
> -    }
> -}
> -
> -void
> -_cogl_pipeline_vertend_layer_change_notify (CoglPipeline *owner,
> -                                            CoglPipelineLayer *layer,
> -                                            CoglPipelineLayerState change)
> -{
> -  /* NB: The comment in fragend_layer_change_notify applies here too */
> -  if (owner->progend != COGL_PIPELINE_PROGEND_UNDEFINED)
> -    {
> -      const CoglPipelineProgend *progend =
> -        _cogl_pipeline_progends[owner->progend];
> -      const CoglPipelineVertend *vertend =
> -        _cogl_pipeline_vertends[progend->vertend];
> -
> -      if (vertend->layer_pre_change_notify)
> -        vertend->layer_pre_change_notify (owner, layer, change);
> -    }
> -}
> -
> -void
> -_cogl_pipeline_progend_layer_change_notify (CoglPipeline *owner,
> -                                            CoglPipelineLayer *layer,
> -                                            CoglPipelineLayerState change)
> -{
> -  const CoglPipelineProgend *progend =
> -    _cogl_pipeline_progends[owner->progend];
> -
> -  if (progend->layer_pre_change_notify)
> -    progend->layer_pre_change_notify (owner, layer, change);
> -}
> -
>  typedef struct
>  {
>    /* The layer we are trying to find */
> --
> 1.7.11.3.g3c3efa5
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl