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

[Giuseppe Bilotta]

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.rohar at 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.
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.
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.)

>> >> +
>> >> +#ifdef XRANDR
>> >> +    if (XRRQueryExtension (dpy, &event_base, &error_base) &&
>> >> +        XRRQueryVersion (dpy, &major, &minor) &&
>> >> +        (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).

>> 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
>> <version> 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 rotated output:

  output: eDP-1
    dimensions:    1800x3200 pixels (346x194 millimeters)
    resolution:    132x419 dots per inch

whereas non-rotated:

  output: eDP-1
    dimensions:    3200x1800 pixels (346x194 millimeters)
    resolution:    235x236 dots per inch

Example from my aforementioned xdpi, rotated:

        eDP-1: 1800x3200 pixels, (R, connected) 194x346 mm: 236x235
dpi, 92x92 dpcm, dot pitch 0.11mm

right side up:

        eDP-1: 3200x1800 pixels, (U, connected) 346x194 mm: 235x236
dpi, 92x92 dpcm, dot pitch 0.11mm

The resolution should be rotational invariant (the monitor dot pitch
does not change if I put it on its side),
but the RANDR protocol doesn't take care of swapping the physical
dimensions, so you have to do it yourself.

>> Also (please don't hate me), for RANDR versions 1.5 or higher we
>> should include RANDR monitors too (possibly in place of individual
>> outputs, I'm not sure about this actually, it depends on how detailed
>> we want to be; I would go with both, but then again I'm an information
>> junkie).
>
> I know that I RANDR 1.5+ supports "monitors" which is needed for display
> port outputs, but unfortunately I do not have such configuration for
> testing. And I do not like idea to write and send code without real
> testing.

If no monitors are manually created, the X server will automatically generate
one per output, so you should be able to test the code even without an
actual MST monitor.

-- 
Giuseppe "Oblomov" Bilotta