diff --git a/js/common/view/GeometricOpticsRulerNode.ts b/js/common/view/GeometricOpticsRulerNode.ts index 83117963..3936efb2 100644 --- a/js/common/view/GeometricOpticsRulerNode.ts +++ b/js/common/view/GeometricOpticsRulerNode.ts @@ -2,7 +2,11 @@ //TODO there appears to be a lot of unnecessary complication here for Creator pattern /** - * Scenery node for a movable ruler in the Geometric Optics simulation + * GeometricOpticsRulerNode is the view of a ruler. Responsibilities include: + * + * - It wraps a scenery-phet.RulerNode, which is re-created when the zoom level changes. As the zoom level is changed, + * the RulerNode's view dimensions remain constant, but it's tick marks change. + * - It handles dragging, including dragging back to the toolbox from whence it came. * * @author Sarah Chang (Swarthmore College) * @author Chris Malley (PixelZoom, Inc.) @@ -30,50 +34,61 @@ const MINIMUM_VISIBLE_LENGTH = GeometricOpticsConstants.RULER_MINIMUM_VISIBLE_LE class GeometricOpticsRulerNode extends Node { - // bounds of the toolbox - toolboxBounds: Bounds2; - readonly ruler: Ruler; - private readonly rulerOptions: any; //TYPESCRIPT any + private readonly ruler: Ruler; + private toolboxBounds: Bounds2; private readonly dragListener: DragListener; /** * @param ruler + * @param zoomTransformProperty + * @param absoluteScaleProperty * @param visibleBoundsProperty - * @param toolboxBounds - * @param modelViewTransform * @param options */ - constructor( ruler: Ruler, visibleBoundsProperty: Property, toolboxBounds: Bounds2, - modelViewTransform: ModelViewTransform2, options?: any ) { + constructor( ruler: Ruler, zoomTransformProperty: Property, + absoluteScaleProperty: Property, visibleBoundsProperty: Property, + options?: any ) { options = merge( { rulerOptions: { opacity: 0.8, minorTicksPerMajorTick: 4, - majorTickDistance: 10, // in model coordinate (cm) majorTickFont: new PhetFont( 13 ), insetsWidth: 0 } }, options ); + assert && assert( options.rulerOptions.tickMarksOnBottom === undefined ); + options.tickMarksOnBottom = ruler.isVertical(); + assert && assert( options.rotation === undefined ); options.rotation = ruler.isVertical() ? -Math.PI / 2 : 0; super( options ); this.ruler = ruler; - this.rulerOptions = options.rulerOptions; - this.toolboxBounds = toolboxBounds; + this.toolboxBounds = Bounds2.NOTHING; // to be set later via setToolboxBounds - // create and add a scenery-phet.RulerNode - this.setRulerNode( modelViewTransform, this.rulerOptions ); + // Create a RulerNode subcomponent to match absoluteScale. + // absoluteScaleProperty is derived from zoomTransformProperty, so we only need to observe absoluteScaleProperty. + absoluteScaleProperty.link( absoluteScale => { + + // update model + ruler.scaleLength( 1 / absoluteScale ); + + // update view + this.removeAllChildren(); + this.addChild( createRulerNode( this.ruler.length, zoomTransformProperty.value, absoluteScale, options.rulerOptions ) ); + } ); - // set the initial position and visibility - this.setPosition(); + this.updatePosition(); this.setInitialVisibility(); - // {DerivedProperty.} dragBounds Property for ruler: the view width and the height of the ruler - // will NOT change throughout the simulation + // update ruler node position based on ruler model position + // the GeometricOpticRulerNode exists from the lifetime of the simulation, so need to unlink. + ruler.positionProperty.link( () => this.updatePosition() ); + + // Drag bounds for the ruler, to keep some part of the ruler inside the visible bounds of the ScreenView. const rulerDragBoundsProperty = new DerivedProperty( [ visibleBoundsProperty ], ( visibleBounds: Bounds2 ) => { @@ -95,21 +110,19 @@ class GeometricOpticsRulerNode extends Node { -this.height ); } } ); + rulerDragBoundsProperty.link( dragBounds => { + ruler.positionProperty.value = dragBounds.closestPointTo( ruler.positionProperty.value ); + } ); this.dragListener = new DragListener( { cursor: 'pointer', useInputListenerCursor: true, positionProperty: ruler.positionProperty, dragBoundsProperty: rulerDragBoundsProperty, - start: () => { - - // move this node on top of all the nodes - this.moveToFront(); - }, - + start: () => this.moveToFront(), end: ( event: SceneryEvent ) => { - // return ruler to toolbox if the pointer is within the toolbox + // Return ruler to toolbox if the pointer is within the toolbox. assert && assert( event.pointer.point instanceof Vector2 ); if ( this.toolboxBounds.containsPoint( this.globalToParentPoint( event.pointer.point as Vector2 ) ) ) { this.visible = false; @@ -117,29 +130,21 @@ class GeometricOpticsRulerNode extends Node { } } ); this.addInputListener( this.dragListener ); - - // update ruler node position based on ruler model position - // the GeometricOpticRulerNode exists from the lifetime of the simulation, so need to unlink. - ruler.positionProperty.link( () => this.setPosition() ); - - // prevent the ruler from escaping the visible bounds - rulerDragBoundsProperty.link( dragBounds => { - ruler.positionProperty.value = dragBounds.closestPointTo( ruler.positionProperty.value ); - } ); } /** * Resets the visibility and position of this node */ public reset(): void { - this.setPosition(); //TODO why isn't it sufficient to reset Ruler.positionProperty? + this.updatePosition(); //TODO why isn't it sufficient to reset Ruler.positionProperty? this.setInitialVisibility(); //TODO add visibleProperty to Ruler model element } - /** - * Sets the position of the ruler - */ - public setPosition(): void { + public setToolboxBounds( toolboxBounds: Bounds2 ): void { + this.toolboxBounds = toolboxBounds; + } + + public updatePosition(): void { if ( this.ruler.isVertical() ) { // set initial position of the ruler - leftBottom since rotated 90 degrees @@ -159,20 +164,6 @@ class GeometricOpticsRulerNode extends Node { this.dragListener.press( event, this ); } - /** - * Adds a new scenery-phet.RulerNode to the parent, detaching the previous RulerNode. - * @param modelViewTransform - * @param options - */ - public setRulerNode( modelViewTransform: ModelViewTransform2, options?: any ): void { - - // remove previous instances of rulerNode - this.removeAllChildren(); - - // add and create new - this.addChild( this.getRulerNode( modelViewTransform, options ) ); - } - //TODO why is this needed? /** * Sets the visibility of this node to false @@ -180,54 +171,50 @@ class GeometricOpticsRulerNode extends Node { private setInitialVisibility(): void { this.visible = false; } +} - /** - * Returns a scenery-phet.RulerNode appropriate for the model view transform - * @param modelViewTransform - * @param options - */ - private getRulerNode( modelViewTransform: ModelViewTransform2, options?: any ): Node { +/** + * Creates a scenery-phet.RulerNode appropriate for the modelViewTransform and zoom scale. + * @param rulerLength + * @param modelViewTransform + * @param absoluteScale + * @param options + */ +function createRulerNode( rulerLength: number, modelViewTransform: ModelViewTransform2, absoluteScale: number, options?: any ): Node { - options = merge( {}, this.rulerOptions, options ); + options = merge( {}, options ); - // define the length ruler - const rulerWidth = modelViewTransform.modelToViewDeltaX( this.ruler.length ); + assert && assert( options.majorTickDistance === undefined ); + options.majorTickDistance = 10 / absoluteScale; // in model coordinate (cm) - // separation between the major ticks mark - const majorTickWidth = modelViewTransform.modelToViewDeltaX( - options.majorTickDistance ); + // define the length ruler + const rulerWidth = modelViewTransform.modelToViewDeltaX( rulerLength ); - const numberOfMajorTicks = Math.floor( rulerWidth / majorTickWidth ) + 1; + // separation between the major ticks mark + const majorTickWidth = modelViewTransform.modelToViewDeltaX( options.majorTickDistance ); - // create major ticks label - const majorTickLabels = []; - for ( let i = 0; i < numberOfMajorTicks; i++ ) { + // create major ticks label + const numberOfMajorTicks = Math.floor( rulerWidth / majorTickWidth ) + 1; + const majorTickLabels = []; + for ( let i = 0; i < numberOfMajorTicks; i++ ) { - const majorTickInterval = options.majorTickDistance; + const majorTickInterval = options.majorTickDistance; - // skip labels on every other major ticks - if ( i % 2 === 0 ) { - majorTickLabels[ i ] = Utils.toFixed( i * majorTickInterval, 0 ); - } - else { - majorTickLabels[ i ] = ''; - } + // skip labels on every other major ticks + if ( i % 2 === 0 ) { + majorTickLabels[ i ] = Utils.toFixed( i * majorTickInterval, 0 ); } + else { + majorTickLabels[ i ] = ''; + } + } - // set the units at the end of ruler - const rulerOptions = merge( { - unitsMajorTickIndex: numberOfMajorTicks - 3 - }, options ); + // set the units at the end of ruler + assert && assert( options.unitsMajorTickIndex === undefined ); + options.unitsMajorTickIndex = numberOfMajorTicks - 3; - return new RulerNode( - rulerWidth, - GeometricOpticsConstants.RULER_HEIGHT, - majorTickWidth, - majorTickLabels, - geometricOpticsStrings.centimeters, - rulerOptions - ); - } + return new RulerNode( rulerWidth, GeometricOpticsConstants.RULER_HEIGHT, + majorTickWidth, majorTickLabels, geometricOpticsStrings.centimeters, options ); } geometricOptics.register( 'GeometricOpticsRulerNode', GeometricOpticsRulerNode ); diff --git a/js/common/view/GeometricOpticsRulersLayer.ts b/js/common/view/GeometricOpticsRulersLayer.ts deleted file mode 100644 index 94426996..00000000 --- a/js/common/view/GeometricOpticsRulersLayer.ts +++ /dev/null @@ -1,128 +0,0 @@ -// Copyright 2021, University of Colorado Boulder - -//TODO get rid of GeometricOpticsRulersLayer, move responsibilities into GeometricOpticsRulerNode -/** - * A layer that contains 1 horizontal ruler and 1 vertical ruler - * Responsible for updating the rulers when zooming - * - * @author Martin Veillette - * @author Chris Malley (PixelZoom, Inc.) - */ - -import Property from '../../../../axon/js/Property.js'; -import Bounds2 from '../../../../dot/js/Bounds2.js'; -import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js'; -import { Node } from '../../../../scenery/js/imports.js'; -import geometricOptics from '../../geometricOptics.js'; -import Ruler from '../model/Ruler.js'; -import GeometricOpticsRulerNode from './GeometricOpticsRulerNode.js'; - -class GeometricOpticsRulersLayer extends Node { - - readonly toolboxBounds: Bounds2; - horizontalRulerNode: GeometricOpticsRulerNode; - verticalRulerNode: GeometricOpticsRulerNode; - - /** - * @param horizontalRuler - * @param verticalRuler - * @param visibleBoundsProperty - * @param absoluteScaleProperty - * @param modelViewTransformProperty - * @param options - */ - constructor( horizontalRuler: Ruler, verticalRuler: Ruler, visibleBoundsProperty: Property, - absoluteScaleProperty: Property, modelViewTransformProperty: Property, - options?: any ) { - - super( options ); - - // set to infinity, will be properly initialized after constructor is called - //TODO since this is set after construction by GeometricOpticsScreen and it passed to GeometricOpticsRulerNode, it might preferable to have toolboxBoundsProperty - this.toolboxBounds = new Bounds2( Number.NEGATIVE_INFINITY, - Number.NEGATIVE_INFINITY, - Number.POSITIVE_INFINITY, - Number.POSITIVE_INFINITY ); - - //TODO creating a new GeometricOpticsRulerNode each time the scale changes will be a problem for PhET-iO - /** - * Returns a GeometricOpticsRulerNode - * @param ruler - * @param absoluteScale - */ - const createRulerNode = ( ruler: Ruler, absoluteScale: number ): GeometricOpticsRulerNode => { - - const rulerOptions = getOptions( ruler, absoluteScale ); - - return new GeometricOpticsRulerNode( - ruler, - visibleBoundsProperty, - this.toolboxBounds, - modelViewTransformProperty.value, - rulerOptions ); - }; - - /** - * Returns the appropriate options for the scale - * It also updates the length of the ruler as a side effect - * @param ruler - * @param absoluteScale - */ - //TYPESCRIPT return type - const getOptions = ( ruler: Ruler, absoluteScale: number ) => { - - // we want to scale model length inversely as the scale such that the view length remains the same - ruler.scaleLength( 1 / absoluteScale ); - - // only vertical ruler has tick marks on bottom - const tickMarksOnBottom = ruler.isVertical(); - - return { - majorTickDistance: 10 / absoluteScale, // in model coordinate (cm) - tickMarksOnBottom: tickMarksOnBottom - }; - }; - - /** - * Update the GeometricOpticsRulerNode based on the new scale - * @param rulerNode - * @param absoluteScale - */ - const updateRulerNode = ( rulerNode: GeometricOpticsRulerNode, absoluteScale: number ): void => { - - // generate new options for the ruler node - const rulerOptions = getOptions( rulerNode.ruler, absoluteScale ); - - // set a new RulerNode within GeometricOpticsRulerNode - rulerNode.setRulerNode( modelViewTransformProperty.value, rulerOptions ); - }; - - this.horizontalRulerNode = createRulerNode( horizontalRuler, absoluteScaleProperty.value ); - this.verticalRulerNode = createRulerNode( verticalRuler, absoluteScaleProperty.value ); - this.addChild( this.horizontalRulerNode ); - this.addChild( this.verticalRulerNode ); - - // update rulers when scale changes - absoluteScaleProperty.link( absoluteScale => { - - // update the horizontal ruler based on the new scale - updateRulerNode( this.horizontalRulerNode, absoluteScale ); - - // update the vertical ruler based on the new scale - updateRulerNode( this.verticalRulerNode, absoluteScale ); - } ); - } - - public dispose(): void { - assert && assert( false, 'dispose is not supported, exists for the lifetime of the sim' ); - super.dispose(); - } - - public reset(): void { - this.horizontalRulerNode.reset(); - this.verticalRulerNode.reset(); - } -} - -geometricOptics.register( 'GeometricOpticsRulersLayer', GeometricOpticsRulersLayer ); -export default GeometricOpticsRulersLayer; \ No newline at end of file diff --git a/js/common/view/GeometricOpticsScreenView.ts b/js/common/view/GeometricOpticsScreenView.ts index 45d6b1cd..ffea86b7 100644 --- a/js/common/view/GeometricOpticsScreenView.ts +++ b/js/common/view/GeometricOpticsScreenView.ts @@ -28,7 +28,7 @@ import GeometricOpticsModel from '../model/GeometricOpticsModel.js'; import DebugPointNode from './DebugPointNode.js'; import FocalPointNode from './FocalPointNode.js'; import GeometricOpticsControlPanel from './GeometricOpticsControlPanel.js'; -import GeometricOpticsRulersLayer from './GeometricOpticsRulersLayer.js'; +import RulersLayer from './RulersLayer.js'; import LabelsNode from './LabelsNode.js'; import LightRaysNode from './LightRaysNode.js'; import OpticalAxis from './OpticalAxis.js'; @@ -126,13 +126,8 @@ class GeometricOpticsScreenView extends ScreenView { // Things that are outside the Experiment Area ===================================================================== // create Rulers - const rulersLayer = new GeometricOpticsRulersLayer( - model.horizontalRuler, - model.verticalRuler, - this.visibleBoundsProperty, - absoluteScaleProperty, - zoomTransformProperty - ); + const rulersLayer = new RulersLayer( model.horizontalRuler, model.verticalRuler, + zoomTransformProperty, absoluteScaleProperty, this.visibleBoundsProperty ); // create control panel at the bottom of the screen const controlPanel = new GeometricOpticsControlPanel( model.representationProperty, model.optic, @@ -149,8 +144,8 @@ class GeometricOpticsScreenView extends ScreenView { tandem: config.tandem.createTandem( 'toolbox' ) } ); - // Tell the rulersLayer where the toolbox is. - rulersLayer.toolboxBounds.set( toolbox.bounds ); + // Tell the rulers where the toolbox is. + rulersLayer.setToolboxBounds( toolbox.bounds ); // radio buttons for the shape of the optic const opticShapeRadioButtonGroup = new OpticShapeRadioButtonGroup( model.optic, { @@ -413,6 +408,7 @@ class GeometricOpticsScreenView extends ScreenView { } } + //TODO make this a private function /** * Returns a model-view transform appropriate for the zoom level * @param zoomLevel diff --git a/js/common/view/RulersLayer.ts b/js/common/view/RulersLayer.ts new file mode 100644 index 00000000..9ea9e093 --- /dev/null +++ b/js/common/view/RulersLayer.ts @@ -0,0 +1,61 @@ +// Copyright 2021, University of Colorado Boulder + +/** + * A layer that contains 1 horizontal ruler and 1 vertical ruler + * + * @author Martin Veillette + * @author Chris Malley (PixelZoom, Inc.) + */ + +import Property from '../../../../axon/js/Property.js'; +import Bounds2 from '../../../../dot/js/Bounds2.js'; +import ModelViewTransform2 from '../../../../phetcommon/js/view/ModelViewTransform2.js'; +import { Node } from '../../../../scenery/js/imports.js'; +import geometricOptics from '../../geometricOptics.js'; +import Ruler from '../model/Ruler.js'; +import GeometricOpticsRulerNode from './GeometricOpticsRulerNode.js'; + +class RulersLayer extends Node { + + horizontalRulerNode: GeometricOpticsRulerNode; + verticalRulerNode: GeometricOpticsRulerNode; + + /** + * @param horizontalRuler + * @param verticalRuler + * @param zoomTransformProperty + * @param absoluteScaleProperty + * @param visibleBoundsProperty + * @param options + */ + constructor( horizontalRuler: Ruler, verticalRuler: Ruler, zoomTransformProperty: Property, + absoluteScaleProperty: Property, visibleBoundsProperty: Property, options?: any ) { + + super( options ); + + this.horizontalRulerNode = new GeometricOpticsRulerNode( horizontalRuler, + zoomTransformProperty, absoluteScaleProperty, visibleBoundsProperty ); + this.verticalRulerNode = new GeometricOpticsRulerNode( verticalRuler, + zoomTransformProperty, absoluteScaleProperty, visibleBoundsProperty ); + this.addChild( this.horizontalRulerNode ); + this.addChild( this.verticalRulerNode ); + } + + public dispose(): void { + assert && assert( false, 'dispose is not supported, exists for the lifetime of the sim' ); + super.dispose(); + } + + public reset(): void { + this.horizontalRulerNode.reset(); + this.verticalRulerNode.reset(); + } + + public setToolboxBounds( toolboxBounds: Bounds2 ): void { + this.horizontalRulerNode.setToolboxBounds( toolboxBounds ); + this.verticalRulerNode.setToolboxBounds( toolboxBounds ); + } +} + +geometricOptics.register( 'RulersLayer', RulersLayer ); +export default RulersLayer; \ No newline at end of file diff --git a/js/common/view/RulersToolbox.ts b/js/common/view/RulersToolbox.ts index 54a54670..42fc00f3 100644 --- a/js/common/view/RulersToolbox.ts +++ b/js/common/view/RulersToolbox.ts @@ -18,7 +18,7 @@ import { Node } from '../../../../scenery/js/imports.js'; import Panel from '../../../../sun/js/Panel.js'; import Tandem from '../../../../tandem/js/Tandem.js'; import geometricOptics from '../../geometricOptics.js'; -import GeometricOpticsRulersLayer from './GeometricOpticsRulersLayer.js'; +import RulersLayer from './RulersLayer.js'; import GeometricOpticsRulerNode from './GeometricOpticsRulerNode.js'; class RulersToolbox extends Panel { @@ -27,7 +27,7 @@ class RulersToolbox extends Panel { * @param rulersLayer * @param options */ - constructor( rulersLayer: GeometricOpticsRulersLayer, options?: any ) { + constructor( rulersLayer: RulersLayer, options?: any ) { options = merge( {