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

pinch to zoom gesture is broken - due to Skew changes #2496

Closed
mobidev111 opened this issue Sep 24, 2015 · 4 comments · Fixed by #2599
Closed

pinch to zoom gesture is broken - due to Skew changes #2496

mobidev111 opened this issue Sep 24, 2015 · 4 comments · Fixed by #2599
Labels

Comments

@mobidev111
Copy link

The pinch to zoom gesture is broken in current master.

Root cause: canvas_gestures.mixin.js has not been updated to changes introduced with the Skew changes

The Skew changes in #2482 introduced a new parameter "dim" to the _setObjectScale() method: cda0611#diff-9dc132ed9a0787568f773999ba577e2bR637

canvas_gestures.mixin.js has not been updated to reflect this change: https://github.com/kangax/fabric.js/blob/4a6001e3561de546c24b6f8cc4b61db990c43367/src/mixins/canvas_gestures.mixin.js#L140

     this._setObjectScale(new fabric.Point(t.scaleX * s * dim.x, t.scaleY * s * dim.y),
        t, lockScalingX, lockScalingY, null, target.get('lockScalingFlip'));

The lacking parameter causes an exception during calculation.

Just adding the missing parameter unfortunately does not resolve the gesture issue - please investigate.

@asturur
Copy link
Member

asturur commented Sep 24, 2015

When you add parameter, do you have still calculation error?
I have no gesture device, so i cannot test properly.

starting on https://github.com/kangax/fabric.js/blob/4a6001e3561de546c24b6f8cc4b61db990c43367/src/mixins/canvas_gestures.mixin.js#L136

If you have time to spare:

var constraintPosition = target.translateToOriginPoint(target.getCenterPoint(), t.originX, t.originY),
-        dim = target._getNonTransformedDimensions();
+        dim = target._getTransformedDimensions();

-      this._setObjectScale(new fabric.Point(t.scaleX * s * dim.x, t.scaleY * s * dim.y),
-        t, lockScalingX, lockScalingY, null, target.get('lockScalingFlip'));
+      this._setObjectScale(new fabric.Point(dim.x * s, dim.y * s),
+        t, lockScalingX, lockScalingY, null, target.get('lockScalingFlip'), dim);

and if you want to try it on a skewed object to see if it feels ok.

@kangax
Copy link
Member

kangax commented Oct 11, 2015

ping @mobidev111

@kangax
Copy link
Member

kangax commented Oct 11, 2015

Actually I see @mobidev111 said that "Just adding the missing parameter unfortunately does not resolve the gesture issue" so we'll need to check more

@mobidev111
Copy link
Author

@asturur regarding touch devices:

a) cheap hardware - the easiest way would be e.g. a cheap android tablet with current android version.
This enables you to do nice remote debugging - you inspect the DOM of the device from your development machine:
https://developer.chrome.com/devtools/docs/remote-debugging

b) simulate multi-touch - you can simulate touch devices in the Chrome browser.
https://developer.chrome.com/devtools/docs/device-mode#touch-emulation
https://frontify.com/blog/how-to-emulate-touch-events-in-chrome/

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 a pull request may close this issue.

3 participants