Re: [PATCH] xcb: make use of _cairo_surface_is_xcb to check for surface type

Hi,

On 30.06.2014 14:03, Ravi Nanjundappa wrote:
> Introduced a new inline function _cairo_surface_is_xcb() as similar to
> _cairo_surface_is_image() and used the same to check for xcb surface type
> 
> Signed-off-by: Ravi Nanjundappa <[email protected]>
> ---
>  src/cairo-xcb-surface.c |   20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cairo-xcb-surface.c b/src/cairo-xcb-surface.c
> index c900edc..e3230c7 100644
> --- a/src/cairo-xcb-surface.c
> +++ b/src/cairo-xcb-surface.c
> @@ -78,6 +78,22 @@ slim_hidden_proto (cairo_xcb_surface_create_with_xrender_format);
>   * Since: 1.12
>   **/
>  
> +
> +/**
> + * _cairo_surface_is_xcb:
> + * @surface: a #cairo_surface_t
> + *
> + * Checks if a surface is an #cairo_xcb_surface_t
> + *
> + * Return value: %TRUE if the surface is an xcb surface
> + **/
> +static inline cairo_bool_t
> +_cairo_surface_is_xcb (const cairo_surface_t *surface)
> +{
> +    /* _cairo_surface_nil sets a NULL backend so be safe */
> +    return surface->backend && surface->backend->type == CAIRO_SURFACE_TYPE_XCB;
> +}


Does this new "->backend != NULL" check make a difference (= does this fix any
bugs?) or is it just copied from _cairo_surface_is_image()?

>  cairo_surface_t *
>  _cairo_xcb_surface_create_similar (void			*abstract_other,
>  				   cairo_content_t	 content,
> @@ -1432,7 +1448,7 @@ cairo_xcb_surface_set_size (cairo_surface_t *abstract_surface,
>      }
>  
>  
> -    if (abstract_surface->type != CAIRO_SURFACE_TYPE_XCB) {
> +    if (! _cairo_surface_is_xcb (abstract_surface)) {
>  	_cairo_surface_set_error (abstract_surface,
>  				  _cairo_error (CAIRO_STATUS_SURFACE_TYPE_MISMATCH));
>  	return;
> @@ -1486,7 +1502,7 @@ cairo_xcb_surface_set_drawable (cairo_surface_t *abstract_surface,
>      }
>  
>  
> -    if (abstract_surface->type != CAIRO_SURFACE_TYPE_XCB) {
> +    if (! _cairo_surface_is_xcb (abstract_surface)) {
>  	_cairo_surface_set_error (abstract_surface,
>  				  _cairo_error (CAIRO_STATUS_SURFACE_TYPE_MISMATCH));
>  	return;

What's special about these two places?

According to git grep, there are lots of other matches for
CAIRO_SURFACE_TYPE_XCB, e.g. in cairo-xcb-surface-core.c or
cairo-xcb-surface-render.c (and in cairo-xlib-xcb-surface.c, but I don't think
that this counts). Some of these check ->type, others do source->backend->type.

Since I don't have much time right now:

subsurfaces/cairo_surface_create_for_rectangle() creates a surface where ->type
is the "original" type and ->backend->type is CAIRO_SURFACE_TYPE_SUBSURFACE,
right? So we should always use ->backend->type internally so that subsurfaces
can't mess things up, right? (= This patch fixes an actual bug, but doesn't fix
all the places where we have this bug)

Sorry for this chaotic mail, I don't have much time right now.

Cheers,
Uli
-- 
Sent from my Game Boy.
-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo