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

Fix Issue Where Large Images Get Vertically Squashed In iOS6 & iOS7 #654

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mpchadwick
Copy link

This pull request implements the detectVerticalSquash() function from the ios-imagefile-megapixel library (https://github.com/stomita/ios-imagefile-megapixel) to fix a bug in iOS6 & iOS7 where large images get vertically squashed when added to the canvas. This issue is likely to occur when adding image from the iPhone file system to the canvas.

Function is called on the image only if the Kinetic.UA.browser indicates that the browser is webkit, and is not called on cross origin images or images with an SVG source as getImageData() would raise a security exception.

A test was also added to confirm that the image is not being vertically squashed.

@mpchadwick
Copy link
Author

I've been thinking that this might actually make more sense as an option...

var img = new Kinetic.Image({
    fixIOSVerticalSquash: true
})

Then we can take out the conditional logic and eliminate the processing overhead for users who don't need to worry about this. For those who do have the issue they'll be able to address it easily without having to integrate another library into their code.

@ericdrowell - Your thoughts?

If you agree I can update this PR.

@valeroAlbatera
Copy link

I have the same problem...when I add a image in a canvas image with kinetics...my image is squashed in iphone 5

@valeroAlbatera
Copy link

Any help?

@ericdrowell
Copy link
Owner

I'll take a look at this when I can. The right way to go is to to do something like this in Kinetic.Image:

if (Kinetic.UA.browser === 'ios') {
// override some methods
}

this way the logic is only executed once, when KineticJS is first setup

@ghost ghost assigned ericdrowell Nov 4, 2013
@valeroAlbatera
Copy link

Hi, thanks for your answer!!
Im novice in programming also more with kinetics, I cant overrride methods because It is so difficult for me.
I achieved to fix the problem with the squash canvas image with this:
function detectVerticalSquash(img) { var iw = img.naturalWidth, ih = img.naturalHeight; var canvas = document.createElement('canvas'); canvas.width = 1; canvas.height = ih; var ctx = canvas.getContext('2d'); ctx.drawImage(img, 0, 0); var data = ctx.getImageData(0, 0, 1, ih).data; // search image edge pixel position in case it is squashed vertically. var sy = 0; var ey = ih; var py = ih; while (py > sy) { var alpha = data[(py - 1) * 4 + 3]; if (alpha === 0) { ey = py; } else { sy = py; } py = (ey + sy) >> 1; } var ratio = (py / ih); return (ratio===0)?1:ratio; }var verticalSquashRatio = detectVerticalSquash(img);var testHeight = 332.598425197 / verticalSquashRatio;
var image1 = new Kinetic.Image({ x: 9, y: 8, width: 238.11023622, height: testHeight, name: "image", image:img, crop: [getX, getY, getWidth, getHeight] });

But....When I add text to the canvas image I have also the same problems and I dont know how fix....it is strange!!
Thanks&Regards!!

Date: Sun, 3 Nov 2013 16:48:53 -0800
From: [email protected]
To: [email protected]
CC: [email protected]
Subject: Re: [KineticJS] Fix Issue Where Large Images Get Vertically Squashed In iOS6 & iOS7 (#654)

I'll take a look at this when I can. The right way to go is to to do something like this in Kinetic.Image:

if (Kinetic.UA.browser === 'ios') {

// override some methods

}

this way the logic is only executed once, when KineticJS is first setup


Reply to this email directly or view it on GitHub.

@mpchadwick
Copy link
Author

@ericdrowell Good point. I had the logic in drawFunc() where it would be executed multiple times.

I have modified this PR so that the logic is executed only once in the ___init() at which point height is modified accordingly, if necessary.

In addition to checking Kinetic.UA.browser (which returns 'webkit' on iOS rather than 'ios'), we also need to make sure that the image source is not an SVG and that it isn't cross origin as detectVerticalSquash() uses ctx.getImageData() and will raise a security exception in those instances.

Let me know if anything else needs to modified here or if this is good to go. Thanks.

@ericdrowell
Copy link
Owner

Thanks, I'll look at this again this week. Stay tuned.

@valeroAlbatera
Copy link

Sorry ericdrowell, do you know when the problems with ios7 should be fix?? Thanks

@ericdrowell
Copy link
Owner

Hiya, yes, with v5.0.0. Here's the release schedule:

https://github.com/ericdrowell/KineticJS/wiki/Release-Schedule

@valeroAlbatera
Copy link

Amm...Ok. ericdrowell...more or less the day? do you have a beta version? Im working in a personal project and I need it to finish and fix the problem with ios7 !! x(

Kind Regards

@scheng2013
Copy link

Hi guys, When will we meet the next release?

@scheng2013
Copy link

Do you have another way to resolve it now?

@franAlbatera
Copy link

Hi ericdrowell,

Any idea when kinetics is finally updated?? I need it!! Regards!!

@ericdrowell
Copy link
Owner

I'm still in the process of wrapping up the new documentation for all of the API changes in v5.0.0. It's the biggest release in KineticJS's history. Since this seems like a pretty burning issue, I'll still try to get this fix in before I release. I plan to build a beta release and start doing integration testing with Html5CanvasTutorials by the end of this week. If things go smoothly, v5.0.0 should be released during the weekend, maybe sooner.

@scheng2013
Copy link

Hi ericdrowell,

Thank you for this information.

Thanks,

Sean

Date: Tue, 7 Jan 2014 16:15:15 -0800
From: [email protected]
To: [email protected]
CC: [email protected]
Subject: Re: [KineticJS] Fix Issue Where Large Images Get Vertically Squashed In iOS6 & iOS7 (#654)

I'm still in the process of wrapping up the new documentation for all of the API changes in v5.0.0. It's the biggest release in KineticJS's history. Since this seems like a pretty burning issue, I'll still try to get this fix in before I release. I plan to build a beta release and start doing integration testing with Html5CanvasTutorials by the end of this week. If things go smoothly, v5.0.0 should be released during the weekend, maybe sooner.

¡ª
Reply to this email directly or view it on GitHub.

@valeroAlbatera
Copy link

Hi ericdrowell, I see that there are not any solution for ios problem in v5.0.0 ? We have to wait until March?

@scheng2013
Copy link

Hi Eric,

Let's say it. Is this issue caused by IOS or KineticJS? Can we use viewport attribute to solve it? Any feedback?

Thanks,

Sean

Date: Sun, 12 Jan 2014 02:45:25 -0800
From: [email protected]
To: [email protected]
CC: [email protected]
Subject: Re: [KineticJS] Fix Issue Where Large Images Get Vertically Squashed In iOS6 & iOS7 (#654)

Hi ericdrowell, I see that there are not any solution for ios problem in v5.0.0 ? We have to wait until March?

¡ª
Reply to this email directly or view it on GitHub.

@ericdrowell
Copy link
Owner

v5.0.0 was too big of a release, so I had to cut out a lot of things that I was planning on putting in (including a fix for this ticket).

I'm planning on taking a week off and letting v5.0.0 stew a bit more to see if there are any issues (there's already one that I just fixed). I'll release v5.0.1 the following week (end of January) which will be a small patch that only contains high priority bug fixes. Here's the revised release schedule so far:

https://github.com/ericdrowell/KineticJS/wiki/Release-Schedule

Stay tuned!

@scheng2013
Copy link

Got it.

Date: Sun, 12 Jan 2014 21:58:02 -0800
From: [email protected]
To: [email protected]
CC: [email protected]
Subject: Re: [KineticJS] Fix Issue Where Large Images Get Vertically Squashed In iOS6 & iOS7 (#654)

v5.0.0 was too big of a release, so I had to cut out a lot of things that I was planning on putting in (including a fix for this ticket).

I'm planning on taking a week off and letting v5.0.0 stew a bit more to see if there are any issues (there's already one that I just fixed). I'll release v5.0.1 the following week (end of January) which will be a small patch that only contains high priority bug fixes. Here's the revised release schedule so far:

https://github.com/ericdrowell/KineticJS/wiki/Release-Schedule

Stay tuned!

¡ª
Reply to this email directly or view it on GitHub.

@NachoVarga
Copy link

@ericdrowell: I discovered that this fix

ctx.drawImage(img, sx, sy, sw, sh, dx, dy, dw, dh / vertSquashRatio);

is only working correctly when you display the whole image. As soon as you display only part of the image, there will be a displacement on the canvas.

If you fix it like this

ctx.drawImage(img, sx * vertSquashRatio, sy * vertSquashRatio, 
                   sw * vertSquashRatio, sh * vertSquashRatio, 
                   dx, dy, dw, dh );

it will work in both cases. (I edited the comment on stackoverflow accordingly.)

I am brand new to github and have no experiences with pull request, etc. Also, it seems the release date is approaching rapidly. So I just leave a comment here and hope there is still time to include it in the next release.

@ericdrowell
Copy link
Owner

Haven't forgotten about this thread. I spent a few minutes looking into this, and it's pretty clear that this change could have a lot of side effects. I feel that it's a fairly high risk change, which means that I need to spend a solid couple of nights reverse engineering stomita's library, and making sure that it doesn't cause side effects with pixel ratio adjustments, event handling, caching, data urls, the toImage method, filters, image size getters, etc.

In the past, these types of hacky work arounds for a bug with a specific device, browser, or OS often times aren't worth the effort because they eventually get fixed. However, there's a lot of people on this thread that clearly need a fix, so I will continue to investigate.

I don't want this to hold back the 5.0.1 patch, which fixes a lot of issues that affects everyone. So, unfortunately I'll have to push this fix to 5.0.2 and I will get to it asap.

For those of you that need a fix now, the changes in this pull request look like they are in the right direction. I'd suggest forking the KineticJS repo and trying to pull the changes in, and see if it fixes your issues. Even if it does cause side effects, you might not notice them depending on the features of KineticJS that you're using.

@scheng2013
Copy link

Eric,

You know IOS is a big eco-system, so it's important to me about our project. Can you schedule the version 5.0.2 in middle of Feb? Thanks.

May you build a seperate version for IOS squash image issue?

@lavrton
Copy link
Contributor

lavrton commented Jan 23, 2014

@scheng2013
Copy link

lavrton,

Thank you for this thread. You know we use Kimage class of kineticjs framework, so we need to know how to use this 3rd js to solve it and where the good place in kineticjs code is?

1 similar comment
@scheng2013
Copy link

lavrton,

Thank you for this thread. You know we use Kimage class of kineticjs framework, so we need to know how to use this 3rd js to solve it and where the good place in kineticjs code is?

@lavrton lavrton added the bug label Mar 9, 2014
@lavrton
Copy link
Contributor

lavrton commented Apr 12, 2014

Hello, folks! Update on this thread. As we know this is a REAL bug of iOS webview. And as @ericdrowell said it may be dangerous to include such workarounds to KineticJS codebase.

BUT! I found very good solution. You should know, that you can pass not only Image objects to Kinetic.Image. Also you may pass <canvas> element! So there is working way:

  1. Create special 'buffer' canvas (with same size as original image)
  2. Draw the image into the canvas with some extra workarounds
  3. Create Kinetic.Image with the canvas.

Code:

    // detect scale ratio
    function detectVerticalSquash(img) {
      var iw = img.width, ih = img.height;
      var canvas = document.createElement('canvas');
      canvas.width = 1;
      canvas.height = ih;
      var ctx = canvas.getContext('2d');
      ctx.drawImage(img, 0, 0);
      var data = ctx.getImageData(0, 0, 1, ih).data;
      // search image edge pixel position in case it is squashed vertically.
      var sy = 0;
      var ey = ih;
      var py = ih;
      while (py > sy) {
          var alpha = data[(py - 1) * 4 + 3];
          if (alpha === 0) {
              ey = py;
          } else {
              sy = py;
          }
          py = (ey + sy) >> 1;
      }
      var ratio = (py / ih);
      return (ratio===0)?1:ratio;
    }

    // create canvas to replace with image
    function generateCanvas(image){
        var canvas = document.createElement('canvas');
        var context = canvas.getContext('2d');
        canvas.width = image.width;
        canvas.height = image.height;
        var vertSquashRatio = detectVerticalSquash(image);
        console.log(image);
        context.drawImage(image, 0, 0, 
                           image.width, image.height / vertSquashRatio);
        return(canvas);
    }

    var img = new Image();
    img.onload = function() {
      var stage = new Kinetic.Stage({
          container: 'con',
          width: 1000,
          height: 1000
      });
      var layer = new Kinetic.Layer();
      stage.add(layer);
      var image = new Kinetic.Image({
        image : generateCanvas(img),
        width : 200,
        height : 200,
        draggable : true
      });
      layer.add(image);
      layer.draw();
    }
    img.src = 'diana2.jpg';

I tested this code on iPad, and it is fixing the bug. Can someone approve?

@confile
Copy link

confile commented Apr 12, 2014

To fix this bug you can also use this lib: https://github.com/stomita/ios-imagefile-megapixel

I had problems on some Android browsers when using this lib. On iOS it works fine.

@icaliman
Copy link

@lavrton I tested your fix. Big images are not drawn on stage on iPhone 3GS, iOS 6.1.6

@lavrton
Copy link
Contributor

lavrton commented May 20, 2014

@icaliman I tested my solution on iPad - and problem was gone. Do you have any errors?

@icaliman
Copy link

I can not see the console from my iPhone but, I don't think there are errors. I am drawing many things on canvas but only big images are not visible. Small images are shown.

@icaliman
Copy link

@lavrton I just discovered http://jsconsole.com/ and debugged with it. There are no errors.

@icaliman
Copy link

@lavrton on my iPhone detectVerticalSquash returns incorect vertical squash.

@icaliman
Copy link

I am fixing this issue like so:

var image = new Kinetic.Image({
  image: imageObj
});

var vertSquashRatio = detectVerticalSquash(imageObj);
if (vertSquashRatio != 1) {
  var h = image.height();
  image.___height = image.height;
  image.height = function() {
    return arguments.length ? image.___height(arguments[0] / vertSquashRatio) : image.___height() * vertSquashRatio
  }
  image.height(h);
}

but vertSquashRatio seems to be incorrect, and the image is scaled incorrectly.
@lavrton can you check my code?

@lavrton
Copy link
Contributor

lavrton commented May 20, 2014

What is value of vertSquashRatio? What are you expecting? Did you try my way with replacing image object by fixed canvas object?

@icaliman
Copy link

I tried your fix but it does not work on my iPhone #654 (comment)

Value of vertSquashRatio for some images is 0.5 and this is correct, but for others it is 0.375.

@lavrton
Copy link
Contributor

lavrton commented May 20, 2014

0.375 is not correct? May be wrong calculation if image has transparent area on left edge.

@icaliman
Copy link

Yes 0.375 is not correct. Images are taken by iPhone, they don't have transparent area.

It is another problem, I have images that was taken in landscape and portrait mode but all of them are drawn on canvas in landscape mode (why?). And here appears the problem: for images that was taken in portrait mode vertSquashRatio is 0.375.

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

Successfully merging this pull request may close these issues.

9 participants