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

Pixel drift when creating sprite frames from sprite sheet #52099

Closed
yrk06 opened this issue Aug 25, 2021 · 5 comments · Fixed by #52461
Closed

Pixel drift when creating sprite frames from sprite sheet #52099

yrk06 opened this issue Aug 25, 2021 · 5 comments · Fixed by #52461

Comments

@yrk06
Copy link
Contributor

yrk06 commented Aug 25, 2021

Godot version

3.3.stable.oficial

System information

Windows 10, GLS3, GTX 1080 v471.68

Issue description

The built in sprite sheet splitter has some pixel drift when trying to use large sprite sheets, for example:
This is the sprite sheet in my image editor
Tileset_error

It has 7 sprites in a line and 10 sprites in a column, but when trying to split it in the sprite frames editor, the following happens:

Tileset_error_engine

As you can see in the bottom rows, it splits the sheet in a position off-grid (earlier than it should).

Considering how the artifact is most noticeable in bigger sprite sheets, I believe this pixel drift has to do with the rounding made by the splitting algorithm and it builds up as the size gets bigger

Steps to reproduce

Create a big sprite sheet (in my case I used a 7x10 sprite sheet) and try to create sprite frames using it

Minimal reproduction project

No response

Bugsquad edit (keywords for easier searching): SpriteFrames

@kleonc
Copy link
Member

kleonc commented Aug 25, 2021

Considering how the artifact is most noticeable in bigger sprite sheets, I believe this pixel drift has to do with the rounding made by the splitting algorithm and it builds up as the size gets bigger

Sizes of images/frames are integer only, there's no rounding involved in here (but there's truncation). So most likely the problem here is the size of your sprite sheet (height not being a multiply of 10). For example if height of your sprite sheet is 428 pixels and you've set the number of vertical frames to 10, then frame's height won't be 42.8, it will be truncated to 42 (which will result in last 428 - 10 * 42 = 8 rows of pixels not being a part of any frame).
To confirm that's the issue please provide your sprite sheet (or just tell what's its size).

@yrk06
Copy link
Contributor Author

yrk06 commented Aug 25, 2021

The sprite sheet size is 168x279px, it's made of 7x10 sprites and each sprite is 24x28 in size.
I'd provide the sprite sheet but since it is not mine I'd rather just provide the size to avoid any licensing issues

(but there's truncation)

I got confused with the names, sorry, but what I mean is basically what you wrote in the response

then frame's height won't be 42.8, it will be truncated to 42

A possible (and maybe more intuitive?) solution could be instead of using number of rows and columns, have the sprite size as the input which will give always the correct sprites regardless of the sheet size, and the algorithm could just ignore any leftover space ( if the image dimension is not a multiple of the sprite size and there's some leftover space on the edge of the sheet)

@kleonc
Copy link
Member

kleonc commented Aug 25, 2021

The sprite sheet size is 168x279px, it's made of 7x10 sprites and each sprite is 24x28 in size.

What you've stated here is impossible without some frames overlapping each other, (7, 10) * (24, 28) = (168, 280) > (168, 279) and that's why it works like it does. According to the height of the texture (279) max valid integer frame height (assuming non-overlapping frames) is 27 because 10 * 27 = 270 <= 279 and 10 * 28 = 280 > 279. That's why your frames end up with (24, 27) size, not (24, 28) size.

A possible (and maybe more intuitive?) solution could be instead of using number of rows and columns, have the sprite size as the input which will give always the correct sprites regardless of the sheet size, and the algorithm could just ignore any leftover space ( if the image dimension is not a multiple of the sprite size and there's some leftover space on the edge of the sheet)

This shouldn't be hard to implement, I'll look into it.

@yrk06
Copy link
Contributor Author

yrk06 commented Aug 25, 2021

What you've stated here is impossible without some frames overlapping each other, (7, 10) * (24, 28) = (168, 280) > (168, 279) and that's why it works like it does. According to the height of the texture (279) max valid integer frame height (assuming non-overlapping frames) is 27 because 10 * 27 = 270 <= 279 and 10 * 28 = 280 > 279. That's why your frames end up with (24, 27) size, not (24, 28) size.

You are right, I've just checked it and there was one line of pixels missing from the sprite sheet, funnily enough all sprite sheets I used to test this were missing one line or one column and I didn't notice it.

Now on the solution I proposed, I don't believe anymore that it should be changed. But there could be the options to split based on the sprite size (which would prevent this error) or have the editor display a warning, telling the user that the truncation is hapening (saying that either the row and column values are wrong, or the file dimensions are wrong)

@kleonc
Copy link
Member

kleonc commented Aug 25, 2021

Now on the solution I proposed, I don't believe anymore that it should be changed. But there could be the options to split based on the sprite size (which would prevent this error) or have the editor display a warning, telling the user that the truncation is hapening (saying that either the row and column values are wrong, or the file dimensions are wrong)

Yeah, I was thinking about adding some toggle to change the mode or something like that, not about replacing the current mode in favor on the new one. And mode where you enter frame size wouldn't solve the issue with a too small sprite sheet. In your example if you'd enter (24, 28) frame size then it would produce 7x9 frames, not 7x10.

Adding some warnings/info seems like a good idea, indeed.

I've just started to take a look at the proposals mentioning SpriteFrames and here's one: godotengine/godot-proposals#1695 proposing just what you've suggested in here.


@Calinou, All in all, there's no bug in here. However, the problem is that first/last vertical/horizontal lines are not being drawn so it's not visible where the frame really ends:
firefox_my2Z6m5qDqq firefox_my2Z6m5qDq

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

Successfully merging a pull request may close this issue.

4 participants