Sim can crash when taking screenshots

KatieWoe commented 2 years ago

For https://github.com/phetsims/qa/issues/804 and https://github.com/phetsims/qa/issues/805. Seen on iPadOS 15.5. It didn't seem to happen on the published version of the sim. I also didn't see it on Win 11 Chrome

When taking screenshots from the phet menu, it would take a noticeably long time. This was especially the case if the function machine had three slots. If you try to drag objects while the sim is thinking it will sometimes freeze completely.

pixelzoom commented 2 years ago

@KatieWoe With features like this that are common to all sims, it would be helpful to know if other sims (in master, or published around the same time) are experiencing the same problem. Is that the case with this feature? There's absolutely nothing about the "screenshots" feature that is specific to this sim.

KatieWoe commented 2 years ago

I was able to get something similar on Geometric Optics without the freezing, but with the long wait times. However, instead all screenshots after the first one when I tried to replicate the bug were empty with a size 0mb size. Checking master will need to wait since phettest is down.

pixelzoom commented 2 years ago

I was able to get something similar on Geometric Optics without the freezing, ...

Production? A dev version?.... I need details.

KatieWoe commented 2 years ago

Production, sorry, I thought I included that. I wasn't able to test dev versions or master at the time since that part of the website was down.

pixelzoom commented 2 years ago

I can't reproduce this in 1.2.0-dev.7 or master. On my iPad6 with iPadOS 15.5 and Safari, taking a screenshot takes barely more than 1 second.

Questions:

Roughly how long, in seconds, is "a noticeably long time"?
Which test device are you seeing this with, and what model iPad is it?
Does it occur with other iPads?

pixelzoom commented 2 years ago

@KatieWoe says:

as much as 3 seconds to take a screenshot (until the "Do you want to download..." dialog appears)
iPad Air 2 test device
@kathy-phet has the other iPad (iPad6?) running iPadOS 15.5, and @KatieWoe needs to get it back from her

pixelzoom commented 2 years ago

I had no problems taking screenshots of Geometric Optics 1.1.1, and there are no empty (0kb) screenshots.

@KatieWoe is also seeing slow screenshots with Fourier 1.0.4.

I don't know how to proceed with this. There is nothing sim-specific about screenshots.

pixelzoom commented 2 years ago

@KatieWoe also clarified that by "freeze", she does't mean that it just stops responding while taking a screenshot. The sim becomes permanently unresponsive, and must be reloaded.

@KatieWoe also demonstrated that the second attempt to take a screenshot will freeze the sim, requiring a reload.

Perhaps this is a problem that's specific to iPad Air. In https://github.com/phetsims/function-builder/issues/147#issuecomment-1135201400, @jessegreenberg indicated that he has an iPad Air 2. @jessegreenberg would you mind testing 1.2.0-dev.7 to see if you can reproduce the Screenshot problems that @KatieWoe has described?

pixelzoom commented 2 years ago

I don't know if it's significant, but https://github.com/phetsims/function-builder/issues/147 (performance problem with pan/zoom) was also reported on iPad Air 2 by @KatieWoe, confirmed on iPad Air 2 by @jessegreenberg. And I could not reproduce on iPad6. Is there something that both Screenshot and Pan/Zoom are using that is having problems on iPad Air 2?

pixelzoom commented 2 years ago

I wondered:

... Is there something that both Screenshot and Pan/Zoom are using that is having problems on iPad Air 2?

Looking at ScreenshotGenerator.js, I see that the screenshot feature is based on canvas. And in phetsims/function-builder#147, @jessegreenberg narrowed down the pan/zoom performance problem to use of canvas. So there does appear to be a relationship here.

Assigning to @samreid (author of ScreenshotGenerator.js) and @jonathanolson for input.

pixelzoom commented 2 years ago

In Slack, @samreid said:

I just wrote an adapter that calls into scenery.

jessegreenberg commented 2 years ago

