Support for other image croppers/comparisons? #12

Closed
dwabyick opened this Issue Oct 18, 2013 · 9 comments

Comments

Projects
None yet
3 participants
@dwabyick
Contributor

dwabyick commented Oct 18, 2013

Great work on Hardy! We're looking forward to using it for our visual testing.

One challenge we have is with speed of running the tests. I took a swing at swapping out the PhantomJS based crop+compare functions with one that relies on GraphicsMagick (or ImageMagick). Performance is way faster.

For my tests on OSX, I'm getting about a 7X improvement in speed, at least.

Current
real 1m9.337s
user 0m27.102s
sys 0m6.516s

My fork
real 0m11.261s
user 0m0.883s
sys 0m0.359s

I imagine you might want to keep the PhantomJS testing for some scenarios, though I've noticed that even on Travis ImageMagick is supported.

I'm wondering what approach you would consider for a Pull Request. Direct swap is obviously easiest, but it could probably be configurable.

Cheers,
-Daniel

@frio80

This comment has been minimized.

Show comment
Hide comment
@frio80

frio80 Oct 18, 2013

+1. There's a noticeable difference the speed here. This could really be beneficial for the image cropping and comparisons.

frio80 commented Oct 18, 2013

+1. There's a noticeable difference the speed here. This could really be beneficial for the image cropping and comparisons.

@thingsinjars

This comment has been minimized.

Show comment
Hide comment
@thingsinjars

thingsinjars Oct 19, 2013

Owner

That is a pretty considerable improvement in speed. Off the top of my head, I'm not sure what the best approach would be. I'd most likely want to keep the PhantomJS tool compatible even if ImageMagick was the default just so there's a fallback for systems without it.

Of course, that shouldn't be hard seeing as they're only a couple of lines each.

In fact, that might be the way to go. Use ImageMagick if it's there and fallback to GhostKnife if not. It's not the simplest development solution but the aim of Hardy is to be as easy as possible for the non-technical user so it should always have a default available. No reason why those who want to can't take advantage of the speed improvements, though.

I'll have a play with the fork and see what the install procedures are like for IM.

(btw, it's super cool that you've looked into this aspect of it, thanks)

Owner

thingsinjars commented Oct 19, 2013

That is a pretty considerable improvement in speed. Off the top of my head, I'm not sure what the best approach would be. I'd most likely want to keep the PhantomJS tool compatible even if ImageMagick was the default just so there's a fallback for systems without it.

Of course, that shouldn't be hard seeing as they're only a couple of lines each.

In fact, that might be the way to go. Use ImageMagick if it's there and fallback to GhostKnife if not. It's not the simplest development solution but the aim of Hardy is to be as easy as possible for the non-technical user so it should always have a default available. No reason why those who want to can't take advantage of the speed improvements, though.

I'll have a play with the fork and see what the install procedures are like for IM.

(btw, it's super cool that you've looked into this aspect of it, thanks)

@dwabyick

This comment has been minimized.

Show comment
Hide comment
@dwabyick

dwabyick Oct 21, 2013

Contributor

Thanks. Yeah, I definitely understand the compatibility urge. It does keep it simple to make it an automatic fallback. One more forward-thinking option would be to make the crop and comparison tool a pluggable API of some sort.

Also, I built an extension (not yet on my branch) that uses GM to create a visual diff of the changes and outputs that file. I'll share that soon, but there's a pull request in GM that needs to get in. ;-)

Contributor

dwabyick commented Oct 21, 2013

Thanks. Yeah, I definitely understand the compatibility urge. It does keep it simple to make it an automatic fallback. One more forward-thinking option would be to make the crop and comparison tool a pluggable API of some sort.

Also, I built an extension (not yet on my branch) that uses GM to create a visual diff of the changes and outputs that file. I'll share that soon, but there's a pull request in GM that needs to get in. ;-)

@thingsinjars

This comment has been minimized.

Show comment
Hide comment
@thingsinjars

thingsinjars Oct 21, 2013

Owner

