cstefanache / angular2-img-cropper

Angular 2 Image Cropper
MIT License
364 stars 135 forks source link

Wrong cropper dimensions when cropAspect == sourceAspect [BUG] #29

Closed gfranco93 closed 8 years ago

gfranco93 commented 8 years ago

When cropAspect == sourceAspect the cropper dimensions are smaller than they should be. Plunker: https://embed.plnkr.co/GdlRu0q3EEWEnvnwRFom/

The following code is wrong: (Extracted from imageCropper, lines 667 to 688)

if (cropAspect > sourceAspect) {
   var imageH = Math.min(w * sourceAspect, h);
    var cropW = imageH / cropAspect;
    tlPos = PointPool.instance.borrow(cX - cropW / 2, cY + imageH / 2);
    trPos = PointPool.instance.borrow(cX + cropW / 2, cY + imageH / 2);
    blPos = PointPool.instance.borrow(cX - cropW / 2, cY - imageH / 2);
    brPos = PointPool.instance.borrow(cX + cropW / 2, cY - imageH / 2);
} else if (cropAspect < sourceAspect) {
    var imageW = Math.min(h / sourceAspect, w);
    var cropH = imageW * cropAspect;
    tlPos = PointPool.instance.borrow(cX - imageW / 2, cY + cropH / 2);
    trPos = PointPool.instance.borrow(cX + imageW / 2, cY + cropH / 2);
    blPos = PointPool.instance.borrow(cX - imageW / 2, cY - cropH / 2);
    brPos = PointPool.instance.borrow(cX + imageW / 2, cY - cropH / 2);
} else {
    var imageW = Math.min(h, w);
    var cropH = imageW * cropAspect;
    tlPos = PointPool.instance.borrow(cX - imageW / 2, cY + cropH / 2);
    trPos = PointPool.instance.borrow(cX + imageW / 2, cY + cropH / 2);
    blPos = PointPool.instance.borrow(cX - imageW / 2, cY - cropH / 2);
    brPos = PointPool.instance.borrow(cX + imageW / 2, cY - cropH / 2);
}

I believe deleting the 'else' block and changing the 'else if' to just 'else' solves the problem. I'm going to submit a PR in a few hours.

cstefanache commented 8 years ago

image

Are you talking about the fact that the resulting output is not filling the cropped canvas?

image

cstefanache commented 8 years ago

Also, is this affecting your production code? Do you need it released on NPM asap?

gfranco93 commented 8 years ago

Don't worry, it just affects the dev code.

I'm talking about the fact that the cropper selector size (I don't know if that's the proper way to call it, I'm referring to the white rectangle) differs from the size of the picture. The picture size is 200x150, and I setted the cropper selector size to 200x150, so it should cover the whole picture.

If I didn't make myself clear, you should try changing (in the plunker) the cropper selector size to 201x150 and see the difference it makes.

cstefanache commented 8 years ago

Thanks @gfranco93