Re: [AD] some small patches

[Elias Pschernig <elias@xxxxxxxxxx>]

On Sat, 2002-11-02 at 22:12, Eric Botcazou wrote:
> > 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.

Yes, probably. I just always get confused by the PALETTE / RGB*
difference - I'll see if it can be changed.

> - can't we get rid of the 332 palette too ?

It's only an example, it actually uses the current palette - just in the
program for which I needed it, I use the 332 palette (which is the most
useful palette IMO and should not be get rid of).

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

When I think about it, I like it more :) I already use the IDLE message
to update the stream. Now if d_dialog_proc would use the above, and
broadcasts IDLE message in between - my problem with it would be solved
and I only need one place to update streams instead of the extra menu
callback.

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

Ah, yes, they were supposed to. I added the ifs when I got lots of
asserts from the scare_mouse_area calls - and forgot that it now
unbalances the unscare_mouse calls. I must think about if it's better to
remove the asserts, or add a flag.

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

I'll shift it by one pixel.

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

Ok, I'll apply it tomorrow.

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

.. At least, not even a spelling checker would have seen it :)

--
Elias Pschernig