-
Notifications
You must be signed in to change notification settings - Fork 13
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
Support aseprite #20
Conversation
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? |
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.
Footnotes |
There was a problem hiding this 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); |
There was a problem hiding this comment.
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)), |
There was a problem hiding this comment.
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.
ok, files has been renamed and the code has been removed now. |
aseprite header