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

rotation seems not to resize correctly #49

Closed
speediro opened this issue Oct 13, 2015 · 9 comments
Closed

rotation seems not to resize correctly #49

speediro opened this issue Oct 13, 2015 · 9 comments

Comments

@speediro
Copy link

When rotating an image where the width is considerably larger than the height over an arbitrary number of degrees, JIMP returns a largely empty rotated image where the new height and width are switched and the image looks correctly rotated. I already tried just switching h and w in the advancedRotate function, but then rotate seems to crop first after this do the rotate so you end up with a cropped result anyway.

var Jimp = require("jimp");

// open a file called "hb.png"
var hb = new Jimp("message.png", function (err) {
    if (err) throw err;

    this.clone().rotate(13).write("./output/msg-rotate-13.png")
});

message

msg-rotate-13

@oliver-moran
Copy link
Collaborator

@speediro - I'm not sure if I understand you correctly but does this answer your question:

When we were developing the rotate function there were two approaches:

  1. We could rotate and resize the image accordingly
  2. We could rotate the image but not resize it

The first approach is non-destructive: all of your original image is still visible but the dimensions of the image will stretch to accommodate the rotation.

The second approach is destructive: the width and height of the image will not have change but after rotation some of your image will outside of the image bounds.

The default behavior is 1. The way to call behavior 2 is to pass false as a second argument to the rotate method:

image.rotate( 13 ); // rotate by 13 degrees and resize accordingly
image.rotate( 13, false ); // rotate by 13 degrees don't resize

Does that answer your issue?

@speediro
Copy link
Author

@oliver-moran - not really.
The examples I posted show the result I get when doing the first.
As you can see, it does not really keep everything of the image. It created a very high image yet rotates over the correct angle.
Can you try my code with the first image I posted?
You should get the second image...

@oliver-moran
Copy link
Collaborator

OK. Sounds like a bug so.

I'll be able to test it (and probably fix it) tonight.

oliver-moran added a commit that referenced this issue Oct 13, 2015
@oliver-moran
Copy link
Collaborator

Fixed. Will do a release tonight with this fix. Thanks for spotting it.

@oliver-moran
Copy link
Collaborator

This is published on NPM. Thanks for spotting it.

@speediro
Copy link
Author

@oliver-moran, nice, looks like it behaves correctly now.And thanks for JIMP and the quick resolution.

@speediro
Copy link
Author

@oliver-moran , might have found another issue with rotation...
Can you check the following test? Is it possible that there are remnants of the first blitting op?

var lenna = new Jimp("lenna.png", function (err) {
    if (err) throw err;
    this.crop(0,200,512,200).write("./output/lenna-cropped.png").rotate(33).write("./output/lenna-rotate-33.png")
});

---edit---
I think I found the issue... I changed (still new at git, is it a pull request I have to create?)

        var max = (this.bitmap.width > this.bitmap.height) ? this.bitmap.width : this.bitmap.height;
        this.resize(max, max);

to

        var max= Math.max(w,h,this.bitmap.width,this.bitmap.height)
        this.resize(max, max);

@oliver-moran
Copy link
Collaborator

Nice! Yes, do a pull request and I'll pull it in.

@oliver-moran oliver-moran reopened this Oct 15, 2015
speediro pushed a commit to speediro/jimp that referenced this issue Oct 15, 2015
oliver-moran added a commit that referenced this issue Oct 29, 2015
Bug fix rotate method #49: use max dimension
@oliver-moran
Copy link
Collaborator

The fix for this has been pulled in. Will appear in the next release. Thanks!

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