phetsims / plinko-probability

"Plinko Probability" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
5 stars 7 forks source link

[iPad2] performance drop with play button #26

Open brroberts1231 opened 8 years ago

brroberts1231 commented 8 years ago

For phetsims/tasks#630 Test device: Tycho

Operating System: iOS 9.3.3

Browser: Safari

Problem description: Sim slows down substantially for a short time when you press the play button. This can be seen most easily by repeatedly pressing the play button to drop individual balls quickly.

Screenshots: https://drive.google.com/file/d/0B9-NgawDrEu3STBnSTJ1T0ZURGs/view?usp=sharing

Troubleshooting information (do not edit): Name: ‪Plinko Probability‬ URL: http://www.colorado.edu/physics/phet/dev/html/plinko-probability/1.0.0-dev.5/plinko-probability_en.html Version: 1.0.0-dev.5 2016-06-27 15:36:39 UTC Features missing: fullscreen User Agent: Mozilla/5.0 (iPad; CPU OS 9_3_3 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Version/9.0 Mobile/13G21 Safari/601.1 Language: en-us Window: 1024x671 Pixel Ratio: 1/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 IMGSGX543-124.1) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.00) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 8 uniform: 128 Texture: size: 4096 imageUnits: 8 (vertex: 8, combined: 8) Max viewport: 4096x4096 OES_texture_float: true Dependencies JSON: {"assert":{"sha":"7d27130a","branch":"master"},"axon":{"sha":"b71cc337","branch":"master"},"babel":{"sha":"0ce3b9c8","branch":"master"},"brand":{"sha":"f0b1f7da","branch":"master"},"chipper":{"sha":"e044eccd","branch":"master"},"dot":{"sha":"c9c05a5a","branch":"master"},"joist":{"sha":"fa5db70c","branch":"master"},"kite":{"sha":"dce9765e","branch":"master"},"phet-core":{"sha":"c48bf320","branch":"master"},"phetcommon":{"sha":"35ef94ad","branch":"master"},"plinko-probability":{"sha":"af3b08d0","branch":"master"},"scenery":{"sha":"4476eb95","branch":"master"},"scenery-phet":{"sha":"52b783d9","branch":"master"},"sherpa":{"sha":"77506dc3","branch":"master"},"sun":{"sha":"0d7e743b","branch":"master"},"tandem":{"sha":"b3c68afa","branch":"master"}}

phet-steele commented 7 years ago

No change in 1.0.0-dev.6

veillette commented 7 years ago

Using the Ipad2, and unhooking the listener attached to the PlayButton, then the sim frame rate could go down as low as 12 fps while tapping the PlayButton.

This happens in requirejs and build mode.

I wonder if it is related to the gradients in the Play Button.

veillette commented 7 years ago

Using a flatAppereanceStrategy, then the lowest number of frames I could get was 40 fps (with no listener).

veillette commented 7 years ago

And hooking back the listener but with the flatAppereanceStrategy the lowest fps I could get was 30fps.

pixelzoom commented 7 years ago

@veillette I'm surprised that changing the appearance of the play button has any affect on performance. Any chance that you have a Canvas that's overlapping the play button, and causing it to be redrawn on every frame?

veillette commented 7 years ago

There are two canvas nodes in the simulation. One for the pegs and one for the rendering of the balls. The peg canvas is limited to the galton board. It is a rectangle that just encompasses the triangular galton board.

The ballsLayerNode canvas is redrawn at every frame but it does not overlap with the play button. The ballsLayerNode canvas bounds are given by the union of the galtonBoard and the histogramBounds.

