Hi there,

In addition to the sound tinkering in my Pull Request I've been playing with code to stream off the shelf .bmp bitmap assets from the SD card to the VDP. It's lead me to two suggestions, the first of which I think is worthwhile regardless of the second:

Bitmap allocation in memory

At the moment you mandate bitmaps to be malloc'd in SRAM, which makes perfect sense if you're expecting them to be less than at most 64x64 for sprite work. But if you want to load in a one-off logo or similar you quickly run out of SRAM and enter undefined behavior territory - usually it just fails silently. PSRAM runs at around about 12MHz equivalent, so at the resolutions we're dealing with it's more than fast enough to stream bitmap data back to the canvas/framebuffer when demanded. I suggest the following change:

if (height*width > 4096) dataptr = (void *)heap_caps_malloc(sizeof(uint32_t)*width*height, MALLOC_CAP_8BIT); //PSRAM

else dataptr = (void *)heap_caps_malloc(sizeof(uint32_t)*width*height, MALLOC_CAP_8BIT | MALLOC_CAP_INTERNAL); // SRAM

bitmaps[current_bitmap].data = (uint8_t *)dataptr; //In either case, assign it.

I've even tested with all bitmaps malloc'd to PSRAM and it doesn't seem to impact any of my existing code or the various sprite demos. But keeping smaller assets in SRAM as before seems a reasonable balance regardless, just in case.

BMP mode

As I mentioned, this came about because I was playing the best way to load in assets from SD on the ez80 side. We don't have the luxury of being able to ingest assets into RAM on the ez80 in order to play with them and stream them in the way the VDP expects for the existing examples. BMP files are the simplest off the shelf asset, and even they have two issues that are hard to get around when you're ingesting a file top-down one char at a time: pixel data is stored bottom-up, and in the form BGR.

My suggestion is a special variant of cmd == 1 in the bitmap/sprite handling code, cmd == 100:

if(cmd == 100) {
//
// BMP mode. Read data to the new databuffer, then flip the whole map vertically (because BMP is stored bottom-up).
//
 	for(n = 0; n < width*height; n++) ((uint32_t *)bitmaps[current_bitmap].data)[n] = (uint32_t)0xFF000000 | ((uint32_t)readByte() << 16) | ((uint32_t)readByte() << 8) | ((uint32_t)readByte());
 	flip((uint32_t *)bitmaps[current_bitmap].data, width, height);
 	debug_log("vdu_sys_sprites: BMP bitmap %d - data received - width %d, height %d\n\r", current_bitmap, width, height);
}

This approach assumes the ez80 doesn't stream an alpha byte for the bitmap, since .bmp shouldn't contain one. It assumes 0xFF, but you could expand this code easily to toggle the alpha value based on the pixel's colour.

The flip() function in question is a dirt simple function to flip any framebuffer vertically, could be done without a function but is exactly the sort of thing we would struggle to do effectively on the ez80:

void flip(uint32_t* framebuffer, int width, int height) {
    uint32_t* row_buffer = (uint32_t*) malloc(sizeof(uint32_t) * width);
    int row_size = width * sizeof(uint32_t);

    for (int y = 0; y < height / 2; y++) {
        uint32_t* top_row = framebuffer + y * width;
        uint32_t* bottom_row = framebuffer + (height - y - 1) * width;

        memcpy(row_buffer, top_row, row_size);
        memcpy(top_row, bottom_row, row_size);
        memcpy(bottom_row, row_buffer, row_size);
    }

    free(row_buffer);
}

As with the sound PR, I recognise there's a balance here between keeping true to the Agon's design spirit with the ez80 firmly in control and the VDP providing the equivalent functions of a period-correct video or sound IC, and making it possible to fully utilise the hardware combination. The UART link helpfully acts as a downward pressure on going too crazy here, since it takes ~10 seconds to transfer a 128x128 BMP file.

Regards,

Greg