phetsims / fourier-making-waves

"Fourier: Making Waves" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
6 stars 3 forks source link

Sim will not load on macOS 10.15.7 (Catalina) + Safari 15 #202

Closed Nancy-Salpepi closed 2 years ago

Nancy-Salpepi commented 2 years ago

Test device iMac (processor: 3.1 GHz Quad-Core Intel Core i7)

Operating System macOS Catalina 10.15.7

Browser safari (15.0)

Problem description The sim will not load. I was able to open other sims on PhET website without issue. I also do not see this problem on my MacBook Air + safari.

Visuals

Screen Shot 2021-10-01 at 11 28 58 AM

Troubleshooting information !!!!! DO NOT EDIT !!!!! Name: ‪Fourier: Making Waves‬ URL: https://phet-dev.colorado.edu/html/fourier-making-waves/1.0.0-rc.1/phet/fourier-making-waves_all_phet.html Version: 1.0.0-rc.1 2021-09-28 15:44:23 UTC Features missing: touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.0 Safari/605.1.15 Language: en-US Window: 1199x675 Pixel Ratio: 2/1 WebGL: WebGL 1.0 GLSL: WebGL GLSL ES 1.0 (1.0) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 31 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
pixelzoom commented 2 years ago

This is a general audio problem that @jbphet described in 9/30/21 dev meeting. See https://github.com/phetsims/tambo/issues/151. @jbphet please advise on how to proceed for Fourier 1.0. High priority because this sim is in RC testing.

pixelzoom commented 2 years ago

@arouinfar and I discussed this issue.

If phetsims/tambo#151 is a problem for published sims, and the intention is to address it soon via a batch maintenance release, then we would feel OK publishing Fourier 1.0 with this known problem.

Fixing sound files will involve patching many repos. That's going to take me a lot of time to patch, and th chances that something might not get patched into the 1.0 branch are high. This also seems to fail on one very specific test platform that I do not have available for testing, so I can't test whether the patch is correct and effective. So I would prefer to handle this via a future batch maintenance release.

jbphet commented 2 years ago

I've implemented better error handling for audio decode errors, and I think this should allow the sim to load when this occurs. See https://github.com/phetsims/tambo/issues/151#issuecomment-933845043 for more information.

jbphet commented 2 years ago

