thatcher / openseadragon

This project has moved to its new github organization at github.com/openseadragon, please join us!
http://openseadragon,github.com/
37 stars 14 forks source link

Two finger pinch-to-zoom appears to be calculating the zoom point incorrectly #22

Closed chrisbartley closed 11 years ago

chrisbartley commented 11 years ago

If you zoom with the scroll wheel, the image correctly zooms about the mouse pointer.

However, with a touch interface (e.g. mobile Chrome, desktop Chrome 24, or desktop Firefox 18 on Windows 7 with a multitouch screen), the zoom midpoint is incorrect. You can see the behavior by testing with the sample image on the project home page. Put fingers on either side of some small feature and drag one out so that the image zooms. See the attached image for an example. You'll soon see that the viewer zooms in to some other part of the image, and the feature you initially wanted to zoom in on is no longer visible.

It looks to be that you may be calculating the zoom midpoint (i.e. the midpoint between the two touch points) every time a touchmove event is handled. If so, the solution is to calculate the zoom midpoint at the beginning of the pinch, and hold on to it and use it through the entire duration of the pinch. I had this exact problem when adding touch support to Seadragon (before I knew about this project...[sigh]). You can see the result at http://multitouch.cmucreatelab.org/index2.html and the code at https://github.com/CMU-CREATE-Lab/multitouch/

pinch-to-zoom

iangilman commented 11 years ago

Good point! So you're saying you've fixed this in your branch of the old Seadragon Ajax? Can you point me to that part of the code in your version? Would be great to have here as well.

thatcher commented 11 years ago

yeah sorry I need to get some additional touch screen resources. Like Ian said, if you can point us to a patch I'll try to reintegrate it in a way that makes sense with the current code base. I have to admit I didnt even realize these touch interfaces were available. Can you also point us to a couple resources about these interfaces?

Thanks, Thatcher

chrisbartley commented 11 years ago

Although I did branch Seadragon Ajax, it wasn't for touch support. I chose instead to layer touch support on top of Seadragon, to reduce coupling (we need multitouch support for some of our other, non-seadragon applications). My touch support classes are pretty simple, but meet our needs. It knows how to deal with W3C touch events (as well as the legacy Mozilla touch events) and can process them into tap, pan, and pinch gestures. So, for any browser supporting the W3C touch event spec, you can just instantiate the TouchManager and add event listeners to it to handle tap, pan, and pinch events (I currently don't handle or care about gestures with 3 or more fingers--not needed by any of our applications at the moment).

To wire it all together, first instantiate the TouchManager:

var touchManager = new org.cmucreatelab.multitouch.TouchManager();

Then add the listeners for the W3C touch events (and legacy Mozilla, if you care about that) to Seadragon, so that the TouchManager can handle them (I do this upon viewer open, with a flag to make sure they don't get added more than once. See the source for my seadragon example for details):

var isMozillaWithLegacyTouchSupport = (jQuery['browser']['mozilla'] == true) && (parseInt(jQuery['browser']['version']) < 18);

var touchSensitiveElement = viewer.drawer.elmt;

if (isMozillaWithLegacyTouchSupport)
   {
   Seadragon.Utils.addEvent(touchSensitiveElement, "MozTouchDown", touchManager.onTouchStartMozilla);
   Seadragon.Utils.addEvent(touchSensitiveElement, "MozTouchMove", touchManager.onTouchMoveMozilla);
   Seadragon.Utils.addEvent(touchSensitiveElement, "MozTouchUp", touchManager.onTouchEndMozilla);
   }
else
   {
   Seadragon.Utils.addEvent(touchSensitiveElement, "touchstart", touchManager.onTouchStart);
   Seadragon.Utils.addEvent(touchSensitiveElement, "touchmove", touchManager.onTouchMove);
   Seadragon.Utils.addEvent(touchSensitiveElement, "touchend", touchManager.onTouchEndOrCancel);
   Seadragon.Utils.addEvent(touchSensitiveElement, "touchcancel", touchManager.onTouchEndOrCancel);
   }

Then register your event listeners with the TouchManager:

touchManager.addEventListener('tap', tapEventHandler);
touchManager.addEventListener('pan', panEventHandler);
touchManager.addEventListener('pinch-start', pinchStartEventHandler);
touchManager.addEventListener('pinch', pinchEventHandler);

And, finally, define your TouchManager event listeners:

var pinchMidpoint = null;

function tapEventHandler(touchPoint, elapsedTouchTime, event)
   {
   if (viewer.isOpen() && viewer.viewport)
      {
      var seadragonTouchPoint = touchPoint.toSeadragonPoint();
      var viewerPositionInPixelCoords = Seadragon.Utils.getElementPosition(viewer.elmt);
      var touchPositionInPixelCoords = seadragonTouchPoint.minus(viewerPositionInPixelCoords);
      var touchPointInSeadragonCoords = SeadragonUtils.convertPageCoordsToSeadragonCoords(touchPositionInPixelCoords, viewer);
      // do something interesting here...
      }
   }

function panEventHandler(pixelDeltaFromPrevious, event)
   {
   if (viewer.isOpen() && viewer.viewport)
      {
      var panBy = viewer.viewport.deltaPointsFromPixels(pixelDeltaFromPrevious);
      viewer.viewport.panBy(panBy);
      }
   }

function pinchStartEventHandler(touch1, touch2, midpoint, event)
   {
   if (viewer.isOpen() && viewer.viewport && midpoint != null)
      {
      // record the current midpoint in Seadragon coords so all future pinch events during this
      // pinch will zoom about this point
      pinchMidpoint = SeadragonUtils.convertPageCoordsToSeadragonCoords(midpoint.toSeadragonPoint(), viewer);
      }
   }

function pinchEventHandler(scaleDeltaX, scaleDeltaY, scaleDeltaXY, touch1, touch2, event)
   {
   // clamp the zoom scale delta to within [.8, 1.2]
   scaleDeltaXY = Math.max(.8, Math.min(1.2, scaleDeltaXY));
   if (viewer.isOpen() && viewer.viewport && pinchMidpoint != null)
      {
      viewer.viewport.zoomBy(scaleDeltaXY, pinchMidpoint);
      }
   }

But, really, the important bits for you guys is what's going on in the pinchStartEventHandler() and pinchEventHandler() functions. The former records the pinch midpoint when the pinch begins, and the latter then uses that midpoint as the point to zoom about during the entire duration of the pinch. If you recalculate the pinch midpoint during every step of the pinch, you'll see the drift. Which is what I think is currently going on with Open Seadragon. It's cool you've already added touch support to your fork of seadragon, so you should just stick with it rather than using my stuff. Looks like all that's missing is the recording and usage of the initial pinch midpoint.

Hope this helps.

best,

Chris

thatcher commented 11 years ago

thanks chris, this should be enough for me to patch the touch handling routine. I'll ping you when we get this in trunk.

thathcer

thatcher commented 11 years ago

please see https://github.com/openseadragon/openseadragon/issues/17

thatcher commented 11 years ago

this is fixed in master openseadragon/openseadragon see https://github.com/openseadragon/openseadragon/issues/17

damienkeating commented 8 years ago

Seems the problem reappears!

I experienced the same behaviour by testing with the sample image on the project home page.

Do you think we can re-open it?

iangilman commented 8 years ago

@damienkeating Work on OpenSeadragon has moved to this repository:

https://github.com/openseadragon/openseadragon/

The recurrence of this issue is tracked here:

https://github.com/openseadragon/openseadragon/issues/788

...and there is a pull request:

https://github.com/openseadragon/openseadragon/pull/793

See the discussion there about next steps; perhaps you could help out?