from:"Pali Rohár"

Re: [PATCH v2] Fix checking for valid argument of --dpi

2019-10-14 Thread Pali Rohár

On Wednesday 24 April 2019 16:29:25 Pali Rohár wrote:
> On Sunday 30 December 2018 14:11:13 Pali Rohár wrote:
> > On Sunday 30 December 2018 13:51:10 Walter Harms wrote:
> > > 
> > > 
> > > > Pali Rohár  hat am 29. Dezember 2018 um 18:45
> > > > geschrieben:
> > > > 
> > > > 
> > > > On Monday 26 November 2018 22:17:24 Pali Rohár wrote:
> > > > > Function strtod() sets strtod_error to the pointer of the first 
> > > > > invalid
> > > > > character and therefore it does not have to be first character from 
> > > > > input.
> > > > > When input is valid then it points to nul byte. Conversion error is
> > > > > indicated by setted errno. Zero-length argument, non-positive DPI or 
> > > > > DPI
> > > > > with trailing or leading whitespaces is invalid too.
> > > > > 
> > > > > Update also error message about invalid argument.
> > > > > 
> > > > > Signed-off-by: Pali Rohár 
> > > > > 
> > > > > --
> > > > > 
> > > > > Changes since v1:
> > > > > * Get same error message for `xrandr --dpi ''` and `xrandr --dpi ' '`
> > > > > * Make the check for dpi <= 0
> > > > > * Do not accept leading whitespaces (trailing were already disallowed)
> > > > > ---
> > > > >  xrandr.c | 6 --
> > > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/xrandr.c b/xrandr.c
> > > > > index ce3cd91..4baa075 100644
> > > > > --- a/xrandr.c
> > > > > +++ b/xrandr.c
> > > > > @@ -40,6 +40,7 @@
> > > > >  #include 
> > > > >  #include 
> > > > >  #include 
> > > > > +#include 
> > > > >  
> > > > >  #ifdef HAVE_CONFIG_H
> > > > >  #include "config.h"
> > > > > @@ -3118,8 +3119,9 @@ main (int argc, char **argv)
> > > > >   if (!strcmp ("--dpi", argv[i])) {
> > > > >   char *strtod_error;
> > > > >   if (++i >= argc) argerr ("%s requires an argument\n", 
> > > > > argv[i-1]);
> > > > > + errno = 0;
> > > > >   dpi = strtod(argv[i], _error);
> > > > > - if (argv[i] == strtod_error)
> > > > > + if (!argv[i][0] || isspace(argv[i][0]) || *strtod_error || 
> > > > > errno ||
> > > > > dpi <= 0)
> > > > >   {
> > > > >   dpi = 0.0;
> > > > >   dpi_output_name = argv[i];
> > > 
> > > 
> > > Why spending so much effort ? 
> > 
> > Which "much effort"?
> > 
> > This is to ensure that incorrect param or garbage supplied by user is
> > not interpreted as number.
> > 
> > > Using atoi(argv[i]) and checking for an invalid value seems fine.
> > 
> > atoi does not signal when garbage is on input. And dpi can be
> > fractional, so atoi cannot be used for it.
> > 
> > > Special cases like NAN do not matter here.
> > > 
> > > BTW: leading whitespaces are ignored with strtod() (so far i know)
> > 
> > But strtod does not signal that they were ignored. Moreover trailing are
> > not accepted. So I chose implementation which is consistent for trailing
> > and leading whitespaces.
> 
> Any other review comments for this patch?
> 
> I would like to know if this patch could be applied or if there are some
> other problems which needs to be resolved.

ping, just a reminder for this my patch...

> > > just my 2 cents
> > > re,
> > >  wh
> > > 
> > > 
> > > 
> > > > > @@ -3569,7 +3571,7 @@ main (int argc, char **argv)
> > > > >   XRROutputInfo   *output_info;
> > > > >   XRRModeInfo *mode_info;
> > > > >   if (!dpi_output)
> > > > > - fatal ("Cannot find output %s\n", dpi_output_name);
> > > > > + fatal ("%s is not valid DPI nor valid output\n", 
> > > > > dpi_output_name);
> > > > >   output_info = dpi_output->output_info;
> > > > >   mode_info = dpi_output->mode_info;
> > > > >   if (output_info && mode_info && output_info->mm_height)
> > > > 
> > > > Hello, can you review this v2 patch?
> 

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH v2] Fix checking for valid argument of --dpi

2019-04-24 Thread Pali Rohár

On Sunday 30 December 2018 14:11:13 Pali Rohár wrote:
> On Sunday 30 December 2018 13:51:10 Walter Harms wrote:
> > 
> > 
> > > Pali Rohár  hat am 29. Dezember 2018 um 18:45
> > > geschrieben:
> > > 
> > > 
> > > On Monday 26 November 2018 22:17:24 Pali Rohár wrote:
> > > > Function strtod() sets strtod_error to the pointer of the first invalid
> > > > character and therefore it does not have to be first character from 
> > > > input.
> > > > When input is valid then it points to nul byte. Conversion error is
> > > > indicated by setted errno. Zero-length argument, non-positive DPI or DPI
> > > > with trailing or leading whitespaces is invalid too.
> > > > 
> > > > Update also error message about invalid argument.
> > > > 
> > > > Signed-off-by: Pali Rohár 
> > > > 
> > > > --
> > > > 
> > > > Changes since v1:
> > > > * Get same error message for `xrandr --dpi ''` and `xrandr --dpi ' '`
> > > > * Make the check for dpi <= 0
> > > > * Do not accept leading whitespaces (trailing were already disallowed)
> > > > ---
> > > >  xrandr.c | 6 --
> > > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/xrandr.c b/xrandr.c
> > > > index ce3cd91..4baa075 100644
> > > > --- a/xrandr.c
> > > > +++ b/xrandr.c
> > > > @@ -40,6 +40,7 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >  
> > > >  #ifdef HAVE_CONFIG_H
> > > >  #include "config.h"
> > > > @@ -3118,8 +3119,9 @@ main (int argc, char **argv)
> > > > if (!strcmp ("--dpi", argv[i])) {
> > > > char *strtod_error;
> > > > if (++i >= argc) argerr ("%s requires an argument\n", 
> > > > argv[i-1]);
> > > > +   errno = 0;
> > > > dpi = strtod(argv[i], _error);
> > > > -   if (argv[i] == strtod_error)
> > > > +   if (!argv[i][0] || isspace(argv[i][0]) || *strtod_error || 
> > > > errno ||
> > > > dpi <= 0)
> > > > {
> > > > dpi = 0.0;
> > > > dpi_output_name = argv[i];
> > 
> > 
> > Why spending so much effort ? 
> 
> Which "much effort"?
> 
> This is to ensure that incorrect param or garbage supplied by user is
> not interpreted as number.
> 
> > Using atoi(argv[i]) and checking for an invalid value seems fine.
> 
> atoi does not signal when garbage is on input. And dpi can be
> fractional, so atoi cannot be used for it.
> 
> > Special cases like NAN do not matter here.
> > 
> > BTW: leading whitespaces are ignored with strtod() (so far i know)
> 
> But strtod does not signal that they were ignored. Moreover trailing are
> not accepted. So I chose implementation which is consistent for trailing
> and leading whitespaces.

Any other review comments for this patch?

I would like to know if this patch could be applied or if there are some
other problems which needs to be resolved.

> > just my 2 cents
> > re,
> >  wh
> > 
> > 
> > 
> > > > @@ -3569,7 +3571,7 @@ main (int argc, char **argv)
> > > > XRROutputInfo   *output_info;
> > > > XRRModeInfo *mode_info;
> > > > if (!dpi_output)
> > > > -   fatal ("Cannot find output %s\n", dpi_output_name);
> > > > +   fatal ("%s is not valid DPI nor valid output\n", 
> > > > dpi_output_name);
> > > > output_info = dpi_output->output_info;
> > > > mode_info = dpi_output->mode_info;
> > > > if (output_info && mode_info && output_info->mm_height)
> > > 
> > > Hello, can you review this v2 patch?

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH v2] Fix checking for valid argument of --dpi

2018-12-30 Thread Pali Rohár

On Sunday 30 December 2018 13:51:10 Walter Harms wrote:
> 
> 
> > Pali Rohár  hat am 29. Dezember 2018 um 18:45
> > geschrieben:
> > 
> > 
> > On Monday 26 November 2018 22:17:24 Pali Rohár wrote:
> > > Function strtod() sets strtod_error to the pointer of the first invalid
> > > character and therefore it does not have to be first character from input.
> > > When input is valid then it points to nul byte. Conversion error is
> > > indicated by setted errno. Zero-length argument, non-positive DPI or DPI
> > > with trailing or leading whitespaces is invalid too.
> > > 
> > > Update also error message about invalid argument.
> > > 
> > > Signed-off-by: Pali Rohár 
> > > 
> > > --
> > > 
> > > Changes since v1:
> > > * Get same error message for `xrandr --dpi ''` and `xrandr --dpi ' '`
> > > * Make the check for dpi <= 0
> > > * Do not accept leading whitespaces (trailing were already disallowed)
> > > ---
> > >  xrandr.c | 6 --
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/xrandr.c b/xrandr.c
> > > index ce3cd91..4baa075 100644
> > > --- a/xrandr.c
> > > +++ b/xrandr.c
> > > @@ -40,6 +40,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  
> > >  #ifdef HAVE_CONFIG_H
> > >  #include "config.h"
> > > @@ -3118,8 +3119,9 @@ main (int argc, char **argv)
> > >   if (!strcmp ("--dpi", argv[i])) {
> > >   char *strtod_error;
> > >   if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> > > + errno = 0;
> > >   dpi = strtod(argv[i], _error);
> > > - if (argv[i] == strtod_error)
> > > + if (!argv[i][0] || isspace(argv[i][0]) || *strtod_error || errno ||
> > > dpi <= 0)
> > >   {
> > >   dpi = 0.0;
> > >   dpi_output_name = argv[i];
> 
> 
> Why spending so much effort ? 

Which "much effort"?

This is to ensure that incorrect param or garbage supplied by user is
not interpreted as number.

> Using atoi(argv[i]) and checking for an invalid value seems fine.

atoi does not signal when garbage is on input. And dpi can be
fractional, so atoi cannot be used for it.

> Special cases like NAN do not matter here.
> 
> BTW: leading whitespaces are ignored with strtod() (so far i know)

But strtod does not signal that they were ignored. Moreover trailing are
not accepted. So I chose implementation which is consistent for trailing
and leading whitespaces.

> just my 2 cents
> re,
>  wh
> 
> 
> 
> > > @@ -3569,7 +3571,7 @@ main (int argc, char **argv)
> > >   XRROutputInfo   *output_info;
> > >   XRRModeInfo *mode_info;
> > >   if (!dpi_output)
> > > - fatal ("Cannot find output %s\n", dpi_output_name);
> > > + fatal ("%s is not valid DPI nor valid output\n", 
> > > dpi_output_name);
> > >   output_info = dpi_output->output_info;
> > >   mode_info = dpi_output->mode_info;
> > >   if (output_info && mode_info && output_info->mm_height)
> > 
> > Hello, can you review this v2 patch?
> > 
> > -- 
> > Pali Rohár
> > pali.ro...@gmail.com
> > ___
> > 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

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature
___
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

Re: [PATCH v2] Fix checking for valid argument of --dpi

2018-12-29 Thread Pali Rohár

On Monday 26 November 2018 22:17:24 Pali Rohár wrote:
> Function strtod() sets strtod_error to the pointer of the first invalid
> character and therefore it does not have to be first character from input.
> When input is valid then it points to nul byte. Conversion error is
> indicated by setted errno. Zero-length argument, non-positive DPI or DPI
> with trailing or leading whitespaces is invalid too.
> 
> Update also error message about invalid argument.
> 
> Signed-off-by: Pali Rohár 
> 
> --
> 
> Changes since v1:
> * Get same error message for `xrandr --dpi ''` and `xrandr --dpi ' '`
> * Make the check for dpi <= 0
> * Do not accept leading whitespaces (trailing were already disallowed)
> ---
>  xrandr.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/xrandr.c b/xrandr.c
> index ce3cd91..4baa075 100644
> --- a/xrandr.c
> +++ b/xrandr.c
> @@ -40,6 +40,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #ifdef HAVE_CONFIG_H
>  #include "config.h"
> @@ -3118,8 +3119,9 @@ main (int argc, char **argv)
>   if (!strcmp ("--dpi", argv[i])) {
>   char *strtod_error;
>   if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> + errno = 0;
>   dpi = strtod(argv[i], _error);
> - if (argv[i] == strtod_error)
> + if (!argv[i][0] || isspace(argv[i][0]) || *strtod_error || errno || 
> dpi <= 0)
>   {
>   dpi = 0.0;
>   dpi_output_name = argv[i];
> @@ -3569,7 +3571,7 @@ main (int argc, char **argv)
>   XRROutputInfo   *output_info;
>   XRRModeInfo *mode_info;
>   if (!dpi_output)
> - fatal ("Cannot find output %s\n", dpi_output_name);
> + fatal ("%s is not valid DPI nor valid output\n", 
> dpi_output_name);
>   output_info = dpi_output->output_info;
>   mode_info = dpi_output->mode_info;
>   if (output_info && mode_info && output_info->mm_height)

Hello, can you review this v2 patch?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature
___
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

[PATCH v2] Fix checking for valid argument of --dpi

2018-11-26 Thread Pali Rohár

Function strtod() sets strtod_error to the pointer of the first invalid
character and therefore it does not have to be first character from input.
When input is valid then it points to nul byte. Conversion error is
indicated by setted errno. Zero-length argument, non-positive DPI or DPI
with trailing or leading whitespaces is invalid too.

Update also error message about invalid argument.

Signed-off-by: Pali Rohár 

--

Changes since v1:
* Get same error message for `xrandr --dpi ''` and `xrandr --dpi ' '`
* Make the check for dpi <= 0
* Do not accept leading whitespaces (trailing were already disallowed)
---
 xrandr.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xrandr.c b/xrandr.c
index ce3cd91..4baa075 100644
--- a/xrandr.c
+++ b/xrandr.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -3118,8 +3119,9 @@ main (int argc, char **argv)
if (!strcmp ("--dpi", argv[i])) {
char *strtod_error;
if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
+   errno = 0;
dpi = strtod(argv[i], _error);
-   if (argv[i] == strtod_error)
+   if (!argv[i][0] || isspace(argv[i][0]) || *strtod_error || errno || 
dpi <= 0)
{
dpi = 0.0;
dpi_output_name = argv[i];
@@ -3569,7 +3571,7 @@ main (int argc, char **argv)
XRROutputInfo   *output_info;
XRRModeInfo *mode_info;
if (!dpi_output)
-   fatal ("Cannot find output %s\n", dpi_output_name);
+   fatal ("%s is not valid DPI nor valid output\n", 
dpi_output_name);
output_info = dpi_output->output_info;
mode_info = dpi_output->mode_info;
if (output_info && mode_info && output_info->mm_height)
-- 
2.11.0

___
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

Re: [PATCH app/xrandr] Fix checking for valid argument of --dpi

2018-10-18 Thread Pali Rohár

On Thursday 18 October 2018 09:03:27 Giuseppe Bilotta wrote:
> Hello,
> 
> On Thu, Apr 12, 2018 at 8:53 PM Pali Rohár  wrote:
> > -   if (++i >= argc) argerr ("%s requires an argument\n", 
> > argv[i-1]);
> > +   if (++i >= argc || !argv[i][0]) argerr ("%s requires an 
> > argument\n", argv[i-1]);
> 
> I don't think this change is necessary, if arg[i][0] is NULL it means
> there _was_ an argument, but it was empty. Getting different error
> messages from `xrandr --dpi ''` and `xrandr --dpi ' '` doesn't seem
> like a good idea to me.

strtod() does not signal error for empty string argument:

  char *endptr = NULL;
  double ret;
  errno = 0;
  ret = strtod("", );
  printf("ret=%lf errno=%d end=%x\n", ret, errno, endptr[0]);

  ret=0.00 errno=0 end=0

But with explicit check for zero or negative return value, it is not
really needed.

> > +   errno = 0;
> > dpi = strtod(argv[i], _error);
> > -   if (argv[i] == strtod_error)
> > +   if (*strtod_error || errno || dpi == 0)
> 
> While we're at it, I would make the check for dpi <= 0, since negative
> values aren't valid either (in fact, negative values are effectively a
> no-op, since they set the DPI from the current framebuffer settings,
> and then set the virtual framebuffer physical dimensions from the
> DPI).
> 
> Cheers,
> 
> Giuseppe Bilotta

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH app/xdpyinfo v4] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-10-17 Thread Pali Rohár

Hello, can you review 4th version of this patch below?

On Monday 07 May 2018 23:34:22 Pali Rohár wrote:
> XServer with enabled XRANDR 1.2 extension does not provide correct
> dimensions from DisplayWidthMM() and DisplayHeightMM() calls anymore.
> Values are calculated from fixed DPI 96.
> 
> Therefore when XRANDR 1.2 extension is enabled, present and user requested
> for it, instead use XRRGetScreenResources() and XRRGetOutputInfo() calls to
> get correct dimensions and resolution information. Core dimensions from
> DisplayWidthMM() and DisplayHeightMM() are still reported.
> 
> Otherwise when XRANDR 1.2 extension is enabled, present and user did not
> request for it (which is default), show note that printed dimension does
> not have to be correct and instruct user to run xdpyinfo with -ext RANDR.
> 
> Also update manual page and add information that xdpyinfo does not provide
> correct information about DPI.
> 
> Signed-off-by: Pali Rohár 
> ---
> Changes since v3:
> * Always show core x screen output
> 
> Changes since v2:
> * Fixed dimensions calculation when rotation is active
> * Show XRANDR output only when explicitly requested
> * Update manpage
> 
> Changes since v1:
> * Fixed detection of presence of XRANDR 1.2
> * Fixed resolution calculation when dimensions are zero
> ---
>  Makefile.am  |   2 +
>  configure.ac |  12 ++
>  man/xdpyinfo.man |   7 +++
>  xdpyinfo.c   | 129 
> ++-
>  4 files changed, 140 insertions(+), 10 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 2f21dda..496094e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -35,6 +35,7 @@ AM_CFLAGS = \
>   $(DPY_XCOMPOSITE_CFLAGS) \
>   $(DPY_XINERAMA_CFLAGS) \
>   $(DPY_DMX_CFLAGS) \
> + $(DPY_XRANDR_CFLAGS) \
>   $(DPY_XTST_CFLAGS)
>  
>  xdpyinfo_LDADD = \
> @@ -49,6 +50,7 @@ xdpyinfo_LDADD = \
>   $(DPY_XCOMPOSITE_LIBS) \
>   $(DPY_XINERAMA_LIBS) \
>   $(DPY_DMX_LIBS) \
> + $(DPY_XRANDR_LIBS) \
>   $(DPY_XTST_LIBS)
>  
>  xdpyinfo_SOURCES =   \
> diff --git a/configure.ac b/configure.ac
> index 73dce26..4473faa 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -132,6 +132,18 @@ else
>   echo "without dmx"
>  fi
>  
> +AC_ARG_WITH(xrandr, AS_HELP_STRING([--without-xrandr],[Disable xrandr 1.2 
> support.]),
> + [USE_XRANDR="$withval"], [USE_XRANDR="yes"])
> +if test "x$USE_XRANDR" != "xno" ; then
> + PKG_CHECK_MODULES(DPY_XRANDR, xrandr >= 1.2,
> + [SAVE_CPPFLAGS="$CPPFLAGS"
> + CPPFLAGS="$CPPFLAGS $DPY_XRANDR_CFLAGS $DPY_X11_CFLAGS"
> + AC_CHECK_HEADERS([X11/extensions/Xrandr.h],,,[#include 
> ])
> + CPPFLAGS="$SAVE_CPPFLAGS"],[echo "not found"])
> +else
> + echo "without xrandr 1.2"
> +fi
> +
>  PKG_CHECK_MODULES(DPY_XTST, xtst,
>   [SAVE_CPPFLAGS="$CPPFLAGS"
>   CPPFLAGS="$CPPFLAGS $DPY_XTST_CFLAGS $DPY_X11_CFLAGS"
> diff --git a/man/xdpyinfo.man b/man/xdpyinfo.man
> index c3d5c6d..5df44ea 100644
> --- a/man/xdpyinfo.man
> +++ b/man/xdpyinfo.man
> @@ -51,6 +51,13 @@ Detailed information about a particular extension is 
> displayed with the
>  \fBall\fP, information about all extensions supported by both \fIxdpyinfo\fP
>  and the server is displayed.
>  .PP
> +Do not use this utility for determining dimensions of monitor when XRANDR 
> 1.2+
> +extension is enabled for X screen, because it does not provide them. For
> +determining physical dimensions or DPI of particular monitor use either
> +.IR xrandr (__appmansuffix__)
> +utility or call xdpyinfo with parameter \fB\-ext RANDR\fP (supported since
> +xdpyinfo version 1.3.3).
> +.PP
>  If \fB-version\fP is specified, xdpyinfo prints its version and exits, 
> without
>  contacting the X server.
>  .SH ENVIRONMENT
> diff --git a/xdpyinfo.c b/xdpyinfo.c
> index 152e32c..8d24b40 100644
> --- a/xdpyinfo.c
> +++ b/xdpyinfo.c
> @@ -76,6 +76,10 @@ in this Software without prior written authorization from 
> The Open Group.
>  #  define DMX
>  # endif
>  
> +# if HAVE_X11_EXTENSIONS_XRANDR_H
> +#  define XRANDR
> +# endif
> +
>  #endif
>  
>  #ifdef WIN32
> @@ -137,6 +141,9 @@ in this Software without prior written authorization from 
> The Open Group.
>  #ifdef DMX
>  #include 
>  #endif
> +#ifdef XRANDR
> +#include 
> +#endif
>  #include 
>  #include 
>  #include 
> @@ -442,6 +449,10 @@ print_visual_info(XVisualInfo *vip)
>   vip->bits_

Re: [PATCH app/xrandr] Fix checking for valid argument of --dpi

2018-10-17 Thread Pali Rohár

Gentle reminder for a patch which I sent 5 months ago...

On Monday 07 May 2018 23:38:10 Pali Rohár wrote:
> Hello, can you review my patch below?
> 
> On Thursday 12 April 2018 20:52:21 Pali Rohár wrote:
> > Function strtod() sets strtod_error to the pointer of the first invalid
> > character and therefore it does not have to be first character from input.
> > When input is valid then it points to nul byte. Conversion error is
> > indicated by setted errno. Zero-length argument and zero DPI is invalid
> > too.
> > 
> > Update also error message about invalid argument.
> > 
> > Signed-off-by: Pali Rohár 
> > ---
> >  xrandr.c | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xrandr.c b/xrandr.c
> > index 7f1e867..1960bbf 100644
> > --- a/xrandr.c
> > +++ b/xrandr.c
> > @@ -3115,9 +3115,10 @@ main (int argc, char **argv)
> > }
> > if (!strcmp ("--dpi", argv[i])) {
> > char *strtod_error;
> > -   if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> > +   if (++i >= argc || !argv[i][0]) argerr ("%s requires an 
> > argument\n", argv[i-1]);
> > +   errno = 0;
> > dpi = strtod(argv[i], _error);
> > -   if (argv[i] == strtod_error)
> > +   if (*strtod_error || errno || dpi == 0)
> > {
> > dpi = 0.0;
> > dpi_output_name = argv[i];
> > @@ -3567,7 +3568,7 @@ main (int argc, char **argv)
> > XRROutputInfo   *output_info;
> > XRRModeInfo *mode_info;
> > if (!dpi_output)
> > -   fatal ("Cannot find output %s\n", dpi_output_name);
> > +   fatal ("%s is not valid DPI nor valid output\n", 
> > dpi_output_name);
> > output_info = dpi_output->output_info;
> > mode_info = dpi_output->mode_info;
> > if (output_info && mode_info && output_info->mm_height)
> 

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature
___
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

Re: [PATCH app/xrandr] Fix checking for valid argument of --dpi

2018-05-07 Thread Pali Rohár

Hello, can you review my patch below?

On Thursday 12 April 2018 20:52:21 Pali Rohár wrote:
> Function strtod() sets strtod_error to the pointer of the first invalid
> character and therefore it does not have to be first character from input.
> When input is valid then it points to nul byte. Conversion error is
> indicated by setted errno. Zero-length argument and zero DPI is invalid
> too.
> 
> Update also error message about invalid argument.
> 
> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> ---
>  xrandr.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/xrandr.c b/xrandr.c
> index 7f1e867..1960bbf 100644
> --- a/xrandr.c
> +++ b/xrandr.c
> @@ -3115,9 +3115,10 @@ main (int argc, char **argv)
>   }
>   if (!strcmp ("--dpi", argv[i])) {
>   char *strtod_error;
> - if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
> + if (++i >= argc || !argv[i][0]) argerr ("%s requires an 
> argument\n", argv[i-1]);
> + errno = 0;
>   dpi = strtod(argv[i], _error);
> - if (argv[i] == strtod_error)
> + if (*strtod_error || errno || dpi == 0)
>   {
>   dpi = 0.0;
>   dpi_output_name = argv[i];
> @@ -3567,7 +3568,7 @@ main (int argc, char **argv)
>   XRROutputInfo   *output_info;
>   XRRModeInfo *mode_info;
>   if (!dpi_output)
> - fatal ("Cannot find output %s\n", dpi_output_name);
> + fatal ("%s is not valid DPI nor valid output\n", 
> dpi_output_name);
>   output_info = dpi_output->output_info;
>   mode_info = dpi_output->mode_info;
>   if (output_info && mode_info && output_info->mm_height)

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature
___
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

[PATCH app/xdpyinfo v4] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-05-07 Thread Pali Rohár

XServer with enabled XRANDR 1.2 extension does not provide correct
dimensions from DisplayWidthMM() and DisplayHeightMM() calls anymore.
Values are calculated from fixed DPI 96.

Therefore when XRANDR 1.2 extension is enabled, present and user requested
for it, instead use XRRGetScreenResources() and XRRGetOutputInfo() calls to
get correct dimensions and resolution information. Core dimensions from
DisplayWidthMM() and DisplayHeightMM() are still reported.

Otherwise when XRANDR 1.2 extension is enabled, present and user did not
request for it (which is default), show note that printed dimension does
not have to be correct and instruct user to run xdpyinfo with -ext RANDR.

Also update manual page and add information that xdpyinfo does not provide
correct information about DPI.

Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
Changes since v3:
* Always show core x screen output

Changes since v2:
* Fixed dimensions calculation when rotation is active
* Show XRANDR output only when explicitly requested
* Update manpage

Changes since v1:
* Fixed detection of presence of XRANDR 1.2
* Fixed resolution calculation when dimensions are zero
---
 Makefile.am  |   2 +
 configure.ac |  12 ++
 man/xdpyinfo.man |   7 +++
 xdpyinfo.c   | 129 ++-
 4 files changed, 140 insertions(+), 10 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 2f21dda..496094e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -35,6 +35,7 @@ AM_CFLAGS = \
$(DPY_XCOMPOSITE_CFLAGS) \
$(DPY_XINERAMA_CFLAGS) \
$(DPY_DMX_CFLAGS) \
+   $(DPY_XRANDR_CFLAGS) \
$(DPY_XTST_CFLAGS)
 
 xdpyinfo_LDADD = \
@@ -49,6 +50,7 @@ xdpyinfo_LDADD = \
$(DPY_XCOMPOSITE_LIBS) \
$(DPY_XINERAMA_LIBS) \
$(DPY_DMX_LIBS) \
+   $(DPY_XRANDR_LIBS) \
$(DPY_XTST_LIBS)
 
 xdpyinfo_SOURCES = \
diff --git a/configure.ac b/configure.ac
index 73dce26..4473faa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,6 +132,18 @@ else
echo "without dmx"
 fi
 
+AC_ARG_WITH(xrandr, AS_HELP_STRING([--without-xrandr],[Disable xrandr 1.2 
support.]),
+   [USE_XRANDR="$withval"], [USE_XRANDR="yes"])
+if test "x$USE_XRANDR" != "xno" ; then
+   PKG_CHECK_MODULES(DPY_XRANDR, xrandr >= 1.2,
+   [SAVE_CPPFLAGS="$CPPFLAGS"
+   CPPFLAGS="$CPPFLAGS $DPY_XRANDR_CFLAGS $DPY_X11_CFLAGS"
+   AC_CHECK_HEADERS([X11/extensions/Xrandr.h],,,[#include 
])
+   CPPFLAGS="$SAVE_CPPFLAGS"],[echo "not found"])
+else
+   echo "without xrandr 1.2"
+fi
+
 PKG_CHECK_MODULES(DPY_XTST, xtst,
[SAVE_CPPFLAGS="$CPPFLAGS"
CPPFLAGS="$CPPFLAGS $DPY_XTST_CFLAGS $DPY_X11_CFLAGS"
diff --git a/man/xdpyinfo.man b/man/xdpyinfo.man
index c3d5c6d..5df44ea 100644
--- a/man/xdpyinfo.man
+++ b/man/xdpyinfo.man
@@ -51,6 +51,13 @@ Detailed information about a particular extension is 
displayed with the
 \fBall\fP, information about all extensions supported by both \fIxdpyinfo\fP
 and the server is displayed.
 .PP
+Do not use this utility for determining dimensions of monitor when XRANDR 1.2+
+extension is enabled for X screen, because it does not provide them. For
+determining physical dimensions or DPI of particular monitor use either
+.IR xrandr (__appmansuffix__)
+utility or call xdpyinfo with parameter \fB\-ext RANDR\fP (supported since
+xdpyinfo version 1.3.3).
+.PP
 If \fB-version\fP is specified, xdpyinfo prints its version and exits, without
 contacting the X server.
 .SH ENVIRONMENT
diff --git a/xdpyinfo.c b/xdpyinfo.c
index 152e32c..8d24b40 100644
--- a/xdpyinfo.c
+++ b/xdpyinfo.c
@@ -76,6 +76,10 @@ in this Software without prior written authorization from 
The Open Group.
 #  define DMX
 # endif
 
+# if HAVE_X11_EXTENSIONS_XRANDR_H
+#  define XRANDR
+# endif
+
 #endif
 
 #ifdef WIN32
@@ -137,6 +141,9 @@ in this Software without prior written authorization from 
The Open Group.
 #ifdef DMX
 #include 
 #endif
+#ifdef XRANDR
+#include 
+#endif
 #include 
 #include 
 #include 
@@ -442,6 +449,10 @@ print_visual_info(XVisualInfo *vip)
vip->bits_per_rgb);
 }
 
+#ifdef XRANDR
+static Bool print_xrandr = False;
+#endif
+
 static void
 print_screen_info(Display *dpy, int scr)
 {
@@ -455,6 +466,14 @@ print_screen_info(Display *dpy, int scr)
 double xres, yres;
 int ndepths = 0, *depths = NULL;
 unsigned int width, height;
+Bool has_xrandr = False;
+#ifdef XRANDR
+int event_base, error_base;
+int major, minor;
+XRRScreenResources *res;
+XRROutputInfo *output;
+XRRCrtcInfo *crtc;
+#endif
 
 /*
  * there are 2.54 centimeters to an inch; so there are 25.4 millimeters.
@@ -464,18 +483,93 @@ print_screen_info(Display *dpy, int scr)
  * = N * 25.4 pixels / M inch
  */
 
-xres = double) Disp

Re: [PATCH app/xdpyinfo v3] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-05-07 Thread Pali Rohár

On Wednesday 18 April 2018 15:45:20 Giuseppe Bilotta wrote:
> On Wed, Apr 18, 2018 at 3:33 PM, Pali Rohár <pali.ro...@gmail.com> wrote:
> > On Thursday 12 April 2018 16:34:15 Adam Jackson wrote:
> >> This should print the RANDR data in a separate stanza after the main
> >> output, like the other extensions do. Again: the purpose of the core of
> >> xdpyinfo is to tell you what the connection block says. Don't make it
> >> print something else.
> >
> > This patch does not change anything in the output when command line
> > option for RANDR is not used. Therefore you would get same output as
> > before (without applying patch).
> >
> > And when RANDR is explicitly requested then it outputs correct dimension
> > information. Yes, it hides what is reported by connection block, but the
> > main problem is that this tools is not already used like you said. Users
> > and also scripts expects that they would get correct monitor/output
> > dimension from xdpyinfo and not something which do not match with their
> > physical monitor device.
> >
> > As Giuseppe said, this seems like a good compromise. When no parameter
> > is specified then xdpyinfo reports exactly same data as without this
> > patch. And with this patch which adds support for optional RANDR
> > parameter, then it reports dimensions for each monitor/output correctly.
> > So users would see what they are already expecting and want.
> 
> 
> No, in the RANDR case you are still replacing the core output, which
> is not what I suggested. Instead, the RANDR information should be
> provided _separately_ and _in addition to_ the core output. So instead
> of defining a useless print_none_info, put the RANDR code in
> print_randr_info and add _that_ to the known_extensions array.

Ok. I will put both core and randr information into output. But still I
think that those dimension information should be at one place and not
separated to different parts. It is also hard to find them when reading
output or parse.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature
___
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

Re: [PATCH app/xdpyinfo v3] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-04-18 Thread Pali Rohár

On Thursday 12 April 2018 16:34:15 Adam Jackson wrote:
> This should print the RANDR data in a separate stanza after the main
> output, like the other extensions do. Again: the purpose of the core of
> xdpyinfo is to tell you what the connection block says. Don't make it
> print something else.

This patch does not change anything in the output when command line
option for RANDR is not used. Therefore you would get same output as
before (without applying patch).

And when RANDR is explicitly requested then it outputs correct dimension
information. Yes, it hides what is reported by connection block, but the
main problem is that this tools is not already used like you said. Users
and also scripts expects that they would get correct monitor/output
dimension from xdpyinfo and not something which do not match with their
physical monitor device.

As Giuseppe said, this seems like a good compromise. When no parameter
is specified then xdpyinfo reports exactly same data as without this
patch. And with this patch which adds support for optional RANDR
parameter, then it reports dimensions for each monitor/output correctly.
So users would see what they are already expecting and want.

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

[PATCH app/xdpyinfo v3] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-04-12 Thread Pali Rohár

XServer with enabled XRANDR 1.2 extension does not provide correct
dimensions from DisplayWidthMM() and DisplayHeightMM() calls anymore.
Values are calculated from fixed DPI 96.

Therefore when XRANDR 1.2 extension is enabled, present and user requested
for it, instead use XRRGetScreenResources() and XRRGetOutputInfo() calls to
get correct dimensions and resolution information.

Otherwise when XRANDR 1.2 extension is enabled, present and user did not
request for it (which is default), show note that printed dimension does
not have to be correct and instruct user to run xdpyinfo with -ext RANDR.

Also update manual page and add information that xdpyinfo does not provide
correct information about DPI.

Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
Changes since v2:
* Fixed dimensions calculation when rotation is active
* Show XRANDR output only when explicitly requested
* Update manpage

Changes since v1:
* Fixed detection of presence of XRANDR 1.2
* Fixed resolution calculation when dimensions are zero
---
 Makefile.am  |   2 +
 configure.ac |  12 ++
 man/xdpyinfo.man |   7 
 xdpyinfo.c   | 117 ++-
 4 files changed, 128 insertions(+), 10 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 2f21dda..496094e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -35,6 +35,7 @@ AM_CFLAGS = \
$(DPY_XCOMPOSITE_CFLAGS) \
$(DPY_XINERAMA_CFLAGS) \
$(DPY_DMX_CFLAGS) \
+   $(DPY_XRANDR_CFLAGS) \
$(DPY_XTST_CFLAGS)
 
 xdpyinfo_LDADD = \
@@ -49,6 +50,7 @@ xdpyinfo_LDADD = \
$(DPY_XCOMPOSITE_LIBS) \
$(DPY_XINERAMA_LIBS) \
$(DPY_DMX_LIBS) \
+   $(DPY_XRANDR_LIBS) \
$(DPY_XTST_LIBS)
 
 xdpyinfo_SOURCES = \
diff --git a/configure.ac b/configure.ac
index 73dce26..4473faa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,6 +132,18 @@ else
echo "without dmx"
 fi
 
+AC_ARG_WITH(xrandr, AS_HELP_STRING([--without-xrandr],[Disable xrandr 1.2 
support.]),
+   [USE_XRANDR="$withval"], [USE_XRANDR="yes"])
+if test "x$USE_XRANDR" != "xno" ; then
+   PKG_CHECK_MODULES(DPY_XRANDR, xrandr >= 1.2,
+   [SAVE_CPPFLAGS="$CPPFLAGS"
+   CPPFLAGS="$CPPFLAGS $DPY_XRANDR_CFLAGS $DPY_X11_CFLAGS"
+   AC_CHECK_HEADERS([X11/extensions/Xrandr.h],,,[#include 
])
+   CPPFLAGS="$SAVE_CPPFLAGS"],[echo "not found"])
+else
+   echo "without xrandr 1.2"
+fi
+
 PKG_CHECK_MODULES(DPY_XTST, xtst,
[SAVE_CPPFLAGS="$CPPFLAGS"
CPPFLAGS="$CPPFLAGS $DPY_XTST_CFLAGS $DPY_X11_CFLAGS"
diff --git a/man/xdpyinfo.man b/man/xdpyinfo.man
index c3d5c6d..5df44ea 100644
--- a/man/xdpyinfo.man
+++ b/man/xdpyinfo.man
@@ -51,6 +51,13 @@ Detailed information about a particular extension is 
displayed with the
 \fBall\fP, information about all extensions supported by both \fIxdpyinfo\fP
 and the server is displayed.
 .PP
+Do not use this utility for determining dimensions of monitor when XRANDR 1.2+
+extension is enabled for X screen, because it does not provide them. For
+determining physical dimensions or DPI of particular monitor use either
+.IR xrandr (__appmansuffix__)
+utility or call xdpyinfo with parameter \fB\-ext RANDR\fP (supported since
+xdpyinfo version 1.3.3).
+.PP
 If \fB-version\fP is specified, xdpyinfo prints its version and exits, without
 contacting the X server.
 .SH ENVIRONMENT
diff --git a/xdpyinfo.c b/xdpyinfo.c
index 152e32c..ac2526f 100644
--- a/xdpyinfo.c
+++ b/xdpyinfo.c
@@ -76,6 +76,10 @@ in this Software without prior written authorization from 
The Open Group.
 #  define DMX
 # endif
 
+# if HAVE_X11_EXTENSIONS_XRANDR_H
+#  define XRANDR
+# endif
+
 #endif
 
 #ifdef WIN32
@@ -137,6 +141,9 @@ in this Software without prior written authorization from 
The Open Group.
 #ifdef DMX
 #include 
 #endif
+#ifdef XRANDR
+#include 
+#endif
 #include 
 #include 
 #include 
@@ -442,6 +449,10 @@ print_visual_info(XVisualInfo *vip)
vip->bits_per_rgb);
 }
 
+#ifdef XRANDR
+static Bool print_xrandr = False;
+#endif
+
 static void
 print_screen_info(Display *dpy, int scr)
 {
@@ -455,6 +466,14 @@ print_screen_info(Display *dpy, int scr)
 double xres, yres;
 int ndepths = 0, *depths = NULL;
 unsigned int width, height;
+Bool has_xrandr = False;
+#ifdef XRANDR
+int event_base, error_base;
+int major, minor;
+XRRScreenResources *res;
+XRROutputInfo *output;
+XRRCrtcInfo *crtc;
+#endif
 
 /*
  * there are 2.54 centimeters to an inch; so there are 25.4 millimeters.
@@ -464,18 +483,81 @@ print_screen_info(Display *dpy, int scr)
  * = N * 25.4 pixels / M inch
  */
 
-xres = double) DisplayWidth(dpy,scr)) * 25.4) /
-   ((double) DisplayWidthMM(dpy,scr)));
-yres = double) DisplayHeight(dpy,scr)) * 25.4) /
-

[PATCH app/xrandr] Fix checking for valid argument of --dpi

2018-04-12 Thread Pali Rohár

Function strtod() sets strtod_error to the pointer of the first invalid
character and therefore it does not have to be first character from input.
When input is valid then it points to nul byte. Conversion error is
indicated by setted errno. Zero-length argument and zero DPI is invalid
too.

Update also error message about invalid argument.

Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
 xrandr.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/xrandr.c b/xrandr.c
index 7f1e867..1960bbf 100644
--- a/xrandr.c
+++ b/xrandr.c
@@ -3115,9 +3115,10 @@ main (int argc, char **argv)
}
if (!strcmp ("--dpi", argv[i])) {
char *strtod_error;
-   if (++i >= argc) argerr ("%s requires an argument\n", argv[i-1]);
+   if (++i >= argc || !argv[i][0]) argerr ("%s requires an 
argument\n", argv[i-1]);
+   errno = 0;
dpi = strtod(argv[i], _error);
-   if (argv[i] == strtod_error)
+   if (*strtod_error || errno || dpi == 0)
{
dpi = 0.0;
dpi_output_name = argv[i];
@@ -3567,7 +3568,7 @@ main (int argc, char **argv)
XRROutputInfo   *output_info;
XRRModeInfo *mode_info;
if (!dpi_output)
-   fatal ("Cannot find output %s\n", dpi_output_name);
+   fatal ("%s is not valid DPI nor valid output\n", 
dpi_output_name);
output_info = dpi_output->output_info;
mode_info = dpi_output->mode_info;
if (output_info && mode_info && output_info->mm_height)
-- 
2.11.0

___
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

Re: [PATCH app/xdpyinfo v2] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-04-08 Thread Pali Rohár

On Sunday 08 April 2018 19:22:28 Giuseppe Bilotta wrote:
> On Sun, Apr 8, 2018 at 4:33 PM, Pali Rohár <pali.ro...@gmail.com> wrote:
> > The main problem is that majority of users use xdpyinfo for getting DPI
> > of their monitors.
> 
> And in the case of multiple monitors, they'll have to get used to
> using `xdpyinfo -ext RANDR`, provided support for that information is
> offered this way. I think that's a good compromise between backwards
> compatibility and providing the correct information.
> 
> > And xdpyinfo reports totally bogus values.
> 
> The values reported by xdpyinfo aren't bogus, they are what the core
> protocol is providing.

For users as human readable output from xdpyinfo, those values are
bogus. For users it does not matter how xdpyinfo obtain those values.
Just that it provides values which do not match reality.

I understand that those values comes from X server and xdpyinfo just
print what it receive. But for end-users of xdpyinfo this is really
irrelevant. That tool worked correctly prior changes in X server (do not
remember version).

> > There are plenty of bugs and lot of reports about this problem.
> 
> Because people are using the wrong tool.

I agree, but you can look at it from other side. This tool worked with
older X servers. If it is stopped working with new X servers, then
either tool itself should write information about it or do not report
those values at all.

Providing wrong information without any warning either in --help,
manpage or in stdout is really the worst solution.

> > Really what is the purpose of reporting bogus values?
> 
> As mentioned, the purpose of xdpyinfo is to report what the core
> protocol reports (modulo use of the -ext flag and related ones).

The main problem is that this is not documented, nor written anywhere.
And output of xdpyinfo does not looks like core information for
end-users.

What end user reads? He sees resolution (which for with the most common
variant for one monitor) matches what he has configured and the he sees
DPI which does not match his monitor. So it is fully bogus for him.

> Now why does core report bogus values by default?

I understand reasons, for multimontitor setups with different DPIs
function DisplayWidthMM() and DisplayHeightMM() report meaningless
values (or at least values which should not be used for anything in
"correctly" written new applications).

> The root of the issue is that in the case of multiple monitors with
> potentially inconsistent DPI values, the core protocol value is
> ambiguous at best. It also has the downside that its value is only
> communicated at connection time, so it doesn't dynamically change even
> when the circumstances change (e.g. resolution change, move to a
> different output with a different DPI, etc). Clients need to be aware
> of the possibilities that different outputs may have different DPI
> values, and that the values can change, and that requires RANDR
> support and listening to the appropriate change notification masks.

Agree.

> So it is important that xdpyinfo keeps reporting what is reporting
> because (1) it's its purpose and (2) it's the only way to get what
> legacy (non-RANDR-aware) clients get.

Ok, it makes sense to retrieve this information, but for sure it should
not be used by new applications, scripts and also users to retrieve DPI.

But main problem is that xdpyinfo does not look like for end-users that
it reports "legacy" values which end-users should not read for xrandr
1.2+ display.

> Now one could argue that in the case of single output X11 could
> automatically set the DPI to the one of the only connected output
> (something I actually agree with), but that's (a) a separate issue and
> (b) not without its downsides (e.g. should it automatically change
> whenever the output changes? what should be done when a new output
> gets connected? what should be done when an output gets disconnected
> and we're left with one output again? etc).

Those are separate issues, which are really out-of-scope of this
discussion. Personally I like idea that DisplayWidthMM() and
DisplayHeightMM() are calculated to 96 DPI as 96 DPI is sane default for
legacy applications. And special case for one monitor setup is bad
because it would change behavior of applications when more then one
monitor is connected.

-- 
Pali Rohár
pali.ro...@gmail.com

signature.asc
Description: PGP signature
___
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

Re: [PATCH app/xdpyinfo v2] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-04-08 Thread Pali Rohár

On Wednesday 04 April 2018 11:12:53 Adam Jackson wrote:
> code exists that depends on xdpyinfo's output.

Exactly and that code expects that xdpyinfo provides correct output. But
it is broken since XServer for RandR 1.2+ capable screens started
reporting fixed values.

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature
___
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

Re: [PATCH app/xdpyinfo v2] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-04-08 Thread Pali Rohár

On Wednesday 04 April 2018 16:00:39 Adam Jackson wrote:
> On Wed, 2018-04-04 at 21:30 +0200, Giuseppe Bilotta wrote:
> > On Wed, Apr 4, 2018 at 5:12 PM, Adam Jackson <a...@nwnk.net> wrote:
> > > On Tue, 2018-04-03 at 22:15 +0200, Pali Rohár wrote:
> > > > Gently reminder of this patch. Can you comment/review it?
> > > 
> > > Nack. The whole point of (that part of) xdpyinfo is to show you what
> > > was sent in the initial connection block. xrandr already exists, and
> > > code exists that depends on xdpyinfo's output.
> > 
> > Would it make sense to add the output from RANDR via an explicit `-ext
> > RANDR` query similar to what is already done for XINERAMA,
> > XInputExtension, etc?
> 
> If it's going to be in xdpyinfo at all - and I kinda don't think it
> needs to be, given xrandr already exists, but I don't insist on that -
> then yes, it should be behind -ext RANDR.
> 
> - ajax

The main problem is that majority of users use xdpyinfo for getting DPI
of their monitors. And xdpyinfo reports totally bogus values. There are
plenty of bugs and lot of reports about this problem.

Really what is the purpose of reporting bogus values?

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: PGP signature
___
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

Re: [PATCH app/xdpyinfo v2] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-04-04 Thread Pali Rohár

On Wednesday 04 April 2018 13:59:15 Giuseppe Bilotta wrote:
> Hello,
> 
> I took the liberty of moving the last paragraph of your email to the
> top because the reply I'm giving to that part covers both.
> 
> On Wed, Apr 4, 2018 at 11:26 AM, Pali Rohár <pali.ro...@gmail.com> wrote:
> >> >> -printf ("  dimensions:%dx%d pixels (%dx%d millimeters)\n",
> >> >> - XDisplayWidth (dpy, scr),  XDisplayHeight (dpy, scr),
> >> >> - XDisplayWidthMM(dpy, scr), XDisplayHeightMM (dpy, scr));
> >> >> -printf ("  resolution:%dx%d dots per inch\n",
> >> >> - (int) (xres + 0.5), (int) (yres + 0.5));
> >>
> >> I would unconditionally show the core output, regardless of RANDR
> >> state. Even if it's fictitious when RANDR is enabled,
> >> it can still be useful to spot inconsistencies. It also ensures that
> >> the xdpyinfo output is "less" broken by this patch (search for
> >> xdpyinfo grep resolution to get an idea of how used this is as a debug 
> >> tool).
> >
> > XRANDR code below provides that 'grep resolution' support correctly and
> > in the same format. It is there for all those scripts which expects
> > resolution and dimensions to be correct.
> 
> But that's a different piece of information. You mention:
> 
> > This patch is just to fix long standing bug, that scripts which parse
> > xdpyinfo output and users who looking at it too just get wrong
> > information about dimensions and DPI.
> 
> For users, having both pieces of information would be better than
> having just the
> RANDR one, especially for debugging, as that's exactly what the server
> is providing
> via its core protocol, and what legacy (non-RANDR-aware) clients will
> get and use.

Agree.

> For scripts, the situation is a bit hairier, but I wonder how such a
> script would work on a
> server with multiple active outputs, where you patch introduces
> multiple “resolution”
> lines anyway.

Multiple resolution lines are there already. Just enable more Xscreens.
Such setups were common in past for dual head or for Xinerama
configurations. Old script needs to be aware of it, otherwise there were
broken in past too.

> If you think it's important for the primary RANDR output information
> to be first,
> maybe you can move the core information to _after_ the RANDR information,
> but I still think it's important enough that it should stick around.
> 
> (And of course it would be better to fix the scripts themselves, but I
> assume not all of them are
> open source.)

I have no idea, but I was told that more people do (or did?)
'xdpyinfo | grep resolution' optionally with '| head -1' to get current
display DPI.

But I have no problem to modify output of xdpyinfo to provide:

1. Information from XRANDR1.2+
   for all outputs, primary first
2. Information from core

> >> >> +
> >> >> +#ifdef XRANDR
> >> >> +if (XRRQueryExtension (dpy, _base, _base) &&
> >> >> +XRRQueryVersion (dpy, , ) &&
> >> >> +(major > 1 || (major == 1 && minor >= 2)) &&
> >> >> +(res = XRRGetScreenResources (dpy, RootWindow (dpy, scr
> >> >> +{
> >>
> >> I'm pondering whether it would be a good idea to print the RANDR
> >> version here, something like:
> >> printf("  RANDR (%d.%d) outputs:\n", major, minor);
> >> and then nesting the per-output data even more.
> >
> > I as a user, would expect to find XRANDR version in "xrandr" utility
> > output, not in xdpyinfo output.
> >
> > But if you really think that it would make sense to have it also in
> > xdpyinfo, it can be included somewhere...
> 
> I'm not sure if it'd be useful or not, really. We could do without.
> (I do show it in my xdpi <https://github.com/Oblomov/xdpi/>, but as I said,
> I'm an information junkie).
> 
> (BTW, I don't think xrandr actually prints the version).

Seems, yes xrandr output does not contain it.

> >> This doesn't work correctly when the display is rotated, since the
> >> width/height in pixel will follow the orientation,
> >> but the physical sizes won't. You should probably take that into
> >> account, and possibly show the output orientation (e.g. in the
> >> dimensions line, or in the name line if you do add the "RANDR
> >>  outputs" header).
> >
> > Hm... I have not thinking about it. If this is a real problem I can look
> > at it and try to fix it.
> 
> Here's how it looks for a

Re: [PATCH app/xdpyinfo v2] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-04-04 Thread Pali Rohár

Hi!

On Wednesday 04 April 2018 09:47:59 Giuseppe Bilotta wrote:
> Hello Pali,
> 
> I like the idea of this patch, wasn't aware of its existence, thanks
> for pinging me about it. A few comments below:
> 
> On Tue, Apr 3, 2018 at 10:15 PM, Pali Rohár <pali.ro...@gmail.com> wrote:
> >> @@ -455,27 +462,75 @@ print_screen_info(Display *dpy, int scr)
> >>  double xres, yres;
> >>  int ndepths = 0, *depths = NULL;
> >>  unsigned int width, height;
> >> -
> >> -/*
> >> - * there are 2.54 centimeters to an inch; so there are 25.4 
> >> millimeters.
> >> - *
> >> - * dpi = N pixels / (M millimeters / (25.4 millimeters / 1 inch))
> >> - * = N pixels / (M inch / 25.4)
> >> - * = N * 25.4 pixels / M inch
> >> - */
> 
> You may want to keep this comment here, rather than in the else block below,
> since the formula applies to any conversion, regardless if it's core
> or per-output DPI.

Ok, no problem.

> >> -xres = double) DisplayWidth(dpy,scr)) * 25.4) /
> >> - ((double) DisplayWidthMM(dpy,scr)));
> >> -yres = double) DisplayHeight(dpy,scr)) * 25.4) /
> >> - ((double) DisplayHeightMM(dpy,scr)));
> >> +#ifdef XRANDR
> >> +int event_base, error_base;
> >> +int major, minor;
> >> +XRRScreenResources *res = NULL;
> >> +XRROutputInfo *output;
> >> +XRRCrtcInfo *crtc;
> >> +#endif
> >>
> >>  printf ("\n");
> >>  printf ("screen #%d:\n", scr);
> >> -printf ("  dimensions:%dx%d pixels (%dx%d millimeters)\n",
> >> - XDisplayWidth (dpy, scr),  XDisplayHeight (dpy, scr),
> >> - XDisplayWidthMM(dpy, scr), XDisplayHeightMM (dpy, scr));
> >> -printf ("  resolution:%dx%d dots per inch\n",
> >> - (int) (xres + 0.5), (int) (yres + 0.5));
> 
> I would unconditionally show the core output, regardless of RANDR
> state. Even if it's fictitious when RANDR is enabled,
> it can still be useful to spot inconsistencies. It also ensures that
> the xdpyinfo output is "less" broken by this patch (search for
> xdpyinfo grep resolution to get an idea of how used this is as a debug tool).

XRANDR code below provides that 'grep resolution' support correctly and
in the same format. It is there for all those scripts which expects
resolution and dimensions to be correct.

> >> +
> >> +#ifdef XRANDR
> >> +if (XRRQueryExtension (dpy, _base, _base) &&
> >> +XRRQueryVersion (dpy, , ) &&
> >> +(major > 1 || (major == 1 && minor >= 2)) &&
> >> +(res = XRRGetScreenResources (dpy, RootWindow (dpy, scr
> >> +{
> 
> I'm pondering whether it would be a good idea to print the RANDR
> version here, something like:
> printf("  RANDR (%d.%d) outputs:\n", major, minor);
> and then nesting the per-output data even more.

I as a user, would expect to find XRANDR version in "xrandr" utility
output, not in xdpyinfo output.

But if you really think that it would make sense to have it also in
xdpyinfo, it can be included somewhere...

> >> +for (i = 0; i < res->noutput; ++i) {
> >> +output = XRRGetOutputInfo (dpy, res, res->outputs[i]);
> >> +if (!output || !output->crtc || output->connection != 
> >> RR_Connected)
> >> +continue;
> >> +
> >> +crtc = XRRGetCrtcInfo (dpy, res, output->crtc);
> >> +if (!crtc) {
> >> +XRRFreeOutputInfo (output);
> >> +continue;
> >> +}
> >> +
> >> +printf ("  output: %s\n", output->name);
> >> +printf ("dimensions:%ux%u pixels (%lux%lu 
> >> millimeters)\n",
> >> +crtc->width, crtc->height, output->mm_width, 
> >> output->mm_height);
> >> +
> >> +if (output->mm_width && output->mm_height) {
> >> +xres = double) crtc->width) * 25.4) / ((double) 
> >> output->mm_width));
> >> +yres = double) crtc->height) * 25.4) / ((double) 
> >> output->mm_height));
> >> +} else {
> >> +xres = 0;
> >> +yres = 0;
> >> +}
> >> +printf ("

Re: [PATCH app/xdpyinfo v2] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-04-03 Thread Pali Rohár

Gently reminder of this patch. Can you comment/review it?

On Tuesday 16 May 2017 22:04:57 Pali Rohár wrote:
> XServer with enabled XRANDR 1.2 extension does not provide correct
> dimensions from DisplayWidthMM() and DisplayHeightMM() calls anymore.
> Values are calculated from fixed DPI 96.
> 
> Therefore when XRANDR 1.2 extension is enabled and present, instead use
> XRRGetScreenResources() and XRRGetOutputInfo() calls to get correct
> dimensions and resolution information.
> 
> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> ---
> Changes since v1:
> * Fixed detection of presence of XRANDR 1.2
> * Fixed resolution calculation when dimensions are zero
> ---
>  Makefile.am  |2 ++
>  configure.ac |   12 
>  xdpyinfo.c   |   91 
> ++
>  3 files changed, 87 insertions(+), 18 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 2f21dda..496094e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -35,6 +35,7 @@ AM_CFLAGS = \
>   $(DPY_XCOMPOSITE_CFLAGS) \
>   $(DPY_XINERAMA_CFLAGS) \
>   $(DPY_DMX_CFLAGS) \
> + $(DPY_XRANDR_CFLAGS) \
>   $(DPY_XTST_CFLAGS)
>  
>  xdpyinfo_LDADD = \
> @@ -49,6 +50,7 @@ xdpyinfo_LDADD = \
>   $(DPY_XCOMPOSITE_LIBS) \
>   $(DPY_XINERAMA_LIBS) \
>   $(DPY_DMX_LIBS) \
> + $(DPY_XRANDR_LIBS) \
>   $(DPY_XTST_LIBS)
>  
>  xdpyinfo_SOURCES =   \
> diff --git a/configure.ac b/configure.ac
> index 73dce26..4473faa 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -132,6 +132,18 @@ else
>   echo "without dmx"
>  fi
>  
> +AC_ARG_WITH(xrandr, AS_HELP_STRING([--without-xrandr],[Disable xrandr 1.2 
> support.]),
> + [USE_XRANDR="$withval"], [USE_XRANDR="yes"])
> +if test "x$USE_XRANDR" != "xno" ; then
> + PKG_CHECK_MODULES(DPY_XRANDR, xrandr >= 1.2,
> + [SAVE_CPPFLAGS="$CPPFLAGS"
> + CPPFLAGS="$CPPFLAGS $DPY_XRANDR_CFLAGS $DPY_X11_CFLAGS"
> + AC_CHECK_HEADERS([X11/extensions/Xrandr.h],,,[#include 
> ])
> + CPPFLAGS="$SAVE_CPPFLAGS"],[echo "not found"])
> +else
> + echo "without xrandr 1.2"
> +fi
> +
>  PKG_CHECK_MODULES(DPY_XTST, xtst,
>   [SAVE_CPPFLAGS="$CPPFLAGS"
>   CPPFLAGS="$CPPFLAGS $DPY_XTST_CFLAGS $DPY_X11_CFLAGS"
> diff --git a/xdpyinfo.c b/xdpyinfo.c
> index 152e32c..409968c 100644
> --- a/xdpyinfo.c
> +++ b/xdpyinfo.c
> @@ -76,6 +76,10 @@ in this Software without prior written authorization from 
> The Open Group.
>  #  define DMX
>  # endif
>  
> +# if HAVE_X11_EXTENSIONS_XRANDR_H
> +#  define XRANDR
> +# endif
> +
>  #endif
>  
>  #ifdef WIN32
> @@ -137,6 +141,9 @@ in this Software without prior written authorization from 
> The Open Group.
>  #ifdef DMX
>  #include 
>  #endif
> +#ifdef XRANDR
> +#include 
> +#endif
>  #include 
>  #include 
>  #include 
> @@ -455,27 +462,75 @@ print_screen_info(Display *dpy, int scr)
>  double xres, yres;
>  int ndepths = 0, *depths = NULL;
>  unsigned int width, height;
> -
> -/*
> - * there are 2.54 centimeters to an inch; so there are 25.4 millimeters.
> - *
> - * dpi = N pixels / (M millimeters / (25.4 millimeters / 1 inch))
> - * = N pixels / (M inch / 25.4)
> - * = N * 25.4 pixels / M inch
> - */
> -
> -xres = double) DisplayWidth(dpy,scr)) * 25.4) /
> - ((double) DisplayWidthMM(dpy,scr)));
> -yres = double) DisplayHeight(dpy,scr)) * 25.4) /
> - ((double) DisplayHeightMM(dpy,scr)));
> +#ifdef XRANDR
> +int event_base, error_base;
> +int major, minor;
> +XRRScreenResources *res = NULL;
> +XRROutputInfo *output;
> +XRRCrtcInfo *crtc;
> +#endif
>  
>  printf ("\n");
>  printf ("screen #%d:\n", scr);
> -printf ("  dimensions:%dx%d pixels (%dx%d millimeters)\n",
> - XDisplayWidth (dpy, scr),  XDisplayHeight (dpy, scr),
> - XDisplayWidthMM(dpy, scr), XDisplayHeightMM (dpy, scr));
> -printf ("  resolution:%dx%d dots per inch\n",
> - (int) (xres + 0.5), (int) (yres + 0.5));
> +
> +#ifdef XRANDR
> +if (XRRQueryExtension (dpy, _base, _base) &&
> +XRRQueryVersion (dpy, , ) &&
> +(major > 1 || (major == 1 && minor >= 2)) &&
> +(res = XRRGetScreenResources (dpy, RootWindow (dpy, scr
> +{
> +for (i = 0; i < res->noutput; ++i)

Re: [PATCH v3 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2018-03-23 Thread Pali Rohár

On Monday 12 March 2018 09:19:57 Giuseppe Bilotta wrote:
> On Sat, Mar 10, 2018 at 4:23 PM, Pali Rohár <pali.ro...@gmail.com> wrote:
> > Explicitly document and make it clear that those options do not change
> > DPI of some monitor output. Also state that these options have no useful
> > meaning for multi-monitor configuration.
> >
> > Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> 
> FWIW,
> 
> Reviewed-by: Giuseppe Bilotta <giuseppe.bilo...@gmail.com>

Ok, can you merge this patch?

> > ---
> >  man/xrandr.man | 19 +++
> >  1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/man/xrandr.man b/man/xrandr.man
> > index aa82724..1291bad 100644
> > --- a/man/xrandr.man
> > +++ b/man/xrandr.man
> > @@ -257,14 +257,25 @@ fit within this size. When this option is not 
> > provided, xrandr computes the
> >  smallest screen size that will hold the set of configured outputs; this
> >  option provides a way to override that behaviour.
> >  .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
> > -Sets the reported values for the physical size of the screen. Normally,
> > +Sets the value reported as physical size of the X screen as a whole
> > +(union of all configured monitors). In configurations with multiple
> > +monitors with different DPIs, the value has no physical meaning, but
> > +it may be used by some legacy clients which do not support RandR
> > +version 1.2 to compute a reference font scaling. Normally,
> >  xrandr resets the reported physical size values to keep the DPI constant.
> > -This overrides that computation.
> > +This overrides that computation. Default DPI value is 96.
> >  .IP "\-\-dpi \fIdpi\fP"
> >  .IP "\-\-dpi \fIfrom-output\fP"
> > -This also sets the reported physical size values of the screen, it uses 
> > either
> > +This also sets the value reported as physical size of the X screen as a 
> > whole
> > +(union of all configured monitors). In configurations with multiple
> > +monitors with different DPIs, the value has no physical meaning, but
> > +it may be used by some legacy clients which do not support RandR
> > +version 1.2 to compute a reference font scaling. This option uses either
> >  the specified DPI value, or the DPI of the given output, to compute an 
> > appropriate
> > -physical size using whatever pixel size will be set.
> > +physical size using whatever pixel size will be set. Typical values are
> > +the default (96 DPI), the DPI of the only monitor in single-monitor
> > +configurations, or the DPI of the primary monitor in multi-monitor
> > +configurations.
> >  .IP "\-\-newmode \fIname\fP \fImode\fP"
> >  New modelines can be added to the server and then associated with outputs.
> >  This option does the former. The \fImode\fP is specified using the ModeLine
> > --
> > 2.11.0
> >
> 
> 
> 

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

[PATCH v3 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2018-03-10 Thread Pali Rohár

Explicitly document and make it clear that those options do not change
DPI of some monitor output. Also state that these options have no useful
meaning for multi-monitor configuration.

Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
 man/xrandr.man | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/man/xrandr.man b/man/xrandr.man
index aa82724..1291bad 100644
--- a/man/xrandr.man
+++ b/man/xrandr.man
@@ -257,14 +257,25 @@ fit within this size. When this option is not provided, 
xrandr computes the
 smallest screen size that will hold the set of configured outputs; this
 option provides a way to override that behaviour.
 .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
-Sets the reported values for the physical size of the screen. Normally,
+Sets the value reported as physical size of the X screen as a whole
+(union of all configured monitors). In configurations with multiple
+monitors with different DPIs, the value has no physical meaning, but
+it may be used by some legacy clients which do not support RandR
+version 1.2 to compute a reference font scaling. Normally,
 xrandr resets the reported physical size values to keep the DPI constant.
-This overrides that computation.
+This overrides that computation. Default DPI value is 96.
 .IP "\-\-dpi \fIdpi\fP"
 .IP "\-\-dpi \fIfrom-output\fP"
-This also sets the reported physical size values of the screen, it uses either
+This also sets the value reported as physical size of the X screen as a whole
+(union of all configured monitors). In configurations with multiple
+monitors with different DPIs, the value has no physical meaning, but
+it may be used by some legacy clients which do not support RandR
+version 1.2 to compute a reference font scaling. This option uses either
 the specified DPI value, or the DPI of the given output, to compute an 
appropriate
-physical size using whatever pixel size will be set.
+physical size using whatever pixel size will be set. Typical values are
+the default (96 DPI), the DPI of the only monitor in single-monitor
+configurations, or the DPI of the primary monitor in multi-monitor
+configurations.
 .IP "\-\-newmode \fIname\fP \fImode\fP"
 New modelines can be added to the server and then associated with outputs.
 This option does the former. The \fImode\fP is specified using the ModeLine
-- 
2.11.0

___
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

Re: [PATCH app/xdpyinfo v2] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2018-03-09 Thread Pali Rohár

Hi! I would like to remind this patch which fixes dimensions values in
xdpyinfo output which are broken for many years...

On Tuesday 16 May 2017 22:04:57 Pali Rohár wrote:
> XServer with enabled XRANDR 1.2 extension does not provide correct
> dimensions from DisplayWidthMM() and DisplayHeightMM() calls anymore.
> Values are calculated from fixed DPI 96.
> 
> Therefore when XRANDR 1.2 extension is enabled and present, instead use
> XRRGetScreenResources() and XRRGetOutputInfo() calls to get correct
> dimensions and resolution information.
> 
> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> ---
> Changes since v1:
> * Fixed detection of presence of XRANDR 1.2
> * Fixed resolution calculation when dimensions are zero
> ---
>  Makefile.am  |2 ++
>  configure.ac |   12 
>  xdpyinfo.c   |   91 
> ++
>  3 files changed, 87 insertions(+), 18 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 2f21dda..496094e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -35,6 +35,7 @@ AM_CFLAGS = \
>   $(DPY_XCOMPOSITE_CFLAGS) \
>   $(DPY_XINERAMA_CFLAGS) \
>   $(DPY_DMX_CFLAGS) \
> + $(DPY_XRANDR_CFLAGS) \
>   $(DPY_XTST_CFLAGS)
>  
>  xdpyinfo_LDADD = \
> @@ -49,6 +50,7 @@ xdpyinfo_LDADD = \
>   $(DPY_XCOMPOSITE_LIBS) \
>   $(DPY_XINERAMA_LIBS) \
>   $(DPY_DMX_LIBS) \
> + $(DPY_XRANDR_LIBS) \
>   $(DPY_XTST_LIBS)
>  
>  xdpyinfo_SOURCES =   \
> diff --git a/configure.ac b/configure.ac
> index 73dce26..4473faa 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -132,6 +132,18 @@ else
>   echo "without dmx"
>  fi
>  
> +AC_ARG_WITH(xrandr, AS_HELP_STRING([--without-xrandr],[Disable xrandr 1.2 
> support.]),
> + [USE_XRANDR="$withval"], [USE_XRANDR="yes"])
> +if test "x$USE_XRANDR" != "xno" ; then
> + PKG_CHECK_MODULES(DPY_XRANDR, xrandr >= 1.2,
> + [SAVE_CPPFLAGS="$CPPFLAGS"
> + CPPFLAGS="$CPPFLAGS $DPY_XRANDR_CFLAGS $DPY_X11_CFLAGS"
> + AC_CHECK_HEADERS([X11/extensions/Xrandr.h],,,[#include 
> ])
> + CPPFLAGS="$SAVE_CPPFLAGS"],[echo "not found"])
> +else
> + echo "without xrandr 1.2"
> +fi
> +
>  PKG_CHECK_MODULES(DPY_XTST, xtst,
>   [SAVE_CPPFLAGS="$CPPFLAGS"
>   CPPFLAGS="$CPPFLAGS $DPY_XTST_CFLAGS $DPY_X11_CFLAGS"
> diff --git a/xdpyinfo.c b/xdpyinfo.c
> index 152e32c..409968c 100644
> --- a/xdpyinfo.c
> +++ b/xdpyinfo.c
> @@ -76,6 +76,10 @@ in this Software without prior written authorization from 
> The Open Group.
>  #  define DMX
>  # endif
>  
> +# if HAVE_X11_EXTENSIONS_XRANDR_H
> +#  define XRANDR
> +# endif
> +
>  #endif
>  
>  #ifdef WIN32
> @@ -137,6 +141,9 @@ in this Software without prior written authorization from 
> The Open Group.
>  #ifdef DMX
>  #include 
>  #endif
> +#ifdef XRANDR
> +#include 
> +#endif
>  #include 
>  #include 
>  #include 
> @@ -455,27 +462,75 @@ print_screen_info(Display *dpy, int scr)
>  double xres, yres;
>  int ndepths = 0, *depths = NULL;
>  unsigned int width, height;
> -
> -/*
> - * there are 2.54 centimeters to an inch; so there are 25.4 millimeters.
> - *
> - * dpi = N pixels / (M millimeters / (25.4 millimeters / 1 inch))
> - * = N pixels / (M inch / 25.4)
> - * = N * 25.4 pixels / M inch
> - */
> -
> -xres = double) DisplayWidth(dpy,scr)) * 25.4) /
> - ((double) DisplayWidthMM(dpy,scr)));
> -yres = double) DisplayHeight(dpy,scr)) * 25.4) /
> - ((double) DisplayHeightMM(dpy,scr)));
> +#ifdef XRANDR
> +int event_base, error_base;
> +int major, minor;
> +XRRScreenResources *res = NULL;
> +XRROutputInfo *output;
> +XRRCrtcInfo *crtc;
> +#endif
>  
>  printf ("\n");
>  printf ("screen #%d:\n", scr);
> -printf ("  dimensions:%dx%d pixels (%dx%d millimeters)\n",
> - XDisplayWidth (dpy, scr),  XDisplayHeight (dpy, scr),
> - XDisplayWidthMM(dpy, scr), XDisplayHeightMM (dpy, scr));
> -printf ("  resolution:%dx%d dots per inch\n",
> - (int) (xres + 0.5), (int) (yres + 0.5));
> +
> +#ifdef XRANDR
> +if (XRRQueryExtension (dpy, _base, _base) &&
> +XRRQueryVersion (dpy, , ) &&
> +(major > 1 || (major == 1 && minor >= 2)) &&
> +(res = XRRGetScreenResources (dpy, RootWindow (dpy, scr
> +{
> +

Re: [PATCH v2 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-11-02 Thread Pali Rohár

On Wednesday 01 November 2017 16:35:04 Adam Jackson wrote:
> On Wed, 2017-11-01 at 21:04 +0100, Giuseppe Bilotta wrote:
> 
> > (Heck, I would even argue that the core DPI should be set to the
> > primary monitor DPI by default, regardless of whether the system has
> > one or more than one monitor.)
> 
> The primary monitor can change at runtime. The core DPI is only sent at
> initial connection. What would you like to do?
> 
> Also: what's stopping us from fixing libXft instead?

But back to my documentation update patch. It is needed to rework it?
And if yes, how,

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH v2 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-11-01 Thread Pali Rohár

On Wednesday 01 November 2017 21:04:17 Giuseppe Bilotta wrote:
> On Wed, Nov 1, 2017 at 11:25 AM, Pali Rohár <pali.ro...@gmail.com> wrote:
> > Hello, can somebody process this my patch?
> >
> > There are no objections for a long time and Walter already reviewed it.
> >
> > On Monday 07 August 2017 18:45:22 walter harms wrote:
> >> Sorry, i had the idea you already got my ack.
> >>
> >> Reviewed-by: Walter Harms wha...@bfs.de
> >>
> >> Am 07.08.2017 12:56, schrieb Pali Rohár:
> >> > walter, any other objections? I fixed problematic parts which you 
> >> > pointed.
> >> >
> >> > On Monday 24 July 2017 20:34:39 Pali Rohár wrote:
> >> >> Explicitly document and make it clear that those options do not change
> >> >> DPI of some monitor output. Also state that these options have no useful
> >> >> meaning for multi-monitor configuration.
> 
> Personally, I disagree with the statement that the core DPI value has
> no useful meaning in multi-monitor configuration.

Hi! I did not write such thing.

The whole documentation is talking about physical size of the X Screen.

There is no "core DPI". There is just resolution of the X Screen and
physical size (in mm) of the X Screen. And IIRC X Server does not report
any value "DPI" for X Screen.

And as I added in patch, that physical size in mm of the X Screen does
not have any coordinate meaning as it does not match any physical
reality of display.

So... now I'm thinking if documentation update by me is really good if
there are already people who interpreted it not in way as I thought :-(

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH v2 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-11-01 Thread Pali Rohár

Hello, can somebody process this my patch?

There are no objections for a long time and Walter already reviewed it.

On Monday 07 August 2017 18:45:22 walter harms wrote:
> Sorry, i had the idea you already got my ack.
> 
> Reviewed-by: Walter Harms wha...@bfs.de
> 
> Am 07.08.2017 12:56, schrieb Pali Rohár:
> > walter, any other objections? I fixed problematic parts which you pointed.
> > 
> > On Monday 24 July 2017 20:34:39 Pali Rohár wrote:
> >> Explicitly document and make it clear that those options do not change
> >> DPI of some monitor output. Also state that these options have no useful
> >> meaning for multi-monitor configuration.
> >>
> >> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> >> ---
> >>  man/xrandr.man |   14 ++
> >>  1 file changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/man/xrandr.man b/man/xrandr.man
> >> index 65ccc2a..a5476d4 100644
> >> --- a/man/xrandr.man
> >> +++ b/man/xrandr.man
> >> @@ -232,14 +232,20 @@ fit within this size. When this option is not 
> >> provided, xrandr computes the
> >>  smallest screen size that will hold the set of configured outputs; this
> >>  option provides a way to override that behaviour.
> >>  .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
> >> -Sets the reported values for the physical size of the screen. Normally,
> >> +Sets the reported values for the physical size of the whole X screen (not 
> >> particular
> >> +output!) which consists of all configured monitors. Physical size of the 
> >> X screen does
> >> +not have any meaning when monitors with different DPI are configured. 
> >> Normally,
> >>  xrandr resets the reported physical size values to keep the DPI constant.
> >> -This overrides that computation.
> >> +This overrides that computation. This option should be calculated from DPI
> >> +value 96 as it not have any useful meaning for multi-monitor 
> >> configuration.
> >>  .IP "\-\-dpi \fIdpi\fP"
> >>  .IP "\-\-dpi \fIfrom-output\fP"
> >> -This also sets the reported physical size values of the screen, it uses 
> >> either
> >> +This also sets the reported physical size values of the whole X screen 
> >> (not particular
> >> +output!) which consists of all configured monitors. Physical size of the 
> >> X screen does not
> >> +have any meaning when monitors with different DPI are configured. This 
> >> option uses either
> >>  the specified DPI value, or the DPI of the given output, to compute an 
> >> appropriate
> >> -physical size using whatever pixel size will be set.
> >> +physical size using whatever pixel size will be set (default 96 DPI). It 
> >> does not
> >> +have any useful meaning for multi-monitor configuration.
> >>  .IP "\-\-newmode \fIname\fP \fImode\fP"
> >>  New modelines can be added to the server and then associated with outputs.
> >>  This option does the former. The \fImode\fP is specified using the 
> >> ModeLine
> > 

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH v2 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-09-26 Thread Pali Rohár

Hi! I would like to remind this patch.

On Friday 18 August 2017 19:15:25 Pali Rohár wrote:
> PING xorg-devel list.
> 
> Is something older needed to rework? If not, can you apply this patch?
> 
> On Monday 07 August 2017 18:45:22 walter harms wrote:
> > Sorry, i had the idea you already got my ack.
> > 
> > Reviewed-by: Walter Harms wha...@bfs.de
> > 
> > Am 07.08.2017 12:56, schrieb Pali Rohár:
> > > walter, any other objections? I fixed problematic parts which you pointed.
> > > 
> > > On Monday 24 July 2017 20:34:39 Pali Rohár wrote:
> > >> Explicitly document and make it clear that those options do not change
> > >> DPI of some monitor output. Also state that these options have no useful
> > >> meaning for multi-monitor configuration.
> > >>
> > >> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> > >> ---
> > >>  man/xrandr.man |   14 ++
> > >>  1 file changed, 10 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/man/xrandr.man b/man/xrandr.man
> > >> index 65ccc2a..a5476d4 100644
> > >> --- a/man/xrandr.man
> > >> +++ b/man/xrandr.man
> > >> @@ -232,14 +232,20 @@ fit within this size. When this option is not 
> > >> provided, xrandr computes the
> > >>  smallest screen size that will hold the set of configured outputs; this
> > >>  option provides a way to override that behaviour.
> > >>  .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
> > >> -Sets the reported values for the physical size of the screen. Normally,
> > >> +Sets the reported values for the physical size of the whole X screen 
> > >> (not particular
> > >> +output!) which consists of all configured monitors. Physical size of 
> > >> the X screen does
> > >> +not have any meaning when monitors with different DPI are configured. 
> > >> Normally,
> > >>  xrandr resets the reported physical size values to keep the DPI 
> > >> constant.
> > >> -This overrides that computation.
> > >> +This overrides that computation. This option should be calculated from 
> > >> DPI
> > >> +value 96 as it not have any useful meaning for multi-monitor 
> > >> configuration.
> > >>  .IP "\-\-dpi \fIdpi\fP"
> > >>  .IP "\-\-dpi \fIfrom-output\fP"
> > >> -This also sets the reported physical size values of the screen, it uses 
> > >> either
> > >> +This also sets the reported physical size values of the whole X screen 
> > >> (not particular
> > >> +output!) which consists of all configured monitors. Physical size of 
> > >> the X screen does not
> > >> +have any meaning when monitors with different DPI are configured. This 
> > >> option uses either
> > >>  the specified DPI value, or the DPI of the given output, to compute an 
> > >> appropriate
> > >> -physical size using whatever pixel size will be set.
> > >> +physical size using whatever pixel size will be set (default 96 DPI). 
> > >> It does not
> > >> +have any useful meaning for multi-monitor configuration.
> > >>  .IP "\-\-newmode \fIname\fP \fImode\fP"
> > >>  New modelines can be added to the server and then associated with 
> > >> outputs.
> > >>  This option does the former. The \fImode\fP is specified using the 
> > >> ModeLine
> > > 
> 

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH v2 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-08-20 Thread Pali Rohár

PING xorg-devel list.

Is something older needed to rework? If not, can you apply this patch?

On Monday 07 August 2017 18:45:22 walter harms wrote:
> Sorry, i had the idea you already got my ack.
> 
> Reviewed-by: Walter Harms wha...@bfs.de
> 
> Am 07.08.2017 12:56, schrieb Pali Rohár:
> > walter, any other objections? I fixed problematic parts which you pointed.
> > 
> > On Monday 24 July 2017 20:34:39 Pali Rohár wrote:
> >> Explicitly document and make it clear that those options do not change
> >> DPI of some monitor output. Also state that these options have no useful
> >> meaning for multi-monitor configuration.
> >>
> >> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> >> ---
> >>  man/xrandr.man |   14 ++
> >>  1 file changed, 10 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/man/xrandr.man b/man/xrandr.man
> >> index 65ccc2a..a5476d4 100644
> >> --- a/man/xrandr.man
> >> +++ b/man/xrandr.man
> >> @@ -232,14 +232,20 @@ fit within this size. When this option is not 
> >> provided, xrandr computes the
> >>  smallest screen size that will hold the set of configured outputs; this
> >>  option provides a way to override that behaviour.
> >>  .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
> >> -Sets the reported values for the physical size of the screen. Normally,
> >> +Sets the reported values for the physical size of the whole X screen (not 
> >> particular
> >> +output!) which consists of all configured monitors. Physical size of the 
> >> X screen does
> >> +not have any meaning when monitors with different DPI are configured. 
> >> Normally,
> >>  xrandr resets the reported physical size values to keep the DPI constant.
> >> -This overrides that computation.
> >> +This overrides that computation. This option should be calculated from DPI
> >> +value 96 as it not have any useful meaning for multi-monitor 
> >> configuration.
> >>  .IP "\-\-dpi \fIdpi\fP"
> >>  .IP "\-\-dpi \fIfrom-output\fP"
> >> -This also sets the reported physical size values of the screen, it uses 
> >> either
> >> +This also sets the reported physical size values of the whole X screen 
> >> (not particular
> >> +output!) which consists of all configured monitors. Physical size of the 
> >> X screen does not
> >> +have any meaning when monitors with different DPI are configured. This 
> >> option uses either
> >>  the specified DPI value, or the DPI of the given output, to compute an 
> >> appropriate
> >> -physical size using whatever pixel size will be set.
> >> +physical size using whatever pixel size will be set (default 96 DPI). It 
> >> does not
> >> +have any useful meaning for multi-monitor configuration.
> >>  .IP "\-\-newmode \fIname\fP \fImode\fP"
> >>  New modelines can be added to the server and then associated with outputs.
> >>  This option does the former. The \fImode\fP is specified using the 
> >> ModeLine
> > 

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH v2 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-08-07 Thread Pali Rohár

walter, any other objections? I fixed problematic parts which you pointed.

On Monday 24 July 2017 20:34:39 Pali Rohár wrote:
> Explicitly document and make it clear that those options do not change
> DPI of some monitor output. Also state that these options have no useful
> meaning for multi-monitor configuration.
> 
> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> ---
>  man/xrandr.man |   14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/man/xrandr.man b/man/xrandr.man
> index 65ccc2a..a5476d4 100644
> --- a/man/xrandr.man
> +++ b/man/xrandr.man
> @@ -232,14 +232,20 @@ fit within this size. When this option is not provided, 
> xrandr computes the
>  smallest screen size that will hold the set of configured outputs; this
>  option provides a way to override that behaviour.
>  .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
> -Sets the reported values for the physical size of the screen. Normally,
> +Sets the reported values for the physical size of the whole X screen (not 
> particular
> +output!) which consists of all configured monitors. Physical size of the X 
> screen does
> +not have any meaning when monitors with different DPI are configured. 
> Normally,
>  xrandr resets the reported physical size values to keep the DPI constant.
> -This overrides that computation.
> +This overrides that computation. This option should be calculated from DPI
> +value 96 as it not have any useful meaning for multi-monitor configuration.
>  .IP "\-\-dpi \fIdpi\fP"
>  .IP "\-\-dpi \fIfrom-output\fP"
> -This also sets the reported physical size values of the screen, it uses 
> either
> +This also sets the reported physical size values of the whole X screen (not 
> particular
> +output!) which consists of all configured monitors. Physical size of the X 
> screen does not
> +have any meaning when monitors with different DPI are configured. This 
> option uses either
>  the specified DPI value, or the DPI of the given output, to compute an 
> appropriate
> -physical size using whatever pixel size will be set.
> +physical size using whatever pixel size will be set (default 96 DPI). It 
> does not
> +have any useful meaning for multi-monitor configuration.
>  .IP "\-\-newmode \fIname\fP \fImode\fP"
>  New modelines can be added to the server and then associated with outputs.
>  This option does the former. The \fImode\fP is specified using the ModeLine

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

[PATCH v2 app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-07-24 Thread Pali Rohár

Explicitly document and make it clear that those options do not change
DPI of some monitor output. Also state that these options have no useful
meaning for multi-monitor configuration.

Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
 man/xrandr.man |   14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/man/xrandr.man b/man/xrandr.man
index 65ccc2a..a5476d4 100644
--- a/man/xrandr.man
+++ b/man/xrandr.man
@@ -232,14 +232,20 @@ fit within this size. When this option is not provided, 
xrandr computes the
 smallest screen size that will hold the set of configured outputs; this
 option provides a way to override that behaviour.
 .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
-Sets the reported values for the physical size of the screen. Normally,
+Sets the reported values for the physical size of the whole X screen (not 
particular
+output!) which consists of all configured monitors. Physical size of the X 
screen does
+not have any meaning when monitors with different DPI are configured. Normally,
 xrandr resets the reported physical size values to keep the DPI constant.
-This overrides that computation.
+This overrides that computation. This option should be calculated from DPI
+value 96 as it not have any useful meaning for multi-monitor configuration.
 .IP "\-\-dpi \fIdpi\fP"
 .IP "\-\-dpi \fIfrom-output\fP"
-This also sets the reported physical size values of the screen, it uses either
+This also sets the reported physical size values of the whole X screen (not 
particular
+output!) which consists of all configured monitors. Physical size of the X 
screen does not
+have any meaning when monitors with different DPI are configured. This option 
uses either
 the specified DPI value, or the DPI of the given output, to compute an 
appropriate
-physical size using whatever pixel size will be set.
+physical size using whatever pixel size will be set (default 96 DPI). It does 
not
+have any useful meaning for multi-monitor configuration.
 .IP "\-\-newmode \fIname\fP \fImode\fP"
 New modelines can be added to the server and then associated with outputs.
 This option does the former. The \fImode\fP is specified using the ModeLine
-- 
1.7.9.5

___
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

Re: [PATCH app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-07-24 Thread Pali Rohár

On Sunday 28 May 2017 23:33:59 Pali Rohár wrote:
> Explicitly document and make it clear that those options does not change
> DPI of some monitor output. Also state that these options have no useful
> meaning for multi-monitor configuration.
> 
> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> ---
>  man/xrandr.man |   14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/man/xrandr.man b/man/xrandr.man
> index 55ea5dd..fec4a1a 100644
> --- a/man/xrandr.man
> +++ b/man/xrandr.man
> @@ -226,14 +226,20 @@ fit within this size. When this option is not provided, 
> xrandr computes the
>  smallest screen size that will hold the set of configured outputs; this
>  option provides a way to override that behaviour.
>  .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
> -Sets the reported values for the physical size of the screen. Normally,
> +Sets the reported values for the physical size of the whole X screen (not 
> particular
> +output!) which consists of all configured monitors. Physical size of the X 
> screen does
> +not have any meaning when two monitors with different DPI are configured. 
> Normally,
>  xrandr resets the reported physical size values to keep the DPI constant.
> -This overrides that computation.
> +This overrides that computation. This option should be calculated from DPI
> +value 96 as it not have any useful meaning for multi-monitor configuration.
>  .IP "\-\-dpi \fIdpi\fP"
>  .IP "\-\-dpi \fIfrom-output\fP"
> -This also sets the reported physical size values of the screen, it uses 
> either
> +This also sets the reported physical size values of the whole X screen (not 
> particular
> +output!) which consists of all configured monitors. Physical size of the X 
> screen does not
> +have any meaning when two monitors with different DPI are configured. This 
> option uses either
>  the specified DPI value, or the DPI of the given output, to compute an 
> appropriate
> -physical size using whatever pixel size will be set.
> +physical size using whatever pixel size will be set. This option should be 
> set to
> +default value 96 as it not have any useful meaning for multi-monitor 
> configuration.
>  .IP "\-\-newmode \fIname\fP \fImode\fP"
>  New modelines can be added to the server and then associated with outputs.
>  This option does the former. The \fImode\fP is specified using the ModeLine

PING, can somebody review this patch?

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-06-28 Thread Pali Rohár

On Saturday 10 June 2017 21:19:32 Pali Rohár wrote:
> On Sunday 28 May 2017 23:33:59 Pali Rohár wrote:
> > Explicitly document and make it clear that those options does not
> > change DPI of some monitor output. Also state that these options
> > have no useful meaning for multi-monitor configuration.
> 
> I would like to remind this patch. Can you review/comment it?

Any opinion on this patch?

-- 
Pali Rohár
pali.ro...@gmail.com

signature.asc
Description: This is a digitally signed message part.
___
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

Re: [PATCH app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-06-11 Thread Pali Rohár

On Sunday 28 May 2017 23:33:59 Pali Rohár wrote:
> Explicitly document and make it clear that those options does not
> change DPI of some monitor output. Also state that these options
> have no useful meaning for multi-monitor configuration.

I would like to remind this patch. Can you review/comment it?

> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> ---
>  man/xrandr.man |   14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/man/xrandr.man b/man/xrandr.man
> index 55ea5dd..fec4a1a 100644
> --- a/man/xrandr.man
> +++ b/man/xrandr.man
> @@ -226,14 +226,20 @@ fit within this size. When this option is not
> provided, xrandr computes the smallest screen size that will hold
> the set of configured outputs; this option provides a way to
> override that behaviour.
>  .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
> -Sets the reported values for the physical size of the screen.
> Normally, +Sets the reported values for the physical size of the
> whole X screen (not particular +output!) which consists of all
> configured monitors. Physical size of the X screen does +not have
> any meaning when two monitors with different DPI are configured.
> Normally, xrandr resets the reported physical size values to keep
> the DPI constant. -This overrides that computation.
> +This overrides that computation. This option should be calculated
> from DPI +value 96 as it not have any useful meaning for
> multi-monitor configuration. .IP "\-\-dpi \fIdpi\fP"
>  .IP "\-\-dpi \fIfrom-output\fP"
> -This also sets the reported physical size values of the screen, it
> uses either +This also sets the reported physical size values of the
> whole X screen (not particular +output!) which consists of all
> configured monitors. Physical size of the X screen does not +have
> any meaning when two monitors with different DPI are configured.
> This option uses either the specified DPI value, or the DPI of the
> given output, to compute an appropriate -physical size using
> whatever pixel size will be set.
> +physical size using whatever pixel size will be set. This option
> should be set to +default value 96 as it not have any useful meaning
> for multi-monitor configuration. .IP "\-\-newmode \fIname\fP
> \fImode\fP"
>  New modelines can be added to the server and then associated with
> outputs. This option does the former. The \fImode\fP is specified
> using the ModeLine

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.
___
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

[PATCH app/xrandr] Document that --dpi and --fbmm options set DPI of whole X screen

2017-05-28 Thread Pali Rohár

Explicitly document and make it clear that those options does not change
DPI of some monitor output. Also state that these options have no useful
meaning for multi-monitor configuration.

Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
 man/xrandr.man |   14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/man/xrandr.man b/man/xrandr.man
index 55ea5dd..fec4a1a 100644
--- a/man/xrandr.man
+++ b/man/xrandr.man
@@ -226,14 +226,20 @@ fit within this size. When this option is not provided, 
xrandr computes the
 smallest screen size that will hold the set of configured outputs; this
 option provides a way to override that behaviour.
 .IP "\-\-fbmm \fIwidth\fPx\fIheight\fP"
-Sets the reported values for the physical size of the screen. Normally,
+Sets the reported values for the physical size of the whole X screen (not 
particular
+output!) which consists of all configured monitors. Physical size of the X 
screen does
+not have any meaning when two monitors with different DPI are configured. 
Normally,
 xrandr resets the reported physical size values to keep the DPI constant.
-This overrides that computation.
+This overrides that computation. This option should be calculated from DPI
+value 96 as it not have any useful meaning for multi-monitor configuration.
 .IP "\-\-dpi \fIdpi\fP"
 .IP "\-\-dpi \fIfrom-output\fP"
-This also sets the reported physical size values of the screen, it uses either
+This also sets the reported physical size values of the whole X screen (not 
particular
+output!) which consists of all configured monitors. Physical size of the X 
screen does not
+have any meaning when two monitors with different DPI are configured. This 
option uses either
 the specified DPI value, or the DPI of the given output, to compute an 
appropriate
-physical size using whatever pixel size will be set.
+physical size using whatever pixel size will be set. This option should be set 
to
+default value 96 as it not have any useful meaning for multi-monitor 
configuration.
 .IP "\-\-newmode \fIname\fP \fImode\fP"
 New modelines can be added to the server and then associated with outputs.
 This option does the former. The \fImode\fP is specified using the ModeLine
-- 
1.7.9.5

___
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

[PATCH app/xrandr] Document format of --dpi option in non-ambiguous way

2017-05-28 Thread Pali Rohár

Slash in previous documentation could be misunderstood as part of the
--dpi command line option. So fix it.

Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
 man/xrandr.man |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/man/xrandr.man b/man/xrandr.man
index 5742286..55ea5dd 100644
--- a/man/xrandr.man
+++ b/man/xrandr.man
@@ -41,7 +41,8 @@ xrandr \- primitive command line interface to RandR extension
 [\-\-prop]
 [\-\-fb \fIwidth\fPx\fIheight\fP]
 [\-\-fbmm \fIwidth\fPx\fIheight\fP]
-[\-\-dpi \fIdpi\fP/\fIoutput\fP]
+[\-\-dpi \fIdpi\fP]
+[\-\-dpi \fIfrom-output\fP]
 [\-\-newmode \fIname\fP \fImode\fP]
 [\-\-rmmode \fIname\fP]
 [\-\-addmode \fIoutput\fP \fIname\fP]
@@ -228,7 +229,8 @@ option provides a way to override that behaviour.
 Sets the reported values for the physical size of the screen. Normally,
 xrandr resets the reported physical size values to keep the DPI constant.
 This overrides that computation.
-.IP "\-\-dpi \fIdpi\fP/\fIoutput\fP"
+.IP "\-\-dpi \fIdpi\fP"
+.IP "\-\-dpi \fIfrom-output\fP"
 This also sets the reported physical size values of the screen, it uses either
 the specified DPI value, or the DPI of the given output, to compute an 
appropriate
 physical size using whatever pixel size will be set.
-- 
1.7.9.5

___
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

[PATCH app/xdpyinfo v2] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2017-05-16 Thread Pali Rohár

XServer with enabled XRANDR 1.2 extension does not provide correct
dimensions from DisplayWidthMM() and DisplayHeightMM() calls anymore.
Values are calculated from fixed DPI 96.

Therefore when XRANDR 1.2 extension is enabled and present, instead use
XRRGetScreenResources() and XRRGetOutputInfo() calls to get correct
dimensions and resolution information.

Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
Changes since v1:
* Fixed detection of presence of XRANDR 1.2
* Fixed resolution calculation when dimensions are zero
---
 Makefile.am  |2 ++
 configure.ac |   12 
 xdpyinfo.c   |   91 ++
 3 files changed, 87 insertions(+), 18 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 2f21dda..496094e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -35,6 +35,7 @@ AM_CFLAGS = \
$(DPY_XCOMPOSITE_CFLAGS) \
$(DPY_XINERAMA_CFLAGS) \
$(DPY_DMX_CFLAGS) \
+   $(DPY_XRANDR_CFLAGS) \
$(DPY_XTST_CFLAGS)
 
 xdpyinfo_LDADD = \
@@ -49,6 +50,7 @@ xdpyinfo_LDADD = \
$(DPY_XCOMPOSITE_LIBS) \
$(DPY_XINERAMA_LIBS) \
$(DPY_DMX_LIBS) \
+   $(DPY_XRANDR_LIBS) \
$(DPY_XTST_LIBS)
 
 xdpyinfo_SOURCES = \
diff --git a/configure.ac b/configure.ac
index 73dce26..4473faa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,6 +132,18 @@ else
echo "without dmx"
 fi
 
+AC_ARG_WITH(xrandr, AS_HELP_STRING([--without-xrandr],[Disable xrandr 1.2 
support.]),
+   [USE_XRANDR="$withval"], [USE_XRANDR="yes"])
+if test "x$USE_XRANDR" != "xno" ; then
+   PKG_CHECK_MODULES(DPY_XRANDR, xrandr >= 1.2,
+   [SAVE_CPPFLAGS="$CPPFLAGS"
+   CPPFLAGS="$CPPFLAGS $DPY_XRANDR_CFLAGS $DPY_X11_CFLAGS"
+   AC_CHECK_HEADERS([X11/extensions/Xrandr.h],,,[#include 
])
+   CPPFLAGS="$SAVE_CPPFLAGS"],[echo "not found"])
+else
+   echo "without xrandr 1.2"
+fi
+
 PKG_CHECK_MODULES(DPY_XTST, xtst,
[SAVE_CPPFLAGS="$CPPFLAGS"
CPPFLAGS="$CPPFLAGS $DPY_XTST_CFLAGS $DPY_X11_CFLAGS"
diff --git a/xdpyinfo.c b/xdpyinfo.c
index 152e32c..409968c 100644
--- a/xdpyinfo.c
+++ b/xdpyinfo.c
@@ -76,6 +76,10 @@ in this Software without prior written authorization from 
The Open Group.
 #  define DMX
 # endif
 
+# if HAVE_X11_EXTENSIONS_XRANDR_H
+#  define XRANDR
+# endif
+
 #endif
 
 #ifdef WIN32
@@ -137,6 +141,9 @@ in this Software without prior written authorization from 
The Open Group.
 #ifdef DMX
 #include 
 #endif
+#ifdef XRANDR
+#include 
+#endif
 #include 
 #include 
 #include 
@@ -455,27 +462,75 @@ print_screen_info(Display *dpy, int scr)
 double xres, yres;
 int ndepths = 0, *depths = NULL;
 unsigned int width, height;
-
-/*
- * there are 2.54 centimeters to an inch; so there are 25.4 millimeters.
- *
- * dpi = N pixels / (M millimeters / (25.4 millimeters / 1 inch))
- * = N pixels / (M inch / 25.4)
- * = N * 25.4 pixels / M inch
- */
-
-xres = double) DisplayWidth(dpy,scr)) * 25.4) /
-   ((double) DisplayWidthMM(dpy,scr)));
-yres = double) DisplayHeight(dpy,scr)) * 25.4) /
-   ((double) DisplayHeightMM(dpy,scr)));
+#ifdef XRANDR
+int event_base, error_base;
+int major, minor;
+XRRScreenResources *res = NULL;
+XRROutputInfo *output;
+XRRCrtcInfo *crtc;
+#endif
 
 printf ("\n");
 printf ("screen #%d:\n", scr);
-printf ("  dimensions:%dx%d pixels (%dx%d millimeters)\n",
-   XDisplayWidth (dpy, scr),  XDisplayHeight (dpy, scr),
-   XDisplayWidthMM(dpy, scr), XDisplayHeightMM (dpy, scr));
-printf ("  resolution:%dx%d dots per inch\n",
-   (int) (xres + 0.5), (int) (yres + 0.5));
+
+#ifdef XRANDR
+if (XRRQueryExtension (dpy, _base, _base) &&
+XRRQueryVersion (dpy, , ) &&
+(major > 1 || (major == 1 && minor >= 2)) &&
+(res = XRRGetScreenResources (dpy, RootWindow (dpy, scr
+{
+for (i = 0; i < res->noutput; ++i) {
+output = XRRGetOutputInfo (dpy, res, res->outputs[i]);
+if (!output || !output->crtc || output->connection != RR_Connected)
+continue;
+
+crtc = XRRGetCrtcInfo (dpy, res, output->crtc);
+if (!crtc) {
+XRRFreeOutputInfo (output);
+continue;
+}
+
+printf ("  output: %s\n", output->name);
+printf ("dimensions:%ux%u pixels (%lux%lu millimeters)\n",
+crtc->width, crtc->height, output->mm_width, 
output->mm_height);
+
+if (output->mm_width && output->mm_height) {
+xres = double) crtc-&g

Re: [PATCH app/xdpyinfo] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2017-05-09 Thread Pali Rohár

On Monday 08 May 2017 09:55:08 Eric Anholt wrote:
> That would also not be accepted.  You don't get to rewrite output like
> this in old CLI tools that have had consistent behavior for decades.  It
> causes breakage for systems expecting old behavior, even when that
> behavior is unfortunate and wrong.

In that case my patch should be suitable. It does not change format of
lines which reports dimensions and resolutions. It just use XRANDR
extension to query correct information.

It is really a problem if xdpyinfo query for information via XRANDR
extension which provides correct data instead of old XScreen information
which are not valid on XServer with XRANDR >= 1.2 support? And if yes,
why?

Old applications/scripts expects that xdpyinfo provides on its output
correct dimension and resolution information. And I'm trying to fix it.
But the only way to get correct information is to query XRANDR.

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH app/xdpyinfo] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2017-05-07 Thread Pali Rohár

On Saturday 06 May 2017 13:28:17 Julien Cristau wrote:
> On Fri, May  5, 2017 at 10:08:14 +0200, Pali Rohár wrote:
> > PING.
> > 
> > I would like to know if there is some problem with this and
> > something needs to be reworked or if it can be accepted.
> > 
> > Currently xdpyinfo report bogus dimensions and resolution values
> > and lot of people complain about this problem. It should be fixed.
> 
> Yes, there's a problem with this.  xrandr is the tool to get monitor
> dimensions, xdpyinfo serves a different purpose.  You could add
> support for xdpyinfo -ext RANDR, but changing the default output is
> a bad idea IMO.

But xdpyinfo already output dimensions and that value is incorrect.

If you do not agree that it should be fixed, then I can send patch to 
totally remove dimensions information from xdpyinfo to not provide 
incorrect and bogus values anymore.

-- 
Pali Rohár
pali.ro...@gmail.com

signature.asc
Description: This is a digitally signed message part.
___
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

Re: [PATCH app/xdpyinfo] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2017-05-06 Thread Pali Rohár

PING.

I would like to know if there is some problem with this and something
needs to be reworked or if it can be accepted.

Currently xdpyinfo report bogus dimensions and resolution values and lot
of people complain about this problem. It should be fixed.

On Wednesday 12 April 2017 18:29:19 Pali Rohár wrote:
> Current usage of DisplayWidthMM() and DisplayHeightMM() does not make sense
> for multi-monitor configuration. In most cases DPI is set to 96 as there is
> no sane value.
> 
> Instead when XRANDR 1.2 extension is supported, report dimensions and
> resolution information per XRANDR monitor output. It should provide
> correct DPI value.
> 
> Lot of users complains about incorrect DPI reported by xdpyinfo, see bug:
> https://bugs.freedesktop.org/show_bug.cgi?id=23705
> 
> Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> ---
> Without this patch `xdpyinfo | grep -A 4 ^screen` reports:
> 
> screen #0:
>   dimensions:1600x900 pixels (423x238 millimeters)
>   resolution:96x96 dots per inch
>   depths (7):24, 1, 4, 8, 15, 16, 32
>   root window id:0xf8
> 
> Where DPI and also monitor dimensions in millimeters is incorrect.
> After applying this patch `xdpyinfo | grep -A 4 ^screen` reports:
> 
> screen #0:
>   output: eDP1
> dimensions:1600x900 pixels (310x170 millimeters)
> resolution:131x134 dots per inch
>   depths (7):24, 1, 4, 8, 15, 16, 32
> 
> And both DPI and monitor dimensions (for eDP1) are correct.
> ---
>  Makefile.am  |2 ++
>  configure.ac |   12 
>  xdpyinfo.c   |   86 
> ++
>  3 files changed, 82 insertions(+), 18 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 2f21dda..496094e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -35,6 +35,7 @@ AM_CFLAGS = \
>   $(DPY_XCOMPOSITE_CFLAGS) \
>   $(DPY_XINERAMA_CFLAGS) \
>   $(DPY_DMX_CFLAGS) \
> + $(DPY_XRANDR_CFLAGS) \
>   $(DPY_XTST_CFLAGS)
>  
>  xdpyinfo_LDADD = \
> @@ -49,6 +50,7 @@ xdpyinfo_LDADD = \
>   $(DPY_XCOMPOSITE_LIBS) \
>   $(DPY_XINERAMA_LIBS) \
>   $(DPY_DMX_LIBS) \
> + $(DPY_XRANDR_LIBS) \
>   $(DPY_XTST_LIBS)
>  
>  xdpyinfo_SOURCES =   \
> diff --git a/configure.ac b/configure.ac
> index 73dce26..4473faa 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -132,6 +132,18 @@ else
>   echo "without dmx"
>  fi
>  
> +AC_ARG_WITH(xrandr, AS_HELP_STRING([--without-xrandr],[Disable xrandr 1.2 
> support.]),
> + [USE_XRANDR="$withval"], [USE_XRANDR="yes"])
> +if test "x$USE_XRANDR" != "xno" ; then
> + PKG_CHECK_MODULES(DPY_XRANDR, xrandr >= 1.2,
> + [SAVE_CPPFLAGS="$CPPFLAGS"
> + CPPFLAGS="$CPPFLAGS $DPY_XRANDR_CFLAGS $DPY_X11_CFLAGS"
> + AC_CHECK_HEADERS([X11/extensions/Xrandr.h],,,[#include 
> ])
> + CPPFLAGS="$SAVE_CPPFLAGS"],[echo "not found"])
> +else
> + echo "without xrandr 1.2"
> +fi
> +
>  PKG_CHECK_MODULES(DPY_XTST, xtst,
>   [SAVE_CPPFLAGS="$CPPFLAGS"
>   CPPFLAGS="$CPPFLAGS $DPY_XTST_CFLAGS $DPY_X11_CFLAGS"
> diff --git a/xdpyinfo.c b/xdpyinfo.c
> index 152e32c..7a75fdc 100644
> --- a/xdpyinfo.c
> +++ b/xdpyinfo.c
> @@ -76,6 +76,10 @@ in this Software without prior written authorization from 
> The Open Group.
>  #  define DMX
>  # endif
>  
> +# if HAVE_X11_EXTENSIONS_XRANDR_H
> +#  define XRANDR
> +# endif
> +
>  #endif
>  
>  #ifdef WIN32
> @@ -137,6 +141,9 @@ in this Software without prior written authorization from 
> The Open Group.
>  #ifdef DMX
>  #include 
>  #endif
> +#ifdef XRANDR
> +#include 
> +#endif
>  #include 
>  #include 
>  #include 
> @@ -455,27 +462,70 @@ print_screen_info(Display *dpy, int scr)
>  double xres, yres;
>  int ndepths = 0, *depths = NULL;
>  unsigned int width, height;
> -
> -/*
> - * there are 2.54 centimeters to an inch; so there are 25.4 millimeters.
> - *
> - * dpi = N pixels / (M millimeters / (25.4 millimeters / 1 inch))
> - * = N pixels / (M inch / 25.4)
> - * = N * 25.4 pixels / M inch
> - */
> -
> -xres = double) DisplayWidth(dpy,scr)) * 25.4) /
> - ((double) DisplayWidthMM(dpy,scr)));
> -yres = double) DisplayHeight(dpy,scr)) * 25.4) /
> - ((double) DisplayHeightMM(dpy,scr)));
> +#ifdef XRANDR
> +int event_base, error_base;
> +int major, minor;
> +XRRScreenResources *res = NULL;
> +XRROutputInfo *ou

Re: [PATCH app/xdpyinfo] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2017-04-21 Thread Pali Rohár

On Thursday 13 April 2017 12:58:52 Michel Dänzer wrote:
> On 13/04/17 12:52 PM, Keith Packard wrote:
> > Pali Rohár <pali.ro...@gmail.com> writes:
> > 
> >> Current usage of DisplayWidthMM() and DisplayHeightMM() does not make sense
> >> for multi-monitor configuration. In most cases DPI is set to 96 as there is
> >> no sane value.
> >>
> >> Instead when XRANDR 1.2 extension is supported, report dimensions and
> >> resolution information per XRANDR monitor output. It should provide
> >> correct DPI value.
> > 
> > I'd be happy for this to be reported as additional information, but I
> > suspect there are numerous shell scripts which parse the old information
> > which will get confused by any change in the format.
> 
> In which case it should probably only be printed with -ext RANDR, to be
> consistent with other extensions.

I do not agree as xdpyinfo on Xservers with XRANDR 1.2 support output
invalid resolution and dimension information. When XRANDR 1.2 support
then those information are available only via XRANDR extension.

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

Re: [PATCH app/xdpyinfo] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2017-04-13 Thread Pali Rohár

On Wednesday 12 April 2017 21:52:46 Keith Packard wrote:
> Pali Rohár <pali.ro...@gmail.com> writes:
> 
> > Current usage of DisplayWidthMM() and DisplayHeightMM() does not make sense
> > for multi-monitor configuration. In most cases DPI is set to 96 as there is
> > no sane value.
> >
> > Instead when XRANDR 1.2 extension is supported, report dimensions and
> > resolution information per XRANDR monitor output. It should provide
> > correct DPI value.
> 
> I'd be happy for this to be reported as additional information, but I
> suspect there are numerous shell scripts which parse the old information
> which will get confused by any change in the format.

I tried to make sure that format would not be broken by shell scripts,
which uses grep. What could happen is just more found lines which
matches... but that could happen also now when there are more screens.
So scripts needs to already handle it.

But what is bad... currently xdpyinfo reports nonsense values when
XRANDR 1.2 extension is supported. So I would really suggest to stop
reporting dimensions values when XRANDR 1.2 is used.

Lot of people are complaining about output fro xdpyinfo and thins that X
server itself is broken, but just xdpyinfo is.

-- 
Pali Rohár
pali.ro...@gmail.com
___
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

[PATCH app/xdpyinfo] Use XRANDR 1.2 extension for reporting dimensions and resolution per output

2017-04-12 Thread Pali Rohár

Current usage of DisplayWidthMM() and DisplayHeightMM() does not make sense
for multi-monitor configuration. In most cases DPI is set to 96 as there is
no sane value.

Instead when XRANDR 1.2 extension is supported, report dimensions and
resolution information per XRANDR monitor output. It should provide
correct DPI value.

Lot of users complains about incorrect DPI reported by xdpyinfo, see bug:
https://bugs.freedesktop.org/show_bug.cgi?id=23705

Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
---
Without this patch `xdpyinfo | grep -A 4 ^screen` reports:

screen #0:
  dimensions:1600x900 pixels (423x238 millimeters)
  resolution:96x96 dots per inch
  depths (7):24, 1, 4, 8, 15, 16, 32
  root window id:0xf8

Where DPI and also monitor dimensions in millimeters is incorrect.
After applying this patch `xdpyinfo | grep -A 4 ^screen` reports:

screen #0:
  output: eDP1
dimensions:1600x900 pixels (310x170 millimeters)
resolution:131x134 dots per inch
  depths (7):24, 1, 4, 8, 15, 16, 32

And both DPI and monitor dimensions (for eDP1) are correct.
---
 Makefile.am  |2 ++
 configure.ac |   12 
 xdpyinfo.c   |   86 ++
 3 files changed, 82 insertions(+), 18 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 2f21dda..496094e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -35,6 +35,7 @@ AM_CFLAGS = \
$(DPY_XCOMPOSITE_CFLAGS) \
$(DPY_XINERAMA_CFLAGS) \
$(DPY_DMX_CFLAGS) \
+   $(DPY_XRANDR_CFLAGS) \
$(DPY_XTST_CFLAGS)
 
 xdpyinfo_LDADD = \
@@ -49,6 +50,7 @@ xdpyinfo_LDADD = \
$(DPY_XCOMPOSITE_LIBS) \
$(DPY_XINERAMA_LIBS) \
$(DPY_DMX_LIBS) \
+   $(DPY_XRANDR_LIBS) \
$(DPY_XTST_LIBS)
 
 xdpyinfo_SOURCES = \
diff --git a/configure.ac b/configure.ac
index 73dce26..4473faa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -132,6 +132,18 @@ else
echo "without dmx"
 fi
 
+AC_ARG_WITH(xrandr, AS_HELP_STRING([--without-xrandr],[Disable xrandr 1.2 
support.]),
+   [USE_XRANDR="$withval"], [USE_XRANDR="yes"])
+if test "x$USE_XRANDR" != "xno" ; then
+   PKG_CHECK_MODULES(DPY_XRANDR, xrandr >= 1.2,
+   [SAVE_CPPFLAGS="$CPPFLAGS"
+   CPPFLAGS="$CPPFLAGS $DPY_XRANDR_CFLAGS $DPY_X11_CFLAGS"
+   AC_CHECK_HEADERS([X11/extensions/Xrandr.h],,,[#include 
])
+   CPPFLAGS="$SAVE_CPPFLAGS"],[echo "not found"])
+else
+   echo "without xrandr 1.2"
+fi
+
 PKG_CHECK_MODULES(DPY_XTST, xtst,
[SAVE_CPPFLAGS="$CPPFLAGS"
CPPFLAGS="$CPPFLAGS $DPY_XTST_CFLAGS $DPY_X11_CFLAGS"
diff --git a/xdpyinfo.c b/xdpyinfo.c
index 152e32c..7a75fdc 100644
--- a/xdpyinfo.c
+++ b/xdpyinfo.c
@@ -76,6 +76,10 @@ in this Software without prior written authorization from 
The Open Group.
 #  define DMX
 # endif
 
+# if HAVE_X11_EXTENSIONS_XRANDR_H
+#  define XRANDR
+# endif
+
 #endif
 
 #ifdef WIN32
@@ -137,6 +141,9 @@ in this Software without prior written authorization from 
The Open Group.
 #ifdef DMX
 #include 
 #endif
+#ifdef XRANDR
+#include 
+#endif
 #include 
 #include 
 #include 
@@ -455,27 +462,70 @@ print_screen_info(Display *dpy, int scr)
 double xres, yres;
 int ndepths = 0, *depths = NULL;
 unsigned int width, height;
-
-/*
- * there are 2.54 centimeters to an inch; so there are 25.4 millimeters.
- *
- * dpi = N pixels / (M millimeters / (25.4 millimeters / 1 inch))
- * = N pixels / (M inch / 25.4)
- * = N * 25.4 pixels / M inch
- */
-
-xres = double) DisplayWidth(dpy,scr)) * 25.4) /
-   ((double) DisplayWidthMM(dpy,scr)));
-yres = double) DisplayHeight(dpy,scr)) * 25.4) /
-   ((double) DisplayHeightMM(dpy,scr)));
+#ifdef XRANDR
+int event_base, error_base;
+int major, minor;
+XRRScreenResources *res = NULL;
+XRROutputInfo *output;
+XRRCrtcInfo *crtc;
+#endif
 
 printf ("\n");
 printf ("screen #%d:\n", scr);
-printf ("  dimensions:%dx%d pixels (%dx%d millimeters)\n",
-   XDisplayWidth (dpy, scr),  XDisplayHeight (dpy, scr),
-   XDisplayWidthMM(dpy, scr), XDisplayHeightMM (dpy, scr));
-printf ("  resolution:%dx%d dots per inch\n",
-   (int) (xres + 0.5), (int) (yres + 0.5));
+
+#ifdef XRANDR
+if (XRRQueryExtension (dpy, _base, _base) &&
+XRRQueryVersion (dpy, , ) &&
+(major >= 1 || (major == 1 && minor >= 2)) &&
+(res = XRRGetScreenResources (dpy, RootWindow (dpy, scr
+{
+for (i = 0; i < res->noutput; ++i) {
+output = XRRGetOutputInfo (dpy, res, res->outputs[i]);
+if (!output || !output->crtc || output->connection != RR_Connected)
+

Re: xserver dependency on crypto library because of a hashmap

2014-06-09 Thread Pali Rohár


Hello Remi,

Rémi Cardona remi at gentoo.org wrote:

Le dimanche 08 juin 2014 à 15:46 +0200, Marek Behun a écrit :
 300 lines of code only to
 wrap external library calls. In those 300 lines one could write some
 simpler, faster hashmap hash function (isn't crc32 or something simpler
 good enough for this?), 


Back in our bugzilla, your only concern seemed to be about our package
depending on OpenSSL. While I understand that concern with all the
recent security flaws in that lib, do you have any numbers to back your
new-found concern regarding speed?



I think that security flaws found in openssl/gnutls last days/months is 
very good reason to not use it - when it is not needed.



As for using something else, SHA1 was introduced nearly 7 years ago,
precisely to replace a custom XOR hash:

commit 19b3b1fd8feb343a690331cafe88ef10b34b9d98
Author: Carl Worth cworth at cworth.org
Date:   Tue Jul 31 17:04:13 2007 -0700

See this thread for some reasoning
http://lists.x.org/archives/xorg/2007-August/026730.html



I still do not understand what cryptographic safe hash function solving in 
hash map of glyphs. Why there is need to use sha1 hash function for hash 
map? I have never seen use of sha1 hash function for implementing hash map. 
For me this is really overkill.


CCing Carl, please can you explain your decision for sha1? Maybe I did not 
catch something important.



 or one could copy the entire code for sha1 from another library.

commit a39377cbcbd3091095efbeab25bec18ae520147e
Author: Keith Packard keithp at keithp.com
Date:   Tue Sep 23 09:22:07 2008 -0700

Revert Render: Use built-in SHA1 library

This reverts commit d3bd31fddff7894f89ba80a3cdddff49aff08db8.

X.org should not be providing a custom SHA1 implementation.


Bundled libraries are distributions' worst nightmares and this
particular debate has been settled.



And using external buggy and insecure libraries is nightmare for admins... 
And specially for programs which run under root (and iopl too). I 
understand that distributions want to use dynamic linking and avoiding 
duplicate code...


But now xserver has big #ifdef for more crypto libraries. Why not to add 
another #ifdef for using bundled sha1 implementation? This will not affect 
any distributions and solve problem of dynamic linking with some ssl 
library (which is itself not needed for xserver).


CCing Keith, what do you think about it?


 Depending on external crypto library because of a hashmap is insane for
 Christ's sake.

I fail to see the insanity of depending on other libraries when they fit
the bill.



I think this is ridiculous. Xserver using only sha1 functions from openssl 
library, no encryption/decryption...



Rémi


PS: Please add me to CC, I'm not subscribed to this list.

--
Pali Rohár
pali.ro...@gmail.com
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

43 matches

Mail list logo