@pixelzoom - I've unassigned myself from this issue. It's up to you whether you'd like to incorporate these changes into the current RC for this sim, but I think there will likely be a maintenance release at some point for all sims to address this issue. I'll be doing that work under the parent GitHub issue (https://github.com/phetsims/tambo/issues/151).

pixelzoom commented 2 years ago

Slack update from @jbphet:

There will likely be a batch MR eventually, but not yet, and I don't know when it will be. I haven't converted all the audio files, I just did all the ones in repos used for Fourier, because it served as a good test case and could be a fix for you if you wanted it. I'd like to do a bit more investigation to see if we can figure out why the decoding is failing before going too much further.

@arouinfar @kathy-phet how do you want to proceed? The options are:

(1) Block Fourier 1.0 until phetsims/tambo#151 is fully addressed and closed. (2) Cherry-pick a partial solution that I can't test. (3) Publish with this as a known problem, and fully address it in a batch MR.

My recommendation is (3). In a Zoom discussion with @arouinfar earlier today, I understood that she was also on board with (3), but wanted to give her the chance to change her mind in light of this update.

pixelzoom commented 2 years ago

(2) Cherry-pick a partial solution that I can't test.

Patching Fourier 1.0 involves cherry-picking 5 shas, for 5 different repos (each requiring a new branch). The shas can be found in the commits immediately above this comment: https://github.com/phetsims/tambo/issues/151#issuecomment-933845043.

After patching, I can't test the 1.0 branch, because I don't have a machine that exhibits the failure. I'd have to go outside of the typical process to give @Nancy-Salpepi a build that she could test (maybe a one-off?) Or just add it to the next RC test and cross our fingers.

kathy-phet commented 2 years ago

I asked JB the timeline for the maintenance release. It seems like a pretty serious bug, so I wouldn't want to do (3) if its going to be a while without a fix.

arouinfar commented 2 years ago

I can reproduce the bug on my machine running Catalina/Safari 15.0. The sim will load with the solution currently in master, albeit it loads very slowly the first time and the only sounds that work are those associated with the harmonics.

I'm fine with option (3) if the batch MR would take place shortly after publishing 1.0. If the MR is ready to go out before then, we'll probably need to go with option (2). I don't want to delay the batch MR because there are currently 11 published sims that are affected by the bug.

If we decide to proceed with (2), I will happily test the next build (whether a one-off or rc.2) before it goes to QA.

arouinfar commented 2 years ago

Update for @pixelzoom:

@jbphet and @jonathanolson are currently working on the batch MR for https://github.com/phetsims/tambo/issues/151 and will include the current release branch of fourier-making-waves.

pixelzoom commented 2 years ago

Thanks @arouinfar. I'll label this as "on-hold" and await further instructions from @jbphet and @jonathanolson.

pixelzoom commented 2 years ago

https://github.com/phetsims/tambo/issues/151#issuecomment-934780833 describes the plan for addressing this issue:

We discussed this issue in today's sound design meeting and plan to move forward with a maintenance release with the commits above. This will mean that for now, sims running on Catalina and Safari 15 will have no or partial sound, and may have longer load times, but won't crash. We will cross our fingers that Apple will fix this reasonably quickly.

On 10/5 and 10/6, @jbphet and @jonathanolson did a batch maintenance release.

On 10/6, I got the "all clear" to proceed with Fourier. From Slack dev-public channel:

John Blanco: Today at 12:25 PM It's now okay for work to resume on unpublished release branches. Published release branches are still off limits while we > create patched RCs, have QA test them, and then re-publish.

Kathy Perkins 1 hour ago does that mean Fourier RC branch is good to go?

John Blanco:house_with_garden: 30 minutes ago Yes.

Any repository whose shas was changed in dependencies.json now has a "fourier-making-waves-1.0" branch. The commits that updated Fourier 1.0 dependencies.json are https://github.com/phetsims/fourier-making-waves/commit/62bd11c6f150726ca1dd0372efad0604f0d745e6 and https://github.com/phetsims/fourier-making-waves/commit/e716acd7b79431a7e23ffc80c06c5fc417a6d0fe.

pixelzoom commented 2 years ago

Slack:

Chris Malley 1:38 PM https://github.com/phetsims/tambo/issues/151#issuecomment-934780833 says:

We discussed this issue in today’s sound design meeting and plan to move forward with a maintenance release with the commits above. This will mean that for now, sims running on Catalina and Safari 15 will have no or partial sound, and may have longer load times, but won’t crash.

@jbphet For the purposes of instructing QA how to verify in the next Fourier RC, can you elaborate on “no or partial sound”? Does this mean that sound files will not play (which includes most UI sounds), but OscillatorSoundGenerator sounds will play (like the Fourier Series sound) ?

John Blanco: 1:44 PM Yes, that's exactly what it means. Any sound that uses a .mp3 as its source will not work, but synthesized sound will.

Chris Malley 1:44 PM Great, thanks.

pixelzoom commented 2 years ago

@arouinfar @Nancy-Salpepi please verify this one-off version, using a system that is running macOS 10.15.7 + Safari 15.0:

https://phet-dev.colorado.edu/html/fourier-making-waves/1.0.0-verify202.1/phet/fourier-making-waves_en_phet.html

screenshot_1327
arouinfar commented 2 years ago

Verify that the sim starts, and there are no errors in the browser console.

The sim starts and is significantly faster to load than master. However, there are still warnings in the console. I think this is still acceptable since these warnings show up for the sims that were a part of the MR, and @jbphet has seen them in my console during discussions.

This pair of warnings repeats dozens of times in the console.

[Warning] decode of audio data failed, using stubbed sound, error: null (fourier-making-waves_en_phet.html, line 874)
[Warning] promise rejection caught for audio decode, error = EncodingError: Decoding failed (fourier-making-waves_en_phet.html, line 874)

Everything else is working as expected, so I've checked them off. The only sounds I heard were those associated with the harmonics. I didn't hear any general UI sounds.

pixelzoom commented 2 years ago

Thanks @arouinfar. Warning in the console are fine.

Nancy-Salpepi commented 2 years ago

I had all of the same results as @arouinfar. Sorry for the delay (dinner).

pixelzoom commented 2 years ago

Great, this is ready for verification in the next RC.

To verify, follow the check list in https://github.com/phetsims/fourier-making-waves/issues/202#issuecomment-937077779.

KatieWoe commented 2 years ago

While it works on 10.15, 10.13 is broken and needs the fix in https://github.com/phetsims/qa/issues/718

pixelzoom commented 2 years ago

On Slack dev-public, @jbphet said:

And yes, you should be able to proceed with a Fourier RC, and I think it would be wise to list the issue that triggered the MR as something that QA should verify so that we can have confidence that the patches really made it onto the branch.

It looks looks like this was patched into Fourier 1.0 in https://github.com/phetsims/fourier-making-waves/commit/1bdf6f17f11c5da157bb4de23b6f8228fddb22aa and https://github.com/phetsims/fourier-making-waves/commit/0449a61dbd8819d73c7163a736df2c243badfac2.

I'll also note that I've looked through https://github.com/phetsims/qa/issues/718, https://github.com/phetsims/tambo/issues/151, and https://github.com/phetsims/tambo/issues/152. Nowhere can I find a description of the new problem, how it was addressed, whether it's a permanent fix or workaround, etc. @jbphet this seems like something that's important to document.

pixelzoom commented 2 years ago

See https://github.com/phetsims/qa/issues/718#issuecomment-941555189 for @jbphet's summary of the additional sound problem that was discovered, resulting in another MR. This new problem was discovered before testing of Fourier 1.0.0-rc.2 began, and caused me to write in https://github.com/phetsims/qa/issues/714#issuecomment-938212647:

The fix for https://github.com/phetsims/fourier-making-waves/issues/202 (phetsims/tambo#151) apparently introduced a problem on other platforms. So another MR will be needed, and another RC will be needed after this one. In the meantime, skip https://github.com/phetsims/fourier-making-waves/issues/202, and verify as many of the other issues as you can.

pixelzoom commented 2 years ago

To verify in the next RC, use whatever procedure you used to verify the sound MR in https://github.com/phetsims/qa/issues/718.

KatieWoe commented 2 years ago

Looks ok on Safari 15 and Safari 13 in rc.3