Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Voicing delayed or absent with Chromebook #303

Closed
Nancy-Salpepi opened this issue Sep 2, 2021 · 15 comments
Closed

Voicing delayed or absent with Chromebook #303

Nancy-Salpepi opened this issue Sep 2, 2021 · 15 comments
Assignees

Comments

@Nancy-Salpepi
Copy link

Test device
Chromebook

Browser
Chrome

Problem description
phetsims/qa#702

Voicing is delayed or requires more than 1 click sometimes to work. Once it starts working, the delay is lessened.

Steps to reproduce
Here is one example: Using a chromebook--turn on voicing in preferences. You should get a delay here.

Visuals

voicedelay44.mov
voicingdelay2.mov

Troubleshooting information:
!!!!! DO NOT EDIT !!!!!
Name: ‪Gravity Force Lab: Basics‬
URL: https://phet-dev.colorado.edu/html/gravity-force-lab-basics/1.1.0-rc.2/phet/gravity-force-lab-basics_all_phet.html
Version: 1.1.0-rc.2 2021-08-30 16:27:03 UTC
Features missing: applicationcache, applicationcache, touch
User Agent: Mozilla/5.0 (X11; CrOS x86_64 13982.82.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.157 Safari/537.36
Language: en-US
Window: 1357x609
Pixel Ratio: 1/1
WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium)
GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium)
Vendor: WebKit (WebKit WebGL)
Vertex: attribs: 16 varying: 32 uniform: 4096
Texture: size: 16384 imageUnits: 32 (vertex: 32, combined: 192)
Max viewport: 16384x16384
OES_texture_float: true

@jessegreenberg
Copy link
Contributor

Thanks @Nancy-Salpepi - would you be able to see if this is happening in the published version of john-travoltage too? I don't have access to a chromebook so I need to think about how I can investigate further.

@Nancy-Salpepi
Copy link
Author

@jessegreenberg I saw 2 things with the published John Travoltage sim:

  1. If I move between the quick info options while one is being read aloud, voicing does not tell me what option I am now on. If I interrupt the voicing after moving the arm or leg and switch focus between them, voicing works as it should.

  2. When turning on voicing for the first time, it takes 2 seconds for voicing to actually start. However, on subsequent attempts, it happens without a delay.

voicingpublishedJT.mov
jtex2.mov

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Sep 22, 2021

@Nancy-Salpepi helped me remote access into a chromebook. I cannot hear output with remote access but I can see when the voicing responses are coming out with ?printVoicingResponses. ?printVoicingResponses puts the output text in the console when the actual utterance from the browser starts speaking, so it is a good indication of the output. Using this I can confirm that it is very delayed the first time voicing is used, it usually takes 5-6 seconds to produce speech.

I was also able to produce the issue where there is no output while clicking on the reading block.

@jessegreenberg
Copy link
Contributor

Just noting some thoughts about things we could improve (or not):

  1. Can we reuse the speechSynthesisUtterance so that we don't recreate one every speech? Maybe this will be faster, maybe not.
  2. What if on the first input to the sim we trigger some empty speech so our first speech synthesis isn't the one used to say that voicing is enabled. This would kind of be like "revving the engine".
  3. I checked whether we were sending too many utterances to the queue causing a delay. In these cases in this issue we are not.
  4. I thought about using different voices but wasn't really able to test this against the issue presented here because in order to change the voice I need to enable voicing first. I also can't hear if the different voices render faster.
  5. I verified that the first Voicing output happens synchronously with input. Having the first output done asynchronously has been problematic for us in the past so I wanted to check this wasn't still an issue.

Option 2 seems the most promising, ill give that a go.

@jessegreenberg
Copy link
Contributor

Here is the patch to try:

Index: js/accessibility/voicing/voicingManager.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/accessibility/voicing/voicingManager.js	(revision d73d6fc643306e149c3eede8706e6d107c978397)
+++ js/accessibility/voicing/voicingManager.js	(date 1632352884035)
@@ -21,6 +21,7 @@
 import stripEmbeddingMarks from '../../../../phet-core/js/stripEmbeddingMarks.js';
 import Announcer from '../../../../utterance-queue/js/Announcer.js';
 import Utterance from '../../../../utterance-queue/js/Utterance.js';
+import Display from '../../display/Display.js';
 import scenery from '../../scenery.js';
 import globalKeyStateTracker from '../globalKeyStateTracker.js';
 import KeyboardUtils from '../KeyboardUtils.js';
