Closed ghost closed 2 years ago
@samreid while this sim is less likely to be used on iPads considering its more advanced nature, iPad Air performance being poor is disconcerting, so I would say this issue warrants attention. We do not want to get in a performance rathole, but I am marking high priority to indicate it is worth a couple hours of investigation to see if the source of the performance bottleneck can at least be identified.
@lmulhall-phet which device were you testing on? I spent quite a bit of time testing on the iPad Air (can't remember the device name, but I think it was the the one with the sticker that warns not to update iOS), for https://github.com/phetsims/wave-interference/issues/315#issuecomment-454556621 and it was running smoothly. @samreid is your iPad Air running the latest iOS? Looks like I was testing on iOS 11.4.
@lmulhall-phet which device were you testing on?
@arouinfar Leibniz
I've been testing on iPad Air 2 "Dalton" which is running iOS 11.4.1. I'm a bit confused because I thought iPad Air 1 was our lowest platform (and that's what I thought I was using) but was surprised to learn I have iPad Air 2 instead. @KatieWoe or @arouinfar or @ariel-phet can you help clarify?
When I go use dev.66 Interference Screen, Sound + Particles, I'm getting 45FPS on iPad Air 2 "Dalton" 11.4.1.
According to the PhET Asset Inventory, Leibniz is running iOS 9.3.5. However in the issue title, @lmulhall-phet said the test was run on "Latest iOS". @KatieWoe or @lmulhall-phet can you please clarify which is correct?
Leibniz is running iOS 12, it was upgraded when iOS 12 came out. The simplified inventory reflects this, and I'll go in and change the full inventory. We don't have any iPad Air 1's that I am aware of. Our lowest device is Tycho, an iOS 9 iPad 2. Beyond that, it is an iPad 4 with iOS 10.
I'm a bit surprised that the iOS 11.4 performance is so much better than the iOS 12 performance. @KatieWoe is Leibniz our only iOS 12 device?
We have one other. Give me a minute to check it.
It seems ok on our other device. I'm going to go troubleshoot Leibniz
Thanks @KatieWoe!
From slack:
Kathryn Woessner [9:06 AM] Even after restarting Leibniz I'm still seeing the same behavior. I'll keep working at it, and there's a new update so I'll see if that does anything. I only saw the performance issue with sound on the first two screens.
Amy Rouinfar [9:17 AM] So strange that the two iOS 12 devices are behaving differently Are they different iPad models?
Kathryn Woessner [9:19 AM] Yes. Leibniz is iPad Air 2. The other is iPad (2018) Updating, restarting, and clearing Safari history hasn't cleared it up, but it may be a bit better than when Liam posted the video.
Further info: Confirmed that iPad Air 2 iOS 11 device in office did not show issue. Issue still present in iPad Air 2 iOS 12, but not in iPad (2018) iOS 12 device. After methods it seems like the behavior might be a bit better, but is still present. See video. https://drive.google.com/file/d/1zjiypySCLIr_S5nrVQWRj1O2It0OR8aT/view?usp=sharing
Does the screen capture slow down the frame rate? What is the reported FPS with ?profiler
(while not recording)?
Not much if it does. That's fairly representative. With profiler it usually reads 59 or 60 fps, but when playing sound on Waves or Interference it drops to about 5 fps, or even 3 if using particles. Checking the other iOS 12 device it drops to about 14 fps.
I decided to do a bit more profiler testing and saw that on my laptop there was no fps drop and on the iOS 11 it drops to anywhere from the high 30s to the low 50s, or more of a drop if on interference screen. I think it may be an iOS 12 problem that our second device is better at handling.
@ariel-phet and @arouinfar do you want me to check out an iOS 12 iPad Air for investigation before RC.1?
@samreid my suggestion would be to look at this on campus on Thursday during when design time meeting would be (since you are not on the agenda). See what you can find at that time on campus and lets discuss. We can also plan to finish dev meeting early, so perhaps you and @jonathanolson can look together.
Slack conversation with @KatieWoe
Kathryn Woessner [8:51 AM] I saw you won't be here today. If you wanted to look at https://github.com/phetsims/wave-interference/issues/322 we could try a zoom call later today.
Sam Reid [8:55 AM] Sounds good, thanks. My first thought was going to be commenting out different parts of the code to see what’s taking the most time.
Kathryn Woessner [8:55 AM] Let me know when works best for you.
Sam Reid [8:57 AM] Do you mind testing performance on https://phet-dev.colorado.edu/html/wave-interference/1.0.0-dev.56/phet/wave-interference_en_phet.html ? That version has no temporal masking and the lower lattice size.
Kathryn Woessner [8:57 AM] Give me a minute to check
Sam Reid [8:59 AM] Also https://phet-dev.colorado.edu/html/wave-interference/1.0.0-dev.62/phet/wave-interference_en_phet.html has temporal masking but the lower lattice size.
Kathryn Woessner [9:03 AM] Low frame rates occur in 56 and 62
Sam Reid [9:04 AM] That’s odd but also good news! Maybe the problem is the graphics phase.
Kathryn Woessner [9:05 AM] Let me know if you want me to try anything else
Sam Reid [9:07 AM] I’m creating a new version that disables one of the graphics phases. Should be published in a moment or two. Can you please test https://phet-dev.colorado.edu/html/wave-interference/1.0.0-dev.67/phet/wave-interference_en_phet.html?skipLatticeCanvasNode
The query parameter disables the canvas paint call in the wave area. So you won’t see the waves, but we can still read the frame rate. If this is faster, then WebGL may help.
Kathryn Woessner [9:13 AM] The waves don't even appear. And the speaker still seem jittery Sorry. Didn't read second part It might be faster. Let me find something to compare
Sam Reid [9:14 AM] What is the frame rate? Use &profiler Sorry, I should have included that in the URL in the first place
Kathryn Woessner [9:16 AM] Drops to about 5 With single speaker
Sam Reid [9:16 AM] Wow, so apparently the canvas isn’t the problem.
Kathryn Woessner [9:16 AM] It takes awhile though
Sam Reid [9:17 AM] Can you try https://phet-dev.colorado.edu/html/wave-interference/1.0.0-dev.67/phet/wave-interference_en_phet.html?latticeSize=50&profiler and report the frame rate?
Kathryn Woessner [9:20 AM] 8-10, but it looks very weird as well, like a checkerboard.
Sam Reid [9:21 AM] Do other animated sims have performance problems on that iPad?
Kathryn Woessner [9:21 AM] Not that I know of. I need to go start setting up. I'll have the device with me during the meeting, so I can run any more tests you need.
Sam Reid [9:22 AM] OK thanks. I’m not sure what to try next, but I’ll let you know if I think of something.
Discussion of tests mentioned in preceding comment:
Is the performance problem only for the Sound scene? It may be related to animating or rendering the sound particles.
@KatieWoe can you please test these and report frame rates?
It seems the slow part is rendering the particles, which is this code:
context.transform( 1 / RESOLUTION, 0, 0, 1 / RESOLUTION, 0, 0 );
this.model.soundScene.soundParticles.forEach( soundParticle => {
// Red particles are shown on a grid
const isRed = ( soundParticle.i % 4 === 2 && soundParticle.j % 4 === 2 );
const sphereImage = isRed ? this.redSphereImage : this.whiteSphereImage;
context.drawImage(
sphereImage,
RESOLUTION * ( this.model.soundScene.modelViewTransform.modelToViewX( soundParticle.x ) ) - sphereImage.width / 2,
RESOLUTION * ( this.model.soundScene.modelViewTransform.modelToViewY( soundParticle.y ) ) - sphereImage.height / 2
);
} );
@jonathanolson any ideas how to optimize that? Or should we go for WebGL? If I go for WebGL, should I use SCENERY/Image or SCENERY/WebGLNode?
I slacked @ariel-phet:
It looks like the problematic part in iPad Air 2/iOS 12 is rendering the sound particles in Canvas. Should I investigate a WebGL renderer for the sound particle layer before RC.1?
For micro-optimization, turning the forEach/callback into an actual for loop might help.
Using Image (with renderer:webgl) seems like hopefully the easier route that might be good to investigate first. WebGLNode only makes sense if that is not feasible, or gives bad performance.
This version uses for loop instead of forEach: https://phet-dev.colorado.edu/html/wave-interference/1.0.0-dev.69/phet/wave-interference_en_phet.html?profiler
@KatieWoe can you please test its performance and report frame rate?
3-15 fps looks laggy
Sam Reid [11:03 AM] Can you please try https://phet-dev.colorado.edu/html/wave-interference/1.0.0-dev.70/phet/wave-interference_en_phet.html?profiler and report frame rate? Do the particles leak out of the box?
Kathryn Woessner [11:08 AM] Around 60 FPS and they are going out of the box
I asked on slack dev-public:
Does
renderer: 'webgl'
not supportclipArea
?
@KatieWoe reported:
I can drop to around 40, but still looks fairly smooth. When there are wave present and you switch from waves to particles or both there seems to be an odd flash or movement that happens too quickly to distinguish, but is a bit noticeable
@ariel-phet checked the behavior and reported:
Looks good to me
This workaround is a pretty messy hack. It uses a technology webkitClipPath
that as far as I know we have never used like this. It also depends on scenery internal details, for the naming of the webgl layer, and it assumes there is only one webgl layer. Currently it is only enabled for mobile safari, because it was generating too much garbage for firefox, which already had reasonable performance. The best long term solution would be https://github.com/phetsims/scenery/issues/937. Another possibility would be to rewrite using WebGLNode and do the clipping in the fragment shader, but that could take 1-2 days for a first draft and would create a lot more complex code that would require maintenance (see for example NEURON/ParticlesWebGLNode which is 390 lines of WebGL).
@ariel-phet do you want to move forward with this code, or work on one of the longer term solutions? If we want to move forward with this, it would be great to request @jonathanolson or another developer to take a look and see what other flaws it has.
UPDATE: I should have mentioned the other option of moving forward with RC.1 with slow performance on iOS 12.
@samreid if the workaround is stable (even if hacky) it is fine for 1.0
This sim will be continued to be worked on, and a longer term solution should be discussed, but not for RC.1
If you feel the workaround is acceptable (it certainly looked reasonable to me from a user perspective) and is not causing any major bugs, I think we should roll with it for 1.0 and then have a longer discussion once 1.0 is published since we have talked about using WebGl for this sim in general for better performance.
I asked @jbphet and @zepumph to take a look and they expressed concern that this is a nonstandard solution and looks pretty fragile. We were interested in the idea of changing the canvas size to fit, or a better way to monitor the bounds. And we agreed it would be great for @jonathanolson to discuss before we take SHAs for RC.1.
UPDATE: to clarify, the reason this is a risky solution is that it could fail in ways we don't anticipate.
@jonathanolson reported:
That looks like it should work but will be fragile and will need browser testing.
I also created a new branch to investigate using WebGLNode directly, see https://github.com/phetsims/wave-interference/issues/329. I was hoping to get something working quickly. However, as expected, that approach still seems like it would to take 1-2 days to get a rough draft, and I'm noticing there is a lot of duplicated effort between it and WebGLBlock, so it's not a "win win".
@ariel-phet how do you recommend to proceed?
I discussed this with @ariel-phet and @KatieWoe on zoom this morning. Our plan is that @KatieWoe will perform more thorough testing of the approach in dev.73, testing things like rotating the device, sleep/waking the device, and generally more testing to see if the approach is stable. If there are minor concerns, they will be reviewed with @ariel-phet to see if they are acceptable. If that phase seems reasonable, we will create RC.1 for Wave Interference with the fragile approach in dev.73.
We also discussed the approach in https://github.com/phetsims/wave-interference/issues/329 which subclasses WebGLNode for the rendering. We determined that there isn't enough time to continue development on that for RC.1 and the longer term solution of https://github.com/phetsims/scenery/issues/937. When the time is right, we will schedule a meeting with @ariel-phet, myself, and @jonathanolson to discuss that approach.
So I'll begin cleaning up some of the testing code that was added in the last day or so to prepare for RC.1.
I also noted that the WebGL and clipping code is only running on mobile safari, so prior tests on other platforms are still valid.
I slightly refactored and published https://phet-dev.colorado.edu/html/wave-interference/1.0.0-dev.74/phet/wave-interference_en_phet.html. @KatieWoe this doesn't invalidate prior testing on dev.73, but can you please use dev.74 for any additional dev testing?
@samreid @ariel-phet After about 2 hours of testing, both Liam and I saw this phenomenon on both the waves and interference screens, at least 3 times: How far off the particles are varies each time it happens. Hitting the orange reset button does not solve the problem. We were unable to pin down the exact causes of the issue. Here is what we found so far:
Sorry we weren't able to narrow it down more.
We also saw what the odd flash I saw with @ariel-phet when switching from wave to particle was. If the sim is paused when you do this, you see this:
May not be connected, but also saw some variant of https://github.com/phetsims/wave-interference/issues/319. If you have the continuous wave going with Both particles and switch to pulse and fire a pulse, end of the continuous wave disappears too soon.
From slack:
Sam Reid [3:00 PM] Katie and Liam pointed out 3 issues:
I’ll take a quick look at each one to see if I can correct it. But to clarify, which ones of those are acceptable for 1.0? Also, what is the soonest time that you would start testing a Wave Interference RC? That will give me a window of time for investigation. (edited)
Kathryn Woessner [3:02 PM] Maybe Monday if the dev for efac goes well today. But Fractions also has its RC
Sam Reid [3:03 PM] OK please keep me in the loop so I can make sure the Wave Interference RC is ready when you are.
Ariel Paul [3:58 PM] My understanding was none of these were easy to reproduce (they happened but no clear steps)
Kathryn Woessner [3:59 PM]
Ariel Paul [4:00 PM] Yes, 2 is fine, that is the "flash"
Kathryn Woessner [4:02 PM] quality assurance Posted using /giphy As for 1.
Ariel Paul [4:08 PM] Please prioritize 1 and 3 for Sam @samreid realistically Tuesday morning would be appropriate for RC
Kathryn Woessner [4:10 PM] Yeah. I don't know how to feel about 1. Since we still don't know what caused it I don't know how common it will be. May be worth it to have someone not expecting it use it for awhile and see if they see it. But it probably will be fine for 1.0 since it will be updated soon. Depends on which we want less. Poor iOS 12 performance or the chance of this bug.
Ariel Paul [4:12 PM] I want poor performance less. Doe Reset fix 1?
Kathryn Woessner [4:12 PM] No You need to refresh the page
Ariel Paul [4:12 PM] But Refreshing the browser does...fair enough I still want the chance of the bug more than poor performance
Sam Reid [4:18 PM] We could disable clipping for ipads. It would look like this:
Ariel Paul [4:19 PM] @samreid ask @arouinfar if she is OK with that. I could live with it for 1.0 (I think on sound it is not too crazy and I expect iPads to be lower use for this sim)
QA found this buggy clipping behavior: https://github.com/phetsims/wave-interference/issues/322#issuecomment-457728933 @ariel and I are wondering how you feel about disabling clipping altogether for iPads. It would look like this (see above):
Amy Rouinfar [4:22 PM] I could definitely live with that
Sam Reid [4:22 PM] @ariel said:
@samreid ask @arouinfar if she is OK with that. I could live with it for 1.0 (I think on sound it is not too crazy and I expect iPads to be lower use for this sim)
Amy Rouinfar [4:23 PM] Seems better than the bugs QA found
Sam Reid [4:23 PM] OK I’ll disable clipping for the particles, only for iPad. Thanks for your quick consultation!
Amy Rouinfar [4:23 PM] Sounds good :slightly_smiling_face:
Clipping removed, and I opened a new issue about restoring the clip area (at lower priority). Closing.
I tried removing the empty Rectangle from the sound screen particle view, and could not tell any difference, with or without webgl enabled, so I removed it. Flagging for a closer look during next QA test.
For Dev.24. This issue was seen mostly on iOS 12 if I recall correctly. This platform is less supported now. @samreid should we discuss what this test should look like?
Things look ok in dev.24
This seems ok and has been published. Closing
For https://github.com/phetsims/QA/issues/264. Performance isn't very good. See video.
https://drive.google.com/open?id=1hV6cgK10tV_uVH7XnzRJNGIyCbDjusGB