phetsims / ph-scale

"pH Scale" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/ph-scale
GNU General Public License v3.0
0 stars 8 forks source link

ratio view shouldn't update when pH hasn't changed #25

Closed pixelzoom closed 10 years ago

pixelzoom commented 10 years ago

When the pH remains constant and the solution volume changes, the 'ratio' view shouldn't change. That is, new molecules shouldn't be drawn. Changing the solution volume should just change the clipping region to show more/less of the ratio view.

Unfortunately, CanvasNode.paintCanvas gets called when anything within its bounds is changed. So changing the solution volume is causing a paintCanvas to occur, and redrawing the molecules.

jonathanolson commented 10 years ago

Canvas clipping basically "doesn't draw" pixels outside of the clipping shape (they aren't "hidden" and can be displayed by changing the clip).

To accomplish that, there are a few options that come to mind:

  1. Don't change the positions when only the volume changes (use a random number generator that takes as input the particle number and a nonce value that increments when the ratio SHOULD change)
  2. Use BAM-style Canvas fitted to the maximum area, and apply a CSS clip. The Canvas will then render the entire area (even hidden parts) on every change, and the clip will show parts of it.
  3. Use BAM-style Canvas with option (1)
  4. Use CanvasNode with the maximum bounds necessary (render entire area), and force Scenery to put another layer in front that covers up the ratio view with the background color. Then tweaks to the front layer's shape will show/hide, instead of clipping.
pixelzoom commented 10 years ago

Oops, put the wrong issue number in my commit message. Reopening this issue.

pixelzoom commented 10 years ago

Option 1 seems the most promising/robust. Notes from Skype chat with @jonathanolson:

[3/5/14 1:16:28 PM] Chris Malley: have we identified a seeded random number generator yet? [3/5/14 1:18:56 PM] Jonathan Olson: well, any hash function could be used for that if we don't want to store particle locations [3/5/14 1:19:30 PM] Jonathan Olson: since particles are on integer boundaries, you could have an 'update' step using http://en.wikipedia.org/wiki/Lehmer_random_number_generator with the Park-Miller numbers [3/5/14 1:19:52 PM] Jonathan Olson: e.g. store an array of random integers [3/5/14 1:20:02 PM] Jonathan Olson: whenever you want to change the positions, apply the step function to each integer [3/5/14 1:20:21 PM] Jonathan Olson: to get the position out, do it mod N, where N is the width or height (bounds) of what you need [3/5/14 1:20:50 PM] Jonathan Olson: or use the mersenne twister code that SR was using [3/5/14 1:20:56 PM] Jonathan Olson: seed it with the frame "number" [3/5/14 1:21:11 PM] Jonathan Olson: and step through its sequence every time you render

pixelzoom commented 10 years ago

Decided to store molecule coordinates, and only update the coordinates when the pH changes.

Fixed, closing.

pixelzoom commented 10 years ago

Reopening.

Y2C reported:

This issue is still occurring in 1.0.0-dev.19 when draining a solution that was previously diluted. The fix you implemented worked for undiluted solutions, but after a solution has been diluted, the ratio view image updates dynamically as the solution drains, even though the pH remains constant.

I confirmed. Happens with smallish volumes of solution. The pH is changing slightly. But since I don't see it change on the pH meter, I'm guessing this is caused by some intermediate state that occurs when the solution volume changes. Probably due to adjusting soluteVolume and waterVolume non-atomically, so that there is an intermediate (and inaccurate) pH value.

pixelzoom commented 10 years ago

Confirmed, solute and water volume are being set non-atomically, see Solution.js

// line 134 in drainSolution(…)
this.waterVolumeProperty.set( waterVolume - ( deltaVolume * waterVolume / totalVolume ) );
this.soluteVolumeProperty.set( soluteVolume - ( deltaVolume * soluteVolume / totalVolume ) );

pHProperty is derived from these 2 properties, so it has a bogus value after waterVolumeProperty has changed, but before soluteVolumeProperty has changed.

pixelzoom commented 10 years ago

Fixed by setting water volume and solute volume atomically via a flag in Solution.js. This avoids intermediate states of pH and total volume. It should also improve performance in general while draining the beaker, since there won't be intermediate states that generate notifications.

pixelzoom commented 10 years ago

Rats, broke the drain, it doesn't work correctly after the beaker has been empties once. Reopening.

pixelzoom commented 10 years ago

Fixed AGAIN. Closing.