@@ -173,6 +174,12 @@
       }
     } );
 
+    const engineRevListener = () => {
+      this.speakIgnoringEnabled( new Utterance( { alert: ' ' } ) );
+      Display.userGestureEmitter.removeListener( engineRevListener );
+    };
+    Display.userGestureEmitter.addListener( engineRevListener );
+
     this.initialized = true;
   }
 
@@ -337,6 +344,8 @@
 
     if ( !this.hasSpoken ) {
 
+      console.log( 'speaking' );
+
       // for the first time speaking it must be synchronous and we cannot use TimeoutCallbackObject workarounds yet
       this.getSynth().speak( speechSynthUtterance );
       this.hasSpoken = true;
Index: js/input/Input.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/input/Input.js	(revision d73d6fc643306e149c3eede8706e6d107c978397)
+++ js/input/Input.js	(date 1632352810206)
@@ -1051,7 +1051,9 @@
    * @param {boolean} bubbles
    */
   dispatchPDOMEvent( trail, eventType, domEvent, bubbles ) {
-    Display.userGestureEmitter.emit();
+    if ( eventType !== 'focus' && eventType !== 'focusin' ) {
+      Display.userGestureEmitter.emit();
+    }
 
     // This workaround hopefully won't be here forever, see ParallelDOM.setExcludeLabelSiblingFromInput() and https://github.com/phetsims/a11y-research/issues/156
     if ( !( domEvent.target && domEvent.target.hasAttribute( PDOMUtils.DATA_EXCLUDE_FROM_INPUT ) ) ) {

The focus event (from scripting focus on the screen div) is triggering Display.userGestureEmitter, so I had to prevent that from happening to get the input gesture from actual input.

@jessegreenberg
Copy link
Contributor

Finally coming back to this. This is the patch I want to try

    const revTheEngine = () => {

      // or could this be empty?
      this.speakIgnoringEnabled( new Utterance( {
        alert: 'Something that will let the browser know to be ready for speech'
      } ) );

      // is it OK to immediately cancel?
      // this.cancel();

      Display.userGestureEmitter.removeListener( revTheEngine );
    };
    Display.userGestureEmitter.addListener( revTheEngine );

I think to test I am going to commit this to a one-off branch.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Oct 20, 2021

I tried out a few versions with remote access into a chromebook.

  1. A version with this change, speaking a string on the first input:
    const revTheEngine = () => {

      // or could this be empty?
      // debugger;
      this.speakIgnoringEnabled( new Utterance( {
        alert: 'This is a test alert'
      } ) );

      // is it OK to immediately cancel?
      // this.cancel();

      phet.scenery.Display.userGestureEmitter.removeListener( revTheEngine );
    };
    phet.scenery.Display.userGestureEmitter.addListener( revTheEngine );

The speech from this first alert was delayed, but subsequent Voicing responses actually do seem more predictable. I will note that each time I take a break from interacting with the sim Voicing is slower to happen the next time I interact.

  1. A version with this change, cancelling the content immediateley
    const revTheEngine = () => {

      // or could this be empty?
      this.speakIgnoringEnabled( new Utterance( {
        alert: 'This is a test alert'
      } ) );

      // is it OK to immediately cancel?
      this.cancel();
      console.log( 'alerting, canceling immediately.' );

      phet.scenery.Display.userGestureEmitter.removeListener( revTheEngine );
    };
    phet.scenery.Display.userGestureEmitter.addListener( revTheEngine );

This seemed as effective as the first test. It also had the same issue where letting the sim sit for ~30 seconds made the next Voicing alert happen much slower.

  1. Speaking empty content without cancel. Just an empty string
    const revTheEngine = () => {

      // or could this be empty?
      this.speakIgnoringEnabled( new Utterance( {
        alert: ''
      } ) );

      // is it OK to immediately cancel?
      // this.cancel();
      console.log( 'alerting empty content.' );

      phet.scenery.Display.userGestureEmitter.removeListener( revTheEngine );
    };
    phet.scenery.Display.userGestureEmitter.addListener( revTheEngine );

With this patch, VOicing also seemed quick the first time it was actually requested from the Preferences dialog.

However, it often seemed quick and responsive when testing with https://phet-dev.colorado.edu/html/gravity-force-lab-basics/1.1.0-rc.2/phet/gravity-force-lab-basics_all_phet.html?printVoicingResponses as well.

But all of these suffered from the delay coming back if Voicing isn't used for ~30 seconds. I wonder if there is a way around this...

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Oct 20, 2021

What if we try a workaround where we "keep the engine running" by alerting something empty every few seconds when there is nothing in the voicing queue?

EDIT: Here is a patch to try this:

Index: js/accessibility/voicing/voicingManager.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/accessibility/voicing/voicingManager.js b/js/accessibility/voicing/voicingManager.js
--- a/js/accessibility/voicing/voicingManager.js	(revision 414d8cf0bf3cdabc1a9c8e6eb6b7e091ae8369cf)
+++ b/js/accessibility/voicing/voicingManager.js	(date 1634769109750)
@@ -21,12 +21,15 @@
 import stripEmbeddingMarks from '../../../../phet-core/js/stripEmbeddingMarks.js';
 import Announcer from '../../../../utterance-queue/js/Announcer.js';
 import Utterance from '../../../../utterance-queue/js/Utterance.js';
+// import Display from '../../display/Display.js';
 import scenery from '../../scenery.js';
 import globalKeyStateTracker from '../globalKeyStateTracker.js';
 import KeyboardUtils from '../KeyboardUtils.js';
 
 const DEFAULT_PRIORITY = 1;
 
+const TIME_SINCE_WAKING_ENGINE = 5; // in seconds
+
 const UTTERANCE_OPTION_DEFAULTS = {
 
   // {boolean} - If true and this Utterance is currently being spoken by the speech synth, announcing it
@@ -202,6 +205,21 @@
     // No dispose, as this singleton exists for the lifetime of the runtime.
     stepTimer.addListener( this.stepQueue.bind( this ) );
 
+    const revTheEngine = () => {
+
+      // or could this be empty?
+      this.speakIgnoringEnabled( new Utterance( {
+        alert: ''
+      } ) );
+
+      // is it OK to immediately cancel?
+      // this.cancel();
+      console.log( 'alerting empty content.' );
+
+      phet.scenery.Display.userGestureEmitter.removeListener( revTheEngine );
+    };
+    phet.scenery.Display.userGestureEmitter.addListener( revTheEngine );
+
     this.initialized = true;
   }
 
@@ -248,6 +266,8 @@
             this.speakingProperty.set( true );
             synth.speak( voicingQueueElement.speechSynthUtterance );
 
+            this.timeSinceSpeaking = 0;
+
             this.assertSpeakingPropertyInSync();
           }
 
@@ -298,6 +318,15 @@
       if ( !this.getSynth().speaking && this.voicingQueue.length === 0 && this.safariWorkaroundUtterances.length > 0 ) {
         this.safariWorkaroundUtterances = [];
       }
+
+      this.timeSinceSpeaking += dt;
+      if ( this.voicingQueue.length === 0 && !this.getSynth().speaking && this.timeSinceSpeaking > TIME_SINCE_WAKING_ENGINE ) {
+
+        // speak empty content to keep the Synthesis engine awake for Chromebooks
+        this.speakIgnoringEnabled( new Utterance( {
+          alert: ''
+        } ) );
+      }
     }
   }
 

