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

Support aseprite #20

Merged
merged 3 commits into from
Jan 5, 2023
Merged

Support aseprite #20

merged 3 commits into from
Jan 5, 2023

Conversation

rsuu
Copy link
Contributor

@rsuu rsuu commented Dec 11, 2022

aseprite header

All sorted in little-endian

0..4    u32    File size
4..6    u16    Magic number (0xA5E0)
6..8    u16    Frames 
8..10   u16    Width in pixels
10..12  u16    Height in pixels

@Roughsketch
Copy link
Owner

I really appreciate the PR, but I'm not sure if this belongs in a general use library. I've never heard of this format before and it seems to be a custom format for a paid program and not an open standard.

Is there some history of common usage for this format outside the program?

@rsuu
Copy link
Contributor Author

rsuu commented Dec 17, 2022

I really appreciate the PR, but I'm not sure if this belongs in a general use library. I've never heard of this format before and it seems to be a custom format for a paid program and not an open standard.

Is there some history of common usage for this format outside the program?

  1. Aseprite is open source.
    Build from source: https://github.com/aseprite/aseprite/blob/main/INSTALL.md#compiling

  2. The FLIC animation format was originally developed by Autodesk for use with Autodesk Animator (FLI) and Autodesk Animator Pro (FLC). In 1993 Jim Kent had a Dr. Dobbs article with a source code listing, introducing the FLIC format to the public. 1

More information: https://github.com/AnimatorPro/Animator-Pro#animator-pro-aka

In GIMP: https://gitlab.gnome.org/GNOME/gimp/-/tree/master/plug-ins/file-fli


But .ase and .flc are a bit different.

The format is much like FLI/FLC files, but with different magic number and differents chunks. 2

Footnotes

  1. https://en.m.wikipedia.org/wiki/FLIC_(file_format)

  2. https://github.com/aseprite/aseprite/blob/main/docs/ase-file-specs.md#introduction

@Roughsketch Roughsketch self-requested a review January 5, 2023 03:27
Copy link
Owner

@Roughsketch Roughsketch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a heads up that although this PR is pretty much fine, I'd appreciate if you could handle some small review changes.

The bigger thing is since #22 was merged your tests will need to be changed slightly. Basically just move test/aseprite into tests/images/aseprite and modify your test paths accordingly.

If for some reason you cannot do this before the next release I have no issue cleaning it up myself so don't feel pressured to do it ASAP. And as always thanks for the PR!

src/lib.rs Outdated
} else {
println!("{:?}", &header);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove extra print

@@ -260,7 +266,7 @@ fn bmp_size<R: BufRead + Seek>(reader: &mut R) -> ImageResult<ImageSize> {

fn gif_size(header: &[u8]) -> ImageResult<ImageSize> {
Ok(ImageSize {
width: ((header[6] as usize) | ((header[7] as usize) << 8)),
width: ((header[6] as usize) | ((header[7] as usize) << 8)),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To minimize the diff I'd appreciate removing smaller format only changes like this. I can run a cargo fmt at some point in the future.

Not going to tag every single instance, but there are lots of diffs with just spacing changes. For reference I count 11 blocks of them including this one.

@rsuu
Copy link
Contributor Author

rsuu commented Jan 5, 2023

Just a heads up that although this PR is pretty much fine, I'd appreciate if you could handle some small review changes.

The bigger thing is since #22 was merged your tests will need to be changed slightly. Basically just move test/aseprite into tests/images/aseprite and modify your test paths accordingly.

If for some reason you cannot do this before the next release I have no issue cleaning it up myself so don't feel pressured to do it ASAP. And as always thanks for the PR!

ok, files has been renamed and the code has been removed now.

@Roughsketch Roughsketch merged commit 6381e48 into Roughsketch:master Jan 5, 2023
@rsuu rsuu deleted the main branch January 6, 2023 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants