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

Base64/Canvas Result Cutting off Right Edge #247

Closed
caseyli opened this issue Nov 17, 2016 · 38 comments
Closed

Base64/Canvas Result Cutting off Right Edge #247

caseyli opened this issue Nov 17, 2016 · 38 comments
Labels

Comments

@caseyli
Copy link

caseyli commented Nov 17, 2016

I'm running into an issue with the latest build of croppie.js. (as in, it only started happening with the new build - seemed to not have this issue in a previous build).

It seems to only happen when I drag the image to the left (leaving empty space to the right of the image). When I get the 'canvas' / 'base64' result (without any other arguments) and assign it to an image tag src as a preview, it seems to cut off some pixels.

Here's what it looks like in croppie:

dragimagetoleftsource

And then when I get the result and preview it:

dragimagetoleftresult

If I drag the image to the right (leaving empty space on the left), it doesn't happen:

Croppie:

dragimagetorightsource

and everything works fine:

dragimagetorightresult

What seems consistent with the problem is that if I specify a height and width when calling "result()", it squashes the image into that cut off area.

screen shot 2016-11-17 at 8 31 55 am

Again, this only happens when dragging the image to the left.

Is anybody else seeing this issue?

@caseyli
Copy link
Author

caseyli commented Nov 17, 2016

Looks like I've narrowed down the issue to lines 976 - 992 of croppie.js. The stuff that starts with

start fixing data to send to draw image for enforceBoundary: false

If that's commented out, seems to work fine again.

@thedustinsmith
Copy link
Contributor

Sorry for the delay - Thanks for the very detailed description.

I think the code you're referring to is new. The problem is, Chrome and firefox are fine with drawImage values that exceed the width and height of the image. However, safari and IE return a blank image. So that was my attempt to adjust the values in a way that would make all browsers happy.

I'm trying to replicate the error that you're getting with the images on the demo site, and I can't get it to happen (even when dragging the image to the left). Any thoughts on that?

@caseyli
Copy link
Author

caseyli commented Dec 8, 2016

hey @thedustinsmith , so I've been trying to extract out the code from our codebase to get exactly what I was experiencing, but am actually having issues reproducing outside of our codebase.

I will look into it more to see if it's maybe just something we're doing on our side. We're using croppie with React, and maybe there are some issues with passing the data around.

@tscrady
Copy link

tscrady commented Dec 10, 2016

@caseyli I commented out those 4 if statements and it seems to have worked. For me, I only had that missing image data if I rotated the image.

@chadidi
Copy link

chadidi commented Dec 13, 2016

did any one find a solustion

@bellsenawat
Copy link

@chadidi I found someone face this problem http://stackoverflow.com/questions/36565998/drawimage-show-blank-when-capture-size-over-source-image-in-safari
I trying to find a problem too

@chadidi
Copy link

chadidi commented Dec 13, 2016

@bellsenawat the code in stackoverflow is the same at this plugin at line 976

@bellsenawat
Copy link

bellsenawat commented Dec 13, 2016

@chadidi I dont think so,

    // start fixing data to send to draw image for enforceBoundary: false
    if (left < 0) {
        startX = Math.abs(left);
        left = 0;
    }
    if (top < 0) {
        startY = Math.abs(top);
        top = 0;
    }
    if ((left + width) > self._originalImageWidth) {
        width = self._originalImageWidth - left;
        outWidth = width;
    }
    if ((top + height) > self._originalImageHeight) {
        height = self._originalImageHeight - top;
        outHeight = height;
    }
 ctx.drawImage(this.elements.preview, left, top, width, height, startX, startY, outWidth, outHeight);

and

if (sx < 0) {
sw += sx;
sx = 0;
}

if (sy < 0) {
sh += sy;
sy = 0;
}

if (sx + sw > image.width) {
sw = image.width - sx
}

if (sy + sh > image.height) {
sh = image.height - sy
}
ctx.drawImage(this.elements.preview, sx, sy, sw, sh, dx, dy, dw, dh);

@chadidi
Copy link

chadidi commented Dec 13, 2016

@bellsenawat i changed variables to this and still the same

if (left < 0) {
width += left;
left = 0;
}
if (top < 0) {
height += top;
top = 0;
}
if (left + width > self._originalImageWidth) {
width = self._originalImageWidth - left
}
if (top + height > self._originalImageHeight) {
height = self._originalImageHeight - top
}
ctx.drawImage(this.elements.preview, left, top, width, height, startX, startY, outWidth, outHeight);

@bellsenawat
Copy link

@chadidi yes me too, did you get undefined from self._originalImageWidth and self._originalImageHeight variable ???

I get it and i change it to this.elements.preview.width and this.elements.preview.height

but still the same

@chadidi
Copy link

chadidi commented Dec 13, 2016

@bellsenawat no i didn't get undefined using both of them what size is your images i am usin smal size 168px * 168px and it working 100% but with the big size 1400px * 425px it's not

@thedustinsmith
Copy link
Contributor

If anyone can put together a codepen/fiddle/jsbin demonstrating the issue, I'm sure we can get it fixed. The lines you guys are referring to were implemented to fix a IE and a safari bug with drawing image data outside of the boundaries of the image. I can try to use the code from the stack overflow post, but they "abort" the operation, which I'd like to avoid, if possible.

@bellsenawat
Copy link

@thedustinsmith I will to upload my code to fiddle, by the way I use this lib with angular.js project, i am not sure the problem came from angular or this lib.

@chadidi
Copy link

chadidi commented Dec 13, 2016

@bellsenawat i am sure it's from the library in my case some images crop just fine and some are not and this is my viewport: {width: 1400,height: 425,type: 'square'},

1481642265_chadidi

1481642304_chadidi

@bellsenawat
Copy link

bellsenawat commented Dec 13, 2016

@chadidi I will try in a difference dimensions, Did you set enforceBoundary= false ?
you can see my example cropping image #258

@chadidi
Copy link

chadidi commented Dec 13, 2016

@bellsenawat when i set enforceBoundary: false, it became worse .
and i am using chrome

@bellsenawat
Copy link

@chadidi I have to set false because i need user can zoom out image and can replace image as they want in any place of canvas, so its work perfect for chrome version 2.2, After that I found its show blank image and i update lib to 2.4 it worse for chrome and safari :(

@chadidi
Copy link

chadidi commented Dec 13, 2016

@bellsenawat test it with Microsoft Edge and it's working ;)

@chadidi
Copy link

chadidi commented Dec 16, 2016

did any one found a solution

@bellsenawat
Copy link

@chadidi did you found a solution ?

@bellsenawat
Copy link

@thedustinsmith sorry for late reply, I test on demo and it happen like this issue
I just test in your demo of this lib.

  1. Edit demo.js just add enfore = false
    screen shot 2016-12-22 at 3 12 33 pm

  2. I want to crop image like this

screen shot 2016-12-22 at 3 12 26 pm

  1. this is the result by the way version 2.2 never happen for me
    screen shot 2016-12-22 at 3 12 38 pm

  2. for cropping other possition
    screen shot 2016-12-22 at 3 18 14 pm

@chadidi

@bellsenawat
Copy link

@thedustinsmith Could you test your demo like my previous post ? I think its a problem :(

@thedustinsmith
Copy link
Contributor

@bellsenawat Thanks for the detailed steps. This time of year is really busy for us, so I'll get to it as soon as I can. It just might be a while. Sorry :(

@thedustinsmith
Copy link
Contributor

Alright, I've got a potential fix in master, but I could really use a lot more eyes on it. Does the code in master solve everyone's issue? Does it break anyone's issue they weren't having before?

@thedustinsmith
Copy link
Contributor

Also, I only saw the issue when I had the size option set on the result method. I'm not sure if that's what everyone else was doing, but it's the only way I could get the problem to occur.

@thedustinsmith
Copy link
Contributor

Fixed in 2.4.1

@maxklenk
Copy link

maxklenk commented Mar 7, 2017

Works great for rectangles but my circles are not cropped right:

screen shot 2017-03-07 at 5 53 58 pm
screen shot 2017-03-07 at 5 56 00 pm

@thedustinsmith
Copy link
Contributor

@maxklenk - It seems to be working for me with circles. Can you put together something that demonstrates the issue you're having?

@maxklenk
Copy link

maxklenk commented Mar 7, 2017

Sure, please try this demo.

To reproduce the behavior shown in the screenshot select an area greater than the original image and press the button:
screen shot 2017-03-07 at 11 08 02 pm

@thedustinsmith
Copy link
Contributor

Thanks for the demo - seems like it's the combo of a circle and an image smaller than the viewport.

@thedustinsmith thedustinsmith reopened this Mar 7, 2017
@charlee88
Copy link

I'm having the same issue. Sometimes the image is complete, sometimes the image is black or sometimes is incomplete.

HTML code:
<div class="col-md-3 col-sm-12 col-xs-12 xs-no-padding ">
<div class="form-group no-margin-bottom">
<label class="text-uppercase">Foto extra 1</label>
<input type="file" id="fotoart2" name="fotoart2" title="Sube foto artículo" class="input-round big-input text-lowercase" >
</div>
</div>

<div id="fotoart2cont" class=" col-md-3 col-sm-12 col-xs-12 form-group text-center " >
<label class="text-uppercase margin-five text-center">Vista artículo extra uno </label>
<div id="mifoto2" ></div>
<div class="row center-col">
<div class="col-md-6 col-sm-6 col-xs-6">
<button id="2_izq" class="btn-rotate2 btn highlight-button-dark btn-very-small btn-round margin-five no-margin-right" data-deg="-90">izquierda</button>
</div>
<div class="col-md-6 col-sm-6 col-xs-6">
<button id="2_der" class="btn-rotate2 btn highlight-button-dark btn-very-small btn-round margin-five no-margin-right" data-deg="90">derecha</button>
</div>
<div class="col-md-12 col-sm-12 col-xs-12">
<button id="2_del" class="btn-delete2 btn highlight-button-dark btn-very-small btn-round margin-one no-margin-right">borrar</button>
</div>
</div>
<img id="fototest2">
</div>

   `basic = $('#fotoart2').croppie({
        viewport: {
            width: 150,
            height: 150,
            type: 'square'
        },
        boundary: {
            width: 200,
            height: 200
        },
        enableOrientation: true
    });`

$('#fotoart2').on('change', function(ev) { ev.preventDefault(); readFile(this); });

`function readFile(input) {
    if (input.files && input.files[0]) {
        var reader = new FileReader();
        reader.onload = function(e) {
            basic.croppie('bind', {
                url: e.target.result
            }).then(function() {
                console.log('jQuery bind complete');
                basic.croppie('result', {
                    type: 'canvas',
                    size: {width: 700, height: 700},
                    quality: 0.8,
                    format: 'jpeg'
                }).then(function(resp) {
                    $("#fototest2").attr("src", resp);
                });
            });
        }
        reader.readAsDataURL(input.files[0]);
    }
}`

result
captura de pantalla 2017-04-17 a la s 02 42 48

@stupotmcdoodlepip
Copy link

Is a fix for this still in the works? I can recreate this problem reliably as follows:
Image size is 203x286 pixels, 96dpi
Viewport is 142x179 pixels

Container code:

$photoCropper = $('#croppie-container').croppie({
	url: $("#default-photo").html(),
	viewport: {
		width: 142,
		height: 179,
	},
	boundary: {
		width: 300,
		height: 300
	},
	enableOrientation: true
});

Original image, for reference:
_40577055_anorak203

Upon loading image, before cropping:
before rotate

Rotated clockwise 90 degrees, zoomed and image dragged to the left:
after rotate and zoom

Enter following into Firebug console:
$('.croppie-container').croppie('get')

Result
points: ["177", "26", "286", "163"]
zoom: 1.3026

Now after cropping:
after crop

Croppie version 2.4.1

Rotation code is:
$('.rotate').on('click', function(ev)
{
	$photoCropper.croppie('rotate', parseInt($(this).data('deg')));
});

Then uploading as base64:

function uploadAndSubmit()
{
       $photoCropper.croppie('result', {
           type: 'canvas',
           size: 'original'
       }).then(function (resp) {
           $('#imagebase64').val(resp);
           $('#main-form').submit();
       });
}

Commenting out this code at line 991 in croppie.js fixes my problem, at least on FireFox.
But will that break in iOS / Safari?

if (right > self._originalImageWidth) {
    width = self._originalImageWidth - left;
    outWidth = width;
}
if (bottom > self._originalImageHeight) {
    height = self._originalImageHeight - top;
    outHeight = height;
}

For now I'll leave the quick and dirty hack in there, but it would be nice to have an official fix for this out in the wild.
Thanks for the otherwise awesome plugin by the way!

Stu

jdrydn added a commit to car-throttle/croppie that referenced this issue May 17, 2017
@jdrydn
Copy link

jdrydn commented May 17, 2017

After discovering Croppie & implementing it into my CMS, I kept finding a strange bug regarding the viewport that I could never get around, but the fix above sorted that for me and now it works fine! This was regardless of what I set enforceBoundary to.

@Aditya94A
Copy link

Does this issue still exist?

@jdrydn What "fix" were you referring to exactly?

@jdrydn
Copy link

jdrydn commented Nov 20, 2017

@AdityaAnand1 See this commit from this fork (referenced above my comment) - commenting out a section of code literally fixed this issue.

I haven't looked into this further after "implementing" this "fix", so I have no idea if later commits & PRs merged into this repo sorted this bug.

@thedustinsmith
Copy link
Contributor

This is out of hand. I'm closing this issue. If anyone's problem persists in 2.5.2, let's address it in another issue.

@Aditya94A
Copy link

@thedustinsmith I'm sure future users who stumble onto this issue would appreciate a less ambiguous conclusion that "This is out of hand". 😄

  • Were you able to conclusively find the cause of the bug and fix it?

@thedustinsmith
Copy link
Contributor

@AdityaAnand1 Why don't you let everyone know if the issue is fixed in this branch: https://github.com/Foliotek/Croppie/tree/pre-2.5.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants