gluckgames / pixi-packer

Fast and flexible texture packer for PIXI.js
MIT License
40 stars 6 forks source link

Support node-only image packing #29

Open ericlathrop opened 8 years ago

ericlathrop commented 8 years ago

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 commented 8 years ago

See the discussion at https://github.com/Ensighten/spritesmith/issues/55

ericlathrop commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

First, thanks for looking at this :+1:

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 commented 8 years ago

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

npm ERR! gmsmith@1.1.2 preinstall: `gm -version || convert -version`
npm ERR! spawn ENOENT
npm ERR! 
npm ERR! Failed at the gmsmith@1.1.2 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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

marekventur commented 8 years ago

That looks good! :+1:

ericlathrop commented 8 years ago

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 commented 8 years ago

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

marekventur commented 8 years ago

@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 :(