PhET-iO instrument Pointers #1384
Assignees
Comments
|
Partial progress on moving pointer data into state: Index: js/input/Input.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/input/Input.ts b/js/input/Input.ts
--- a/js/input/Input.ts (revision d008ddaa19226234f301f2f4096039a929e3253c)
+++ b/js/input/Input.ts (date 1647573948266)
@@ -168,7 +168,32 @@
import Tandem from '../../../tandem/js/Tandem.js';
import NullableIO from '../../../tandem/js/types/NullableIO.js';
import NumberIO from '../../../tandem/js/types/NumberIO.js';
-import { BatchedDOMEvent, BatchedDOMEventCallback, BatchedDOMEventType, BrowserEvents, Display, EventIO, Features, IInputListener, Mouse, Node, PDOMInstance, PDOMPointer, PDOMUtils, Pen, Pointer, scenery, SceneryEvent, SceneryListenerFunction, Touch, Trail, WindowTouch } from '../imports.js';
+import {
+ BatchedDOMEvent,
+ BatchedDOMEventCallback,
+ BatchedDOMEventType,
+ BrowserEvents,
+ Display,
+ EventIO,
+ Features,
+ IInputListener,
+ Mouse,
+ Node,
+ PDOMInstance,
+ PDOMPointer,
+ PDOMUtils,
+ Pen,
+ Pointer,
+ scenery,
+ SceneryEvent,
+ SceneryListenerFunction,
+ Touch,
+ Trail,
+ WindowTouch
+} from '../imports.js';
+import Property from '../../../axon/js/Property.js';
+import BooleanProperty from '../../../axon/js/BooleanProperty.js';
+import BooleanIO from '../../../tandem/js/types/BooleanIO.js';
// This is the list of keys that get serialized AND deserialized. NOTE: Do not add or change this without
// consulting the PhET-iO IOType schema for this in EventIO
@@ -326,7 +351,8 @@
// Event listeners are not added until initializeEvents, and are stored in this Map so they can be removed
// again in detachEvents.
private pdomEventListenerMap?: Map<string, ( event: Event ) => void>;
-
+ private readonly mousePointerPointProperty: Property<Vector2 | null>;
+ private readonly mouseIsDownProperty: Property<boolean | null>;
/**
* @param display
@@ -368,6 +394,21 @@
// Declare the Actions that send scenery input events to the PhET-iO data stream. Note they use the default value
// of phetioReadOnly false, in case a client wants to synthesize events.
+ const mouseTandem = options.tandem.createTandem( 'mouse' )
+ this.mousePointerPointProperty = new Property<Vector2 | null>( null, {
+ useDeepEquality: true,
+ tandem: mouseTandem.createTandem( 'pointProperty' ),
+ phetioType: Property.PropertyIO( NullableIO( Vector2.Vector2IO ) ),
+ phetioReadOnly: true
+ } );
+ this.mousePointerPointProperty.debug( 'mpp' );
+ this.mouseIsDownProperty = new Property<boolean | null>( null, {
+ tandem: mouseTandem.createTandem( 'isDownProperty' ),
+ phetioType: Property.PropertyIO( NullableIO( BooleanIO ) ),
+ phetioReadOnly: true
+ } );
+ this.mouseIsDownProperty.debug( 'mouse is down' );
+
this.validatePointersAction = new PhetioAction( () => {
let i = this.pointers.length;
while ( i-- ) {
@@ -402,6 +443,8 @@
const mouse = this.ensureMouse( point );
mouse.id = id;
const pointChanged = mouse.down( point, event );
+ this.mousePointerPointProperty.value = point;
+ this.mouseIsDownProperty.value = true;
this.downEvent<MouseEvent>( mouse, event, pointChanged );
}, {
phetioPlayback: true,
@@ -418,6 +461,7 @@
this.mouseMoveAction = new PhetioAction( ( point: Vector2, event: MouseEvent ) => {
const mouse = this.ensureMouse( point );
mouse.move( point, event );
+ this.mousePointerPointProperty.value = point;
this.moveEvent<MouseEvent>( mouse, event );
}, {
phetioPlayback: true,
@@ -434,6 +478,7 @@
this.mouseOverAction = new PhetioAction( ( point: Vector2, event: MouseEvent ) => {
const mouse = this.ensureMouse( point );
mouse.over( point, event );
+ this.mousePointerPointProperty.value = point;
// TODO: how to handle mouse-over (and log it)... are we changing the pointer.point without a branch change?
}, {
phetioPlayback: true,
@@ -449,6 +494,7 @@
this.mouseOutAction = new PhetioAction( ( point: Vector2, event: MouseEvent ) => {
const mouse = this.ensureMouse( point );
mouse.out( point, event );
+ this.mousePointerPointProperty.value = point;
// TODO: how to handle mouse-out (and log it)... are we changing the pointer.point without a branch change?
}, {
phetioPlayback: true,
@@ -1214,6 +1260,7 @@
sceneryLog && sceneryLog.Input && sceneryLog.Input( `mouseUp(${Input.debugText( point, event )});` );
sceneryLog && sceneryLog.Input && sceneryLog.push();
this.mouseUpAction.execute( point, event );
+ this.mouseIsDownProperty.value = false;
sceneryLog && sceneryLog.Input && sceneryLog.pop();
}
|
|
Here's a patch that adds state for touches and and display overlay for mouse and touches: Index: main/scenery/js/overlays/PointerStateOverlay.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/scenery/js/overlays/PointerStateOverlay.ts b/main/scenery/js/overlays/PointerStateOverlay.ts
new file mode 100644
--- /dev/null (date 1650766691746)
+++ b/main/scenery/js/overlays/PointerStateOverlay.ts (date 1650766691746)
@@ -0,0 +1,28 @@
+// Copyright 2013-2022, University of Colorado Boulder
+
+/**
+ * Displays mouse and touch areas when they are customized. Expensive to display!
+ *
+ * @author Jonathan Olson <jonathan.olson@colorado.edu>
+ */
+
+import { Shape } from '../../../kite/js/imports.js';
+import { scenery, ShapeBasedOverlay, Display, Node, IOverlay } from '../imports.js';
+
+export default class PointerStateOverlay extends ShapeBasedOverlay implements IOverlay {
+ constructor( display: Display, rootNode: Node ) {
+ super( display, rootNode, 'pointerStateOverlay' );
+ }
+
+ addShapes() {
+ const mousePoint = this.display._input!.mousePointerPointProperty.value;
+ if ( mousePoint ) {
+ this.addShape( Shape.circle( mousePoint.x, mousePoint.y, 10 ), 'blue', false );
+ }
+ const touches = this.display._input!.touchesProperty.value;
+ console.log( touches.length, touches );
+ touches.forEach( touch => this.addShape( Shape.circle( touch.x, touch.y, 10 ), 'red', false ) );
+ }
+}
+
+scenery.register( 'PointerStateOverlay', PointerStateOverlay );
Index: main/scenery/js/display/Display.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/scenery/js/display/Display.ts b/main/scenery/js/display/Display.ts
--- a/main/scenery/js/display/Display.ts (revision c9d36d46987a41556ad4f856b975e03b7ddeffee)
+++ b/main/scenery/js/display/Display.ts (date 1650766320691)
@@ -65,6 +65,7 @@
import AriaLiveAnnouncer from '../../../utterance-queue/js/AriaLiveAnnouncer.js';
import UtteranceQueue from '../../../utterance-queue/js/UtteranceQueue.js';
import { scenery, Color, Features, FullScreen, Trail, Utils, scenerySerialize, BackboneDrawable, ChangeInterval, DOMBlock, Drawable, DOMDrawable, Instance, Renderer, Node, PDOMInstance, PDOMSiblingStyle, PDOMUtils, globalKeyStateTracker, FocusManager, KeyboardUtils, PDOMTree, Input, CanvasNodeBoundsOverlay, FittedBlockBoundsOverlay, HighlightOverlay, HitAreaOverlay, PointerAreaOverlay, PointerOverlay, IInputListener, IOverlay, InputOptions, Block, WebGLBlock, CanvasBlock } from '../imports.js';
+import PointerStateOverlay from '../overlays/PointerStateOverlay.js';
export type DisplayOptions = {
// Initial (or override) display width
@@ -224,7 +225,7 @@
// "current" linked-list information so it is up-to-date, but needs to use the "old" information also. We move
// updating the "current" => "old" linked-list information until after syncTree and stitching is complete, and is
// taken care of in an updateDisplay pass.
- private _drawablesToUpdateLinks: Drawable[]
+ private _drawablesToUpdateLinks: Drawable[];
// We store information on {ChangeInterval}s that records change interval
// information, that may contain references. We don't want to leave those references dangling after we don't need
@@ -255,6 +256,7 @@
private _overlays: IOverlay[];
private _pointerOverlay: PointerOverlay | null;
+ private _pointerStateOverlay: PointerStateOverlay | null = null;
private _pointerAreaOverlay: PointerAreaOverlay | null;
private _hitAreaOverlay: HitAreaOverlay | null;
private _canvasAreaBoundsOverlay: CanvasNodeBoundsOverlay | null;
@@ -1169,6 +1171,22 @@
}
}
+ setPointerStateVisible( visibility: boolean ) {
+ const hasOverlay = !!this._pointerStateOverlay;
+
+ if ( visibility !== hasOverlay ) {
+ if ( !visibility ) {
+ this.removeOverlay( this._pointerStateOverlay! );
+ this._pointerStateOverlay!.dispose();
+ this._pointerStateOverlay = null;
+ }
+ else {
+ this._pointerStateOverlay = new PointerStateOverlay( this, this._rootNode! );
+ this.addOverlay( this._pointerStateOverlay );
+ }
+ }
+ }
+
/**
* TODO: reduce code duplication for handling overlays
*/
Index: main/scenery/js/input/Pointer.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/scenery/js/input/Pointer.ts b/main/scenery/js/input/Pointer.ts
--- a/main/scenery/js/input/Pointer.ts (revision c9d36d46987a41556ad4f856b975e03b7ddeffee)
+++ b/main/scenery/js/input/Pointer.ts (date 1650764656617)
@@ -106,13 +106,6 @@
* @param type - the type of the pointer; can different for each subtype
*/
protected constructor( initialPoint: Vector2, initialDownState: boolean, type: PointerType ) {
- assert && assert( initialPoint === null || initialPoint instanceof Vector2 );
- assert && assert( typeof initialDownState === 'boolean' );
-
- assert && assert( typeof type === 'string' );
-
- assert && assert( Object.getPrototypeOf( this ) !== Pointer.prototype, 'Pointer is an abstract class' );
-
this.point = initialPoint;
this.type = type;
this.trail = null;
Index: main/joist/js/SimDisplay.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/joist/js/SimDisplay.js b/main/joist/js/SimDisplay.js
--- a/main/joist/js/SimDisplay.js (revision f170b3c44238382daed60ac1b05d90c017f4bf6e)
+++ b/main/joist/js/SimDisplay.js (date 1650766199273)
@@ -142,6 +142,7 @@
// Pass through query parameters to scenery for showing supplemental information
this.setPointerDisplayVisible( phet.chipper.queryParameters.showPointers );
+ this.setPointerStateVisible( phet.chipper.queryParameters.showPointers );
this.setPointerAreaDisplayVisible( phet.chipper.queryParameters.showPointerAreas );
this.setHitAreaDisplayVisible( phet.chipper.queryParameters.showHitAreas );
this.setCanvasNodeBoundsVisible( phet.chipper.queryParameters.showCanvasNodeBounds );
Index: main/scenery/js/input/Input.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/main/scenery/js/input/Input.ts b/main/scenery/js/input/Input.ts
--- a/main/scenery/js/input/Input.ts (revision c9d36d46987a41556ad4f856b975e03b7ddeffee)
+++ b/main/scenery/js/input/Input.ts (date 1650766951067)
@@ -169,6 +169,9 @@
import NullableIO from '../../../tandem/js/types/NullableIO.js';
import NumberIO from '../../../tandem/js/types/NumberIO.js';
import { BatchedDOMEvent, BatchedDOMEventCallback, BatchedDOMEventType, BrowserEvents, Display, EventIO, Features, IInputListener, Mouse, Node, PDOMInstance, PDOMPointer, PDOMUtils, Pen, Pointer, scenery, SceneryEvent, SceneryListenerFunction, Touch, Trail, WindowTouch } from '../imports.js';
+import Property from '../../../axon/js/Property.js';
+import BooleanIO from '../../../tandem/js/types/BooleanIO.js';
+import ArrayIO from '../../../tandem/js/types/ArrayIO.js';
// This is the list of keys that get serialized AND deserialized. NOTE: Do not add or change this without
// consulting the PhET-iO IOType schema for this in EventIO
@@ -257,8 +260,8 @@
export default class Input {
- display: Display;
- rootNode: Node;
+ readonly display: Display;
+ readonly rootNode: Node;
attachToWindow: boolean;
batchDOMEvents: boolean;
@@ -272,9 +275,9 @@
mouse: Mouse | null;
// All active pointers.
- pointers: Pointer[];
+ readonly pointers: Pointer[];
- pointerAddedEmitter: TinyEmitter<[ Pointer ]>;
+ readonly pointerAddedEmitter: TinyEmitter<[ Pointer ]>;
// Whether we are currently firing events. We need to track this to handle re-entrant cases
// like https://github.com/phetsims/balloons-and-static-electricity/issues/406.
@@ -326,7 +329,9 @@
// Event listeners are not added until initializeEvents, and are stored in this Map so they can be removed
// again in detachEvents.
private pdomEventListenerMap?: Map<string, ( event: Event ) => void>;
-
+ readonly mousePointerPointProperty: Property<Vector2 | null>;
+ readonly mouseIsDownProperty: Property<boolean | null>;
+ readonly touchesProperty: Property<Vector2[]>;
/**
* @param display
@@ -368,6 +373,31 @@
// Declare the Actions that send scenery input events to the PhET-iO data stream. Note they use the default value
// of phetioReadOnly false, in case a client wants to synthesize events.
+ const mouseTandem = options.tandem.createTandem( 'mouse' );
+ this.mousePointerPointProperty = new Property<Vector2 | null>( null, {
+ useDeepEquality: true,
+ tandem: mouseTandem.createTandem( 'pointProperty' ),
+ phetioType: Property.PropertyIO( NullableIO( Vector2.Vector2IO ) ),
+ phetioReadOnly: true
+ } );
+
+ this.mouseIsDownProperty = new Property<boolean | null>( null, {
+ tandem: mouseTandem.createTandem( 'isDownProperty' ),
+ phetioType: Property.PropertyIO( NullableIO( BooleanIO ) ),
+ phetioReadOnly: true
+ } );
+
+ this.touchesProperty = new Property<Array<Vector2>>( [], {
+ phetioType: Property.PropertyIO( ArrayIO( Vector2.Vector2IO ) ),
+ tandem: options.tandem.createTandem( 'touchesProperty' ),
+ phetioReadOnly: true
+ } );
+
+ this.touchesProperty.link( touches => {
+
+ console.log( 'TTT', touches.length, touches );
+ } );
+
this.validatePointersAction = new PhetioAction( () => {
let i = this.pointers.length;
while ( i-- ) {
@@ -376,6 +406,7 @@
this.branchChangeEvents<Event>( pointer, pointer.lastDOMEvent, false );
}
}
+ // this.touchesProperty.value = this.pointers.filter( pointer => pointer instanceof Touch ).map( touch => touch.point );
}, {
phetioPlayback: true,
tandem: options.tandem.createTandem( 'validatePointersAction' ),
@@ -402,6 +433,8 @@
const mouse = this.ensureMouse( point );
mouse.id = id;
const pointChanged = mouse.down( point, event );
+ this.mousePointerPointProperty.value = point;
+ this.mouseIsDownProperty.value = true;
this.downEvent<MouseEvent>( mouse, event, pointChanged );
}, {
phetioPlayback: true,
@@ -418,6 +451,7 @@
this.mouseMoveAction = new PhetioAction( ( point: Vector2, event: MouseEvent ) => {
const mouse = this.ensureMouse( point );
mouse.move( point, event );
+ this.mousePointerPointProperty.value = point;
this.moveEvent<MouseEvent>( mouse, event );
}, {
phetioPlayback: true,
@@ -434,6 +468,7 @@
this.mouseOverAction = new PhetioAction( ( point: Vector2, event: MouseEvent ) => {
const mouse = this.ensureMouse( point );
mouse.over( point, event );
+ this.mousePointerPointProperty.value = point;
// TODO: how to handle mouse-over (and log it)... are we changing the pointer.point without a branch change?
}, {
phetioPlayback: true,
@@ -449,6 +484,7 @@
this.mouseOutAction = new PhetioAction( ( point: Vector2, event: MouseEvent ) => {
const mouse = this.ensureMouse( point );
mouse.out( point, event );
+ this.mousePointerPointProperty.value = point;
// TODO: how to handle mouse-out (and log it)... are we changing the pointer.point without a branch change?
}, {
phetioPlayback: true,
@@ -485,6 +521,7 @@
this.touchStartAction = new PhetioAction( ( id: number, point: Vector2, event: TouchEvent | PointerEvent ) => {
const touch = new Touch( id, point, event );
this.addPointer( touch );
+ this.touchesProperty.value = this.pointers.filter( pointer => pointer instanceof Touch ).map( touch => touch.point );
this.downEvent<TouchEvent | PointerEvent>( touch, event, false );
}, {
phetioPlayback: true,
@@ -505,6 +542,8 @@
const pointChanged = touch.end( point, event );
this.upEvent<TouchEvent | PointerEvent>( touch, event, pointChanged );
this.removePointer( touch );
+
+ this.touchesProperty.value = this.pointers.filter( pointer => pointer instanceof Touch ).map( touch => touch.point );
}
}, {
phetioPlayback: true,
@@ -524,6 +563,8 @@
assert && assert( touch instanceof Touch ); // eslint-disable-line
touch.move( point, event );
this.moveEvent<TouchEvent | PointerEvent>( touch, event );
+
+ this.touchesProperty.value = this.pointers.filter( pointer => pointer instanceof Touch ).map( touch => touch.point );
}
}, {
phetioPlayback: true,
@@ -1214,6 +1255,7 @@
sceneryLog && sceneryLog.Input && sceneryLog.Input( `mouseUp(${Input.debugText( point, event )});` );
sceneryLog && sceneryLog.Input && sceneryLog.push();
this.mouseUpAction.execute( point, event );
+ this.mouseIsDownProperty.value = false;
sceneryLog && sceneryLog.Input && sceneryLog.pop();
}
I have several questions before I can proceed. Would be good to discuss with @zepumph and potentially @jonathanolson:
Also, beware that the upstream and downstream sim will have their own pointers, so be careful not to overlap those states. |
|
Are those just for phet-io state? I'm unclear about the need. Do clients have needs? If it's just for state saving, presumably we can skip all of the Property stuff and just... save the state?
Wouldn't the typical process be for the
Unclear how this would work. It's possible to have multiple pens
The mouse is lazily created, so that wouldn't happen if we have things on the Mouse (but also... is phet-io incompatible with that type of lazy instantiation?) The hardcoded named mouse Properties seem clunky and easy to mess up. Having Properties defined on each Pointer in the normal phet-io way would presumably be easier to maintain? PhetioGroup for pointers, PhetioCapsule for the mouse? |
|
@zepumph and I discussed and agree
|
|
The pointer data can remain in the full state. We don't need a separate channel for this data. |
samreid
added a commit
that referenced
this issue
May 13, 2022
|
Implemented in the commit and working well in the state wrapper (select all instead of featured). I have not added an overlay for it yet. @jonathanolson what is the long-term plan for |
|
I used this patch to display the pointer states, and it worked well. Index: js/input/Input.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/input/Input.ts b/js/input/Input.ts
--- a/js/input/Input.ts (revision 895916a0a9e13d21bc5802ef4f95d95e7fd2bc21)
+++ b/js/input/Input.ts (date 1652453322440)
@@ -2009,7 +2009,11 @@
Input.InputIO = new IOType<Input>( 'InputIO', {
valueType: Input,
- applyState: () => {},
+ applyState: ( input: Input, stateObject ) => {
+
+ // @ts-ignore
+ input.restoredState = stateObject;
+ },
toStateObject: ( input: Input ) => {
return {
pointers: ArrayIOPointerIO.toStateObject( input.pointers )
Index: js/overlays/PointerAreaOverlay.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/overlays/PointerAreaOverlay.ts b/js/overlays/PointerAreaOverlay.ts
--- a/js/overlays/PointerAreaOverlay.ts (revision 895916a0a9e13d21bc5802ef4f95d95e7fd2bc21)
+++ b/js/overlays/PointerAreaOverlay.ts (date 1652453494670)
@@ -16,6 +16,16 @@
}
addShapes(): void {
+
+ if ( this.display._input!.restoredState ) {
+ const state = this.display._input!.restoredState;
+ console.log( state );
+
+ state.pointers.forEach( pointer => {
+ this.addShape( Shape.circle( pointer.point.x, pointer.point.y, 10 ), 'blue', false );
+ } );
+ }
+
new Trail( this.rootNode ).eachTrailUnder( trail => {
const node = trail.lastNode();
if ( !node.isVisible() ) {
Next steps for this issue:
|
|
In #803 @jonathanolson said:
and
I think it would be much better to remove |
samreid
added a commit
that referenced
this issue
May 18, 2022
|
In the commit, I removed PointerIO.isDown from the state. I agree we don't want a deprecated value in the state. For the future, it sounds natural and valuable to include data specific to certain pointers, such as adding an array of downProperties for the mouse as proposed in #803 (comment). However, I don't think we need to add that eagerly and there is not currently a use case for it. And in the future, we may promote a hybrid data stream + states for a richer playback. |
|
Sounds great. I think your patch is awesome at showing at easy it is to input this data into the pointerOverlay. I don't think I'd recommend anything else for now. In the future, we could add some sort of logic hidden behind playbackModeEnabledProperty or something along those lines. But since we currently don't support this, I think it may be more trouble than its worth to commit any more code without further design. I did one final check in the state wrapper. I put the set-state-rate at 5000ms and pressed with 3 fingers and my mouse. Here is what the state looks like: Can we add a line of phetioDocumentation on Input? We don't need to be redundant to the state schema that will be in the API file, but in general don't we add doc? Up to you. Feel free to close. |
samreid
added a commit
that referenced
this issue
May 19, 2022
|
I added brief |
|
Looks really good! I think it is ok to not have a mechanism to parse this state into the sim just yet. We can always build that out later. Closing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
From https://github.com/phetsims/phet-io/issues/1854. We want this data in states since we may use it to playback sims in the future via states.
The text was updated successfully, but these errors were encountered: