[PATCH xserver] glamor: keep gl_fbo and fbo consistent

[Olivier Fourdan]

Hi

> If the pixmap type is neither GLAMOR_TEXTURE_ONLY nor GLAMOR_TEXTURE_DRM
> we might have the fbo field set but the gl_fbo still set to the default
> GLAMOR_FBO_UNATTACHED, which later may fail an assert in
> glamor_upload_picture_to_texture().
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99346
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  glamor/glamor_render.c | 8 ++++++++
>  glamor/glamor_utils.h  | 4 ++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
> index e04dd21..a9ab971 100644
> --- a/glamor/glamor_render.c
> +++ b/glamor/glamor_render.c
> @@ -916,6 +916,10 @@ glamor_composite_choose_shader(CARD8 op,
>          }
>          if (source_pixmap_priv->gl_fbo == GLAMOR_FBO_UNATTACHED) {
>  #ifdef GLAMOR_PIXMAP_DYNAMIC_UPLOAD
> +            if (!GLAMOR_PIXMAP_PRIV_HAS_TEXTURE(source_pixmap_priv)) {
> +                glamor_fallback("no texture in source\n");
> +                goto fail;
> +            }
>              source_needs_upload = TRUE;
>  #else
>              glamor_fallback("no texture in source\n");
> @@ -932,6 +936,10 @@ glamor_composite_choose_shader(CARD8 op,
>          }
>          if (mask_pixmap_priv->gl_fbo == GLAMOR_FBO_UNATTACHED) {
>  #ifdef GLAMOR_PIXMAP_DYNAMIC_UPLOAD
> +            if (!GLAMOR_PIXMAP_PRIV_HAS_TEXTURE(mask_pixmap_priv)) {
> +                glamor_fallback("no texture in mask\n");
> +                goto fail;
> +            }
>              mask_needs_upload = TRUE;
>  #else
>              glamor_fallback("no texture in mask\n");
> diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h
> index 6b88527..636c095 100644
> --- a/glamor/glamor_utils.h
> +++ b/glamor/glamor_utils.h
> @@ -585,6 +585,10 @@
>  
>  #define GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv)    (pixmap_priv->gl_fbo ==
>  GLAMOR_FBO_NORMAL)
>  
> +#define GLAMOR_PIXMAP_PRIV_HAS_TEXTURE(pixmap_priv) (			\
> +				pixmap_priv->type == GLAMOR_TEXTURE_DRM \
> +				|| pixmap_priv->type == GLAMOR_TEXTURE_ONLY)
> +
>  /**
>   * Borrow from uxa.
>   */
> --
> 2.9.3


There is a fundamental logical problem with this patch though, because glamor_upload_picture_to_texture() does:

  assert(glamor_pixmap_is_memory(pixmap));

which is basically priv->type == GLAMOR_MEMORY;

So glamor_upload_picture_to_texture() clearly expects a pixmap of type GLAMOR_MEMORY which my patch avoids, so the patch is clearly not right... I mean, it avoids the assert() failure but it disables the entire logic.

Cheers,
Olivier