The performance is something that is symptomatic of the iPad and the Sun Buttons. This may have slipped under the radar as no other PhET simulations expect someone to be tapping a button. I logged an issue in sun (https://github.com/phetsims/sun/issues/244).

pixelzoom commented 7 years ago

Ideally, we'd like to change the play button back to 3dAppearanceStrategy. @ariel-phet noted the (unexpected) change from 3D to flat when I published 1.0.0-dev.7.

ariel-phet commented 7 years ago

Actually, I knew this was a possibility given the open issues, however we should get the opinion of @amanda-phet

I would be OK with the flat look so long as it is still easy to find. The 3d button does "pop" a bit more, but the sim is fairly straightforward.

amanda-phet commented 7 years ago

The flat design is fine.

pixelzoom commented 7 years ago

62 may be partially responsible for this. The sim is continuously calling paintCanvas, even when nothing on the screen is changing. That may not leave enough time to mutate and repaint the button fill for the 3D look.

ariel-phet commented 7 years ago

If we do go for the "flat" look, lets make sure to center the black triangle of the play button in the circle (it is a little off to the left currently)

pixelzoom commented 7 years ago

lets make sure to center the black triangle of the play button in the circle

I'm afraid that's not currently possible. See https://github.com/phetsims/sun/issues/236, which we agreed to address on 4/22/16, but has not made any progress.

veillette commented 7 years ago

I think it may be related to a commit I did, that should be reverted. I was trying to minimizes the number of scenery nodes, I did not realized that it changed the centering of the play button.

https://github.com/phetsims/plinko-probability/commit/9f3c042d2cd0180e8ffe2359b526e60a1c236608

pixelzoom commented 7 years ago

The centering problem is a general problem with sun buttons, where the content on flat buttons is not centered properly. See phetsims/sun#236.

amanda-phet commented 7 years ago

@ariel-phet I thought it looked off-center as well but when I played with this in illustrator I am more convinced that it is an optical illusion.

screen shot 2016-08-30 at 10 27 39 am
ariel-phet commented 7 years ago

@amanda-phet - sorry, my statement should be the triangle should be "visually centered"

pixelzoom commented 7 years ago

I fairly confident that the 3D appearance of this button is not the primary source of the performance problem. So I've revert the button to 3D for now. After addressing other performance issues, I'll be responsible for verifying that performance is acceptable with 3D appearance.

pixelzoom commented 7 years ago

After the performance improvements in #62, repeatedly pressing the 3D Play button is still a problem. It results in a noticeable pause in animation of the balls and histogram.

I will revery to the flat appearance. But appearance will now be settable via PlinkoProbabilityConstants.PLAY_PAUSE_BUTTON_APPEARANCE_STRATEGY.

pixelzoom commented 7 years ago

These dev versions are identical, with the exception of the play/pause button appearance:

1.0.0-dev.12 - 3D button - animation slows down dramatically when pressing the play button repeatedly

1.0.0-dev.13 - flat button - animation slows down a tiny (insignificant?) amount when pressing the play button repeatedly

pixelzoom commented 7 years ago

To see if this is actually due to the 3D button appearance, I decided to add a similar 3D button to the first screen of rutherford-scattering. I chose rutherford-scattering because it has some things in common with plinko-probability: it has lots of particle-like things moving, and it uses CanvasNode.

This bit of code was added to the end of RutherfordAtomScreenView constructor:

var RoundPushButton = require( 'SUN/buttons/RoundPushButton' );
this.addChild( new RoundPushButton( { 
  baseColor: 'red', 
  left: this.layoutBounds.left + 10, 
  bottom: this.layoutBounds.bottom - 10 
} ) );

A red 3D push button appears in the lower-left corner of the screen, as shown in the screenshot below. With the 'nucleus' scene selected, the light "on", and "Traces" checked, I saw no performance impact when pressing the button repeatedly/spastically.

So now I'm stumped. The test with rutherford-scattering seems to suggest that the work done in the button callback would be responsible for the performance problem. But that wouldn't be affect by switching between 3D and flat appearance.

screenshot_16

pixelzoom commented 7 years ago

I copied PlayButton to rutherford-scattering, so that I'm using the exact same button. This bit of code was added to the end of RutherfordAtomScreenView constructor:

var PlayButton = require( 'RUTHERFORD_SCATTERING/rutherfordatom/view/PlayButton' );
this.addChild( new PlayButton( {
  left: this.layoutBounds.left + 10,
  bottom: this.layoutBounds.bottom - 10
} ) );

Screenshot below shows Play button in lower-let corner. And again, I saw no performance impact when pressing the button repeatedly/spastically.

screenshot_17

pixelzoom commented 7 years ago

@amanda-phet @ariel-phet How do you want to proceed with this one? I'm out of ideas.

ariel-phet commented 7 years ago

The flat button....

I might have some ever so slight concern if this sim had many controls, but with so few controls it is just not really an issue.

It would be good to understand this issue at some point ideally, but since it seems pathologically tied to this sim, I am also not too concerned.

So - lets do two things:

  1. Go with the flat button here
  2. Make a Sun issue and mark it for developer meeting - just so we brain storm as a group and I might have you spend an hour at some point in the future investigating if anyone comes up with a lead worth following.
ariel-phet commented 7 years ago

unassinging @amanda-phet since she already gave the green light to the flat button in https://github.com/phetsims/plinko-probability/issues/26#issuecomment-243486985

pixelzoom commented 7 years ago

@ariel-phet It might be worth having @jonathanolson spend 15 minutes digging around in the DOM to see if there's anything obvious. The DOM structure is still a mystery to me, so I'm unlikely to see the problem even if I'm looking directly at it.

pixelzoom commented 7 years ago

@ariel-phet said:

Go with the flat button here

OK. Not a blocking issue for publication.

Make a Sun issue and mark it for developer meeting

Since this problem is "pathologically tied to this sim", I don't think a sun issue is appropriate. I'm going to leave this issue open, mark it for developer meeting, and we can brainstorm here.

ariel-phet commented 7 years ago

@pixelzoom - I am curious about one WAG for an experiment. What if the play button were taken out of the control panel and put in the lower left such as in Rutherford (most of the time we have not had play buttons in control panels).

Note - I am NOT suggesting we change the design. But if that were an easy experiment to perform, might be worth a try. Or better yet, put a flat button in the control panel and have a 3d button in the lower left (still a play button) so you could try the performance experiment in the same sim.

pixelzoom commented 7 years ago

@ariel-phet suggested:

What if the play button were taken out of the control panel and put in the lower left ....

Yes, worth a try.

pixelzoom commented 7 years ago

@ariel-phet I tried what you suggested in https://github.com/phetsims/plinko-probability/issues/26#issuecomment-246066462. See http://www.colorado.edu/physics/phet/dev/html/plinko-probability/1.0.0-dev.15/plinko-probability_en.html.

In addition to the flat Play button in the upper-right panel of the Intro screen, this version has a 3D Play button in the lower-left corner. Pressing either Play button performs the same action.

On iPad2 + iOS 9.3.5: • animation slows down slightly when repeatedly pressing the flat Play button • animation slows down dramatically when repeatedly pressing the 3D Play button

I also tried putting the 3D Play button in the upper-left, with identical results.

pixelzoom commented 7 years ago

I added query parameter "play3d", which uses the 3D appearance for the Play button.

Flat (default): http://www.colorado.edu/physics/phet/dev/html/plinko-probability/1.0.0-dev.16/plinko-probability_en.html

3D: http://www.colorado.edu/physics/phet/dev/html/plinko-probability/1.0.0-dev.16/plinko-probability_en.html?play3d

pixelzoom commented 7 years ago

Things we haven't done yet:

(1) Brainstorm as a group, scheduled for Thu Sep 15. (https://github.com/phetsims/plinko-probability/issues/26#issuecomment-246062539) (2) Ask @jonathanolson to have a look. This would likely be more efficient than (1). (https://github.com/phetsims/plinko-probability/issues/26#issuecomment-246065836)

@ariel-phet How do you want to proceed? This is the only open issue blocking dev testing. Do you want to do either of the above before dev testing begins?

ariel-phet commented 7 years ago

@pixelzoom - lets proceed with dev testing and a flat button if this is the only remaining issue.

I will ask @jonathanolson to take a look, but he can do that regardless of dev testing. As we have noted this issue appears to be pathological to this sim, you have put quite a bit of intellectual effort into fixing it, and the flat bottom is a completely reasonable solutoin.

pixelzoom commented 7 years ago

@jonathanolson If you have time to take a look... You can switch between flat and 3D buttons using a query parameter, as described in https://github.com/phetsims/plinko-probability/issues/26#issuecomment-246816719. To reproduce the problem, run the sim on iPad2, go to the "Intro" screen, press the Play button repeatedly.

pixelzoom commented 7 years ago

Not a prerequisite for 1.0.0.

jonathanolson commented 7 years ago

Since this is deferred, dropping priority. Probably related to gradients being loaded/unloaded if I need to investigate.