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

[New Feature] Autocrop #71

Merged
merged 1 commit into from
Nov 17, 2015
Merged

[New Feature] Autocrop #71

merged 1 commit into from
Nov 17, 2015

Conversation

marcolino
Copy link
Contributor

Autocrop feature added.
Not particularly smart algorithm, but it looks like it does its job... :-)
I did not update the README...

@marcolino marcolino changed the title First autocrop merge [New feature] First autocrop merge Nov 17, 2015
@marcolino marcolino changed the title [New feature] First autocrop merge [New Feature] First autocrop merge Nov 17, 2015
@marcolino marcolino changed the title [New Feature] First autocrop merge [New Feature] Autocrop Nov 17, 2015
oliver-moran added a commit that referenced this pull request Nov 17, 2015
@oliver-moran oliver-moran merged commit cd30807 into jimp-dev:master Nov 17, 2015
@oliver-moran
Copy link
Collaborator

Love the user of JS labels! I've never seen them used in anger before. Very nice. Merged now. Will test and include in the next release. Thanks!

@marcolino
Copy link
Contributor Author

:-) Thanks to you!

Marco Solari
System Analyst, Software Engineer and IT Consultant at Koinè Sistemi Torino

On 17 November 2015 at 14:01, Oliver Moran notifications@github.com wrote:

Love the user of JS labels! I've never seen them used in anger before.
Very nice. Merged now. Will test and include in the next release. Thanks!


Reply to this email directly or view it on GitHub
#71 (comment).

widthOfPixelsToCrop >= 0 ? widthOfPixelsToCrop : 0;
var heightOfPixelsToCrop = h - (southPixelsToCrop + northPixelsToCrop);
heightOfPixelsToCrop >= 0 ? heightOfPixelsToCrop : 0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Safety checks" do not work. Should probably be

var widthOfPixelsToCrop = w - (westPixelsToCrop + eastPixelsToCrop);
var heightOfPixelsToCrop = h - (southPixelsToCrop + northPixelsToCrop);

widthOfPixelsToCrop = widthOfPixelsToCrop >= 0 ? widthOfPixelsToCrop : 0;
heightOfPixelsToCrop = heightOfPixelsToCrop >= 0 ? heightOfPixelsToCrop : 0;

or more compact

var widthOfPixelsToCrop = Math.max(0, w - westPixelsToCrop - eastPixelsToCrop);
var heightOfPixelsToCrop = Math.max(0, h - southPixelsToCrop - northPixelsToCrop);

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

Successfully merging this pull request may close these issues.

None yet

3 participants