Skip to content

Commit

Permalink
performance optimization - made the pointer image for the individual …
Browse files Browse the repository at this point in the history
…cell only be created when needed, see #117
  • Loading branch information
jbphet committed Nov 4, 2015
1 parent dec5404 commit 37ef8d2
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 21 deletions.
9 changes: 7 additions & 2 deletions js/common/view/table/MultiplicationTableNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ define( function( require ) {
* @param {Array} levelModels - Array of descriptions for each level.
* @param {Array} answerSheet - 2D array that tracks problems that have and haven't been answered
* @param {boolean} animateAnswer - flag that controls whether answer appears to fly into the cell or just appears
* @param {boolean} pointerUsedInCells - flag that indicates whether the cells need to support a pointer finger when highlighted
*
* @constructor
*/
function MultiplicationTableNode( levelProperty, stateProperty, levelModels, answerSheet, animateAnswer ) {
function MultiplicationTableNode( levelProperty, stateProperty, levelModels, answerSheet, animateAnswer, pointerUsedInCells ) {
var self = this;
Node.call( this );

Expand Down Expand Up @@ -114,7 +115,11 @@ define( function( require ) {
self.cells[ levelIndex ][ i ][ j ] = new MultiplierTableHeaderCell( i.toString(), buttonOptions );
}
else {
self.cells[ levelIndex ][ i ][ j ] = new MultiplierTableBodyCell( ( i * j ).toString(), buttonOptions );
self.cells[ levelIndex ][ i ][ j ] = new MultiplierTableBodyCell(
( i * j ).toString(),
pointerUsedInCells,
buttonOptions
);
}
hBoxChildren.push( self.cells[ levelIndex ][ i ][ j ] );
}
Expand Down
32 changes: 16 additions & 16 deletions js/common/view/table/MultiplierTableBodyCell.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,30 +26,30 @@ define( function( require ) {
//TODO: The options should either be consolidated or renamed, since the API makes it such that they can't both be optional.
/**
* @param {Text} contentText - Text label for button.
* @param {boolean} pointerUsed - Flag that indicates whether the point hand should be available in this cell.
* @param {Object} backgroundOptions - Background options for button.
* @param {Object} textOptions - Text options for button.
*
* @constructor
*/
function MultiplierTableBodyCell( contentText, backgroundOptions, textOptions ) {
function MultiplierTableBodyCell( contentText, pointerUsed, backgroundOptions, textOptions ) {
backgroundOptions = _.extend( {
fill: NORMAL_COLOR
}, backgroundOptions );
AbstractCell.call( this, backgroundOptions, textOptions );

// create pointer for active state
// TODO: Seems inefficient and unnecessary that there is an image node created for every cell when they are not used
// for the multiply and divide screens. Consider having either an option for whether the pointer is used or a pool
// of pointers for the various sized needed. Or both.
this._pointer = new Image( pointingHandImage, { visible: false } );
if ( pointerUsed ) {
// create pointer for active state
this._pointer = new Image( pointingHandImage, { visible: false } );

// set position and size for pointer
this._pointer.scale( backgroundOptions.width / this._pointer.getWidth() * 0.75, backgroundOptions.height / this._pointer.getHeight() * 0.75 );
this._pointer.centerX = backgroundOptions.width / 2;
this._pointer.centerY = backgroundOptions.height / 2;
// set position and size for pointer
this._pointer.scale( backgroundOptions.width / this._pointer.getWidth() * 0.75, backgroundOptions.height / this._pointer.getHeight() * 0.75 );
this._pointer.centerX = backgroundOptions.width / 2;
this._pointer.centerY = backgroundOptions.height / 2;

// add pointer to node
this.addChild( this._pointer );
// add pointer to node
this.addChild( this._pointer );
}

this.setText( contentText );
this.hideText();
Expand All @@ -59,19 +59,19 @@ define( function( require ) {
// TODO: active may no longer be needed
active: function() {
this.setBackgroundFill( ACTIVE_COLOR );
this._pointer.visible = false;
this._pointer && ( this._pointer.visible = false );
},
hover: function() {
this.setBackgroundFill( HOVER_COLOR );
this._pointer.visible = true;
this._pointer && ( this._pointer.visible = true );
},
normal: function() {
this.setBackgroundFill( NORMAL_COLOR );
this._pointer.visible = false;
this._pointer && ( this._pointer.visible = false );
},
select: function() {
this.setBackgroundFill( SELECT_COLOR );
this._pointer.visible = false;
this._pointer && ( this._pointer.visible = false );
}
} );
} );
2 changes: 1 addition & 1 deletion js/divide/view/MultiplicationTableDivideNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ define( function( require ) {
*/
function MultiplicationTableDivideNode( problemModel, answerSheet, stateProperty, levelProperty, levelModels ) {
var self = this;
MultiplicationTableNode.call( this, levelProperty, stateProperty, levelModels, answerSheet, true );
MultiplicationTableNode.call( this, levelProperty, stateProperty, levelModels, answerSheet, true, false );

stateProperty.lazyLink( function( state ) {
// set view for multiplication table after choosing left and right multipliers
Expand Down
2 changes: 1 addition & 1 deletion js/factor/view/MultiplicationTableFactorNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ define( function( require ) {
*/
function MultiplicationTableFactorNode( model ) {
var self = this;
MultiplicationTableNode.call( this, model.property( 'level' ), model.property( 'state' ), model.levelModels, model.answerSheet, false );
MultiplicationTableNode.call( this, model.property( 'level' ), model.property( 'state' ), model.levelModels, model.answerSheet, false, true );

// convenience var
var gameState = model.property( 'state' );
Expand Down
2 changes: 1 addition & 1 deletion js/multiply/view/MultiplicationTableMultiplyNode.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ define( function( require ) {
*/
function MultiplicationTableMultiplyNode( problemModel, answerSheet, stateProperty, levelProperty, levelModels ) {
var self = this;
MultiplicationTableNode.call( this, levelProperty, stateProperty, levelModels, answerSheet, true );
MultiplicationTableNode.call( this, levelProperty, stateProperty, levelModels, answerSheet, true, false );
this.problemModel = problemModel;

stateProperty.lazyLink( function( state ) {
Expand Down

0 comments on commit 37ef8d2

Please sign in to comment.