Re: [AD] Update for gfx_mode_select/ex/filter behavior for A4.3.11+

[Edgar <edgarreynaldo@xxxxxxxxxx>]

On 3/8/2009 9:05 AM, Elias wrote :
> As long as it solves the problem, everything is fine to me - require
> that all variables must be initialized to sane values or require that
> they are all initialized to 0 or -1 (then no change in the code is
> required), or require that if *card is GFX_AUTODETECT *w and *h can be
> uninitialized (and don't read their values then).
I just went with both options. I modified the functions to only search 
for initial values when the *parameter is not equivalent to 0 or -1, and 
included this behavior as well as a notification to initialize user 
variables in the documentation. Since searching for GFX_AUTODETECT (0) 
or GFX_TEXT (-1) would always result in not being on the list or being 
first on the list, then both values would always lead to using the first 
entry as the default in the selection list anyway. Take a look at the 
attached patch and let me know if this is suitable.

Regarding any changes to api._tx, I don't see any changes related to 
4.3.10 in there, but in changes._tx instead. Should I just leave 
documenting that part up to you?


Index: docs/src/allegro._tx
===================================================================
--- docs/src/allegro._tx	(revision 11766)
+++ docs/src/allegro._tx	(working copy)
@@ -13211,30 +13211,45 @@
 @xref gfx_mode_select_ex, gfx_mode_select_filter, set_gfx_mode, gui_fg_color
 @shortdesc Displays the Allegro graphics mode selection dialog.
    Displays the Allegro graphics mode selection dialog, which allows the 
-   user to select a screen mode and graphics card. Stores the selection in 
-   the three variables, and returns zero if it was closed with the Cancel 
-   button or non-zero if it was OK'd.
-   If an error occurred, `card' will be set to GFX_NONE and zero will be
-   returned.
+   user to select a screen mode and graphics card.
+   
+   The initial values at the addresses provided by card, w, and h are used as
+   the default selections in the dialog if they are found in the driver and
+   mode lists. If they are not found then the initial selections will be the
+   first in each list. If you wish to ensure that the initial selection is
+   always the first entry, then initialize the data at the addresses passed
+   to the function to the value of 0 or -1. You should always initialize your
+   variables anyway.
+   
+   If the dialog is OK'd, it stores the selections at the addresses passed to
+   the function.
+@retval
+   See the gfx_mode_select_filter function for the return values.
 
-   The initial values of card, w, h are not used.
-
 @@int @gfx_mode_select_ex(int *card, int *w, int *h, int *color_depth);
 @xref gfx_mode_select, gfx_mode_select_filter, set_color_depth, set_gfx_mode
 @xref gui_fg_color
 @eref ex3d, exscn3d, exswitch, exupdate, exzbuf
 @shortdesc Extended version of the graphics mode selection dialog.
-   Extended version of the graphics mode selection dialog, which allows the 
-   user to select the color depth as well as the resolution and hardware 
-   driver.
+   Extended version of the graphics mode selection dialog, which also allows
+   the user to select the color depth.
    
-   This version of the function reads the initial values from the 
-   parameters when it activates so you can specify the default values.
-   In fact, you should be sure not to pass in uninitialised values.
+   As with gfx_mode_select, the values stored at the addresses passed to the
+   function will be used as suggestions for the initial selections in the
+   dialog, defaulting to the first entry in each list if the values are not
+   found. Initialize the data stored at the addresses passed to the function
+   to the value of 0 or -1 if you want to ensure that the initial selection
+   for each list will be the first entry. Always initialize your variables.
 
+   If the dialog is OK'd, it stores the selections at the addresses passed to
+   the function.
+@retval
+   See the gfx_mode_select_filter function for the return values.
+
 @\int @gfx_mode_select_filter(int *card, int *w, int *h, int *color_depth,
 @@                            int (*filter)(int, int, int, int));
-@xref gfx_mode_select, gfx_mode_select_ex, set_color_depth, set_gfx_mode, gui_fg_color
+@xref gfx_mode_select, gfx_mode_select_ex, set_color_depth, set_gfx_mode
+@xref gui_fg_color
 @shortdesc Even more extended version of the graphics mode selection dialog.
    Even more extended version of the graphics mode selection dialog, which
    allows the programmer to customize the contents of the dialog and the user
@@ -13243,13 +13258,45 @@
    return 0 to let the specified quadruplet be added to the list of displayed
    modes.
 
-   This version of the function reads the initial values from the 
-   parameters when it activates so you can specify the default values.
-   In fact, you should be sure not to pass in uninitialised values.
+   As with gfx_mode_select, the values stored at the addresses passed to the
+   function will be used as suggestions for the initial selections in the
+   dialog, defaulting to the first entry in each list if the values are not
+   found. Initialize the data stored at the addresses passed to the function
+   to the value of 0 or -1 if you want to ensure that the initial selection
+   for each list will be the first entry. Always initialize your variables.
 
-   If all the modes were filtered out and the resulting list was empty, `card'
-   will be set to GFX_NONE and non-zero will be returned.
+   If the dialog is OK'd, it stores the selections at the addresses passed to
+   the function.
 
+   Example usage :
+<codeblock>
+   ret = gfx_mode_select_filter(&card, &w, &h, &color_depth, user_filter);
+   if (ret) {/* User okayed dialog or user_filter removed all modes */
+      if (card == GFX_NONE) {
+         // No modes available
+         *card = 0;/* Make sure not to leave *card == GFX_NONE */
+         return -1;
+      }
+      /* Handle changing to new mode here... */
+      
+   } else {/* User cancelled dialog or there was an error (unlikely) */
+      if (card == GFX_NONE) {
+         /* Error, probably out of memory */
+         *card = 0;/* Make sure not to leave *card == GFX_NONE */
+         return -2;
+      }
+      /* Carry on in current graphics mode if that is acceptable */
+   }
+<endblock>
+@retval
+   Returns zero if the user cancelled the dialog or an error occurred. In the
+   case of an error then *card is assigned the value GFX_NONE. The functions
+   return non-zero if the user made a selection OR if all the modes were
+   filtered out. In the case that all of the modes were filtered out, then
+   *card is assigned the value GFX_NONE. This means you should NOT initialize
+   the *card to the value of GFX_NONE, as it could interfere with determining
+   the proper return value.
+
 @@extern int (*@gui_shadow_box_proc)(int msg, struct DIALOG *d, int c);
 @@extern int (*@gui_ctext_proc)(int msg, struct DIALOG *d, int c);
 @@extern int (*@gui_button_proc)(int msg, struct DIALOG *d, int c);
Index: src/modesel.c
===================================================================
--- src/modesel.c	(revision 11766)
+++ src/modesel.c	(working copy)
@@ -611,14 +611,22 @@
 /* gfx_mode_select_filter:
  *  Extended version of the graphics mode selection dialog, which allows the 
  *  user to select the color depth as well as the resolution and hardware 
- *  driver. This version of the function reads the initial values from the 
- *  parameters when it activates, so you can specify the default values.
- *  An optional filter can be passed to check whether a particular entry
- *  should be displayed or not.
+ *  driver. An optional filter can be passed to check whether a particular
+ *  entry should be displayed or not. Initial values for the selections may be
+ *  given at the addresses passed to the function, and the user's selection
+ *  will be stored at those addresses if the dialog is OK'd. Initial values
+ *  at the addresses passed to the function may be set to 0 or -1 to indicate
+ *  not to search for the values but to default to the first entries in each 
+ *  selection list.
+ *  In the case of an error, *card is set to GFX_NONE and FALSE is returned.
+ *  In the case that the filter filtered out all of the modes, *card is set to
+ *  GFX_NONE and TRUE is returned.
  */
 int gfx_mode_select_filter(int *card, int *w, int *h, int *color_depth, FILTER_FUNCTION filter)
 {
    int i, ret, what_driver, what_mode, what_bpp, extd;
+   MODE_LIST* mode_iter;
+   
    ASSERT(card);
    ASSERT(w);
    ASSERT(h);
@@ -647,43 +655,49 @@
       return TRUE;
    }
 
-   /* We try to use the values passed through the argument pointers
-    * as initial settings for the dialog boxes, but only if we have
-    * been called from the extended function.
+   /* The data stored at the addresses passed to this function is used to
+    * search for initial selections in the dialog lists. If the requested
+    * entries are not found, default to the first selection in each list in
+    * order of the driver, the mode w/h, and also the color depth when in
+    * extended mode (called directly or from gfx_mode_select_ex).
     */
-   if (extd) {
-      /* firstly the driver */
-      what_dialog[GFX_DRIVERLIST].d1 = 0;  /* GFX_AUTODETECT */
-
-      for (i=0; i<driver_count; i++) {
+   /* Check for the user suggested driver first */
+   what_driver = 0;/* Default to first entry if not found */
+   /* Don't search for initial values if *card is 0 or -1 */
+   if (!((*card == 0) || (*card == -1))) {
+      for (i = 0 ; i < driver_count ; ++i) {
          if (driver_list[i].id == *card) {
-            what_dialog[GFX_DRIVERLIST].d1 = i;
-            break;
+            what_driver = i;
          }
       }
-
-      what_driver = what_dialog[GFX_DRIVERLIST].d1;
-      what_dialog[GFX_CHANGEPROC].d1 = what_dialog[GFX_DRIVERLIST].d1;
-
-      /* secondly the resolution */
-      what_dialog[GFX_MODELIST].d1 = 0;
-
-      for (i=0; driver_list[what_driver].mode_list[i].w; i++) {
-         if ((driver_list[what_driver].mode_list[i].w == *w)
-              && (driver_list[what_driver].mode_list[i].h == *h)) {
-            what_dialog[GFX_MODELIST].d1 = i;
-            break; 
+   }
+   what_dialog[GFX_DRIVERLIST].d1 = what_driver;
+   what_dialog[GFX_CHANGEPROC].d1 = what_driver;
+   
+   /* Check for suggested resolution dimensions second */
+   what_mode = 0;/* Default to first entry if not found */
+   mode_iter = &(driver_list[what_driver].mode_list[0]);
+   /* Don't search for initial values if *w or *h is 0 or -1 */
+   if (!(((*w == 0) || (*w == -1)) || ((*h == 0) || (*h == -1)))) {
+      for (i = 0 ; i < driver_list[what_driver].mode_count ; ++i) {
+         if ((mode_iter->w == *w) && (mode_iter->h == *h)) {
+            what_mode = i;
+            break;
          }
+         ++mode_iter;
       }
-
-      what_mode = what_dialog[GFX_MODELIST].d1;
-      what_dialog[GFX_CHANGEPROC].d2 = what_dialog[GFX_MODELIST].d1;  /* not d2 */
-
-      /* thirdly the color depth */
-      what_bpp = bpp_index_for_mode(*color_depth, what_driver, what_mode);
-      if (what_bpp < 0)
-         what_bpp = 0;
-
+   }
+   what_dialog[GFX_MODELIST].d1 = what_mode;
+   what_dialog[GFX_CHANGEPROC].d2 = what_mode;
+   
+   /* Check for suggested color depth when in extended mode */
+   if (extd) {
+      what_bpp = 0;
+      /* Don't search for initial values if *color_depth is 0 or -1 */
+      if (!((*color_depth == 0) || (*color_depth == -1))) {
+         what_bpp = bpp_index_for_mode(*color_depth , what_driver , what_mode);
+         if (what_bpp < 0) {what_bpp = 0;} /* Default to first entry if not found */
+      }
       what_dialog[GFX_DEPTHLIST].d1 = what_bpp;
    }
 
@@ -699,14 +713,15 @@
    else
       what_bpp = 0;
 
-   *card = driver_list[what_driver].id;
+   if (ret != GFX_CANCEL) {
+      *card = driver_list[what_driver].id;
+      *w = driver_list[what_driver].mode_list[what_mode].w;
+      *h = driver_list[what_driver].mode_list[what_mode].h;
+      if (extd) {
+         *color_depth = bpp_value_for_mode(what_bpp, what_driver, what_mode);
+      }
+   }
 
-   *w = driver_list[what_driver].mode_list[what_mode].w;
-   *h = driver_list[what_driver].mode_list[what_mode].h;
-
-   if (extd)
-      *color_depth = bpp_value_for_mode(what_bpp, what_driver, what_mode);
-
    destroy_driver_list();
 
    if (ret == GFX_CANCEL)
@@ -718,10 +733,12 @@
 
 
 /* gfx_mode_select_ex:
- *  Extended version of the graphics mode selection dialog, which allows the 
- *  user to select the color depth as well as the resolution and hardware 
- *  driver. This version of the function reads the initial values from the 
- *  parameters when it activates, so you can specify the default values.
+ *  Extended version of the graphics mode selection dialog, which allows the
+ *  user to select the color depth as well as the resolution and hardware
+ *  driver. Initial values for the selections will be looked for at the
+ *  addresses passed to the function, and selections will be stored there if
+ *  the user does not cancel the dialog. See gfx_mode_select_filter for
+ *  details and return values.
  */
 int gfx_mode_select_ex(int *card, int *w, int *h, int *color_depth)
 {
@@ -736,22 +753,17 @@
 
 /* gfx_mode_select:
  *  Displays the Allegro graphics mode selection dialog, which allows the
- *  user to select a screen mode and graphics card. Stores the selection
- *  in the three variables, and returns zero if it was closed with the 
- *  Cancel button, or non-zero if it was OK'd.
+ *  user to select a screen mode and graphics card. Initial values for the
+ *  selection will be looked for at the addresses passed to the function, and
+ *  the selection will be stored in the three variables if the dialog is not
+ *  cancelled. See gfx_mode_select_filter for details and return values.
  */
 int gfx_mode_select(int *card, int *w, int *h)
 {
    ASSERT(card);
    ASSERT(w);
    ASSERT(h);
+   return gfx_mode_select_filter(card, w, h, NULL, NULL);
+}
 
-   /* Make sure these values are not used uninitialised.
-    * This is different to the other gfx_mode_select_* functions.
-    */
-   *card = GFX_AUTODETECT;
-   *w = 0;
-   *h = 0;
 
-   return gfx_mode_select_filter(card, w, h, NULL, NULL);
-}