From 55393e739e553f74706085182ab433eaac603ac9 Mon Sep 17 00:00:00 2001 From: pixelzoom Date: Wed, 26 Aug 2020 12:17:41 -0600 Subject: [PATCH] performance improvements for SpriteInstance hierarchy, #60 --- .../BunnySelectionRectangleSprite.js | 3 +- .../BunnySelectionRectangleSpriteInstance.js | 4 +- .../view/environment/BunnySpriteInstance.js | 4 +- .../environment/OrganismSpriteInstance.js | 86 ++++++++++++------- .../view/environment/ShrubSpriteInstance.js | 4 +- .../view/environment/WolfSpriteInstance.js | 4 +- 6 files changed, 61 insertions(+), 44 deletions(-) diff --git a/js/common/view/environment/BunnySelectionRectangleSprite.js b/js/common/view/environment/BunnySelectionRectangleSprite.js index ed115e40..bf5213be 100644 --- a/js/common/view/environment/BunnySelectionRectangleSprite.js +++ b/js/common/view/environment/BunnySelectionRectangleSprite.js @@ -21,9 +21,8 @@ class BunnySelectionRectangleSprite extends Sprite { /** * @param {HTMLImageElement} maxImage - the largest bunny image - * @param {Object} [options] */ - constructor( maxImage, options ) { + constructor( maxImage ) { assert && assert( maxImage instanceof HTMLImageElement, 'invalid maxImage' ); assert && assert( maxImage.width > 0 && maxImage.height > 0, 'maxImage does not have valid dimensions' ); diff --git a/js/common/view/environment/BunnySelectionRectangleSpriteInstance.js b/js/common/view/environment/BunnySelectionRectangleSpriteInstance.js index e87195bb..3c31c1d0 100644 --- a/js/common/view/environment/BunnySelectionRectangleSpriteInstance.js +++ b/js/common/view/environment/BunnySelectionRectangleSpriteInstance.js @@ -27,9 +27,7 @@ class BunnySelectionRectangleSpriteInstance extends OrganismSpriteInstance { assert && assert( bunny instanceof Bunny, 'invalid bunny' ); assert && assert( sprite instanceof BunnySelectionRectangleSprite, 'invalid sprite' ); - super( bunny, sprite, { - baseScale: NaturalSelectionConstants.BUNNY_IMAGE_SCALE - } ); + super( bunny, sprite, NaturalSelectionConstants.BUNNY_IMAGE_SCALE ); } } diff --git a/js/common/view/environment/BunnySpriteInstance.js b/js/common/view/environment/BunnySpriteInstance.js index b5f64341..28c3c65b 100644 --- a/js/common/view/environment/BunnySpriteInstance.js +++ b/js/common/view/environment/BunnySpriteInstance.js @@ -24,9 +24,7 @@ class BunnySpriteInstance extends OrganismSpriteInstance { assert && assert( bunny instanceof Bunny, 'invalid bunny' ); assert && assert( sprite instanceof Sprite, 'invalid sprite' ); - super( bunny, sprite, { - baseScale: NaturalSelectionConstants.BUNNY_IMAGE_SCALE - } ); + super( bunny, sprite, NaturalSelectionConstants.BUNNY_IMAGE_SCALE ); } } diff --git a/js/common/view/environment/OrganismSpriteInstance.js b/js/common/view/environment/OrganismSpriteInstance.js index 6b2ca556..197b238c 100644 --- a/js/common/view/environment/OrganismSpriteInstance.js +++ b/js/common/view/environment/OrganismSpriteInstance.js @@ -8,60 +8,85 @@ * @author Chris Malley (PixelZoom, Inc.) */ -import Multilink from '../../../../../axon/js/Multilink.js'; -import merge from '../../../../../phet-core/js/merge.js'; import Sprite from '../../../../../scenery/js/util/Sprite.js'; import SpriteInstance from '../../../../../scenery/js/util/SpriteInstance.js'; import naturalSelection from '../../../naturalSelection.js'; import Organism from '../../model/Organism.js'; import XDirection from '../../model/XDirection.js'; +import NaturalSelectionUtils from '../../NaturalSelectionUtils.js'; class OrganismSpriteInstance extends SpriteInstance { /** * @param {Organism} organism * @param {Sprite} sprite - * @param {Object} [options] + * @param {number} baseScale - the base amount to scale, tuned based on the PNG file dimensions */ - constructor( organism, sprite, options ) { + constructor( organism, sprite, baseScale ) { + // args are validated by initialize - assert && assert( organism instanceof Organism, 'invalid organism' ); - assert && assert( sprite instanceof Sprite, 'invalid sprite' ); + super(); - options = merge( { - baseScale: 1 // the base amount to scale, tuned based on the PNG file dimensions - }, options ); + // Set field in super SpriteInstance. Every Organism needs to be both translated and scaled + // because the view is a 2D projection of a 3D model position. + this.transformType = SpriteInstance.TransformType.TRANSLATION_AND_SCALE; - super(); + // @private + this.organismListener = this.updateMatrix.bind( this ); + + this.initialize( organism, sprite, baseScale ); + } + + /** + * Initializes the OrganismSpriteInstance. This is factored out of the constructor (and is public) in case + * we ever want to leverage SpriteInstance's Poolable features. + * @param organism + * @param sprite + * @param baseScale + * @protected for use by Poolable + */ + initialize( organism, sprite, baseScale ) { + + assert && assert( organism instanceof Organism, 'invalid organism' ); + assert && assert( sprite instanceof Sprite, 'invalid sprite' ); + assert && assert( NaturalSelectionUtils.isPositive( baseScale ), `invalid baseScale: ${baseScale}` ); // Set fields in super SpriteInstance this.sprite = sprite; - this.transformType = SpriteInstance.TransformType.TRANSLATION_AND_SCALE; // @public (read-only) this.organism = organism; - // Update position and direction. Must be disposed. - const multilink = new Multilink( - [ organism.positionProperty, organism.xDirectionProperty ], - ( position, xDirection ) => { + // @private + this.baseScale = baseScale; - // compute scale and position, in view coordinates - const viewScale = options.baseScale * organism.modelViewTransform.getViewScale( position.z ); - const viewX = organism.modelViewTransform.modelToViewX( position ); - const viewY = organism.modelViewTransform.modelToViewY( position ); + // Update position and direction, unlink in dispose. Do not use a Multilink or define disposeOrganismSpriteInstance + // because we will be creating a large number of OrganismSpriteInstance instances. + assert && assert( this.organismListener, 'organismListener should exist by now' ); + this.organism.positionProperty.link( this.organismListener ); + this.organism.xDirectionProperty.link( this.organismListener ); + } - // update the matrix in the most efficient way possible - this.matrix.set00( viewScale * XDirection.toSign( xDirection ) ); // reflected to account for x direction - this.matrix.set11( viewScale ); - this.matrix.set02( viewX ); - this.matrix.set12( viewY ); - } ); + /** + * Updates the matrix to match the organism's position and xDirection. + * @private + */ + updateMatrix() { - // @private - this.disposeOrganismSpriteInstance = () => { - multilink.dispose(); - }; + const position = this.organism.positionProperty.value; + const xDirection = this.organism.xDirectionProperty.value; + + // compute scale and position, in view coordinates + const viewScale = this.baseScale * this.organism.modelViewTransform.getViewScale( position.z ); + const viewX = this.organism.modelViewTransform.modelToViewX( position ); + const viewY = this.organism.modelViewTransform.modelToViewY( position ); + + // update the matrix in the most efficient way possible + this.matrix.set00( viewScale * XDirection.toSign( xDirection ) ); // reflected to account for x direction + this.matrix.set11( viewScale ); + this.matrix.set02( viewX ); + this.matrix.set12( viewY ); + assert && assert( this.matrix.isFinite(), 'matrix should be finite' ); } /** @@ -69,7 +94,8 @@ class OrganismSpriteInstance extends SpriteInstance { * @override */ dispose() { - this.disposeOrganismSpriteInstance(); + this.organism.positionProperty.unlink( this.organismListener ); + this.organism.xDirectionProperty.unlink( this.organismListener ); super.dispose && super.dispose(); } } diff --git a/js/common/view/environment/ShrubSpriteInstance.js b/js/common/view/environment/ShrubSpriteInstance.js index 4a4ea339..ebfc9cad 100644 --- a/js/common/view/environment/ShrubSpriteInstance.js +++ b/js/common/view/environment/ShrubSpriteInstance.js @@ -28,9 +28,7 @@ class ShrubSpriteInstance extends OrganismSpriteInstance { assert && assert( tenderSprite instanceof Sprite, 'invalid tenderSprite' ); assert && assert( toughSprite instanceof Sprite, 'invalid toughSprite' ); - super( shrub, isTough ? toughSprite : tenderSprite, { - baseScale: NaturalSelectionConstants.SHRUB_IMAGE_SCALE - } ); + super( shrub, ( isTough ? toughSprite : tenderSprite ), NaturalSelectionConstants.SHRUB_IMAGE_SCALE ); // @private this.tenderSprite = tenderSprite; diff --git a/js/common/view/environment/WolfSpriteInstance.js b/js/common/view/environment/WolfSpriteInstance.js index d6b466d9..09f54dda 100644 --- a/js/common/view/environment/WolfSpriteInstance.js +++ b/js/common/view/environment/WolfSpriteInstance.js @@ -24,9 +24,7 @@ class WolfSpriteInstance extends OrganismSpriteInstance { assert && assert( wolf instanceof Wolf, 'invalid wolf' ); assert && assert( sprite instanceof Sprite, 'invalid sprite' ); - super( wolf, sprite, { - baseScale: NaturalSelectionConstants.WOLF_IMAGE_SCALE - } ); + super( wolf, sprite, NaturalSelectionConstants.WOLF_IMAGE_SCALE ); } }