EDIT: Shockingly, this works.

EDIT: I also tested this without "reving the engine"

Index: js/accessibility/voicing/voicingManager.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/accessibility/voicing/voicingManager.js b/js/accessibility/voicing/voicingManager.js
--- a/js/accessibility/voicing/voicingManager.js	(revision 414d8cf0bf3cdabc1a9c8e6eb6b7e091ae8369cf)
+++ b/js/accessibility/voicing/voicingManager.js	(date 1634769747291)
@@ -21,12 +21,15 @@
 import stripEmbeddingMarks from '../../../../phet-core/js/stripEmbeddingMarks.js';
 import Announcer from '../../../../utterance-queue/js/Announcer.js';
 import Utterance from '../../../../utterance-queue/js/Utterance.js';
+// import Display from '../../display/Display.js';
 import scenery from '../../scenery.js';
 import globalKeyStateTracker from '../globalKeyStateTracker.js';
 import KeyboardUtils from '../KeyboardUtils.js';
 
 const DEFAULT_PRIORITY = 1;
 
+const TIME_SINCE_WAKING_ENGINE = 5; // in seconds
+
 const UTTERANCE_OPTION_DEFAULTS = {
 
   // {boolean} - If true and this Utterance is currently being spoken by the speech synth, announcing it
@@ -201,6 +204,21 @@
 
     // No dispose, as this singleton exists for the lifetime of the runtime.
     stepTimer.addListener( this.stepQueue.bind( this ) );
+    //
+    // const revTheEngine = () => {
+    //
+    //   // or could this be empty?
+    //   this.speakIgnoringEnabled( new Utterance( {
+    //     alert: ''
+    //   } ) );
+    //
+    //   // is it OK to immediately cancel?
+    //   // this.cancel();
+    //   console.log( 'alerting empty content.' );
+    //
+    //   phet.scenery.Display.userGestureEmitter.removeListener( revTheEngine );
+    // };
+    // phet.scenery.Display.userGestureEmitter.addListener( revTheEngine );
 
     this.initialized = true;
   }
