[AD] Locking Behaviour

[Paul Suntsov <siegelords_abode@xxxxxxxxxx>]

I have noticed some seemingly sub-optimal behaviour in the memory blitting functions and the al_lock_bitmap_region function.

For the first, why does the routine try to lock the entirety of the destination instead of only the location that is going to be written to?

For example, in _al_draw_scaled_bitmap_memory these lines are used to accoplish the locking:

if (!al_lock_bitmap(dest, &dst_region, 0)) {
  al_unlock_bitmap(src);
  return;
}

I think that using al_lock_bitmap_region would be better.

Secondly, why not check to see if the region we are drawing to is already locked, instead of failing outright if it's already locked? I don't think there is a function like that yet, but perhaps we could make al_bitmap_region_is_locked and al_bitmap_point_is_locked for the purpose.

Thirdly, if the second point is implemented, both the destination and the source bitmaps should remain locked, I think.
More generally, here's how I'd imagine myself these things working: memory routines and such that require locked bitmaps will first check to see if the regions they want to draw to are locked, locking them if they are not. When they are done, they leave them locked. Now, the routines that require unlocked bitmaps (all hardware accelerated ones, at least) check if their required bitmaps are locked or not, and unlock them themselves if they are. al_flip_display, for example, will make sure to unlock the back and front buffers. I think this sort of behaviour would be optimal in terms of the number of required locks/unlocks (I believe this is the sort of strategy A4 used for release/acquire functions). Now, I know this violates the leave-global-state-as-it-was-when-you-got-it strategy, but I think it makes sense in this situation. Right now the functions are too fail-happy for my liking.

Now, about al_lock_bitmap_region itself, right now it fails if the bitmap is locked already:

if (bitmap->locked)
   return NULL;

Would it perhaps be better to unlock the bitmap in this case, and re-lock it as usual? Just seems like less of a hassle this way (I can't imagine any situation when you wouldn't want to do that upon failure anyway, that couldn't be handled via al_is_bitmap_locked check).

I didn't want to make any patches before asking first, so tell me what you think of the above.

SiegeLord