Re: [AD] WIP compressed texture support patch

["Trent Gamblin" <trent@xxxxxxxxxx>]

I'm not very familiar with compressed textures, but are block width and
block size standard sort of things in this domain? For example would PVR
compressed textures be able to use al_get_pixel_block_*? If they're specific
to DDS, does it make sense to have such generic names for them?
 
What is al_pixel_format_is_video_only? Could this be the same thing as
al_pixel_format_is_compressed? When would it differ?

Does the OpenGL flipping thing in any way potentially cause patent issues? I
notice it's almost decoding and re-encoding rows, but I'm no expert.

Well I obviously can't be very thorough in just an hour or two but looks
like you did a good job in general. These things I mentioned are pretty
minor. The technical details of if there's a bug or not, obviously will take
usage to figure out.

-----Original Message-----
From: SiegeLordEx [mailto:slabode@xxxxxxxxxx] 
Sent: December 8, 2014 9:33 PM
To: Coordination of admins/developers of the game programming library
Allegro
Subject: [AD] WIP compressed texture support patch

My new employer makes it a bit difficult for me to OSS projects (typically,
each patch of mine needs to be cleared, which adds a day or so of lag), so I
decided to get this very WIP (please don't merge this!) patch for compressed
texture support some review.

The general idea behind compressed textures is that they allow 6x to 4x
compression ratios on the GPU with reasonable quality for smooth images
(doubt it works well for pixel art). If your game has a lot of art assets,
this would make it easier to fit it all in VRAM. This patch implements the
DXT1, DXT3 and DXT5 compression formats (see
https://en.wikipedia.org/wiki/S3_Texture_Compression for a quick
introduction).

Conceptually, ALLEGRO_BITMAPs (just 'bitmaps' from now on) act very much
like read only (i.e. FBO-less) bitmaps, both in terms of user UI (i.e. 
drawing into them requires falling back to memory drawing) and backing code
(a lot of the FBO-less code is re-used). The key difference is that you
cannot create compressed memory bitmaps. This has nothing to do with the
algorithms per se (they are trivial), but rather with the fact that they are
patented. It seems to me (IANAL) that it's fine to use compressed textures
in Allegro, as long as compression and decompression is done by the drivers
(those drivers thus deal with the licensing). 
This is, in fact, what this patch does.

Bitmap locking is a bit different with compressed bitmaps. You can lock a
compressed bitmap in a non-locked format (i.e. 
compression/decompressing it depending on what flags you use), but you
cannot lock in a compressed format. This is for two reasons. First, the
drivers don't support compressing a non-compressed video bitmap. Second, the
memory layout of these compressed bitmaps is very different from the memory
layout of the usual non-compressed bitmaps, so re-using the API just
wouldn't work.

Instead, I introduced two new locking functions that lock using block
coordinates. E.g. consider:

ALLEGRO_LOCKED_REGION* al_lock_bitmap_region_blocked(ALLEGRO_BITMAP
*bitmap, int x_block, int y_block, int width_block, int height_block, int
flags)

Calling that function like so:

al_lock_bitmap_region_blocked(bmp, 1, 1, 1, 1, flags);

is roughly identical to:

int format = al_get_bitmap_format(bmp);
int bw = al_get_pixel_block_width(format) al_lock_bitmap_region(bmp, bw, bw,
bw, bw, format, flags);

Couple of things to note. These blocked locking functions do not support
format conversion: this simplifies the code quite a bit. These functions
return a 'regular' ALLEGRO_LOCKED_REGION that works roughly the same way.
The only difference is that rows in the locked memory are composed of
blocks.

These blocked locking functions are useful for a few reasons. First, it
simplifies bitmap cloning (otherwise cloning a compressed bitmap would
involve decompressing and re-compressing it). Second, it allows the user to
perhaps create compressed bitmap atlasses. Third, although you can compress
bitmaps using al_lock_bitmap, the quality of compression is variable between
drivers. Thus, it is best to compress beforehand and then load compressed
textures directly. A common format for this is the DirectX DDS file, so I
wrote a simple loader for it (it works for OpenGL too).

So that's the overview of the good parts. The ugly part of this patch is
that, as you all know, we store OpenGL bitmaps upside down. This means that
during blocked locking of compressed OpenGL bitmaps I need to flip the
pixels within the locked blocks (luckily, the compression format is simple,
so this is easy to do). We should give a little bit of thought about this
and see if we want find a different solution to this (perhaps using texture
coordinate matrices). I don't think this is a practical issue (the code to
deal with this is already there and it works), or that it needs to be solved
now, but it may be neater to avoid this issue entirely. With this talk about
5.2, it'd be nice not to write ourselves into a corner with this question...
I think it might be as simple as simply removing the al_use_tex_matrix from
shaders or w/e its called and making the use of the texture matrix
mandatory.

Another issue is that compression/decompression on Windows requires the D3DX
library. I made it a hard requirement for this feature (it still loads the
necessary function dynamically though).

Anyway, I'd prefer if somebody looked over all this and checked if anything
looks crazy. Don't worry about the formatting, etc, I'll fix it later. I'll
also write documentation and tests. Also, I don't know if this works at all
on Android/iOS/OSX, so it'd be nice if this was tested too (at least to see
if it compiles). I only implemented DXT1/3/5 formats, but there are
others... if the ones I implemented don't work, then perhaps some others
will. The framework should be good enough to handle them (although the
OpenGL flipping bit will need to be implemented for them).

The patch is attached, and you can also look at this: 
https://github.com/SiegeLord/allegro5/compare/compression .

Thanks,

-SL