[PATCH] xserver: add monitor Option "ZoomModes"

vdb at picaros.org vdb at picaros.org
Thu Mar 7 07:07:31 PST 2013


> 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.

Well, I settled on "ZoomModes" since xserver/hw/xfree86 functions with 
'Zoom' in their name relate to the Ctrl+Alt+Keypad-{Plus,Minus} mode 
switch.  The 'Modes' part comes from the Section "Screen"/Subsection 
"Display"/Modes statement.  

> > 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.

Indeed, text modified per suggestions, thank you.

> > 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

> > @@ -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;
> > +}

Agreed, code changed per suggestion.

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

There used to be a function in xf86-video-ati/ to process the
Option "MetaModes" for the driver's internal multiple output support.  

I checked xserver/hw/xfree86/{ common, parser} but couldn't find 
anything reusable.  An option statement with a list value, for example

  Option "ZoomModes" "1600x1200" "1280x1024" "1280x1024" "640x480"

would be ideal.  It's almost there: in common/xf86Opt.h ValueUnion 
add a pointer to a list, add support in parser/Flag.c
xf86parseOption() for list values and extend common/xf86Option.c
ParseOptionValue().

But this added complexity doesn't seem worthwhile for a single Option 
user.  So I settled for a string of mode names and a gettoken() 
function.  

> > +/** 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?

Indeed, text amended per suggestion, thank you.

> > + * 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.

It seems a design choice to match mode names with plain strcmp() 
or strncmp().  Examples in xserver/hw/xfree86: 

common/xf86Config.c modeIsPresent(),
fbdevhw/fbdevhw.c   fbdevHWSetVideoModes(),
modes/xf86Crtc.c    xf86ProbeOutputModes() Option "PreferredMode".

Also the now unused modes/xf86Modes.c xf86ValidateModesUserConfig() 
uses a plain prefix match to filter for modes whose name matches a 
listed "Screen"/"Display"/Modes name.  

> > +                    && !(mode->type & M_T_USERDEF)) { /* no rematch */
> > +                    mode->type |= M_T_USERDEF;
> > +                    break;
> > +                }
> > +
> > +            count++;
> > +            next = gettoken(next, &token, &len);
> > +        }
> > +    }
> > +
> > +    return count;
> > +}

The revised patch v2 is here below.  Servaas.
---
 hw/xfree86/man/xorg.conf.man |   11 +++++
 hw/xfree86/modes/xf86Crtc.c  |   89 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 0 deletions(-)

diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
index 5d92bbe..f9f98dc 100644
--- a/hw/xfree86/man/xorg.conf.man
+++ b/hw/xfree86/man/xorg.conf.man
@@ -1676,6 +1676,17 @@ 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 list.
+Since this output is the output connected to the lowest
+dot-area monitor, as determined from its largest size mode, that
+monitor defines the available zoom modes.
+(RandR 1.2-supporting drivers only)
+.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..a49dc3e 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,90 @@ 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 *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 */
+    if (*len > 0)
+        return &(*token)[*len];
+    else
+        return NULL;
+}
+
+/** 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 specification matches the next output mode
+ * with the same name.
+ *
+ * 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 */
+                    && !(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 +1805,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) {
-- 
1.7.4.5



More information about the xorg-devel mailing list