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 node-only image packing #29

Open
ericlathrop opened this issue Jun 14, 2016 · 12 comments
Open

Support node-only image packing #29

ericlathrop opened this issue Jun 14, 2016 · 12 comments

Comments

@ericlathrop
Copy link

I'd like to avoid requiring users to install ImageMagick for an easier setup experience across win/mac/linux. Since pixi-packer uses the gmsmith engine from spritesmith, maybe we can have a configuration option to use the pixelsmith engine. I'm willing to work on a PR for this if its something that could be accepted.

@ericlathrop
Copy link
Author

See the discussion at twolfson/spritesmith#55

@ericlathrop
Copy link
Author

I hacked out a working proof-of-concept fairly quickly. Still need to make the engine configurable, and add some tests. https://github.com/ericlathrop/pixi-packer/commit/6fb00567fe125f96d9fcc4fd89df57f41353be30

@ericlathrop
Copy link
Author

I wanted to make sure gmsmith would break on a machine without imagemagick without uninstalling imagemagick, so I renamed my convert program and unset PATH, but it still worked! Any ideas on how to fool it?

@marekventur
Copy link
Contributor

First, thanks for looking at this 👍

As you've probably seen, the bits in https://github.com/Gamevy/pixi-packer/blob/master/lib/image_processor.js would need redoing. combine should be easy since there are drop-in replacements, but trim and scale will probably require some work.

The best way of having multiple options might be to go down a similar route as spritesmith's engine modules. Don't worry about that bit for now though, I'm happy to pick up a working branch and separate the modules out.

I've looked at gm's source code, if you want to make sure it can't use gm, you could edit https://github.com/aheckmann/gm/blob/master/lib/command.js#L206 in your node_modules folder and set bin to something nonsensical.

@ericlathrop
Copy link
Author

I tried getting pixi-packer set up on another computer that doesn't have ImageMagick, and npm install failed with:

npm ERR! [email protected] preinstall: `gm -version || convert -version`
npm ERR! spawn ENOENT
npm ERR! 
npm ERR! Failed at the [email protected] preinstall script 'gm -version || convert -version'.

...meaning people can't even install this module without having ImageMagick installed.

I was hoping to just leave gmsmith as the default engine to avoid a breaking change, but we'd have to change the defaualt to pixelsmith in package.json to allow people without ImageMagick to use this. This should probably be a major version bump. Is this acceptable?

@marekventur
Copy link
Contributor

yes, I'm fine with that, version numbers are cheap. There could be two engine-modules, one for gm and one for pure node. Whether or not there should be a default one that has no side-effects (like spritesmith does with pixelsmith) is something we can decide later.

@ericlathrop
Copy link
Author

Just to keep you updated, I was able to write a pure-javascript image scaling function using the get-pixels, save-pixels, and ndarray-warp modules, which is the same underlying code that the pixelsmith engine uses. I'll need to extract some sort of engine adapter to so that when we're using pixelsmith it uses the pure-javascript scaling code, and when we're using gmsmith it uses gm to scale. Once I get that working, I can figure out how to implement trim.

@ericlathrop
Copy link
Author

Here's the scaling code, in case you're curious: https://github.com/ericlathrop/scale-image/blob/master/index.js

@marekventur
Copy link
Contributor

That looks good! 👍

@ericlathrop
Copy link
Author

I've gotten the pure-javascript trim code working. You can see it here: https://github.com/ericlathrop/scale-image/blob/master/trim.js

The next thing to do is integrate these code chunks into pixi-packer in some sort of spritesmith engine adapter layer. I'm headed out of town on vacation, so I probably won't get to that for a week or so.

@marekventur
Copy link
Contributor

That looks great! thanks for doing that, looking forward to seeing it all come together. Enjoy your vacation!

@marekventur
Copy link
Contributor

@ericlathrop Just a heads up, I've done some refactoring on image-processor.js in #33 which removes the unrelated queuing logic from that class.

This should make your work easier, but you might however have to do some rebasing on your fork. Sorry about that :(

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

No branches or pull requests

2 participants