Sounds interesting. After pulling in the config file change (#11) I started to wonder about an API... Hmmm... there's an idea...

Owner

thingsinjars commented Oct 21, 2013

Sounds interesting. After pulling in the config file change (#11) I started to wonder about an API... Hmmm... there's an idea...

@dwabyick

This comment has been minimized.

Show comment
Hide comment
@dwabyick

dwabyick Oct 25, 2013

Contributor

Any thoughts on this one? We can use our own fork/build for now, but I'd love to figure out a way to get this in.

Contributor

dwabyick commented Oct 25, 2013

Any thoughts on this one? We can use our own fork/build for now, but I'd love to figure out a way to get this in.

@dwabyick

This comment has been minimized.

Show comment
Hide comment
@dwabyick

dwabyick Oct 27, 2013

Contributor

I started working to make the compare/crop functions pluggable ... https://github.com/dwabyick/Hardy/tree/image_refactor.

Next steps is to add a graphics-magick equivalent, and make that an option from the config file, or possibly a 'try and fallback' approach.

Feedback appreciated.

Contributor

dwabyick commented Oct 27, 2013

I started working to make the compare/crop functions pluggable ... https://github.com/dwabyick/Hardy/tree/image_refactor.

Next steps is to add a graphics-magick equivalent, and make that an option from the config file, or possibly a 'try and fallback' approach.

Feedback appreciated.

@thingsinjars

This comment has been minimized.

Show comment
Hide comment
@thingsinjars

thingsinjars Oct 27, 2013

Owner

I'm having some difficulty getting graphics-magick to work properly. I had ImageMagick installed in order to use Wraith but I'm getting a lot of dyld errors when I run this fork. I don't think that's anything to do with the fork itself, more likely my machine's setup. I've tried a few times this week but maybe it's just my computer's way of telling me it wants reinstalled from scratch...

I'll try on a different machine this week and reinstall this one. It'll be a good opportunity to try installing everything clean.

Owner

thingsinjars commented Oct 27, 2013

I'm having some difficulty getting graphics-magick to work properly. I had ImageMagick installed in order to use Wraith but I'm getting a lot of dyld errors when I run this fork. I don't think that's anything to do with the fork itself, more likely my machine's setup. I've tried a few times this week but maybe it's just my computer's way of telling me it wants reinstalled from scratch...

I'll try on a different machine this week and reinstall this one. It'll be a good opportunity to try installing everything clean.

@dwabyick

This comment has been minimized.

Show comment
Hide comment
@dwabyick

dwabyick Oct 27, 2013

Contributor

Awesome. I'm working right now on a version that will gracefully fallback from gm to ghost knife.

Theoretically gm can work with image magick as well, but I haven't tried that.

On Oct 27, 2013, at 10:32 AM, Simon Madine notifications@github.com wrote:

I'm having some difficulty getting graphics-magick to work properly. I had ImageMagick installed in order to use Wraith but I'm getting a lot of dyld errors when I run this fork. I don't think that's anything to do with the fork itself, more likely my machine's setup. I've tried a few times this week but maybe it's just my computer's way of telling me it wants reinstalled from scratch...

I'll try on a different machine this week and reinstall this one. It'll be a good opportunity to try installing everything clean.


Reply to this email directly or view it on GitHub.

Contributor

dwabyick commented Oct 27, 2013

Awesome. I'm working right now on a version that will gracefully fallback from gm to ghost knife.

Theoretically gm can work with image magick as well, but I haven't tried that.

On Oct 27, 2013, at 10:32 AM, Simon Madine notifications@github.com wrote:

I'm having some difficulty getting graphics-magick to work properly. I had ImageMagick installed in order to use Wraith but I'm getting a lot of dyld errors when I run this fork. I don't think that's anything to do with the fork itself, more likely my machine's setup. I've tried a few times this week but maybe it's just my computer's way of telling me it wants reinstalled from scratch...

I'll try on a different machine this week and reinstall this one. It'll be a good opportunity to try installing everything clean.


Reply to this email directly or view it on GitHub.

@thingsinjars

This comment has been minimized.

Show comment
Hide comment
@thingsinjars

thingsinjars Oct 29, 2013

Owner

Brought in #19 on db2f726.

I'll bump the version shortly.

Owner

thingsinjars commented Oct 29, 2013

Brought in #19 on db2f726.

I'll bump the version shortly.

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