[AD] linear bitmaps and putpixel

[Sven Sandberg <svens@xxxxxxxxxx>]

Hello,
Our previous discussion about putpixel (part of the thread "clipping 
line algorithm") seems to have self-died with no conclusion. That thread 
discussed many different topics; I'm starting this thread because I want 
a conclusion on one specific topic.


==== The problem ====

There are three types of bitmaps:
 (1) Bitmaps that support bmp_* operations (these are 
bmp_[read|write][_line|8|15|16|24|32]() and 
_[get|put]pixel[|15|16|24|32]()).
 (2) Mode-X screen bitmaps, supporting a very special way to access 
pixels using outport*.
 (3) Bitmaps that support neither bmp_* nor outport*.

Allegro currently has two (related) problems, and these are what I want 
to discuss:
 (A) Some of Allegro's functions don't work if they are given a type 
(3) bitmap.
 (B) There is no way to detect if a bitmap is of type (3). To elaborate 
a bit on this, the following two functions already exist: 
is_linear_bitmap() returns true for type (1) bitmaps, and 
is_planar_bitmap() returns true for type (2) bitmaps. There is no way 
for a bitmap to say that it is of type (3), and it is not clearly 
defined what is_linear_bitmap()/is_planar_bitmap() should return in this 
case.

Another piece of important background info: is_linear_bitmap() is not 
used for any other purpose than to detect if bmp_* can be used. 
is_planar_bitmap(), on the other hand, is used in many places where 
mode-X bitmaps need special treatment in other ways.


==== Suggested solution (on the level of API) ====

The problem is simple, and I proposed the following simple solution:

First, add a way for a bitmap to say that it is of type (3). Both 
is_linear_bitmap() and is_planar_bitmap() should return false for such 
bitmaps. That solves problem (B).

Second, in the few places (rotate.c, gsprite.c and cstretch.c) where 
treatment of type (3) bitmaps is missing, add it. That solves problem 
(A). This is easy; it consists in using getpixel()/putpixel() to access 
the bitmap. So we would have:

   if (is_linear_bitmap(bmp))      { use bmp_* }
   else if (is_planar_bitmap(bmp)) { use outport* }
   else                            { use putpixel }

(currently we only have
   if (is_linear_bitmap(bmp))      { use bmp_* }
   else                            { use outport* }
)

Do people agree up to this point? Note that I have not yet said how to 
represent the information indicating type (3).


==== Suggested implementation of this solution ====

For the actual implementation of my solution to (B), I suggest the 
following:

Add a flag called BMP_ID_LINEAR to BITMAP->id. Update all places that 
create linear BITMAPs so that they set this flag. Update 
is_linear_bitmap() to look for this flag rather than negating the answer 
from is_planar_bitmap().

Rationale:
 - The reason why I think it should be a flag in BITMAP->id is that 
this can be done while maintaining source and binary compatibility. If 
we instead added new fields to BITMAP or BITMAP->vtable, it would break 
source and binary compatibility (source compatibility would only be 
broken for add-ons like AllegroGL that would need to access the new fields).
 - The reason why I think the flag should be set for linear bitmaps (as 
opposed to one that is set for type (3) bitmaps) is that with this 
solution it is more clear that type (3) is the default, while type (1) 
and (2) are special and optional cases. Thus, a person who rolls her own 
version of create_bitmap() and does not set either flag, will be fine 
since Allegro falls back on the vtable's putpixel(). But a person who 
happens to know that bmp_* works on the bitmap, can set the 
BMP_ID_LINEAR flag to indicate this, allowing better performance.

Do people agree up to this point too? If not, please explain what the 
alternatives are! I don't think I understood that from the previous 
thread. If you agree, I can code it.

This grew longer than I thought, I'd better stop now...

--
Sven