[xrandr v2] Select NearestNeighbour filtering for pixel exact scaling
Alex Deucher
alexdeucher at gmail.com
Mon Apr 4 20:41:43 UTC 2016
On Mon, Apr 4, 2016 at 4:34 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> When using pixel-exact scaling from for example running a 3840x2160 monitor
> at 1920x1080 half-resolution upscaling using a bilinear filter
> introduces blur where none is expected.
>
> v2: Allow the user to specify what filter they want using --filter, but
> default to automatic selection.
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=94816
Also:
https://bugs.freedesktop.org/show_bug.cgi?id=94820
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> configure.ac | 2 +-
> xrandr.c | 275 +++++++++++++++++++++++++++++++++++++++++------------------
> 2 files changed, 192 insertions(+), 85 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index dcf7c10..95cc1f6 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -39,7 +39,7 @@ XORG_DEFAULT_OPTIONS
> AC_CHECK_LIB(m,floor)
>
> # Checks for pkg-config packages
> -PKG_CHECK_MODULES(XRANDR, xrandr >= 1.5 xrender x11 xproto >= 7.0.17)
> +PKG_CHECK_MODULES(XRANDR, xrandr >= 1.5 xrender x11 xproto >= 7.0.17 pixman-1)
>
> AC_CONFIG_FILES([
> Makefile
> diff --git a/xrandr.c b/xrandr.c
> index dcfdde0..5c3a326 100644
> --- a/xrandr.c
> +++ b/xrandr.c
> @@ -40,6 +40,7 @@
> #include <inttypes.h>
> #include <stdarg.h>
> #include <math.h>
> +#include <pixman.h>
>
> #ifdef HAVE_CONFIG_H
> #include "config.h"
> @@ -135,6 +136,7 @@ usage(void)
> " --scale <x>x<y>\n"
> " --scale-from <w>x<h>\n"
> " --transform <a>,<b>,<c>,<d>,<e>,<f>,<g>,<h>,<i>\n"
> + " --filter auto,bilinear,nearest\n"
> " --off\n"
> " --crtc <crtc>\n"
> " --panning <w>x<h>[+<x>+<y>[/<track:w>x<h>+<x>+<y>[/<border:l>/<t>/<r>/<b>]]]\n"
> @@ -285,6 +287,7 @@ typedef enum _changes {
> changes_panning = (1 << 10),
> changes_gamma = (1 << 11),
> changes_primary = (1 << 12),
> + changes_filter = (1 << 13),
> } changes_t;
>
> typedef enum _name_kind {
> @@ -305,19 +308,24 @@ typedef struct {
> typedef struct _crtc crtc_t;
> typedef struct _output output_t;
> typedef struct _transform transform_t;
> +typedef struct _filter filter_t;
> typedef struct _umode umode_t;
> typedef struct _output_prop output_prop_t;
> typedef struct _provider provider_t;
> typedef struct _monitors monitors_t;
> typedef struct _umonitor umonitor_t;
>
> -struct _transform {
> - XTransform transform;
> +struct _filter {
> const char *filter;
> int nparams;
> XFixed *params;
> };
>
> +
> +struct _transform {
> + XTransform transform;
> +};
> +
> struct _crtc {
> name_t crtc;
> Bool changing;
> @@ -331,6 +339,7 @@ struct _crtc {
> output_t **outputs;
> int noutput;
> transform_t current_transform, pending_transform;
> + filter_t current_filter, pending_filter;
> };
>
> struct _output_prop {
> @@ -370,6 +379,7 @@ struct _output {
> Bool automatic;
> int scale_from_w, scale_from_h;
> transform_t transform;
> + filter_t filter;
>
> struct {
> float red;
> @@ -707,19 +717,29 @@ init_transform (transform_t *transform)
> memset (&transform->transform, '\0', sizeof (transform->transform));
> for (x = 0; x < 3; x++)
> transform->transform.matrix[x][x] = XDoubleToFixed (1.0);
> - transform->filter = "";
> - transform->nparams = 0;
> - transform->params = NULL;
> +}
> +
> +static void
> +init_filter (filter_t *filter)
> +{
> + filter->filter = "";
> + filter->nparams = 0;
> + filter->params = NULL;
> }
>
> static void
> set_transform (transform_t *dest,
> - XTransform *transform,
> - const char *filter,
> - XFixed *params,
> - int nparams)
> + XTransform *transform)
> {
> dest->transform = *transform;
> +}
> +
> +static void
> +set_filter (filter_t *dest,
> + const char *filter,
> + XFixed *params,
> + int nparams)
> +{
> /* note: this string is leaked */
> dest->filter = strdup (filter);
> dest->nparams = nparams;
> @@ -728,10 +748,25 @@ set_transform (transform_t *dest,
> }
>
> static void
> +auto_filter (output_t *output)
> +{
> + if ((output->changes & changes_filter) == 0) {
> + init_filter (&output->filter);
> + output->filter.filter = "auto";
> + output->changes |= changes_filter;
> + }
> +}
> +
> +static void
> copy_transform (transform_t *dest, transform_t *src)
> {
> - set_transform (dest, &src->transform,
> - src->filter, src->params, src->nparams);
> + set_transform (dest, &src->transform);
> +}
> +
> +static void
> +copy_filter (filter_t *dest, filter_t *src)
> +{
> + set_filter(dest, src->filter, src->params, src->nparams);
> }
>
> static Bool
> @@ -739,6 +774,12 @@ equal_transform (transform_t *a, transform_t *b)
> {
> if (memcmp (&a->transform, &b->transform, sizeof (XTransform)) != 0)
> return False;
> + return True;
> +}
> +
> +static Bool
> +equal_filter (filter_t *a, filter_t *b)
> +{
> if (strcmp (a->filter, b->filter) != 0)
> return False;
> if (a->nparams != b->nparams)
> @@ -1157,6 +1198,40 @@ set_gamma_info(output_t *output)
> XRRFreeGamma(crtc_gamma);
> }
>
> +static Bool
> +pixel_exact(const XTransform *t)
> +{
> + struct pixman_transform inv;
> +
> + if (!pixman_transform_invert(&inv, (struct pixman_transform *)t))
> + return False;
> +
> + /* Any perspective scaling? */
> + if ((inv.matrix[2][0] | inv.matrix[2][1] | inv.matrix[2][2]) != 0x10000)
> + return False;
> +
> + /* Any non-integer translation? */
> + if ((inv.matrix[0][2] | inv.matrix[1][2]) & 0xffff)
> + return False;
> +
> + /* Rotation? */
> + if (inv.matrix[0][1] | inv.matrix[1][0]) {
> + if (inv.matrix[0][0] | inv.matrix[1][1])
> + return False;
> +
> + if ((inv.matrix[1][0] | inv.matrix[0][1]) & 0xffff)
> + return False;
> + } else {
> + if (inv.matrix[1][0] | inv.matrix[0][1])
> + return False;
> +
> + if ((inv.matrix[0][0] | inv.matrix[1][1]) & 0xffff)
> + return False;
> + }
> +
> + return True;
> +}
> +
> static void
> set_output_info (output_t *output, RROutput xid, XRROutputInfo *output_info)
> {
> @@ -1303,15 +1378,29 @@ set_output_info (output_t *output, RROutput xid, XRROutputInfo *output_info)
> output->transform.transform.matrix[0][0] = XDoubleToFixed (sx);
> output->transform.transform.matrix[1][1] = XDoubleToFixed (sy);
> output->transform.transform.matrix[2][2] = XDoubleToFixed (1.0);
> - if (sx != 1 || sy != 1)
> - output->transform.filter = "bilinear";
> - else
> - output->transform.filter = "nearest";
> - output->transform.nparams = 0;
> - output->transform.params = NULL;
> }
> }
>
> + if (!(output->changes & changes_filter))
> + {
> + if (output->crtc_info)
> + copy_filter (&output->filter, &output->crtc_info->current_filter);
> + else
> + init_filter (&output->filter);
> + }
> + else
> + {
> + if (strcmp(output->filter.filter, "auto") == 0)
> + {
> + if (pixel_exact(&output->transform.transform))
> + output->filter.filter = "nearest";
> + else
> + output->filter.filter = "bilinear";
> + if (verbose)
> + printf("Using %s filter\n", output->filter.filter);
> + }
> + }
> +
> /* set primary */
> if (!(output->changes & changes_primary))
> output->primary = output_is_primary(output);
> @@ -1376,7 +1465,8 @@ get_crtcs (void)
> }
> if (XRRGetCrtcTransform (dpy, res->crtcs[c], &attr) && attr) {
> set_transform (&crtcs[c].current_transform,
> - &attr->currentTransform,
> + &attr->currentTransform);
> + set_filter(&crtcs[c].current_filter,
> attr->currentFilter,
> attr->currentParams,
> attr->currentNparams);
> @@ -1385,8 +1475,10 @@ get_crtcs (void)
> else
> {
> init_transform (&crtcs[c].current_transform);
> + init_filter (&crtcs[c].current_filter);
> }
> copy_transform (&crtcs[c].pending_transform, &crtcs[c].current_transform);
> + copy_filter (&crtcs[c].pending_filter, &crtcs[c].current_filter);
> }
> }
>
> @@ -1403,6 +1495,7 @@ crtc_add_output (crtc_t *crtc, output_t *output)
> crtc->rotation = output->rotation;
> crtc->mode_info = output->mode_info;
> copy_transform (&crtc->pending_transform, &output->transform);
> + copy_filter (&crtc->pending_filter, &output->filter);
> }
> if (!crtc->outputs) fatal ("out of memory\n");
> crtc->outputs[crtc->noutput++] = output;
> @@ -1555,7 +1648,7 @@ crtc_disable (crtc_t *crtc)
> }
>
> static void
> -crtc_set_transform (crtc_t *crtc, transform_t *transform)
> +crtc_set_transform (crtc_t *crtc, transform_t *transform, filter_t *filter)
> {
> int major, minor;
>
> @@ -1563,9 +1656,9 @@ crtc_set_transform (crtc_t *crtc, transform_t *transform)
> if (major > 1 || (major == 1 && minor >= 3))
> XRRSetCrtcTransform (dpy, crtc->crtc.xid,
> &transform->transform,
> - transform->filter,
> - transform->params,
> - transform->nparams);
> + filter->filter,
> + filter->params,
> + filter->nparams);
> }
>
> static Status
> @@ -1579,8 +1672,12 @@ crtc_revert (crtc_t *crtc)
> if (dryrun)
> return RRSetConfigSuccess;
>
> - if (!equal_transform (&crtc->current_transform, &crtc->pending_transform))
> - crtc_set_transform (crtc, &crtc->current_transform);
> + if (!equal_transform (&crtc->current_transform, &crtc->pending_transform) ||
> + !equal_filter (&crtc->current_filter, &crtc->pending_filter))
> + crtc_set_transform (crtc,
> + &crtc->current_transform,
> + &crtc->current_filter);
> +
> return XRRSetCrtcConfig (dpy, res, crtc->crtc.xid, CurrentTime,
> crtc_info->x, crtc_info->y,
> crtc_info->mode, crtc_info->rotation,
> @@ -1617,8 +1714,11 @@ crtc_apply (crtc_t *crtc)
> s = RRSetConfigSuccess;
> else
> {
> - if (!equal_transform (&crtc->current_transform, &crtc->pending_transform))
> - crtc_set_transform (crtc, &crtc->pending_transform);
> + if (!equal_transform (&crtc->current_transform, &crtc->pending_transform) ||
> + !equal_filter (&crtc->current_filter, &crtc->pending_filter))
> + crtc_set_transform (crtc,
> + &crtc->pending_transform,
> + &crtc->pending_filter);
> s = XRRSetCrtcConfig (dpy, res, crtc->crtc.xid, CurrentTime,
> crtc->x, crtc->y, mode, crtc->rotation,
> rr_outputs, crtc->noutput);
> @@ -1961,6 +2061,8 @@ check_crtc_for_output (crtc_t *crtc, output_t *output)
> return False;
> if (!equal_transform (&crtc->current_transform, &output->transform))
> return False;
> + if (!equal_filter (&crtc->current_filter, &output->filter))
> + return False;
> }
> else if (crtc->crtc_info->noutput)
> {
> @@ -2978,66 +3080,71 @@ main (int argc, char **argv)
> setit_1_2 = True;
> continue;
> }
> - if (!strcmp ("--scale", argv[i]))
> - {
> - double sx, sy;
> - if (!config_output) argerr ("%s must be used after --output\n", argv[i]);
> - if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> - if (sscanf (argv[i], "%lfx%lf", &sx, &sy) != 2)
> - argerr ("failed to parse '%s' as a scaling factor\n", argv[i]);
> - init_transform (&config_output->transform);
> - config_output->transform.transform.matrix[0][0] = XDoubleToFixed (sx);
> - config_output->transform.transform.matrix[1][1] = XDoubleToFixed (sy);
> - config_output->transform.transform.matrix[2][2] = XDoubleToFixed (1.0);
> - if (sx != 1 || sy != 1)
> - config_output->transform.filter = "bilinear";
> - else
> - config_output->transform.filter = "nearest";
> - config_output->transform.nparams = 0;
> - config_output->transform.params = NULL;
> - config_output->changes |= changes_transform;
> - continue;
> - }
> - if (!strcmp ("--scale-from", argv[i]))
> - {
> - int w, h;
> - if (!config_output) argerr ("%s must be used after --output\n", argv[i]);
> - if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> - if (sscanf (argv[i], "%dx%d", &w, &h) != 2)
> - argerr ("failed to parse '%s' as a scale-from size\n", argv[i]);
> - if (w <=0 || h <= 0)
> - argerr ("--scale-from dimensions must be nonnegative\n");
> - config_output->scale_from_w = w;
> - config_output->scale_from_h = h;
> - config_output->changes |= changes_transform;
> - continue;
> - }
> - if (!strcmp ("--transform", argv[i])) {
> - double transform[3][3];
> - int k, l;
> + if (!strcmp ("--filter", argv[i])) {
> if (!config_output) argerr ("%s must be used after --output\n", argv[i]);
> if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> - init_transform (&config_output->transform);
> - if (strcmp (argv[i], "none") != 0)
> - {
> - if (sscanf(argv[i], "%lf,%lf,%lf,%lf,%lf,%lf,%lf,%lf,%lf",
> - &transform[0][0],&transform[0][1],&transform[0][2],
> - &transform[1][0],&transform[1][1],&transform[1][2],
> - &transform[2][0],&transform[2][1],&transform[2][2])
> - != 9)
> - argerr ("failed to parse '%s' as a transformation\n", argv[i]);
> - init_transform (&config_output->transform);
> - for (k = 0; k < 3; k++)
> - for (l = 0; l < 3; l++) {
> - config_output->transform.transform.matrix[k][l] = XDoubleToFixed (transform[k][l]);
> - }
> - config_output->transform.filter = "bilinear";
> - config_output->transform.nparams = 0;
> - config_output->transform.params = NULL;
> + if (strcmp ("auto", argv[i]) == 0 ||
> + strcmp ("nearest", argv[i]) == 0 ||
> + strcmp ("bilinear", argv[i]) == 0) {
> + init_filter (&config_output->filter);
> + config_output->filter.filter = argv[i];
> + config_output->changes |= changes_filter;
> }
> - config_output->changes |= changes_transform;
> - continue;
> }
> + if (!strcmp ("--scale", argv[i]))
> + {
> + double sx, sy;
> + if (!config_output) argerr ("%s must be used after --output\n", argv[i]);
> + if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> + if (sscanf (argv[i], "%lfx%lf", &sx, &sy) != 2)
> + argerr ("failed to parse '%s' as a scaling factor\n", argv[i]);
> + init_transform (&config_output->transform);
> + config_output->transform.transform.matrix[0][0] = XDoubleToFixed (sx);
> + config_output->transform.transform.matrix[1][1] = XDoubleToFixed (sy);
> + config_output->transform.transform.matrix[2][2] = XDoubleToFixed (1.0);
> + config_output->changes |= changes_transform;
> + auto_filter (config_output);
> + continue;
> + }
> + if (!strcmp ("--scale-from", argv[i]))
> + {
> + int w, h;
> + if (!config_output) argerr ("%s must be used after --output\n", argv[i]);
> + if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> + if (sscanf (argv[i], "%dx%d", &w, &h) != 2)
> + argerr ("failed to parse '%s' as a scale-from size\n", argv[i]);
> + if (w <=0 || h <= 0)
> + argerr ("--scale-from dimensions must be nonnegative\n");
> + config_output->scale_from_w = w;
> + config_output->scale_from_h = h;
> + config_output->changes |= changes_transform;
> + auto_filter (config_output);
> + continue;
> + }
> + if (!strcmp ("--transform", argv[i])) {
> + double transform[3][3];
> + int k, l;
> + if (!config_output) argerr ("%s must be used after --output\n", argv[i]);
> + if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> + init_transform (&config_output->transform);
> + if (strcmp (argv[i], "none") != 0)
> + {
> + if (sscanf(argv[i], "%lf,%lf,%lf,%lf,%lf,%lf,%lf,%lf,%lf",
> + &transform[0][0],&transform[0][1],&transform[0][2],
> + &transform[1][0],&transform[1][1],&transform[1][2],
> + &transform[2][0],&transform[2][1],&transform[2][2])
> + != 9)
> + argerr ("failed to parse '%s' as a transformation\n", argv[i]);
> + init_transform (&config_output->transform);
> + for (k = 0; k < 3; k++)
> + for (l = 0; l < 3; l++) {
> + config_output->transform.transform.matrix[k][l] = XDoubleToFixed (transform[k][l]);
> + }
> + }
> + config_output->changes |= changes_transform;
> + auto_filter(config_output);
> + continue;
> + }
> if (!strcmp ("--off", argv[i])) {
> if (!config_output) argerr ("%s must be used after --output\n", argv[i]);
> set_name_xid (&config_output->mode, None);
> @@ -3802,8 +3909,8 @@ main (int argc, char **argv)
> if (y < 2)
> printf ("\n\t ");
> }
> - if (output->transform.filter)
> - printf ("\n\t filter: %s", output->transform.filter);
> + if (output->filter.filter)
> + printf ("\n\t filter: %s", output->filter.filter);
> printf ("\n");
> }
> if (verbose || properties)
> --
> 2.8.0.rc3
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
More information about the xorg-devel
mailing list