Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cute_aseprite.h - v1.3 Tilesets Discussion #350

Open
RandyGaul opened this issue Jun 8, 2023 · 4 comments
Open

cute_aseprite.h - v1.3 Tilesets Discussion #350

RandyGaul opened this issue Jun 8, 2023 · 4 comments

Comments

@RandyGaul
Copy link
Owner

RandyGaul commented Jun 8, 2023

Thanks to @mathewmariani for the initial PR #349 on parsing tilesets. I went ahead and made some updates to their PR, including a variety of bugfixes 8ce6803.

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
Copy link
Owner Author

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
Copy link
Contributor

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
Copy link
Owner Author

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
Copy link
Owner Author

RandyGaul commented Jul 15, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants