[PATCH] xserver: add monitor Option "ZoomModes"

[Aaron Plattner]

On 11/21/2012 04:12 AM, vdb at picaros.org wrote:
> Add support for the Option "ZoomModes" in a monitor section:
>
> Section "Monitor"
>    Identifier "a21inch"
>    Option "PreferredMode" "1600x1200"
>    Option "ZoomModes" "1600x1200 1280x1024 1280x1024 640x480"
> EndSection

"ZoomModes" seems like an unfortunate name to me, but there's precedent 
for it in the "DontZoom" option so I won't object to it too strongly.

> The option's effect is to search for and mark once each named mode in
> the output modes list.  So the specification order is free and the zoom
> modes sequence follows the order of the output modes list.  All marked
> modes are available via the Ctrl+Alt+Keypad-{Plus,Minus} key
> combination.
>
> See also http://bugs.freedesktop.org/show_bug.cgi?id=17954.
>
> This option has its use for combined monitor and television setups.
> It allows for easy switching between 60 Hz and 50 Hz modes even when a
> monitor refuses to display the input signal.
>
> Is there any support for this enhancement ? The patch is here below.
>
> Signed-off-by: Servaas Vandenberghe <vdb at picaros.org>
> ---
>   hw/xfree86/man/xorg.conf.man |   10 +++++
>   hw/xfree86/modes/xf86Crtc.c  |   87 ++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 97 insertions(+), 0 deletions(-)
>
> diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
> index 5d92bbe..729d301 100644
> --- a/hw/xfree86/man/xorg.conf.man
> +++ b/hw/xfree86/man/xorg.conf.man
> @@ -1676,6 +1676,16 @@ This optional entry specifies a mode to be marked as the preferred initial mode
>   of the monitor.
>   (RandR 1.2-supporting drivers only)
>   .TP 7
> +.BI "Option \*qZoomModes\*q \*q" name " " name " " ... \*q
> +This optional entry specifies modes to be marked as zoom modes.
> +It is possible to switch to the next and previous mode via
> +.BR Ctrl+Alt+Keypad\-Plus " and " Ctrl+Alt+Keypad\-Minus .
> +All these keypad available modes are selected from the screen mode
> +list.  This list is a copy of the compatibility output monitor mode

roff requires each sentence to begin on its own line, though you can add 
additional line breaks in the middle of sentences for readability.

> +list.  Since this output is the output connected to the lowest
> +dot\-area monitor, as determined from its largest size mode, that

Should this be a hyphen ('-') rather than a literal dash ('\-')?

> +monitor defines the available zoom modes.

This only applies to RandR 1.2 drivers, so it should probably get the 
"RandR 1.2-supporting drivers only" text.

