Re: [AD] some small patches

[Eric Botcazou <ebotcazou@xxxxxxxxxx>]

> 1. nopal.diff
> Doesn't create a useless palette when NULL is passed to load_bitmap and
> a truecolor image is loaded. (Useful when loading true-color images into
> 8bit modes, for example when using the generate_332_palette
> truecolor-emulation.)

Good idea, but:
- why to add a new parameter to _fixup_loaded_bitmap() ? Can't we simply pass 
a NULL palette instead ? Of course, it shall never be NULL if the loaded 
bitmap is 8-bit.
- can't we get rid of the 332 palette too ?

> 2. menu.diff
> Provides gui_menu_callback. Extremely useful, for example it's the only
> way I know (but probably I'm overlooking something obvious) to write a
> music player (using streams) with a GUI-menu.

No, I think you're right, there is currently no way to "preempt" the menu 
code. But I wonder if an API similar to that of dialogs would not be better:

   MENU_PLAYER *player = init_menu(menu);

   while (update_menu(player)) {
      /* do whatever you want here */
   }

   return shutdown_menu(player);

What do you, and the other developers, think about that ?

> 3. noflicker.diff
> This reduces the flicker of the mouse cursor when a GUI object somewhere
> else on the screen is redrawn. (For example, in a music player which
> updates a waveform, it now only flickers when over the waveform object,
> instead of all the time.)

Good idea. But I think the scare_mouse/unscare_mouse calls must always be 
balanced.

> 4. euro.diff
> Euro character. Even in X-windows they added it, and my keyboard also
> has it. (Btw., I just sort of merged the C and = characters to get ?, no
> idea if this is the right way).

The bottom cross-stroke is shifted to the left with regard to the top 
cross-stroke.

Document the new character in the header of the file, add a blank line then 
the following line:
  Elias Pschernig added the Euro character.

+/* euro character */
+static FONT_GLYPH* euro_data[] =
+{
+	&f_0x20AC
+};

No tab.

 static FONT_MONO_DATA extended_a_monofont = {
     0x100, 0x180,               /* begin, end characters */
     extended_a_data,            /* the data set */
-    0                           /* next */
+    & euro_monofont             /* next */
 };

No space after the '&'.

Ok with these changes, if you add the very same line to fr.cfg (in the 
[key_altgr] section).

> And this is a small comment in the docs for set_clip, because I got
> caught by it:
> 5. clipcomment.diff

+   Note: If you pass coordinates which would set the clipping rectangle
+   completely out of the bitmap, they will be adjusted to be inside the
+   bitmap.

Too vague. 

+   Although note that it's not possible to
+   set the clipping rectangle to the point in the upper left corner of a
+   bitmap (see above about all zero parameters).

Also note... ?

-- 
Eric Botcazou