From 51c36fd468cdca13d6dd81d7a08941ab9bb3ea5e Mon Sep 17 00:00:00 2001 From: Sam Reid Date: Wed, 6 Apr 2022 15:55:26 -0600 Subject: [PATCH] Fix memory leak, see https://github.com/phetsims/center-and-variability/issues/76 --- js/common/view/CAVScreenView.ts | 66 ++++++++++++++------------------- 1 file changed, 27 insertions(+), 39 deletions(-) diff --git a/js/common/view/CAVScreenView.ts b/js/common/view/CAVScreenView.ts index 323b52ca..6ed650e9 100644 --- a/js/common/view/CAVScreenView.ts +++ b/js/common/view/CAVScreenView.ts @@ -162,6 +162,7 @@ class CAVScreenView extends ScreenView { model.objectGroup.elementDisposedEmitter.addListener( casObject => { const viewNode = map.get( casObject )!; objectNodeGroup.disposeElement( viewNode ); + map.delete( casObject ); } ); this.topCheckboxGroup = new TopRepresentationCheckboxGroup( model, merge( { @@ -182,26 +183,26 @@ class CAVScreenView extends ScreenView { } ); this.addChild( this.playAreaMedianIndicatorNode ); - const updateMedianNode = () => { - const medianValue = model.medianValueProperty.value; - const visible = medianValue !== null && model.isShowingPlayAreaMedianProperty.value; - - if ( visible ) { - - // if there is a ball at that location, go above the ball - const ballsAtLocation = model.objectGroup.filter( casObject => casObject.valueProperty.value === medianValue ); - const modelHeight = ballsAtLocation.length * model.objectType.radius * 2; // assumes no spacing - - const viewHeight = this.modelViewTransform.modelToViewDeltaY( modelHeight ); - - this.playAreaMedianIndicatorNode.centerX = this.modelViewTransform.modelToViewX( medianValue ); - this.playAreaMedianIndicatorNode.bottom = this.modelViewTransform.modelToViewY( 0 ) + viewHeight; - } - this.playAreaMedianIndicatorNode.visible = visible; - }; - model.medianValueProperty.link( updateMedianNode ); - model.objectChangedEmitter.addListener( updateMedianNode ); - model.isShowingPlayAreaMedianProperty.link( updateMedianNode ); + // const updateMedianNode = () => { + // const medianValue = model.medianValueProperty.value; + // const visible = medianValue !== null && model.isShowingPlayAreaMedianProperty.value; + // + // if ( visible ) { + // + // // if there is a ball at that location, go above the ball + // const ballsAtLocation = model.objectGroup.filter( casObject => casObject.valueProperty.value === medianValue ); + // const modelHeight = ballsAtLocation.length * model.objectType.radius * 2; // assumes no spacing + // + // const viewHeight = this.modelViewTransform.modelToViewDeltaY( modelHeight ); + // + // this.playAreaMedianIndicatorNode.centerX = this.modelViewTransform.modelToViewX( medianValue ); + // this.playAreaMedianIndicatorNode.bottom = this.modelViewTransform.modelToViewY( 0 ) + viewHeight; + // } + // this.playAreaMedianIndicatorNode.visible = visible; + // }; + // model.medianValueProperty.link( updateMedianNode ); + // model.objectChangedEmitter.addListener( updateMedianNode ); + // model.isShowingPlayAreaMedianProperty.link( updateMedianNode ); this.medianPredictionNode = new PredictionNode( model.medianPredictionProperty, this.modelViewTransform, model.physicalRange, { center: this.layoutBounds.center, @@ -219,15 +220,7 @@ class CAVScreenView extends ScreenView { } ); this.resetAllButton = new ResetAllButton( { - listener: () => { - this.interruptSubtreeInput(); // cancel interactions that may be in progress - - model.reset(); - - // hide the dragIndicatorArrowNode and reset the flag for if it has been dragged already - objectHasBeenDragged = false; - dragIndicatorArrowNode.visible = false; - }, + listener: this.doReset.bind( this ), right: this.layoutBounds.maxX - CAVConstants.SCREEN_VIEW_X_MARGIN, bottom: this.layoutBounds.maxY - CAVConstants.SCREEN_VIEW_Y_MARGIN, tandem: options.tandem.createTandem( 'resetAllButton' ) @@ -235,16 +228,7 @@ class CAVScreenView extends ScreenView { this.eraseButton = new EraserButton( { tandem: options.tandem.createTandem( 'eraseButton' ), - listener: () => { - - // Interrupt dragging of existing objects - this.interruptSubtreeInput(); - - model.clearData(); - - // hide the dragIndicatorArrowNode but don't reset objectHasBeenDragged - dragIndicatorArrowNode.visible = false; - }, + listener: this.doReset.bind( this ), iconWidth: 26, right: this.resetAllButton.left - CAVConstants.SCREEN_VIEW_X_MARGIN, centerY: this.resetAllButton.centerY @@ -259,6 +243,10 @@ class CAVScreenView extends ScreenView { */ step( dt: number ): void { } + + doReset(): void { + this.model.reset(); + } } centerAndVariability.register( 'CAVScreenView', CAVScreenView );