search

phetsims / projectile-data-lab

"Projectile Data Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 0 forks source link

Multitouch crash #321

Closed samreid closed 5 months ago

samreid commented 5 months ago

From https://github.com/phetsims/projectile-data-lab/issues/290#issuecomment-2083428976 and discovered in https://github.com/phetsims/qa/issues/1079

@KatieWoe said:

I had a fairly bad instance of freezing with multitouch. I saw it on iPad, and don't have a touch screen on my computer, so I don't have a console readout.

Steps:

  1. Go to the third screen and turn on the interval tools.
  2. Drag around one of the ends of the tool,
  3. While dragging, press reset all, make sure the dragging finger is moving at this point
  4. With that finger still down, turn on interval tools again
  5. Drag the down finger over the tool to snag it and drag it. It should catch the "middle" of the tool
  6. Press reset all again.

https://github.com/phetsims/projectile-data-lab/assets/41024075/c4df07dd-004b-4e13-8338-bdabc96698d4

samreid commented 5 months ago
```diff Subject: [PATCH] Avoid infinite loop updating edges during reset during multitouch interaction, see https://github.com/phetsims/projectile-data-lab/issues/321 --- Index: js/measures/view/IntervalToolNode.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/measures/view/IntervalToolNode.ts b/js/measures/view/IntervalToolNode.ts --- a/js/measures/view/IntervalToolNode.ts (revision 550aaa9980fe589bd77123cf60970c5a89040e80) +++ b/js/measures/view/IntervalToolNode.ts (date 1714496286655) @@ -423,7 +423,10 @@ }, edge2DragListenerOptions, dragListenerOptions ) ) ); desiredCenterLocationProperty.lazyLink( ( value: Vector2, oldValue: Vector2 ) => { - if ( this.isCenterDraggingProperty.value ) { + + // During a drag, update the edge positions based on the center drag. During a reset all, this would cause an + // infinite loop, and the edges are reset elsewhere, so this can be avoided during reset all. + if ( this.isCenterDraggingProperty.value && !ResetAllButton.isResettingAllProperty.value ) { let delta = value.x - oldValue.x; const max = Math.max( intervalTool.edge1Property.value, intervalTool.edge2Property.value );
matthew-blackman commented 5 months ago

This has been fixed in the commit @KatieWoe. Please confirm that the crash no longer occurs on main.

KatieWoe commented 5 months ago

Looks good on main

KatieWoe commented 5 months ago

Looks good in rc.3