@@ -248,6 +266,8 @@
             this.speakingProperty.set( true );
             synth.speak( voicingQueueElement.speechSynthUtterance );
 
+            this.timeSinceSpeaking = 0;
+
             this.assertSpeakingPropertyInSync();
           }
 
@@ -298,6 +318,17 @@
       if ( !this.getSynth().speaking && this.voicingQueue.length === 0 && this.safariWorkaroundUtterances.length > 0 ) {
         this.safariWorkaroundUtterances = [];
       }
+
+      // if ( this.enabledProperty.value ) {
+      this.timeSinceSpeaking += dt;
+      if ( this.voicingQueue.length === 0 && !this.getSynth().speaking && this.timeSinceSpeaking > TIME_SINCE_WAKING_ENGINE ) {
+
+        // speak empty content to keep the Synthesis engine awake for Chromebooks
+        this.speakIgnoringEnabled( new Utterance( {
+          alert: ''
+        } ) );
+      }
+      // }
     }
   }
 

EDIT: This seemed to work very well too.

@jessegreenberg
Copy link
Contributor

jessegreenberg commented Oct 20, 2021

This is really good information. The collection of builds I used to test this can be found at https://phet-dev.colorado.edu/html/jg-tests/voicing-tests/. Next is to decide the best way to apply this discovery. And decide if we even should.

@jessegreenberg
Copy link
Contributor

@zepumph and I discussed this workaround and potential ways to improve it.

  1. Don't use speakIgnoringEnabled, just use synth.speak directly to avoid garbage and adding start/end listeners and other work for this workaround.
  2. Can we increase the interval to 10 seconds instead of 5? The less frequent the better, but we don't want to push it all the way to the limit since this behavior is very platform dependent.
  3. This workaround is OK. The worst part about it is that it is used even when the voicingManager is not enabled. But it is worth it to have more readily available SpeechSynthesis on Chromebooks.
  4. Instead of every N seconds, the interval should be relative to how long it has been since the last usage of synth.speak and check that voicingManager.speakingProperty.value is false and that the voicingManager.queue is empty. This way this workaround won't interfere with other things in the queue, and only add to the queue when there is nothing in it.
  5. It should always respect voicingManager.enabledProperty, but will still happen when speechAllowedProperty is false after Replace VoicingManager.speakIgnoringEnabled with a Property controlling speech utterance-queue#69 is done.

@jessegreenberg
Copy link
Contributor

The above commits got this working much better. After testing a more complete change on the Chromebook I decided to use both the stepTimer approach and the userGestureEmitter together so that that we try the workaround on first input instead of waiting 10 seconds.

It isn't perfect but I think it is just about the best we can do. If you turn on Voicing immediately after the first time the workaround is used there is still a slight delay while Chrome enables the feature. But after that it is consistently available.

@jessegreenberg
Copy link
Contributor

Unfortunately this is looking pretty tough to cherry pick into the release branch because GFL:B scenery SHAs do not include big changes to the voicingManager to fix phetsims/scenery#1288.

I am happy we have a workaround in place for Chromebooks moving forward. But since it only impacts that platform, we already have one sim published with this behavior using Voicing, and is only an issue the first time SpeechSynthesis is used by the browser, I am going to recommend it isn't worth destabilizing the GFL:B 1.1 SHAs.

@jessegreenberg
Copy link
Contributor

Confirmed on slack that it is OK to proceed with GFL:B 1.1 without this fix. @Nancy-Salpepi can you please try out Voicing on master on your Chromebook? I am curious if you feel this has made any difference since you first noticed the problem. As mentioned in #303 (comment) I don't think it is a perfect fix but hopefully is improved.

@jessegreenberg jessegreenberg removed their assignment Oct 26, 2021
@Nancy-Salpepi
Copy link
Author

@jessegreenberg Voicing is much improved! I do not notice a delay at all now.

@jessegreenberg
Copy link
Contributor

Thats great to hear @Nancy-Salpepi, thank you! Closing this issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants