A graphical error occured - Githubissues

phetsims / density

"Density" is an educational simulation in HTML5, by PhET Interactive Simulations.

GNU General Public License v3.0

0 stars 6 forks source link

A graphical error occured #44

Closed KatieWoe closed 5 months ago

KatieWoe commented 4 years ago

For https://github.com/phetsims/QA/issues/507. I was opening a new tab to check the sim in master, and as I did both the dev version and the master version seemed to have a graphical error. In the dev version, objects disappeared and an error popped up. Bellow is the error and the console read-out. grapherrorconsole graphicserror The sim had not been open for particularly long when this occurred. Seen on Win 10 Chrome

KatieWoe commented 4 years ago

The page crashes, goes black, and reloads without graphics after about 3 minutes of fuzz testing. thrreminutefuzz

phet-steele commented 4 years ago

@KatieWoe requested that I reproduce, and here are my results:

Took me 5 sim tabs before I lost WebGL context in the oldest sim, which is more or less expected to me. (Win 10 Chrome)

KatieWoe commented 4 years ago

Also noting that when I opened multiple tabs of the dev version in the regular browser (3) I wasn't getting this like I was with one dev in regular and one master copy in incognito.

loganbraywork commented 4 years ago

I also had a graphical error once while testing an issue while master and the dev version were both open. I haven't been able to reproduce it again, but only the two sims were open. I remember it occurred while I had opened ScreenToGif to record the screen, so I had assumed it was resultant of that.

KatieWoe commented 4 years ago

Did some fuzz testing on iPadOS 13 and it crashed in less than a minute.

jonathanolson commented 3 years ago

I've moved the icons to by default be loaded as images (instead of generated). This means we create 5 fewer WebGL contexts, and I believe it may help with this.

@KatieWoe or @phet-steele, could you test https://bayes.colorado.edu/dev/phettest/density/build/phet/density_en_phet.html (I rebuilt after pulling) to see if it behaves a bit better? There is a moderate chance it will behave the same.

Additionally, platform/browser information here helps a lot (even if it's laptop vs desktop, due to GPU differences).

KatieWoe commented 3 years ago

About 5 tabs open before graphical issue occurred on Win 10 Chrome incognito. I also did a fuzz test with iPadOS 14 and the crash and reload was almost immediate.

jonathanolson commented 3 years ago

Testing locally on 2017 Macbook Pro, I have ~5 instances of Density/Buoyancy in my main window, and another 10 running nicely in an incognito window (Chrome 89).

I'll test on my Windows setup and iPhone (with fuzz)

jonathanolson commented 3 years ago

iPhone testing seemed fine while fuzzing, did not crash (watched it for at least a few minutes), 10min of fuzzing seemed fine after.

Windows incognito with 5 tabs, got this:

and the graphics error.

I'll look into seeing whether we can re-load the view based on this.

jonathanolson commented 3 years ago

@kathy-phet and @arouinfar, it seems like this MAY be resolvable, but would take a significant amount (week?) of work. Thoughts on whether this is acceptable, or should I proceed with attempting to resolve this?

arouinfar commented 3 years ago

From what I understand, this only happens if someone opens up 5+ tabs of the sim (not just browser tabs). Is that correct? If so, it doesn't seem like a reasonable use case that's worth spending a week of your time on @jonathanolson.

jonathanolson commented 3 years ago

Is that correct?

That's my current understanding, however other WebGL-using pages could trigger this too. We currently create 3 contexts (although this could potentially be taken down to 1 with a moderate amount of work), so anything else that creates ~12 contexts would likely also do this.

zepumph commented 9 months ago

For this issue @zepumph and @AgustinVallejo will:

[x] Determine how many contexts Buoyancy is currently making. ANSWER: one per screen = 5 + 2 for capability testing = 7
[x] Ensure that all icon generation involving three js is hidden behind the image generation. DONE
[x] Make sure that we are "cleaning up" contexts if we need to create/destroy them (I doubt we do). UPDATE: It seems since we have 5 contexts, we do not need to dynamically manage them. So I will check this one off.
[x] I opened 10 tabs of buoyancy in one window, and 2 more in another window, and didn't reproduce this problem.
[x] Try next time with more contexts using generateIconImages. (especially once we get https://github.com/phetsims/buoyancy/issues/48 done). UPDATE: I copied 10x IntroScreen to get double contexts. It tested fine in mac Chrome unbuilt. Firefox unbuilt went out to lunch 30+ seconds, but built was fine in mac firefox and mac safari. This seems a reasonable stress test, especially since our alternatives are (a) spending a significant amount (week?) to reduce contexts or (b) going to a smaller number of screens.

samreid commented 5 months ago

With this patch, I observed that 7 (?) contexts are created for Buoyancy. I fuzzed 5 minutes and did not see any more created.

```diff Subject: [PATCH] Remove console.log, see https://github.com/phetsims/buoyancy/issues/161 --- Index: buoyancy_en.html IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/buoyancy_en.html b/buoyancy_en.html --- a/buoyancy_en.html (revision 969fe548453bd5aee41bde8c37ee67061e9af808) +++ b/buoyancy_en.html (date 1717012045488) @@ -16,6 +16,36 @@ Githubissues.

Githubissues is a development platform for aggregating issues.