search

phetsims / wave-interference

"Wave Interference" is an educational simulation in HTML5, by PhET Interactive Simulations.
MIT License
19 stars 5 forks source link

Particles outside of box #419

Open ghost opened 5 years ago

ghost commented 5 years ago

Test Device

Hopper

Operating System

iOS 12.4

Browser

Safari

Problem Description

For https://github.com/phetsims/QA/issues/389. I've only seen this in Wave Interference, not in Waves Intro. See the title and visuals. This bug is present in the published version.

Steps to Reproduce

  1. Interference screen.
  2. Select particles radio button or both radio button.

Visuals

IMG_0061

samreid commented 5 years ago

This is a known problem from https://github.com/phetsims/wave-interference/issues/331, but surprising that it only occurred in Wave Interference and not Waves Intro. I'll take a look.

samreid commented 5 years ago

Also, I wonder about the performance improvements @pixelzoom and @jonathanolson discussed for Gas Properties. If we can eliminate the requirement to use WebGL on iOS without sacrificing performance, we can address the clipping problem.

samreid commented 5 years ago

In the preceding commit, I made it so Waves Intro uses WebGL on mobile safari to improve performance, as we decided for Wave Interference. Good catch @lmulhall-phet! I'll move the preceding comment to #331.

pixelzoom commented 5 years ago

@samreid Before investigating WebGL, you might consider making some straightforward optimizations to your for loops. I see many for loops where the exit condition is being reevaluated on every iteration. These types of optimizations resulted in a significant performance improvement (+5fps) for Gas Properties, see https://github.com/phetsims/gas-properties/issues/146#issuecomment-515237667.

First, if your array is large, rather than using i < array.length as your exit condition, iterate in reverse. For example in IntensityGraphPanel.js:

- for ( let i = 0; i < intensityValues.length; i++ ) {
+ for ( let i = intensityValues.length - 1; i >= 0; i-- ) {

Second, if your exit condition is a constant, factor it out, do not put it in the for expression. E.g. in Lattice.js:

- for ( let i = 0; i < this.width - this.dampX * 2; i++ ) {
+ const iMax = this.width - this.dampX * 2;
+ for ( let i = 0; i < iMax; i++ ) {
samreid commented 5 years ago

I measured the profiler frame rate with throttle x4, then replaced all of the for let end conditions with constants and re-ran the profiler. There was not a significant difference in the frame rate for this simulation. I'll leave the code as it is for now.

UPDATE: To clarify, I tested both water waves (38fps) and sound particles (24fps) in requirejs mode.

samreid commented 5 years ago

In #322 we got good performance on the particles via WebGL, but did not add clipping due to the amount of time and complexity it would take to implement. This problem is noted in #331. @arouinfar or @ariel-phet do you want to work on that further before we publish?

arouinfar commented 5 years ago

@samreid this is really a question of priorities and desired publication timeline. I believe the goal was to have these sims published in time for the next school year (which has already started in some parts of the country). I am fine with continuing to defer the clipping issue, but @ariel-phet should make that call.

ariel-phet commented 5 years ago

I am fine with deferring. @samreid perhaps keep this task in mind for an intern.

samreid commented 5 years ago

Deferring sounds great. Some notes for when we revisit this issue: