Pinch to Zoom can move molecule while zooming occurs

[KatieWoe]

Test device Dell Operating System Win 11 Browser Firefox Problem description For https://github.com/phetsims/qa/issues/768 and may be related to https://github.com/phetsims/joist/issues/749. When you start to zoom into the sim with pinch to zoom, the molecule acts as though it is also being dragged. This sometimes stops after a second, but if you accidentally swipe your finger as you start to pinch, the molecule will not freeze. Steps to Reproduce

1. Put two fingers on the touch screen
2. Swipe fingers, don't pick them up
3. Start a pinch to zoom gesture

Visuals

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Molecule Shapes‬ URL: https://phet-dev.colorado.edu/html/molecule-shapes/1.5.0-dev.3/phet/molecule-shapes_all_phet.html Version: 1.5.0-dev.3 2022-01-22 22:59:16 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:96.0) Gecko/20100101 Firefox/96.0 Language: en-US Window: 1280x667 Pixel Ratio: 1.5/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 Vendor: Mozilla (ANGLE (Intel(R) HD Graphics Direct3D11 vs_5_0 ps_5_0, D3D11-30.0.100.9955)) Vertex: attribs: 16 varying: 30 uniform: 4096 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 32767x32767 OES_texture_float: true Dependencies JSON: {}

[jonathanolson]

I believe this should be prevented by the above commit. @KatieWoe can you verify?

[KatieWoe]

The movement that lasts a second before stopping still happens, but the more serious version that doesn't stop does seem fixed.

[jessegreenberg]

@jonathanolson I am curious what you think of this patch as an alternative to your change in https://github.com/phetsims/molecule-shapes/commit/a2b726f46f184e1b8742c4d9979172dad675a05f. The issue is that pan/zoom listener only interrupts attached Pointer listeners, so this was happening in the condition that the listener was not attached to the pointer.

```patch Index: js/common/view/MoleculeShapesScreenView.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/view/MoleculeShapesScreenView.js b/js/common/view/MoleculeShapesScreenView.js --- a/js/common/view/MoleculeShapesScreenView.js (revision a2b726f46f184e1b8742c4d9979172dad675a05f) +++ b/js/common/view/MoleculeShapesScreenView.js (date 1643238067079) @@ -13,14 +13,9 @@ import Sphere3 from '../../../../dot/js/Sphere3.js'; import Vector3 from '../../../../dot/js/Vector3.js'; import ScreenView from '../../../../joist/js/ScreenView.js'; -import ContextLossFailureDialog from '../../../../scenery-phet/js/ContextLossFailureDialog.js'; import ResetAllButton from '../../../../scenery-phet/js/buttons/ResetAllButton.js'; -import { Mouse } from '../../../../scenery/js/imports.js'; -import { animatedPanZoomSingleton } from '../../../../scenery/js/imports.js'; -import { AlignBox } from '../../../../scenery/js/imports.js'; -import { DOM } from '../../../../scenery/js/imports.js'; -import { Rectangle } from '../../../../scenery/js/imports.js'; -import { Utils } from '../../../../scenery/js/imports.js'; +import ContextLossFailureDialog from '../../../../scenery-phet/js/ContextLossFailureDialog.js'; +import { AlignBox, animatedPanZoomSingleton, DOM, Mouse, Rectangle, Utils } from '../../../../scenery/js/imports.js'; import moleculeShapes from '../../moleculeShapes.js'; import MoleculeShapesGlobals from '../MoleculeShapesGlobals.js'; import LabelWebGLView from './3d/LabelWebGLView.js'; @@ -216,7 +211,9 @@ draggedParticleCount++; } // we don't want to rotate while we are dragging any particles - // Additionally, don't rotate if we're zoomed into the sim + // Additionally, don't rotate if we're zoomed into the sim - the pan/zoom listener will interrupt the rotation + // to start a pan, but not until there is a little bit of pointer movement. If we are zoomed in at all + // we don't want to allow movement that will soon just get interrupted. else if ( draggedParticleCount === 0 && animatedPanZoomSingleton.listener.matrixProperty.value.equalsEpsilon( Matrix3.IDENTITY, 1e-7 ) ) { // we rotate the entire molecule with this pointer dragMode = 'modelRotate'; @@ -229,8 +226,9 @@ const lastGlobalPoint = pointer.point.copy(); - const attach = dragMode === 'pairExistingSpherical'; - if ( attach ) { + // If a drag starts on a pair group, input should only be for dragging. Indicate to other listeners that + // behavior is reserved (specifically the pan/zoom listener that should not interrupt for pan). + if ( dragMode === 'pairExistingSpherical' ) { pointer.reserveForDrag(); } @@ -258,10 +256,6 @@ move: function( event, trail ) { if ( dragMode === 'modelRotate' ) { - // Interrupt molecule drags if we're zoomed in, see https://github.com/phetsims/molecule-shapes/issues/213 - if ( !animatedPanZoomSingleton.listener.matrixProperty.value.equalsEpsilon( Matrix3.IDENTITY, 1e-7 ) ) { - this.endDrag( event, trail ); - } const delta = pointer.point.minus( lastGlobalPoint ); lastGlobalPoint.set( pointer.point ); @@ -281,7 +275,7 @@ // not a Scenery event endDrag: onEndDrag, interrupt: onEndDrag - }, attach ); + }, true ); // attach the listener so that it can be interrupted from pan and zoom operations } }; this.backgroundEventTarget.addInputListener( multiDragListener ); ```

I tried to remove the other equalsEpsilon check in multiDragListener. The pan/zoom listener will interrupt rotation to start a pan but only after there is a little bit of pointer movement so we will start to rotate the molecule then quickly interrupt which looks a little odd. I don't see a great way currently to remove that workaround.

[jonathanolson]

I prefer your approach, I think the lack of attachment was for pan-zoom so this works better. Committed above.

[KatieWoe]

On master it looks like I'm still seeing what was seen in https://github.com/phetsims/molecule-shapes/issues/213#issuecomment-1022633858. I think this is generally acceptable, but am reassigning. If you agree that this is acceptable behavior this can be closed.

[Nancy-Salpepi]

I still see what was reported in https://github.com/phetsims/molecule-shapes/issues/213#issuecomment-1022633858 when testing https://github.com/phetsims/qa/issues/785

[jessegreenberg]

I think I am able to see https://github.com/phetsims/molecule-shapes/issues/213#issuecomment-1022633858 if I start dragging with two fingers and drag without making the zoom gesture. Then as soon as I make the zoom gesture dragging is interrupted and we zoom in. That is the only way I have been able to reproduce and I think it is the correct behavior.

@jonathanolson anything else you would like to do for this issue?

[jonathanolson]

This looks good to me, thanks!