Skip to content

Commit

Permalink
eliminate unnecessary reusable vectors, mutate position and make it p…
Browse files Browse the repository at this point in the history
…rivate, #141
  • Loading branch information
pixelzoom committed Mar 28, 2024
1 parent d5e5dc1 commit 7006afe
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 18 deletions.
31 changes: 16 additions & 15 deletions js/common/model/Electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,8 @@ export default class Electron {
private readonly currentAmplitudeProperty: TReadOnlyProperty<number>;
private readonly currentAmplitudeRange: Range;

// Electron's position, relative to the coil's position. This value is changed internally and should not be changed by
// clients. Also be aware that the entire Vector2 instance may be changed (rather than just the x and y values changing).
public position: Vector2;
// Electron's position, relative to the coil's position. This Vector2 is mutated as position changes.
private readonly position: Vector2;

// Ordered collection of the segments that make up the coil
private readonly coilSegments: CoilSegment[];
Expand All @@ -72,10 +71,6 @@ export default class Electron {
// Scale for adjusting speed.
private readonly speedScaleProperty: TReadOnlyProperty<number>;

// Reusable Vector2 instances
private readonly reusablePosition1: Vector2;
private readonly reusablePosition2: Vector2;

public constructor( currentAmplitudeProperty: TReadOnlyProperty<number>, currentAmplitudeRange: Range, providedOptions: ElectronOptions ) {

const options = providedOptions;
Expand All @@ -84,12 +79,9 @@ export default class Electron {
this.currentAmplitudeProperty = currentAmplitudeProperty;
this.currentAmplitudeRange = currentAmplitudeRange;

this.reusablePosition1 = new Vector2( 0, 0 );
this.reusablePosition2 = new Vector2( 0, 0 );

const coilSegment = options.coilSegments[ options.coilSegmentIndex ];

this.position = coilSegment.evaluate( options.coilSegmentPosition, this.reusablePosition1 );
this.position = coilSegment.evaluate( options.coilSegmentPosition );
this.coilSegments = options.coilSegments;
this.coilSegmentIndex = options.coilSegmentIndex;
this.coilSegmentPosition = options.coilSegmentPosition;
Expand All @@ -102,6 +94,14 @@ export default class Electron {
// Nothing to do currently. But this class is allocated dynamically, so keep this method as a bit of defensive programming.
}

public get x(): number {
return this.position.x;
}

public get y(): number {
return this.position.y;
}

/**
* Gets the layer that this electron currently occupies.
*/
Expand Down Expand Up @@ -148,11 +148,12 @@ export default class Electron {
}
assert && assert( COIL_SEGMENT_POSITION_RANGE.contains( this.coilSegmentPosition ) );

// Evaluate the quadratic to determine the electron's position relative to the segment. Use a different reusable
// Vector2 each time that curve.evaluate is called so that position is different for any by-reference comparisons.
// Get the coil segment that this electron currently occupies.
const coilSegment = this.coilSegments[ this.coilSegmentIndex ];
const returnValue = ( this.position === this.reusablePosition1 ) ? this.reusablePosition2 : this.reusablePosition1;
this.position = coilSegment.evaluate( this.coilSegmentPosition, returnValue );

// Evaluate the quadratic to determine the electron's position relative to the segment.
// Note that this mutates this.position.
coilSegment.evaluate( this.coilSegmentPosition, this.position );
}
}

Expand Down
5 changes: 2 additions & 3 deletions js/common/view/ElectronsNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,9 @@ class ElectronSpriteInstance extends SpriteInstance {
if ( this.electron.getLayer() === this.coilLayer ) {

// Move to the electron's position (at the electron's center) and apply inverse scale.
const position = this.electron.position;
this.matrix.rowMajor(
ELECTRON_INVERSE_SCALE, 0, position.x + ELECTRON_RADIUS,
0, ELECTRON_INVERSE_SCALE, position.y + ELECTRON_RADIUS,
ELECTRON_INVERSE_SCALE, 0, this.electron.x + ELECTRON_RADIUS,
0, ELECTRON_INVERSE_SCALE, this.electron.y + ELECTRON_RADIUS,
0, 0, 1
);
assert && assert( this.matrix.isFinite(), 'matrix should be finite' );
Expand Down

0 comments on commit 7006afe

Please sign in to comment.