On 03/07/13 07:07, v...@picaros.org wrote:
On 11/21/2012 04:12 AM, v...@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.

That's fair.

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.

Sounds fine.

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

Got it.  Thanks for checking that.

+                    && !(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.

Thanks for making these changes.  Version 2 looks good to me:
Reviewed-by: Aaron Plattner <aplatt...@nvidia.com>

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


_______________________________________________
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

Reply via email to