buggy "Binary Probability" control

[pixelzoom]

Perhaps this is due to my lack of understanding about how the "Binary Probability" control is supposed to work in the Lab screen. But as I slide it between 0 and 1, I was expecting the flat side of the pegs in the Galton board to rotate from left to right. But if I move the slider very fast, I can get cases where the slider value is 0 and the flat side of the pegs are facing straight up, instead of rotated to the left.

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

For example, if I move the slider slowly, here's the orientation of the pegs at probability = 0:

Here's another orientation of the pegs at probability=0, after moving the slider wildly:

And here's yet another orientation at probability=0, after moving the slider wildly:

@ariel-phet Do you know if this is correct or a bug?

[phet-steele]

@pixelzoom I can say with 100% certainty that this is a bug, however I am not currently observing it on Win 10 Chrome.

[ariel-phet]

Definitely a bug, observed on Win 7 FF

[ariel-phet]

If an "easy" fix is setting a max speed at which the slider can be changed or such, that is fine with me. I have to move it pretty darn fast to see this problem.

[pixelzoom]

I don't have to move it very fast at all (Mac + Chrome) to see the problem. There's no good reason why drag speed should affect this, there's probably a logic (or rounding) bug somewhere. I'll investigate.

[pixelzoom]

This bug is easily reproducible in 1.0.0-dev.6, so not something that I introduced during cleanup.

[pixelzoom]

Here's where the pegs are updated when the probability changes. GaltonBoardNode line 94:

    probabilityProperty.lazyLink( function( newProbability, oldProbability ) {

      // rotate the underlying pegPath
      var newAngle = newProbability * options.rangeRotationAngle;
      var oldAngle = oldProbability * options.rangeRotationAngle;
      var changeAngle = newAngle - oldAngle;
      pegPath.rotateAround( pegPath.center, changeAngle );

      // recreate the image of the peg
      pegPathToImage();

      // update this canvas
      self.invalidatePaint();
    } );

And pegPathToImage is:

    function pegPathToImage() {
      var pegImageHalfSize = Math.ceil( pegRadius ) + 1;
      pegPath.toImage( function( image ) {
        self.pegImage = image;
      }, pegImageHalfSize, pegImageHalfSize, 2 * pegImageHalfSize, 2 * pegImageHalfSize );
    }

Using the toImage functions in scenery has (for me) resulted in unpredictable behavior, as the the image is created asynchronously. And I'm betting that's what's happening here - the image isn't ready when it's time to draw, and we're drawing some older version of the image, for some other probability value.

[pixelzoom]

I'm a little surprised that this requires converting the peg to an image, then drawing multiple pegs with Canvas. Why not create one Image node, then use scenery's DAG feature to render it in multiple places? There are indeed a lot of pegs when rows=26, but I'd still be surprised if this were a performance showstopper.

[veillette]

FYI @pixelzoom, the pegs were rendered (initially) using scenery which worked very well but it proved to be to slow for the iPad2, at least in the way we had done it. Here is a reference to the svg implementation https://github.com/phetsims/plinko-probability/blob/66f08e11daf90702a0ee3e926a6824dd87b83639/js/common/view/GaltonBoardNode.js

The canvas Node implementation was a real can of worms, that gave rise to a lot of bugs for the very reason you mentioned.

[pixelzoom]

Some ideas:

(1) Instead of rotating the peg in scenery, handle the rotation in Canvas. Then the peg image only needs to be created once, and never changed.

(2) Draw all of the pegs using a single scenery.Path, eliminating the need for Canvas.

(3) There's really no reason to redraw the peg shadows when probability changes. If (2) is possible, then draw all of the shadows once as a single scenery.Path. Recompute only when numberOfRows changes.

[pixelzoom]

Summary of optimizations that I did:

• Call toImage for peg and shadow only once, in the default orientation • Handle rotation of the pegs in Canvas • For the Intro screen where the pegs do not rotate, do not link to probabilityProperty, and draw the pegs as a simple circle.

Other fixes: • Properly handle the situation where an asynchronously created image is not yet available, and call invalidatePaint in the toImage callbacks.

[pixelzoom]

After the above changes, the display now stays synchronized with the "Binary Probability" control. And the sim feels responsive on iPad2.

@phet-steele Please take this dev version for a test drive on a few platforms (thanks). http://www.colorado.edu/physics/phet/dev/html/plinko-probability/1.0.0-dev.8/plinko-probability_en.html

[pixelzoom]

@ariel-phet Since @phet-steele is out, would you mind verifying this one? The latest dev version is http://www.colorado.edu/physics/phet/dev/html/plinko-probability/1.0.0-dev.11/plinko-probability_en.html.

[ariel-phet]

Checked on Win FF, chrome and on iPad2. Bug gone, sim extremely responsive on iPad2.

Closing