[xrandr,v2] Select NearestNeighbour filtering for pixel exact scaling

Submitted by Chris Wilson on April 4, 2016, 8:34 p.m.

Details

Message ID 1459802099-17601-1-git-send-email-chris@chris-wilson.co.uk
State New
Series "Select NearestNeighbour filtering for pixel exact scaling"
Headers show

Commit Message

Chris Wilson April 4, 2016, 8:34 p.m.
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
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 configure.ac |   2 +-
 xrandr.c     | 275 +++++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 192 insertions(+), 85 deletions(-)

Patch hide | download patch | download mbox

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)

Comments

Alex Deucher April 4, 2016, 8:41 p.m.
On Mon, Apr 4, 2016 at 4:34 PM, Chris Wilson <chris@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@amd.com>

> Signed-off-by: Chris Wilson <chris@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@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
Matt Turner Sept. 1, 2017, 9:17 p.m.
Was there a reason this did not land?
Chris Wilson Sept. 24, 2017, 8:51 a.m.
Quoting Matt Turner (2017-09-01 22:17:58)
> Was there a reason this did not land?

It needs a touch more work:

"I was wondering, did you (or anyone else) follow up on this patch? I
recently used it with good success, and would be happy if it was
upstream. Especially as I am planning on getting a 4K monitor soon,
and will likely need to run e.g. games at 1080p resolution.

I did have to add a "continue;" at the end of the
if (!strcmp ("--filter", argv[i])) {
block though, or selecting a filter would not work.

As another note, changing the filter only has an effect if the
resolution/scaling is also altered."

My attempt at doing the latter just ended in segv. After which it just
slipped my mind.
-Chris
Michel Dänzer Oct. 2, 2017, 4:20 p.m.
On 24/09/17 10:51 AM, Chris Wilson wrote:
> Quoting Matt Turner (2017-09-01 22:17:58)
>> Was there a reason this did not land?
> 
> It needs a touch more work:

[...]

> As another note, changing the filter only has an effect if the
> resolution/scaling is also altered."

That could be due to X server bugs fixed by:

https://cgit.freedesktop.org/xorg/xserver/commit/?h=server-1.19-branch&id=358f0bcd4f6703302b8895e42e20d1cbdfff102e
https://cgit.freedesktop.org/xorg/xserver/commit/?h=server-1.19-branch&id=0934d56dc804780f3e83ae0153c797d392e6faba