Re: [AD] Rationale behind al_lock_bitmap* parameters

[Elias Pschernig <elias.pschernig@xxxxxxxxxx>]

On Thu, 2008-12-25 at 20:17 -0800, Paul Suntsov wrote:
> Why does al_lock_bitmap_region and al_lock_bitmap require a
> ALLEGRO_LOCKED_REGION* to be passed to it? This is also puzzling as
> the two functions return this very region after they are done filling
> it out (one imagine the return type to be a void in this case). I
> would think it be more logical to have a persistent
> ALLEGRO_LOCKED_REGION that is always attached to the bitmap (I think
> this is the case already), rather than forcing a user to pass a new
> region that simply gets copied to the ALLEGRO_BITMAP's region member
> variable.

My first concern was, what if users keep the pointer around after the
bitmap is unlocked? However, accessing the locked region with the
current API after the bitmap has been unlocked is just as bad. In fact,
with your method, we could set the data field of ALLEGRO_LOCKED_REGION
to NULL after unlocking so it could actually be considered slightly
safer.

> 
> On a similar note, the region parameter should be optional, as the
> primary usage of al_lock_bitmap* is to speed up the memory functions,
> and not to manually alter the bitmap memory, in my mind. In such uses
> the current setup requires a dummy ALLEGRO_LOCKED_REGION whose only
> purpose is to be filled out by the function.
> 
> In my mind, the function signature should look like this for the
> al_lock_bitmap function:
> 
> ALLEGRO_LOCKED_REGION *al_lock_bitmap(ALLEGRO_BITMAP *bitmap, int
> flags)

Well, make it:

ALLEGRO_LOCKED_REGION const *al_lock_bitmap(ALLEGRO_BITMAP *bitmap, int
flags)

-- 
Elias Pschernig <elias@xxxxxxxxxx>