9 comments

[ 3.6 ms ] story [ 32.7 ms ] thread
The readme contains the most important thing missing from the blog post:

Algorithm Overview

1. Find edges using laplace

2. Find regions with a color like skin

3. Find regions high in saturation

4. Generate a set of candidate crops using a sliding window

5. Rank them using an importance function to focus the detail in the center and avoid it in the edges.

6. Output the candidate crop with the highest rank

This is what cargo culting looks like: https://github.com/jwagner/smartcrop.js/blob/master/smartcro... it takes a callback "just for good measure" but is a completely synchronous library.
If the callback were optional, the API call would've been completely fine IMHO.
To give the author the benefit of the doubt, maybe he plans to eventually make the computation happen asynchronously (e.g., in a web worker), or at least maybe he wants to leave that possibility open. Then the API wouldn't need to change.
NO it is not. Curryhoward is absolutely right: The reason behind this decision was to keep a consistent API when smartcrop might get broken into multiple calls to avoid blocking the mainloop for too long.

With that said, this should have been documented more clearly.

It's always easier to layer synchronous semantics on top of asynchronous than the other way around. Maybe the author intends on advanced usage using webworkers or a webservice.
I've been using Reddit's smart cropping code[0], which just uses histograms to achieve a similar effect. If you're looking to crop to a thumbnail size it's orders of magnitude more efficient, while avoiding bad cropping on images where the interesting content is off-centre.

[0] https://github.com/reddit/reddit/blob/e152ff6ad9260d243a8d4e...

While this is cool, the blog and the repo don't really say what this library doesn't do: create good crops of arbitrary images.