paintCanvas is called continuously

[pixelzoom]

As soon as a screen is selected on the Home screen, BallsLayerNode.paintCanvas and GaltonBoardNode.paintCanvas are called continuously. This occurs without having otherwise interacted with the sim, and there's nothing visibly changing on the screen.

[pixelzoom]

I reverted to @ fa37ddb19bb8e9e78a94a96a30b021cc5179b7f8, before I started working on this sim. I see BallsLayerNode.paintCanvas called continuously, but not GaltonBoardNode.paintCanvas.

GaltonBoardNode.paintCanvas started being called continuously with eca6eba687ff6089f25f80ae692ce85eaf877c70, the changes made for #58.

[pixelzoom]

And here's the offending code in PlinkoProbabilityCommonView:

      // Checks if the galtonBoard has been initially painted and if not then paint it.
      if ( !this.galtonBoardNode.isInitiallyPainted ) {
        this.galtonBoardNode.invalidatePaint();
      }
      // update view on model step
      this.ballsNode.invalidatePaint();

[pixelzoom]

GaltonBoardNode.paintCanvas was being called because the isInitiallyPainted flag was removed in the work that I did for #58. So I deleted the call to galtonBoardNode.invalidatePaint because it's no longer needed. That solves half of the problem.

[pixelzoom]

2 fundamental problems with this sim:

(1) It doesn't adhere to the MVC pattern. For the balls, the view isn't notified when the model has changed. Instead, the view redraws itself on every frame (by calling BallsNode.invalidatePaint), whether the model has changed on not. So the sim is doing a lot of work when nothing is happening.

(2) The sim is relying on the call to BallsNode.invalidatePaint in PlinkoProbabilityCommonView.step as the means to clear the balls from the screen when the hopper mode is set (via radio buttons) to "Path" or "None". Instead, it seems like the sim should simply set the BallsNode instance to invisible.

[pixelzoom]

Fixed fundamental problem (2) above, so that the BallsNode instance is made invisible when "Ball" is not selected for the hopper mode. This also prevents BallsNode.paintCanvas from being called when "Path" or "None" is selected (which was in fact happening previously.)

[pixelzoom]

Fundamental problem (1) above was not possible to fix. Doing so would have involved fundamental changes to the model, would have surely destabilized the sim, and was ultimately not worth the effort.

Instead, I added a workaround, summarized as follows:

• The model step function is called on every animation frame, and it calls step for each ball. • If a ball is already at rest in a bin, that ball's step function is a no-op. • If any ball actually moves, the model sets flag someBallMoved = true. • In the view step function, the balls are redrawn only if model.someBallMoved == true. • When the histogram mode is switched to display bins, the balls are redrawn.

So the sim still isn't following the MVC pattern - the model isn't notifying the view when it has changed. But at least the view can consult the model on each time step, and ask it whether the balls need to be redrawn.

[pixelzoom]

The above fixes are demonstrated in this dev version: http://www.colorado.edu/physics/phet/dev/html/plinko-probability/1.0.0-dev.12/plinko-probability_en.html

[pixelzoom]

Tested on iPad2 (mobile Safari) and Mac OS X (Chrome, Safari, Firefox). With the exception of the Play button (#26) everything feels smooth and responsive. Closing this issue.

[pixelzoom]

Reopening because I had another thought about a different way to address this that is more MVC-like. Instead of setting a flag in the model, have an emitter in the model that fires whenever at least 1 ball has moved. BallNode would observe this emitter and call invalidatePaint when it fires. So the model would be informing the view, rather than the view polling the model.

[pixelzoom]

There's also a bug in the previous approach. The erase button is not erasing the ball, it needs to call BallsNode.invalidatePaint.

[pixelzoom]

New approach implemented in above commit. The model now uses an Emitter to notify the view when the balls have moved, and thus need to be redrawn. This approach conforms to the MVC pattern. (Also fixed the issue with the erase button.)

Demonstrated in this dev version: http://www.colorado.edu/physics/phet/dev/html/plinko-probability/1.0.0-dev.14/plinko-probability_en.html

Closing.