[cairo] [PATCH] V2 image: xlib/xcb backends use filtering to match image backend
Uli Schlachter
psychon at znc.in
Tue Sep 23 00:59:23 PDT 2014
Hi,
no idea about if this is a good idea. I'll let other decide about that.
Am 23.09.2014 um 05:21 schrieb Bill Spitzak:
[...]
> diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c
> index e6fdae6..0f77670 100644
> --- a/src/cairo-pattern.c
> +++ b/src/cairo-pattern.c
> @@ -3371,6 +3371,7 @@ _cairo_pattern_analyze_filter (const cairo_pattern_t *pattern,
> if (_cairo_matrix_is_pixel_exact (&pattern->matrix)) {
> pad = 0.;
> optimized_filter = CAIRO_FILTER_NEAREST;
> + break;
> } else {
> /* 0.5 is enough for a bilinear filter. It's possible we
> * should defensively use more for CAIRO_FILTER_BEST, but
This seems unnecessary? There is a "break" after the else branch...
> @@ -3379,6 +3380,22 @@ _cairo_pattern_analyze_filter (const cairo_pattern_t *pattern,
> */
> pad = 0.5;
> optimized_filter = pattern->filter;
> + /* Use BILINEAR for any scale greater than 1/1.35 instead
> + * of GOOD. For scales of 1 and larger this is identical,
> + * for the smaller sizes it was judged that the artifacts
> + * were not worse than the artifacts from a box filer.
> + */
> + if (pattern->filter == CAIRO_FILTER_GOOD) {
> + double h;
> + h = pattern->matrix.xx * pattern->matrix.xx +
> + pattern->matrix.xy * pattern->matrix.xy;
> + if (h < 1.35 * 1.35) {
> + h = pattern->matrix.yx * pattern->matrix.yx +
> + pattern->matrix.yy * pattern->matrix.yy;
> + if (h < 1.35 * 1.35)
> + optimized_filter = CAIRO_FILTER_BILINEAR;
> + }
> + }
> }
> break;
...here.
> diff --git a/src/cairo-xcb-connection.c b/src/cairo-xcb-connection.c
> index b48add1..2d51e14 100644
> --- a/src/cairo-xcb-connection.c
> +++ b/src/cairo-xcb-connection.c
> @@ -77,6 +77,8 @@ typedef struct _cairo_xcb_xid {
>
> #define XCB_RENDER_HAS_PICTURE_TRANSFORM(surface) XCB_RENDER_AT_LEAST((surface), 0, 6)
> #define XCB_RENDER_HAS_FILTERS(surface) XCB_RENDER_AT_LEAST((surface), 0, 6)
> +#define XCB_RENDER_HAS_FILTER_GOOD(surface) FALSE
> +#define XCB_RENDER_HAS_FILTER_BEST(surface) FALSE
This could need some comment explaining things. "Cairo 1.14 changes the meaning
of this and suddenly RENDER doesn't match" or something like that.
Also, what's the relationship between HAS_FILTERS and HAS_FILTER_GOOD/BEST?
Could also need a comment.
> #define XCB_RENDER_HAS_EXTENDED_REPEAT(surface) XCB_RENDER_AT_LEAST((surface), 0, 10)
> #define XCB_RENDER_HAS_GRADIENTS(surface) XCB_RENDER_AT_LEAST((surface), 0, 10)
> @@ -390,6 +392,12 @@ _cairo_xcb_connection_query_render (cairo_xcb_connection_t *connection)
> if (XCB_RENDER_HAS_FILTERS (version))
> connection->flags |= CAIRO_XCB_RENDER_HAS_FILTERS;
>
> + if (XCB_RENDER_HAS_FILTER_GOOD (version))
> + connection->flags |= CAIRO_XCB_RENDER_HAS_FILTER_GOOD;
> +
> + if (XCB_RENDER_HAS_FILTER_BEST (version))
> + connection->flags |= CAIRO_XCB_RENDER_HAS_FILTER_BEST;
> +
> if (XCB_RENDER_HAS_PDF_OPERATORS (version))
> connection->flags |= CAIRO_XCB_RENDER_HAS_PDF_OPERATORS;
>
> @@ -882,6 +890,8 @@ cairo_xcb_device_debug_cap_xrender_version (cairo_device_t *device,
> CAIRO_XCB_RENDER_HAS_COMPOSITE_TRAPEZOIDS |
> CAIRO_XCB_RENDER_HAS_PICTURE_TRANSFORM |
> CAIRO_XCB_RENDER_HAS_FILTERS |
> + CAIRO_XCB_RENDER_HAS_FILTER_GOOD |
> + CAIRO_XCB_RENDER_HAS_FILTER_BEST |
> CAIRO_XCB_RENDER_HAS_PDF_OPERATORS |
> CAIRO_XCB_RENDER_HAS_EXTENDED_REPEAT |
> CAIRO_XCB_RENDER_HAS_GRADIENTS);
> diff --git a/src/cairo-xcb-private.h b/src/cairo-xcb-private.h
> index 2610458..1e1d1ee 100644
> --- a/src/cairo-xcb-private.h
> +++ b/src/cairo-xcb-private.h
> @@ -259,6 +259,8 @@ enum {
> CAIRO_XCB_RENDER_HAS_PDF_OPERATORS = 0x0080,
> CAIRO_XCB_RENDER_HAS_EXTENDED_REPEAT = 0x0100,
> CAIRO_XCB_RENDER_HAS_GRADIENTS = 0x0200,
> + CAIRO_XCB_RENDER_HAS_FILTER_GOOD = 0x0400,
> + CAIRO_XCB_RENDER_HAS_FILTER_BEST = 0x0800,
>
> CAIRO_XCB_HAS_SHM = 0x80000000,
>
> @@ -271,7 +273,9 @@ enum {
> CAIRO_XCB_RENDER_HAS_FILTERS |
> CAIRO_XCB_RENDER_HAS_PDF_OPERATORS |
> CAIRO_XCB_RENDER_HAS_EXTENDED_REPEAT |
> - CAIRO_XCB_RENDER_HAS_GRADIENTS,
> + CAIRO_XCB_RENDER_HAS_GRADIENTS |
> + CAIRO_XCB_RENDER_HAS_FILTER_GOOD |
> + CAIRO_XCB_RENDER_HAS_FILTER_BEST,
> CAIRO_XCB_SHM_MASK = CAIRO_XCB_HAS_SHM
> };
>
> diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c
> index edfa34c..21a89cc 100644
> --- a/src/cairo-xcb-surface-render.c
> +++ b/src/cairo-xcb-surface-render.c
> @@ -394,11 +394,6 @@ _pattern_is_supported (uint32_t flags,
> if (pattern->type == CAIRO_PATTERN_TYPE_SOLID)
> return TRUE;
>
> - if (! _cairo_matrix_is_integer_translation (&pattern->matrix, NULL, NULL)) {
> - if ((flags & CAIRO_XCB_RENDER_HAS_PICTURE_TRANSFORM) == 0)
> - return FALSE;
> - }
Why are you removing this check? This seems like an unrelated change to this
patch. And yes, you aren't moving this check around, for quite some pattern
types you are entirely removing this.
This makes me wonder: Did you run this patch through the test suite? What were
the results? Could you mention so in the commit message?
> switch (pattern->extend) {
> default:
> ASSERT_NOT_REACHED;
> @@ -412,18 +407,22 @@ _pattern_is_supported (uint32_t flags,
> }
>
> if (pattern->type == CAIRO_PATTERN_TYPE_SURFACE) {
> - cairo_filter_t filter;
> -
> - filter = pattern->filter;
> - if (_cairo_matrix_is_pixel_exact (&pattern->matrix))
> - {
> - filter = CAIRO_FILTER_NEAREST;
> - }
> -
> - if (! (filter == CAIRO_FILTER_NEAREST || filter == CAIRO_FILTER_FAST)) {
> - if ((flags & CAIRO_XCB_RENDER_HAS_FILTERS) == 0)
> - return FALSE;
> + switch (pattern->filter) {
> + case CAIRO_FILTER_FAST:
> + case CAIRO_FILTER_NEAREST:
> + return (flags & CAIRO_XCB_RENDER_HAS_PICTURE_TRANSFORM) ||
> + _cairo_matrix_is_integer_translation (&pattern->matrix, NULL, NULL);
> + case CAIRO_FILTER_GOOD:
> + return flags & CAIRO_XCB_RENDER_HAS_FILTER_GOOD;
> + case CAIRO_FILTER_BEST:
> + return flags & CAIRO_XCB_RENDER_HAS_FILTER_BEST;
> + case CAIRO_FILTER_BILINEAR:
> + case CAIRO_FILTER_GAUSSIAN:
> + default:
> + return flags & CAIRO_XCB_RENDER_HAS_FILTERS;
>
> }
> + } else if (pattern->type == CAIRO_PATTERN_TYPE_MESH) {
> + return FALSE;
> } else { /* gradient */
> if ((flags & CAIRO_XCB_RENDER_HAS_GRADIENTS) == 0)
> return FALSE;
> @@ -435,9 +434,8 @@ _pattern_is_supported (uint32_t flags,
> {
> return FALSE;
> }
> + return TRUE;
> }
> -
> - return pattern->type != CAIRO_PATTERN_TYPE_MESH;
Should this become an ASSERT_UNREACHED? If not, what happens when the end of
this function is reached? I'd expect some compiler warnings about a function
without a return statement about the end.
> }
>
> static void
[...]
Cheers,
Uli
--
Happiness can't be found -- it finds you.
- Majic
More information about the cairo
mailing list