RandyGaul / cute_headers

Collection of cross-platform one-file C/C++ libraries with no dependencies, primarily used for games
4.21k stars 264 forks source link

cute_aseprite.h - v1.3 Tilesets Discussion #350

Open RandyGaul opened 1 year ago

RandyGaul commented 1 year ago

Thanks to @mathewmariani for the initial PR https://github.com/RandyGaul/cute_headers/pull/349 on parsing tilesets. I went ahead and made some updates to their PR, including a variety of bugfixes https://github.com/RandyGaul/cute_headers/commit/8ce6803c90d74f4fae9fe22b657eeebc0b24dc20.

For now tiles are stored in ase_cel_t just like pixels. However, there are a few open questions/todos on what to do next to make it really easy to actually use this tile information.

  1. Each cel has some flags for flip on x/y axis, and a flag for 90 degree cw rotation. This unambiguously encodes all flips/rotates possible for square tiles. This is a bit cumbersome and low-level for users to deal with. Instead, should we also convert this information into a set of uv coordinates that are already pre-transformed according to the flags? Is there some other way to express tile instance transforms?
  2. The tileset image itself is currently stored under a void*, in Aseprite's variable-length image encoding format. Pulling pixels out of this is pretty annoying. There is a static helper function in cute_asprite.h's implementation section called s_color that helps with this. Instead, we should probably provide users the tileset image itself as ase_color_t (the return value of s_color).

The user API should look something like this (something like this should probably be in the docs as well as an example):

for (int i = 0; i < ase->frame_count; ++i) {
    ase_frame_t* frame = ase->frames + i;
    for (int j = 0; j < frame->cel_count; ++j) {
        ase_cel_t* cel = frame->cels + j;
        for (int y = 0; y < ase->tileset.tile_h; ++y) {
            for (int x = 0; x < ase->tileset.tile_w; ++x) {
                ase_tile_t* tile = cel->tiles[y * ase->tileset.tile_w + x];
                do_something_with_tile(tile);
            }
        }
    }
}

Where ase_tile_t needs to be created by answering point 1.

For point 2, the tileset pixels are currently stored in ase->tileset.data as a void*. However, it should probably be a pointer to an array of ase_color_t instead.

RandyGaul commented 1 year ago

I don't personally have time to quite make all these changes myself right now, as I'm not currently useing Aseprite tileset feature in any projects. If anyone has time to contribute a PR here that would be greatly appreciated! (@mathewmariani maybe?)

mathewmariani commented 1 year ago

I will have time in July to start looking into these todos.

Another change I would consider looking into would be to separate the zlib decoding into its own function, as the code is duplicated in multiple different spots.

Love the bug fixes! I was thinking of making the same changes to the function names for reading data but felt I would be overstepping.

RandyGaul commented 1 year ago

Good idea. Those two bytes in front of the DEFLATE data are actually zlib header bytes. zlib itself just wraps up the DEFLATE data stream with a couple headers bytes and an adler32 checksum. So that's 2 bytes up-front for the zlib header, and 32-bit checksum at the end. The s_inflate function already handles the 32-bit checksum. Though, we should probably handle the 2 zlib bytes within a refactored s_read_zlib function (or some similar name), and hand that off to s_inflate afterwards.

RandyGaul commented 1 year ago

Looks like there are some linger bugs. Going to have to roll this back for now to a previous version. I had trouble parsing a variety of other aseprite files. Since the userdata chunk refactor was quite significant, I have suspicion this was the culprit.