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

[ Thread Index | Date Index | More lists.liballeg.org/allegro-developers Archives ]


I wrote up a patch for gfx_mode_select, gfx_mode_select_ex and gfx_mode_select_filter to remove the requirement of the second two functions to 'be sure not to pass in uninitialized values'. The new behaviour for all three functions would be to always check for matching values in the driver, mode, and (color_depth) lists from the initial values at the addresses provided by the pointers passed to the three functions, and defaulting to the first entry in the list if not found.

The current behaviour only checks for suggested values in the gfx_mode_select_[ex | filter] functions and also defaults to the first entry. However, the variables what_dialog[GFX_DRIVERLIST].d1 and what_dialog[GFX_MODELIST].d1 are not initialized when called from gfx_mode_select, and so they default to the values currently held by the gfx_mode_dialog DIALOG array.

The current behavior of all three functions also always copies the values of the selections into the address parameters, even when the dialog is cancelled, so I made it so that the values are only copied over when the user okays the dialog.

So basically, I reworked the list index initialization code, made gfx_mode_select adhere to the same policy as gfx_mode_select[ex | filter], made the functions only store the data when OK'd, and reworked the documentation for the three functions to match.

I've attached three files :
gfx_mode_select_filter_modifications.c - The whole source for just the three modified functions. allegro_tx_gmsf_modifications.txt - The whole _tx documentation source for just the 3 functions.

gmsf_mod.diff       - Diff file against the latest SVN for A4.3.11

I attached the .c and the .txt file so you could see the changes in their entirety outside the context of a diff.

Let me know what you guys think about my proposed changes.

Thanks,
Edgar



/* Modified versions of gfx_mode_select_filter and gfx_mode_select to remove
 *  the requirement of 'passing in initialized values' for the list selections.
 *  New behaviour is to always search for initial values at the specified
 *  addresses.
 */

/* Old behaviour of gfx_mode_select_filter :
 *  Only checks for suggested modes when called directly or from gfx_mode_select_ex, but not from gfx_mode_select.
 *  It also doesn't initialize what_dialog[GFX_DRIVERLIST].d1 or what_dialog[GFX_MODELIST].d1 at all when it is 
 *  called from gfx_mode_select (which means the values left over in the dialog are the initial values the next
 *  time that you call gfx_mode_select). The selections are always copied into the addresses held by the pointers, even
 *  when the dialog is cancelled.
 */

/// The lines below replace allegro\src\modesel.c lines 611-758



/* 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. 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.
 *  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);

   clear_keybuf();

   extd = color_depth ? TRUE : FALSE;

   while (gui_mouse_b());

   what_dialog = extd ? gfx_mode_ex_dialog : gfx_mode_dialog;

   what_dialog[GFX_TITLE].dp = (void*)get_config_text("Graphics Mode");
   what_dialog[GFX_OK].dp = (void*)get_config_text("OK");
   what_dialog[GFX_CANCEL].dp = (void*)get_config_text("Cancel");

   ret = create_driver_list(filter);

   if (ret == -1) {
      *card = GFX_NONE;
      return FALSE;
   }

   if (!driver_count) {
      *card = GFX_NONE;
      return TRUE;
   }

   /* 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).
    */
   /* Check for the user suggested driver first */
   what_driver = 0;/* Default to first entry if not found */
   for (i = 0 ; i < driver_count ; ++i) {
      if (driver_list[i].id == *card) {
         what_driver = i;
      }
   }
   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]);
   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_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 = 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;
   }

   centre_dialog(what_dialog);
   set_dialog_color(what_dialog, gui_fg_color, gui_bg_color);
   ret = popup_dialog(what_dialog, GFX_DRIVERLIST);

   what_driver = what_dialog[GFX_DRIVERLIST].d1;
   what_mode = what_dialog[GFX_MODELIST].d1;

   if (extd)
      what_bpp = what_dialog[GFX_DEPTHLIST].d1;
   else
      what_bpp = 0;

   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);
      }
   }

   destroy_driver_list();

   if (ret == GFX_CANCEL)
      return FALSE;
   else 
      return TRUE;
}



/* 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. 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)
{
   ASSERT(card);
   ASSERT(w);
   ASSERT(h);
   ASSERT(color_depth);
   return gfx_mode_select_filter(card, w, h, color_depth, NULL);
}



/* gfx_mode_select:
 *  Displays the Allegro graphics mode selection dialog, which allows the
 *  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);
}


Index: docs/src/allegro._tx
===================================================================
--- docs/src/allegro._tx	(revision 11766)
+++ docs/src/allegro._tx	(working copy)
@@ -13211,30 +13211,38 @@
 @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 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.
 
+   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 +13251,39 @@
    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.
 
-   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
+         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 */
+         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.
+
 @@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,19 @@
 /* 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.
+ *  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 +652,39 @@
       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++) {
-         if (driver_list[i].id == *card) {
-            what_dialog[GFX_DRIVERLIST].d1 = i;
-            break;
-         }
+   /* Check for the user suggested driver first */
+   what_driver = 0;/* Default to first entry if not found */
+   for (i = 0 ; i < driver_count ; ++i) {
+      if (driver_list[i].id == *card) {
+         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]);
+   for (i = 0 ; i < driver_list[what_driver].mode_count ; ++i) {
+      if ((mode_iter->w == *w) && (mode_iter->h == *h)) {
+         what_mode = i;
+         break;
       }
-
-      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;
-
+      ++mode_iter;
+   }
+   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 = 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 +700,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 +720,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 +740,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);
-
-   /* 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);
 }
+
+

/// Modifications to allegro._tx documentation for corresponding changes to
///   the gfx_mode_select_filter and gfx_mode_select functions.

/// To be placed in allegro\docs\src\allegro._tx at lines 13210 - 13251 replacing
///   the current documentation for gfx_mode_select, gfx_mode_select_ex, and
///   gfx_mode_select_filter.



/********************** New version *****************************/

@@int @gfx_mode_select(int *card, int *w, int *h);
@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. 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 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_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 also allows
   the user to select the color depth.
   
   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.

   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
@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
   to select the color depth as well as the resolution and hardware driver.
   `filter' will be passed (card, w, h, color_depth) quadruplets and must
   return 0 to let the specified quadruplet be added to the list of displayed
   modes.

   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.

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







/********************** Old version *****************************/

@@int @gfx_mode_select(int *card, int *w, int *h);
@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.

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

@\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
@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
   to select the color depth as well as the resolution and hardware driver.
   `filter' will be passed (card, w, h, color_depth) quadruplets and must
   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.

   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.



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/