> +.TP 7
>   .BI "Option \*qPosition\*q \*q" x " " y \*q
>   This optional entry specifies the position of the monitor within the X
>   screen.
> diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
> index 154f684..2e46885 100644
> --- a/hw/xfree86/modes/xf86Crtc.c
> +++ b/hw/xfree86/modes/xf86Crtc.c
> @@ -427,6 +427,7 @@ extern XF86ConfigPtr xf86configptr;
>
>   typedef enum {
>       OPTION_PREFERRED_MODE,
> +    OPTION_ZOOM_MODES,
>       OPTION_POSITION,
>       OPTION_BELOW,
>       OPTION_RIGHT_OF,
> @@ -445,6 +446,7 @@ typedef enum {
>
>   static OptionInfoRec xf86OutputOptions[] = {
>       {OPTION_PREFERRED_MODE, "PreferredMode", OPTV_STRING, {0}, FALSE},
> +    {OPTION_ZOOM_MODES, "ZoomModes", OPTV_STRING, {0}, FALSE },
>       {OPTION_POSITION, "Position", OPTV_STRING, {0}, FALSE},
>       {OPTION_BELOW, "Below", OPTV_STRING, {0}, FALSE},
>       {OPTION_RIGHT_OF, "RightOf", OPTV_STRING, {0}, FALSE},
> @@ -1424,6 +1426,88 @@ preferredMode(ScrnInfoPtr pScrn, xf86OutputPtr output)
>       return preferred_mode;
>   }
>
> +/** identify a token
> + * args
> + *   *src     a string with zero or more tokens, e.g. "tok0 tok1",
> + *   **token  stores a pointer to the first token character,
> + *   *len     stores the token length.
> + * return
> + *   a pointer into src[] at the token terminating character, or
> + *   NULL if no token is found.
> + */
> +static const char *
> +gettoken(const char *src, const char **token, int *len)
> +{
> +    const char *next, *delim = " \t";
> +    int skip;
> +
> +    if (!src)
> +        return NULL;
> +
> +    skip = strspn(src, delim);
> +    *token = &src[skip];
> +
> +    *len = strcspn(*token, delim);
> +    /* Support for backslash escaped delimiters could be implemented
> +     * here.
> +     */
> +
> +    /* (*token)[0] != '\0'  <==>  *len > 0 */
> +    next = *len > 0 ? &(*token)[*len] : NULL;

This would probably be clearer written

   if (*len > 0)
       return &(*token)[*len];
   else
       return NULL;

> +
> +    return next;
> +}

It seems surprising that there isn't already a function to do this.

> +/** Check for a user configured zoom mode list, Option "ZoomModes":
> + *
> + * Section "Monitor"
> + *   Identifier "a21inch"
> + *   Option "ZoomModes" "1600x1200 1280x1024 1280x1024 640x480"
> + * EndSection
> + *
> + * Each user mode name is searched for independently so the list
> + * specification order is free.  An output mode is matched at most
> + * once, a mode with an already set M_T_USERDEF type bit is skipped.
> + * Thus a repeat mode name specificaton matches the next output mode.

s/specificaton/specification/

This took me a few reads to make sense.  Maybe add "with the same name" 
to the end of this sentence?

> + * Ctrl+Alt+Keypad-{Plus,Minus} zooms {in,out} by selecting the
> + * {next,previous} M_T_USERDEF mode in the screen modes list, itself
> + * sorted toward lower dot area or lower dot clock frequency, see
> + *   modes/xf86Crtc.c: xf86SortModes() xf86SetScrnInfoModes(), and
> + *   common/xf86Cursor.c: xf86ZoomViewport().
> + */
> +static int
> +processZoomModes(xf86OutputPtr output)
> +{
> +    const char *zoom_modes;
> +    int count = 0;
> +
> +    zoom_modes = xf86GetOptValString(output->options, OPTION_ZOOM_MODES);
> +
> +    if (zoom_modes) {
> +        const char *token, *next;
> +        int len;
> +
> +        next = gettoken(zoom_modes, &token, &len);
> +        while (next) {
> +            DisplayModePtr mode;
> +
> +            for (mode = output->probed_modes; mode; mode = mode->next)
> +                if (!strncmp(token, mode->name, len)  /* prefix match */
> +                    && mode->name[len] == '\0'        /* equal length */

This should probably use xf86NameCmp for consistency with the rest of 
the server's parsing code.  I know that means using strndup or similar; 
sorry.

> +                    && !(mode->type & M_T_USERDEF)) { /* no rematch */
> +                    mode->type |= M_T_USERDEF;
> +                    break;
> +                }
> +
> +            count++;
> +            next = gettoken(next, &token, &len);
> +        }
> +    }
> +
> +    return count;
> +}
> +
>   static void
>   GuessRangeFromModes(MonPtr mon, DisplayModePtr mode)
>   {
> @@ -1719,6 +1803,9 @@ xf86ProbeOutputModes(ScrnInfoPtr scrn, int maxX, int maxY)
>               }
>           }
>
> +        /* Ctrl+Alt+Keypad-{Plus,Minus} zoom mode: M_T_USERDEF mode type */
> +        processZoomModes(output);
> +
>           output->initial_rotation = xf86OutputInitialRotation(output);
>
>           if (debug_modes) {
>

-- 
Aaron