[cairo] [cairo-commit] src/cairo-gl-composite.c src/cairo-gl-device.c src/cairo-gl-dispatch-private.h src/cairo-gl-msaa-compositor.c src/cairo-gl-operand.c src/cairo-gl-private.h src/cairo-gl-surface.c

[Chris Wilson]

On Thu, 17 May 2012 11:16:03 -0700 (PDT), mrobinson at kemper.freedesktop.org (Martin Robinson) wrote:
> +static cairo_bool_t
> +should_fall_back (cairo_gl_surface_t *surface,
> +		  cairo_antialias_t antialias)
> +{
> +    if (antialias == CAIRO_ANTIALIAS_FAST)
> +	return TRUE;
> +    if (antialias == CAIRO_ANTIALIAS_NONE)
> +	return FALSE;
> +    return ! surface->supports_msaa;
> +}

This is a little non-descriptive, and I can't quite judge if it does
what I think it should just by the name. I was expecting something along
the lines of:

  can_use_msaa(cairo_gl_surface_t *surface, cairo_antialias_t antialias)
  {
    if (antialias == CAIRO_ANTIALIAS_NONE)
      return TRUE; /* single sample "MSAA" */
    if (antialias == CAIRO_ANTIALIAS_FAST)
      return surface->supports_msaa;
    if (antialias == CAIRO_ANTIALIAS_DEFAULT)
      return surface->supports_msaa && 
             to_gl_device(surface->base.device)->use_msaa_by_default;
    return FALSE;
  }

> +
>  static cairo_int_status_t
>  _cairo_gl_msaa_compositor_paint (const cairo_compositor_t	*compositor,
>  				 cairo_composite_rectangles_t	*composite)
> @@ -337,6 +348,9 @@ _cairo_gl_msaa_compositor_mask (const cairo_compositor_t	*compositor,
>      cairo_int_status_t status;
>      cairo_operator_t op = composite->op;
>  
> +    if (should_fall_back (dst, CAIRO_ANTIALIAS_GOOD))
s/GOOD/CAIRO_ANTLIAS_DEAFULT/;

Then this reads as
  if (! can_use_msaa (dst, CAIRO_ANTIALIAS_DEFAULT))
    return CAIRO_INT_STATUS_UNUSPPORTED;
for which I hope we both agree it is much easier to understand the intent.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre