Adding Display.refreshSVG / Display.refreshSVGOnNextFrame, improved S…

…VGBlock usage to support lazy creation, and SimDisplay chromium guard to refresh SVG during pan/zoom, see #1507
phetsims · Mar 13, 2024 · 3561b0f · 3561b0f
1 parent 4d9eaff
commit 3561b0f
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 25 deletions.
diff --git a/js/display/Display.ts b/js/display/Display.ts
@@ -68,7 +68,6 @@ import UtteranceQueue from '../../../utterance-queue/js/UtteranceQueue.js';
 import { BackboneDrawable, Block, CanvasBlock, CanvasNodeBoundsOverlay, ChangeInterval, Color, DOMBlock, DOMDrawable, Drawable, Features, FittedBlockBoundsOverlay, FocusManager, FullScreen, globalKeyStateTracker, HighlightOverlay, HitAreaOverlay, Input, InputOptions, Instance, KeyboardUtils, Node, PDOMInstance, PDOMSiblingStyle, PDOMTree, PDOMUtils, Pointer, PointerAreaOverlay, PointerOverlay, Renderer, scenery, SceneryEvent, scenerySerialize, SelfDrawable, TInputListener, TOverlay, Trail, Utils, WebGLBlock } from '../imports.js';
 import TEmitter from '../../../axon/js/TEmitter.js';
 import SafariWorkaroundOverlay from '../overlays/SafariWorkaroundOverlay.js';
-import TinyEmitter from '../../../axon/js/TinyEmitter.js';
 
 type SelfOptions = {
   // Initial (or override) display width
@@ -310,8 +309,12 @@ export default class Display {
   private perfDrawableOldIntervalCount?: number;
   private perfDrawableNewIntervalCount?: number;
 
-  // (scenery-internal)
-  public readonly frameEmitter = new TinyEmitter();
+  // (scenery-internal) When fired, forces an SVG refresh, to try to work around issues
+  // like https://github.com/phetsims/scenery/issues/1507
+  public readonly _refreshSVGEmitter = new Emitter();
+
+  // If true, we will refresh the SVG elements on the next frame
+  private _refreshSVGPending = false;
 
   /**
    * Constructs a Display that will show the rootNode and its subtree in a visual state. Default options provided below
@@ -714,7 +717,11 @@ export default class Display {
 
     PDOMTree.auditPDOMDisplays( this.rootNode );
 
-    this.frameEmitter.emit();
+    if ( this._forceSVGRefresh || this._refreshSVGPending ) {
+      this._refreshSVGPending = false;
+
+      this.refreshSVG();
+    }
 
     sceneryLog && sceneryLog.Display && sceneryLog.pop();
   }
@@ -2089,6 +2096,23 @@ export default class Display {
     return ( instance && instance.trail ) ? instance.trail : null;
   }
 
+  /**
+   * Forces SVG elements to have their visual contents refreshed, by changing state in a non-visually-apparent way.
+   * It should trick browsers into re-rendering the SVG elements.
+   *
+   * See https://github.com/phetsims/scenery/issues/1507
+   */
+  public refreshSVG(): void {
+    this._refreshSVGEmitter.emit();
+  }
+
+  /**
+   * Similar to refreshSVG (see docs above), but will do so on the next frame.
+   */
+  public refreshSVGOnNextFrame(): void {
+    this._refreshSVGPending = true;
+  }
+
   /**
    * Releases references.
    *

diff --git a/js/display/SVGBlock.js b/js/display/SVGBlock.js
@@ -86,27 +86,26 @@ class SVGBlock extends FittedBlock {
 
     // Forces SVG elements to be refreshed every frame, which can force repainting and detect (or potentially in some
     // cases work around) SVG rendering browser bugs. See https://github.com/phetsims/scenery/issues/1507
-    if ( this.display._forceSVGRefresh && !this.forceRefreshListener ) {
-
-      const workaroundGroup = document.createElementNS( svgns, 'g' );
-      this.svg.appendChild( workaroundGroup );
-
-      const workaroundRect = document.createElementNS( svgns, 'rect' );
-      workaroundRect.setAttribute( 'width', '0' );
-      workaroundRect.setAttribute( 'height', '0' );
-      workaroundRect.setAttribute( 'fill', 'none' );
-      workaroundGroup.appendChild( workaroundRect );
-
-      // @private {function} - Forces a color change on the 0x0 rect
-      this.forceRefreshListener = () => {
-        const red = dotRandom.nextIntBetween( 0, 255 );
-        const green = dotRandom.nextIntBetween( 0, 255 );
-        const blue = dotRandom.nextIntBetween( 0, 255 );
-        workaroundRect.setAttribute( 'fill', `rgba(${red},${green},${blue},0.02)` );
-      };
-    }
+    // @private {function} - Forces a color change on the 0x0 rect
+    this.forceRefreshListener = () => {
+      // Lazily add this, so we're not incurring any performance penalties until we actually need it
+      if ( !this.workaroundRect ) {
+        const workaroundGroup = document.createElementNS( svgns, 'g' );
+        this.svg.appendChild( workaroundGroup );
+
+        this.workaroundRect = document.createElementNS( svgns, 'rect' );
+        this.workaroundRect.setAttribute( 'width', '0' );
+        this.workaroundRect.setAttribute( 'height', '0' );
+        this.workaroundRect.setAttribute( 'fill', 'none' );
+        workaroundGroup.appendChild( this.workaroundRect );
+      }
 
-    this.display._forceSVGRefresh && this.display.frameEmitter.addListener( this.forceRefreshListener );
+      const red = dotRandom.nextIntBetween( 0, 255 );
+      const green = dotRandom.nextIntBetween( 0, 255 );
+      const blue = dotRandom.nextIntBetween( 0, 255 );
+      this.workaroundRect.setAttribute( 'fill', `rgba(${red},${green},${blue},0.02)` );
+    };
+    this.display._refreshSVGEmitter.addListener( this.forceRefreshListener );
 
     // reset what layer fitting can do
     Utils.prepareForTransform( this.svg ); // Apply CSS needed for future CSS transforms to work properly.
@@ -392,7 +391,7 @@ class SVGBlock extends FittedBlock {
 
     this.paintCountMap.clear();
 
-    this.display._forceSVGRefresh && this.display.frameEmitter.removeListener( this.forceRefreshListener );
+    this.display._refreshSVGEmitter.removeListener( this.forceRefreshListener );
 
     this.baseTransformGroup.removeChild( this.rootGroup.svgGroup );
     this.rootGroup.dispose();