viniciusgerevini / godot-aseprite-wizard

Godot Editor plugin to help import Aseprite animations to AnimationPlayers, AnimatedSprites and SpriteFrames.
MIT License
891 stars 42 forks source link

Inconsistent AtlasTexture displaying when Trim Mode is active(?) #39

Closed Mickeon closed 2 years ago

Mickeon commented 3 years ago

Hello again! Thank you so much for implementing my proposal and its configurations for every user to enjoy!

I was in the process of importing my work to utilise this plugin. Now, the Trim Mode option is pretty useful for saving space. When enabled however, and I don't know if this is intentional, animations are offset unexpectedly. At least, not in the same way as they're shown in Aseprite. For example, here's a simple ball animation. In Aseprite, and when exported with Trim Mode, the ball moves up and squishes down. https://user-images.githubusercontent.com/66727710/142782410-9d672d4a-3a5f-4f70-acd6-eb656cbd7c67.mp4

However, in Godot, the ball squishes while stationary. https://user-images.githubusercontent.com/66727710/142782519-26b106a4-7261-4585-ad30-3b098b9d3098.mp4

I didn't give up on trimming, since I would've loved that bit extra memory squeezing. I ended up "fixing" this by changing the _create_atlastexture_from_frame() function to take in account how much the sprite was trimmed and applying it to the atlas' margin:

func _create_atlastexture_from_frame(image, frame_data):
    var atlas = AtlasTexture.new()
    var frame = frame_data.frame
    var sprite_source_size = frame_data.spriteSourceSize
    var source_size = frame_data.sourceSize

    atlas.atlas = image
    atlas.region = Rect2(frame.x, frame.y, frame.w, frame.h)
    if (frame_data.trimmed):
        atlas.margin = Rect2(
                sprite_source_size.x, sprite_source_size.y, 
                source_size.w - frame.w, source_size.h - frame.h
        )
    return atlas

Once again, I do not know if this is intentional. Could it be that messing with the AtlasTexture's margin has negative effects on textures not used in SpriteFrames? Or, likely, the original behavior is completely fine, since the sprite is being trimmed in memory, and is something to be expected. I'm not sure, I just wanted to document what I believed to be an inconsistency.

viniciusgerevini commented 3 years ago

Hello! The current behaviour is like that by design, but to be fair I didn't put much thought on the size aspect of it. The behaviour you are suggesting is more concerned with file size and not as much with removing padding from the animation. It could be a good internal improvement, though.

The current behaviour does trimming frame by frame. It becomes useful when you create your animations with movement in Aseprite, but when moving to Godot you wish to control its position via code.

A good example could be a kinematic character with a jumping animation. In Aseprite you would be able to visualise the jump better by having the character changing position across frames, and when importing it in Godot it would be all centralised, so you wouldn't need to adjust the collider to match the position on the sprite.

I don't think it makes sense to expose this as a trimming option as those are mostly visual and the new one would look virtually the same as not using trimming. We could enable it as a file size improvement for non-trimmed files. We could enable it by default and add it as a global configuration, so people can disable it, in case they want the full spritesheet for some reason.

I can't think of any issues with the way you implemented it, but we would have to run some extra tests to be sure.

Thanks for all the suggestions. It helps to make the plugin better for all.

viniciusgerevini commented 3 years ago

Hello @Mickeon . I ran some tests with trimming and I found out that there is no improvement on the resource file by using the proposed solution. In fact, it's even worse.

No big deal, because we are talking about bytes here, but it's worth to note.

Here are some examples. The green underlines are the source pngs. The red underline are the resource files.

Trimming vs no trimming Trimming vs no trimming 2

In the first image, jumper is a small enemy, the canvas size is 29x26. Container has a canvas size of 160x160, but only 80x96 is used.

The second image has a test I made where I created a file with a canvas size of 100x100, 5 frames, having only one pixel printed.

You can see even though the png changes significantly, the resource remains almost the same. Maybe Godot optimises the transparent part in the png already, that's why we don't see much difference. The extra bytes are probably the extra margin info we need to add to each frame for the trimming solution.

There is one benefit on the trimming, though. As mentioned on the issue #32 , Godot has a limit on file width and height. I was working on a boss animation today, and my canvas was too big, blowing this limit. With the internal trimming I was able to import the file with no problem.

I´ll add this as an improvement for the big file problem, but as there is a negative impact on file size (even if small) I'll leave it disabled by default for now.

One extra piece of info. Using the trim mode in the wizard screen (the one that doesn't add margin) saves 17 bytes from the test file. In summary, trimming with margin added ~18%, while just trimming save ~1.7% (meh).

Mickeon commented 3 years ago

Maybe it could be combined with the Aseprite command --sheet-pack if the image is too wide, to make sure that the final image is as small as it can be for those scenarios? Although, that would cause inconsistent image sizes as frames are added or removed, which is barely a problem for SpriteFrames, but for the other exported resource types, it very much is.

A new export variable to control how many columns are generated in the sprite sheet, instead of always being a strip? It could be inserted as the argument of Aseprite's --sheet-columns. In theory that would allow a lot more of the image to be usable, and would even allow the trimming to act vertically. In practice, though, I am not sure. It may be a value most users do not need.

viniciusgerevini commented 2 years ago

Yeah, I'm intending to add --sheet-pack in a upcoming release. I've been trying it and it works well.

About the other resource types, they were added with the importer as a nice-to-have, but as I haven't heard of anyone using that I don't give them the same importance as the main Spritesheet. In other words, I'd rather remove them (or changing their behaviour), than sacrificing features from the main resource.

I'm also fixing an issue that does not allow me to use --ignore-empty. Those two changes may improve file size.

I might not go forward with adding the built-in trimming. As I mentioned, the file size is bigger and does not really solve the problem with sprite dimensions limit, as I can still reach them. I'll have to implement other workarounds for the big sprite issue (or maybe just accepting it as a caveat.