phetsims / color-vision

"Color Vision" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/color-vision
GNU General Public License v3.0
1 stars 7 forks source link

Photons ideally shouldn't get bunched up on slow-framerate devices #18

Closed jonathanolson closed 10 years ago

jonathanolson commented 10 years ago

I was testing with the unoptimized Scenery 0.2 (currently low FPS), and saw the following:

color-vision-slow-framerate

samreid commented 10 years ago

@jonathanolson can you elaborate on what causes this problem in general? I think you once showed me a way to simulate a slow CPU with setTimeout or equivalent. Would be good to confirm that this problem is resolved and we are stepping the model nicely.

jonathanolson commented 10 years ago

Discussed with @aaronsamuel137 in person. It's caused by all photons starting off at an equivalent "x=0" on an animation frame, instead of photons starting randomly between "x=0" and "x=v*dt" (the interval of space where photons that were emitted since the last animation frame could be.

samreid commented 10 years ago

Excellent, thanks. Can you confirm the problem was resolved by 5e0695f if it is still easy for you to run the sim throttled at low FPS?

jonathanolson commented 10 years ago

Now it's performing well when consistently slowed, but I noticed two issues on Chrome/OS X:

  1. Resizing the sim can cause photons to be painted on invalid areas and not cleared (see screenshot below). This is likely to be a Scenery CanvasNode issue: if so, please create an issue on Scenery and I'll fix it.
  2. Frames with larger dt values create gaps in the photon stream (see screenshot) color-vision-photon-issues

To reproduce the gap issue, it's easiest to pull joist master and in the dev console type "makeRandomSlowness()". It randomly sleeps, causing gaps between frames that may mimic GC on slow devices. It's also reproducible by switching to another tab and then switching back to the sim.

aaronsamuel137 commented 10 years ago

Thanks Jonathan, I have noticed the gap issue too, and I'll start looking into solutions. Meanwhile, I'll create a scenery issue for the canvasNode issue.

jonathanolson commented 10 years ago

I've fixed the Scenery-relevant portion of this bug, but color-vision is drawing outside of the CanvasNode's canvasBounds.

Adding the following to SingleBulbPhotonBeamNode constructor:

this.beamBounds = options.canvasBounds;

and in the inner loop for drawing:

        if ( !this.beamBounds.containsPoint( this.photons[i].location ) ) {
          var distance = Math.sqrt( this.beamBounds.minimumDistanceToPointSquared( this.photons[i].location ) );
          console.log( 'distance: ' + distance );
        }

results in drawing regularly outside of the bounds by <15 pixels during normal operation:

distance: 1.4125271380693905 SingleBulbPhotonBeamNode.js?1402513768823:44
distance: 1.2573650848867262 SingleBulbPhotonBeamNode.js?1402513768823:44
distance: 2.134091443717648 SingleBulbPhotonBeamNode.js?1402513768823:44

and after tabbing out for a bit and tabbing back into the sim:

distance: 43747.01960465486 SingleBulbPhotonBeamNode.js?1402513483002:44
distance: 43532.70157186466 SingleBulbPhotonBeamNode.js?1402513483002:44
distance: 43441.10506723528 SingleBulbPhotonBeamNode.js?1402513483002:44
distance: 43802.05773023203 SingleBulbPhotonBeamNode.js?1402513483002:44

Once all drawing is inside the canvasBounds provided to CanvasNode, that specific issue shouldn't occur.

jonathanolson commented 10 years ago

Also, minimumDistanceToPointSquared involves a lot more computation than needed. If required, filter on containsPoint.

aaronsamuel137 commented 10 years ago

It seems that we couldn't just test if the centroid of the photon is inside the bounds, because there might be a case where it is partially outside and partially inside the bounds. We could use this strategy if we reduced the size of the bounds to compensate for the dimensions of the photons.

samreid commented 10 years ago

The implementation for minimumDistanceToPoint squared isn't doing any allocations, just what looks like fast math operations, so in my opinion it would be okay to leave this if the performance on iPad3 is sufficiently fast.

jonathanolson commented 10 years ago

Or for code clarity:

this.beamBounds.minimumDistanceToPointSquared( this.photons[i].location ) === 0

vs

this.beamBounds.containsPoint( this.photons[i].location )
aaronsamuel137 commented 10 years ago

I think this is fixed. Feel free to reopen if you notice it happening again.

jonathanolson commented 10 years ago

Gaps still seem to occur (although the individual photons are jittered), and the density seems highly dependent on the frame-rate (2nd screen, lower framerate is less dense).

By hacking line 389 of Sim.js (Joist) to adjust reported DTs, I can trigger the following cases:

  1. Small dts (high frame-rate) can cause no photons to appear (there's probably a low threshold) color-vision-screen-1-low-dt
  2. Small dts (high frame-rate) can cause a high density on the second screen, and tabbing out causes gaps. color-vision-screen-2-low-dt
aaronsamuel137 commented 10 years ago

I think this is fixed since using the EventTimer. Closing.