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.

Sign up for GitHub

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

Jump to bottom

Adding spritesheet import - fixes #188 #284

Merged
merged 5 commits into from
Jun 4, 2015
Merged

Adding spritesheet import - fixes #188 #284

merged 5 commits into from
Jun 4, 2015

Conversation

CodeMooseUS
Copy link
Contributor

  • Updated the import dialog to allow users to specify the number of frames in the source image (which defaults to 1 x and 1 y)
  • Setting the frame count for x and y will draw a dotted line in the preview that shows where the image will be split into individual frames
  • Changing the frame count will also auto adjust the image width/height so that it displays the size of an individual frame, which can then be manually changed to resize the resultant frames into the desired final size
  • When imported with a frame count above 1, the source image will be split into the different frames and loaded just as if it were an animated gif
  • This allows users to import existing spritesheet pngs, including those produced by the piskel export function (additional work will be needed to specify the frame layout on export - right now a 'square' layout as shown in the image below will still be exported as a single horizontal strip)

image

James Lissiak added 3 commits June 1, 2015 10:29
Adding spritesheet import
48f24c0 
- Updated the import dialog to allow users to specify the number of frames in the image (which defaults to 1 x and 1 y)
- Setting the frame count for x and y will draw a dotted line in the preview that shows where the image will be split into individual frames
- When imported with a frame count above 1, the source image will be split into the different frames and loaded just as if it were an animated gif
- This allows users to import existing spritesheet pngs, including those produced by the piskel export function
Fixing unnecessary whitespace changes
8e4ea84 
- My editor added additional whitespace to several unchanged lines, so I just reverted them
Fixing the preview frame grid stroke
8ff15fd 
- The width/height of the canvas used to draw the frame grid in the preview was incorrect, so the stroke width was too thick
- This change fixes it so the stroke width remains nice and thin by applying the correct canvas size
@juliandescottes
Copy link
Collaborator

Thanks a lot for your contribution. The code change looks good to me, that's great !

I tried out the feature and I just have one comment : many spritesheets I find have some padding on the right/bottom of the image.
Therefore when you split by X rows, Y columns, it's sometimes impossible to properly isolate the frames. (see http://www.swingswingsubmarine.com/wp-content/uploads/2010/11/dude_animation_sheet.png for instance)

I was wondering if we shouldn't go the other way around, and ask for a frame size. User could specify the expected frame width/height, and adjust it using the grid preview you developed. We would then ignore empty/incomplete frames during the import.

But maybe I was just unlucky with the spritesheets I found ?
What do you think ?

@CodeMooseUS
Copy link
Contributor Author

You are right, I guess a lot of spritesheets are packed into images that are powers of 2 in size, which is fine until the actual frame size is not fully divisible by that, as with your example. Mostly I tend to work with frame sizes that are powers of 2 themselves (16, 32, etc) as I know I'm going to be packing them together, so I didn't think about it.

I guess the better approach is to (as you say) allow to user to specify the frame size, and a possible offset value, and only split into frames where it can fit a full frame of that size (leaving out the gap at the right/bottom).

What are your thoughts on the UI for that? Add a new checkbox for 'split into frames' and if checked, reuse the 'Size' as frame size, and have what I've currently got as 'Frames' to be 'Offset' instead?
Or maybe have a completely new set of 'frame size' and 'frame offset' inputs, so that the user can use the existing 'Size' input to specify the output frame size in case they want to resize the resulting split frames?
It might also be useful to make the preview image bigger so its easier to see where the frames are split, but I couldn't figure out a good way to do that and still leave space for all the options we need.

I could potentially add a checkbox for 'ignore empty frames' in case people want to keep the empty ones for some reason, but maybe that is overkill.

I'll take a look later and see what I can come up with. But any ideas you have would be great.

@juliandescottes
Copy link
Collaborator

Ultimately I think the image import and spritesheet import could be really separated.
But in the meantime, a suggestion could be :

https://screenletstore.appspot.com/img/a8412887-0957-11e5-b018-719238540f86.png

  • move the preview to the right, give it more space (screenshot : at 220px max-width & max height)
  • reduce the popup font size (screenshot : down from 1.5em to 1.3em)
  • add a radio to choose between image import and spritesheet import
  • increase a bit the popup size (screenshot : 550x350)

What do you have in mind for the offset ? If there is padding on the top/left of the image, and not only on the bottom/right ?

I could potentially add a checkbox for 'ignore empty frames' in case people want to keep the empty ones for some reason, but maybe that is overkill.

I tend to agree, it's an overkill, let's just skip the empty or incomplete frames.

Updating the spritesheet import to use size
2d9001d 
- The import dialog now allows users to select an option between single image or spritesheet importing
- The spritesheet option allows setting of the size of an indivdual frame and the offset from the left/top from which to start slicing frames
- Selecting the spritesheet option will display a frame slice grid over the preview image to give a quick view of where the frames will be made
- When importing the spritesheet blank (transparent) frames and also partial frames will be ignored
- This allows users to import spritesheets that have been packed into a larger image with excess padding
@CodeMooseUS
Copy link
Contributor Author

I've updated the code so that you can now specify the size and offset as you suggested. The new UI looks like this:
image

The offset will be useful if a spritesheet is packed into a larger image but does not start at the top left.

Also the 'blank' frames are ignored by comparing the canvas's dataUrl to a empty canvas, which will only work if the background color of the spritesheet is transparent. I did this as its faster than comparing all the pixels in each frame to see if it contains anything. This seems fine to me as the workaround is for a user to just delete empty frames themselves after they have imported it if they use a non-transparent background color. But let me know if you have a better idea.

@CodeMooseUS
Copy link
Contributor Author

Also - Thanks for the great tool, I love this thing :)

@juliandescottes
Copy link
Collaborator

That looks really great ! 👍

Made a few comments in the review, if you still have energy to spare :) It should be ready to merge after that.

💡 Food for thoughts : even with the grid, it can be difficult to spot the "good" sprite size. Any ideas on how we could make this easier ? Maybe a live preview of the import ?

Also - Thanks for the great tool, I love this thing :)

That's really nice to hear ! Thanks a lot for your contribution :)

Updating for pull request feedback
5c46cfe 
- Using labels for the import type radio buttons
- Non animated gifs can now be imported as a spritesheet
- Fixing frame slicing to ignore a partial frame while looping
@CodeMooseUS
Copy link
Contributor Author

Thanks. I've updated the code based on your feedback.

A live preview would be a nice addition. I guess the workaround right now is to just import again until you get it right? So not too bad I hope. Shouldn't be an issue for sheets you've made yourself though, as you already know the size.

@juliandescottes
Copy link
Collaborator

Looks good to me, merging !
I will try to release a new version and upload it to piskelapp.com this weekend.

(Do you have a twitter account ? I'd like to mention you when announcing the release on twitter, if you're ok?)

juliandescottes added a commit that referenced this pull request Jun 4, 2015
@juliandescottes
Merge pull request #284 from JALissiak/spritesheetImport
e773f9a 
Adding spritesheet import - fixes #188
@juliandescottes juliandescottes merged commit e773f9a into piskelapp:master Jun 4, 2015
@CodeMooseUS
Copy link
Contributor Author

I don't really use Twitter. Feel free to say "github user JALissiak" or something. But it's not necessary. Just glad I could be useful :)

@CodeMooseUS CodeMooseUS deleted the spritesheetImport branch June 7, 2015 05:09
@juliandescottes juliandescottes mentioned this pull request Jun 7, 2015
Open
@JFC263 JFC263 mentioned this pull request Dec 8, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CodeMooseUS @juliandescottes