I tried on my iPad Air 2 and had similar results as @KatieWoe. It takes ~3 seconds to see the "Do you want to download..." dialog from Safari.

I tried a couple of others (graphing-lines, friction) and it took ~1 second to see the dialog.

To be honest even 3 seconds felt acceptable to me, other apps and actions can take a long time to process on this old device (though generally not in PhET sims).

jessegreenberg commented 2 years ago

I was able to reproduce the sim freeze after taking 7 snapshots on a local version of master. I will plug it in to a mac and see if there is an error or stack trace.

jessegreenberg commented 2 years ago

While tethering my iPad Air 2 to my Mac I didn't see any freeze or error in the dev tools but the safari page did automatically refresh as if it was trying to recover from a problem.

pixelzoom commented 2 years ago

Thanks for testing @jessegreenberg.

pixelzoom commented 2 years ago

Here's a summary of the problem with the Screenshot feature:

A screenshot takes longer than expected, 3 seconds or more.
After the screenshot completes, the sim sometimes freezes and requires a restart.
When taking another screenshot, the sim sometimes freezes before the screenshot completes, and requires a restart.
When tethered to a Mac, there are no error messages in the console.
This problem has been reproduced on multiple iPad Air 2 devices.
This problem cannot be reproduced on my iPad6.
This problem has been reproduced in other sims (recently-published, master).
This problem does not occur in FB 1.0.25, so it's likely a regression.
A problem has been reported with pan/zoom that also seems to be specific to iPad Air 2, and also involves canvas. See phetsims/function-builder#147.
Since this problem crashes the sim, and is relevant to all sims, it is currently blocking.

@jonathanolson thoughts?

jonathanolson commented 2 years ago

When tethered to a Mac, there are no error messages in the console.

Devtools console? I'm curious whether the application failure would report out any more information.

Could be a number of things, not sure if we're going over some memory limit, if the browser process is erroring out, or possibly if it's more of an input failure (somehow) afterward. Debugging while tethered with XCode tools would be my first line of attack.

jonathanolson commented 2 years ago

Also if tethering is not an option, working to reproduce based on cutting screenshot parts out of screenshotting (e.g. screenshotting with Canvas but not saving the file, or the opposite of saving a blank file) to see if the part that causes the crash can be isolated.

jonathanolson commented 2 years ago

Let me know if I should collaborate with someone who has one of the affected devices, or whether I should pick one up(?).

jonathanolson commented 2 years ago

I was looking into something else (https://github.com/phetsims/function-builder/issues/145), and laptop Safari slowed down to a crawl on ?rootRenderer=canvas. The bulk of the time is spent applying a Canvas filter (ColorMatrixFilter of some type) with the per-pixel operations and get/put of data. This is possibly related.

pixelzoom commented 2 years ago

Yes @jonathanolson, you should collaborate with someone who has one of the affected devices, or pick one up. While we can live with the slower performance of canvas, we can't live with it crashing the sim.

amanda-phet commented 2 years ago

I recall this being an issue in another recent sim, but can't remember the sim and can't find an issue about it. But this doesn't seem like the first time we've had screenshot lags - maybe @KatieWoe will remember? It might be helpful to hunt down that thread, if there was one, for any additional context.

KatieWoe commented 2 years ago

I know there was an issue of slow screenshots, but I don't recall there being a freezing or crashing issue associated with it. If I find it I will link it here.

amanda-phet commented 2 years ago

Right, sorry I wasn't clear. Do you remember when we had the slow screenshot issue? Might be useful to reference here to document the history of this issue.

KatieWoe commented 2 years ago

Looking for it now.

KatieWoe commented 2 years ago

Found it https://github.com/phetsims/number-line-operations/issues/78

pixelzoom commented 2 years ago

https://github.com/phetsims/function-builder/issues/147#issuecomment-1137800666 summarizes the performance problem caused by using both renderer: canvas and filters. And the Screenshot feature currently relies on both canvas and filters, so we're stuck with a performance problem on Safari. But 3 seconds for a screenshot is still acceptable, so the performance problem is not a showstopper.

What is a showstopper is that the sim crashes on iPad Air 2. I discussed with @jonathanolson, and he's going to procure an iPad Air 2 for testing. In the meantime, I will likely proceed with creating the FB 1.2 release branch, because we don't want too much time to pass during dev testing.

pixelzoom commented 2 years ago

@KatieWoe Can you please test other sims in master, and other recently-published sims, to see if they crash when taking Screenshots on iPad Air 2 ? If this affects other sims (especially published sims) then it may be best to proceed with publishing FB 1.2 with this known problem, then doing a batch MR for affected sims.

pixelzoom commented 2 years ago

@KatieWoe to be clear, when testing other sims, look for the following:

crashing during/after using the Screenshot feature (this is most important)
empty/0MB screenshots, like what you observed in published Geometric Optics (this is not good, but the sim is still usable)
unusually long time to complete the screenshot (this we can live with)
anything else that seems wrong/odd

KatieWoe commented 2 years ago

Some updates from todays testing:

I tested 5 of the most recent published sims and I was not able to replicate the crashing behavior on them. Since getting the crash could sometimes be hard in the dev test this was found in, this isn't a guarantee the problem isn't there, but I didn't see it.
I briefly tested the problem dev test on iPad Air 2s running iOS 13 and 14. I didn't see the problem there. Though, as noted above, this isn't a guarantee.
I tested on an iPad (6th generation) running iOS 15.4 on the dev test that showed the problem and was able to get the crashing behavior.

All this indicates a problem with iOS 15 rather than iPad Air 2 as we previously suspected. I suspect the difficulty before as that it can sometimes take a few tries to get the bug to occur. Edit: I planned to test on master but it hasn't been working while I try to test. I will update this issue if this problem is resolved (5/26/22).

kathy-phet commented 2 years ago

@KatieWoe - Thanks, were you able to replicate the problem on anything that is published on the website using the iOS 15 devices? (Or only things on dev?)

KatieWoe commented 2 years ago

I was not able to replicate on the website. I tried about 5 of the most recent sims, including prototypes. This bug can be hard to reproduce, so this isn't a guarantee, but I didn't see it.

kathy-phet commented 2 years ago

OK - Let's publish a new version of FB, and see if you see it once its built and published.

pixelzoom commented 2 years ago

@kathy-phet can you please clarify what you mean by "published"? Do you mean ignore this issue for now, publish 1.2 to the website, then see if it's reproducible? I don't know why it would not be reproducible in the production version, when it's been reproduced in a (built) dev version.

jonathanolson commented 2 years ago

I have an iPad Air 2, and I'm able to reproduce this. I'll grab a cord to tether it at home to debug.

KatieWoe commented 2 years ago

Note in https://github.com/phetsims/qa/issues/807: I did manage to get a crashing behavior with the screenshot. However, it took many more tries to replicate.

jonathanolson commented 2 years ago

Using Instruments (OS logging), shows:

0x10d05c380 - GPUProcessProxy::didClose:
0x10d05c380 - GPUProcessProxy::gpuProcessExited: reason=crash

https://github.com/openlayers/openlayers/issues/12908 seems potentially related.

jonathanolson commented 2 years ago

https://bugs.webkit.org/show_bug.cgi?id=231157 also could trigger this, if we're using a larger path in this sim.

jonathanolson commented 2 years ago

Simulator is also showing:

Total canvas memory use exceeds the maximum limit (224 MB).

jonathanolson commented 2 years ago

Reducing the size of Canvases used in the internal rendering-to-Canvas code prevents the crash. Doing layer fitting could potentially resolve this (it might be related to the filters it's applying).

jonathanolson commented 2 years ago

I believe the above approach (layer fitting in the Canvas approach) fixes this. It improves the memory AND performance characteristics of screenshotting (particularly for Safari + filters, which was the main bottleneck).

@KatieWoe can you test master (built) function-builder-basics to see if this resolves the issue on your end? It seems to on my test iPad.

@pixelzoom can you see the changes, and see if it needs review?

KatieWoe commented 2 years ago

On master the screenshots are taken much faster now. They are taken fast enough that if the crashing issue is there I don't think I'll be able to react fast enough to recreate it. This was a first test on unbuilt function builder. Give me a bit to try out a built version.

pixelzoom commented 2 years ago

... They are taken fast enough that if the crashing issue is there I don't think I'll be able to react fast enough to recreate it.

@KatieWoe Recall that using the Screenshot feature multiple times resulted in a crash, see https://github.com/phetsims/scenery/issues/1424. Reaction time is irrelevant in that case. So please make sure that you're testing that.

pixelzoom commented 2 years ago

@pixelzoom can you see the changes, and see if it needs review?

Changes look OK to me (though this is outside my wheelhouse). I'm OK without additional review.

This presumably affects more than just Function Builder. @jonathanolson what's your opinion on how this should be addressed? What published sims are affected? Do we need a batch MR to fix affected sims? Should I patch this into the 1.2 branch for function-builder, or publish 1.2, then address via the MR?

KatieWoe commented 2 years ago

Sorry, it took me a bit of time to get a built version up and running. Taking several screenshots (10 downloaded and several others canceled) did not crash the sim.

jonathanolson commented 2 years ago

I'd recommend patching this into sims that crash at the very minimum. If it seems to speed up screenshots across sims for Safari, I'm also fine patching that in (probably @kathy-phet's decision there).

pixelzoom commented 2 years ago

@jonathanolson I see this error in CT for many sims. energy-forms-and-changes, geometric-optics, graphing-quadratics, molecules-and-light, ... So do we have a new problem here?

geometric-optics : pan-and-zoom-fuzz : unbuilt
https://bayes.colorado.edu/continuous-testing/ct-snapshots/1654727627557/geometric-optics/geometric-optics_en.html?continuousTest=%7B%22test%22%3A%5B%22geometric-optics%22%2C%22pan-and-zoom-fuzz%22%2C%22unbuilt%22%5D%2C%22snapshotName%22%3A%22snapshot-1654727627557%22%2C%22timestamp%22%3A1654739145313%7D&brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000&supportsPanAndZoom=true
Query: brand=phet&ea&fuzz&fuzzPointers=2&memoryLimit=1000&supportsPanAndZoom=true
Uncaught InvalidStateError: Failed to execute 'drawImage' on 'CanvasRenderingContext2D': The image argument is a canvas element with a width or height of 0.
Error: Failed to execute 'drawImage' on 'CanvasRenderingContext2D': The image argument is a canvas element with a width or height of 0.
at drawImage (Node.ts:5367:26)
at renderToCanvasSubtree (Node.ts:5374:16)
at renderToCanvasSubtree (Node.ts:5374:16)
at renderToCanvasSubtree (Node.ts:5374:16)
at renderToCanvasSubtree (Node.ts:5374:16)
at renderToCanvasSubtree (Node.ts:5374:16)
at renderToCanvasSubtree (Node.ts:5374:16)
at renderToCanvasSubtree (ScreenshotGenerator.js:42:17)
at generateScreenshot (PhetMenu.js:207:46)
at callback (MenuItem.ts:136:8)
id: Bayes Puppeteer
Snapshot from 6/8/2022, 4:33:47 PM

jonathanolson commented 2 years ago

Looking into it.

pixelzoom commented 2 years ago

Since this is a general problem (not specific to function-builder), I'm transferring this issue to scenery.

jonathanolson commented 2 years ago

Looks like a lot of these cases need pan-zoom for the failure. Likely something is messing up if things are "off-screen"

phetsims / scenery

Sim can crash when taking screenshots #1424