[PATCH xinput] Rename map-to-crtc to map-to-output

Wed Feb 8 08:01:34 PST 2012

On 02/08/2012 06:07 AM, Peter Hutterer wrote:
> On Tue, Feb 07, 2012 at 10:46:34AM -0800, Aaron Plattner wrote:
>> On 02/07/2012 12:21 AM, Peter Hutterer wrote:
>>> xrandr uses "output", let's be consistent there.
>>>
>>> Signed-off-by: Peter Hutterer<peter.hutterer at who-t.net>
>>> ---
>>>   man/xinput.man  |    4 ++--
>>>   src/transform.c |   16 ++++++++--------
>>>   src/xinput.c    |    6 +++---
>>>   src/xinput.h    |    2 +-
>>>   4 files changed, 14 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/man/xinput.man b/man/xinput.man
>>> index 085a5fc..f70fe8c 100644
>>> --- a/man/xinput.man
>>> +++ b/man/xinput.man
>>> @@ -140,9 +140,9 @@ Set the ClientPointer for the client owning \fIwindow\fP to \fImaster\fP.
>>>   \fImaster\fP must specify a master pointer.
>>>   .PP
>>>   .TP 8
>>> -.B --map-to-crtc \fIdevice\fP \fIcrtc\fP
>>
>> Are any scripts using this old name?
>
> No, this was never in a released version.

Cool.

>>> +.B --map-to-output \fIdevice\fP \fIcrtc\fP
>>>   Restricts the movements of the absolute \fIdevice\fP to the RandR
>>> -\fIcrtc\fP. The CRTC name must match a currently connected CRTC (see
>>> +\fIcrtc\fP. The output name must match a currently connected output (see
>>
>> It might be worth mentioning exactly which crtc this is referring to
>> (i.e., the one the given output is attached to).
>>
>> Also, I don't think it's enough for the output to be connected.  It
>> has to actually be bound to an active crtc, right?
>
> It has to be an active output, so that we get the geometry information. I
> suspect a connected but disabled output would just give us 0/0 (can't test
> this atm).
>
> The reason for this switch is largely to be consistent with the xrandr
> comandline utility, it seemed weird to run xrandr --output VGA0 followed
> by xinput --map-to-crtc. Do you have a better suggestion for the option or
> are do you just want the man page changed?

I don't know if anyone but a pedant would care about the 
connected-and-active vs. connected-but-inactive distinction, so I'm okay 
with the current wording.  :)

>>>   \fIxrandr(__appmansuffix__)\fP). If the NVIDIA binary driver is
>>>   detected or RandR 1.2 or later is not available, a Xinerama output may be
>>
>>
>>>   specified as "HEAD-N", with N being the Xinerama screen number. This option
>>> diff --git a/src/transform.c b/src/transform.c
>>> index c1a065f..cffc3af 100644
>>> --- a/src/transform.c
>>> +++ b/src/transform.c
>>> @@ -136,7 +136,7 @@ set_transformation_matrix(Display *dpy, Matrix *m, int offset_x, int offset_y,
>>>   }
>>>
>>>   static int
>>> -map_crtc_xrandr(Display *dpy, int deviceid, const char *output_name)
>>> +map_output_xrandr(Display *dpy, int deviceid, const char *output_name)
>>>   {
>>>       int i, found = 0;
>>>       int rc = EXIT_FAILURE;
>>
>> Not really related to this change, but this function leaks one or
>> more XRRCrtcInfo structures (they're allocated by XRRGetCrtcInfo, to
>> be freed by the caller).
>
> Thanks, well spotted. Patch should be on the list in a bit.
>
>>> @@ -178,7 +178,7 @@ map_crtc_xrandr(Display *dpy, int deviceid, const char *output_name)
>>>   }
>>>
>>>   static int
>>> -map_crtc_xinerama(Display *dpy, int deviceid, const char *output_name)
>>> +map_output_xinerama(Display *dpy, int deviceid, const char *output_name)
>>>   {
>>>       const char *prefix = "HEAD-";
>>>       XineramaScreenInfo *screens = NULL;
>>> @@ -229,11 +229,11 @@ out:
>>>   }
>>>
>>>   int
>>> -map_to_crtc(Display *dpy, int argc, char *argv[], char *name, char *desc)
>>> +map_to_output(Display *dpy, int argc, char *argv[], char *name, char *desc)
>>>   {
>>>       int opcode, event, error;
>>>       int maj, min;
>>> -    char *crtc_name;
>>> +    char *output_name;
>>>       XIDeviceInfo *info;
>>>
>>>       if (argc<   2)
>>> @@ -249,15 +249,15 @@ map_to_crtc(Display *dpy, int argc, char *argv[], char *name, char *desc)
>>>           return EXIT_FAILURE;
>>>       }
>>>
>>> -    crtc_name = argv[1];
>>> +    output_name = argv[1];
>>>
>>>       /* Check for RandR 1.2. Server bug causes the NVIDIA driver to
>>> -     * report with RandR 1.3 support but it doesn't expose RandR CRTCs.
>>> +     * report with RandR 1.3 support but it doesn't expose RandR outputs.
>>>        * Force Xinerama if NV-CONTROL is present */
>>>       if (XQueryExtension(dpy, "NV-CONTROL",&opcode,&event,&error) ||
>>
>> Eww... this will disallow using RandR outputs on X screens driven by
>> other drivers when the NVIDIA driver is driving any screen.  Could
>> you check for the server-generated output named "default" instead?
>
> Can you expand on this a bit? I don't have a nvidia box at hand right now.
> The "default" output is the one available when the driver doesn't support
> RandR?

In randr/rrinfo.c in RRGetInfo, if it sees pPriv->nSizes > 0 (e.g. 
because the DDX implementation used RRRegisterSize) then it calls 
RRScanOldConfig, which creates the output named "default".  It does this 
so that clients can use the RandR 1.2 interface even with drivers that 
only implement the RandR 1.1 interface.  It's not a bug, it's 
intentional behavior in the server.  Our driver is not specifically 
enabling that behavior, and I don't think there's anything we can do to 
explicitly disable it.

Maybe it would help if the server tagged that output with some unique 
property?

In any case, for the current change,
Reviewed-by: Aaron Plattner <aplattner at nvidia.com>

> Cheers,
>    Peter
>
>>>           !XQueryExtension(dpy, "RANDR",&opcode,&event,&error) ||
>>>           !XRRQueryVersion(dpy,&maj,&min) || (maj * 1000 + min)<   1002)
>>> -       return map_crtc_xinerama(dpy, info->deviceid, crtc_name);
>>> +       return map_output_xinerama(dpy, info->deviceid, output_name);
>>>       else
>>> -       return map_crtc_xrandr(dpy, info->deviceid, crtc_name);
>>> +       return map_output_xrandr(dpy, info->deviceid, output_name);
>>>   }
>>> diff --git a/src/xinput.c b/src/xinput.c
>>> index 80a1789..66b967b 100644
>>> --- a/src/xinput.c
>>> +++ b/src/xinput.c
>>> @@ -104,9 +104,9 @@ static entry drivers[] =
>>>         "<device>",
>>>         test_xi2,
>>>       },
>>> -    { "map-to-crtc",
>>> -      "<device>   <crtc name>",
>>> -      map_to_crtc,
>>> +    { "map-to-output",
>>> +      "<device>   <output name>",
>>> +      map_to_output,
>>>       },
>>>   #endif
>>>       { "list-props",
>>> diff --git a/src/xinput.h b/src/xinput.h
>>> index 94b8f3c..b420e61 100644
>>> --- a/src/xinput.h
>>> +++ b/src/xinput.h
>>> @@ -77,6 +77,6 @@ int change_attachment( Display* display, int argc, char *argv[], char *prog_name
>>>   int float_device( Display* display, int argc, char *argv[], char *prog_name, char *prog_desc);
>>>   int set_clientpointer( Display* display, int argc, char *argv[], char *prog_name, char *prog_desc);
>>>   int test_xi2( Display* display, int argc, char *argv[], char *prog_name, char *prog_desc);
>>> -int map_to_crtc( Display* display, int argc, char *argv[], char *prog_name, char *prog_desc);
>>> +int map_to_output( Display* display, int argc, char *argv[], char *prog_name, char *prog_desc);
>>>
>>>   /* end of xinput.h */