Browse Source

backends: improve consistency of backend removal and backend creation by removing all backends at refresh (#678)

problem isn't completely solved, see #678.
sschmid 5 năm trước cách đây
mục cha
commit
2fc5276d63

+ 19 - 4
src/MusicalScore/Graphical/VexFlow/VexFlowBackend.ts

@@ -33,15 +33,30 @@ export abstract class VexFlowBackend {
     return this.renderer;
   }
 
+  public removeAllChildrenFromContainer(container: HTMLElement): void {
+    while (container.children.length !== 0) {
+      container.removeChild(container.children.item(0));
+    }
+  }
+
+  // note: removing single children to remove all is error-prone, because sometimes a random SVG-child remains.
   public removeFromContainer(container: HTMLElement): void {
     //console.log("backend type: " + this.getVexflowBackendType());
-    if (this.getVexflowBackendType() === Vex.Flow.Renderer.Backends.SVG) {
-      container.removeChild(this.canvas);
-    } else if (this.getVexflowBackendType() === Vex.Flow.Renderer.Backends.CANVAS) {
-      container.removeChild(this.inner);
+    let htmlElementToRemove: HTMLElement = this.canvas; // for SVGBackend
+    if (this.getVexflowBackendType() === Vex.Flow.Renderer.Backends.CANVAS) {
+      htmlElementToRemove = this.inner;
       // for SVG, this.canvas === this.inner, but for Canvas, removing this.canvas causes an error because it's not a child of container,
       // so we have to remove this.inner instead.
     }
+
+    // only remove child if the container has this child, otherwise it will throw an error.
+    for (let i: number = 0; i < container.children.length; i++) {
+      if (container.children.item(i) === htmlElementToRemove) {
+        container.removeChild(htmlElementToRemove);
+        break;
+      }
+    }
+    // there is unfortunately no built-in container.hasChild(child) method.
   }
 
 public abstract getContext(): Vex.IRenderContext;

+ 9 - 3
src/OpenSheetMusicDisplay/OpenSheetMusicDisplay.ts

@@ -231,14 +231,20 @@ export class OpenSheetMusicDisplay {
     }
 
     private createOrRefreshRenderBackend(): void {
-        //console.log("render: need update");
+        // console.log("[OSMD] createOrRefreshRenderBackend()");
+
         // Remove old backends
         if (this.drawer && this.drawer.Backends) {
-            for (const backend of this.drawer.Backends) {
-                backend.removeFromContainer(this.container);
+            // removing single children to remove all is error-prone, because sometimes a random SVG-child remains.
+            // for (const backend of this.drawer.Backends) {
+            //     backend.removeFromContainer(this.container);
+            // }
+            if (this.drawer.Backends[0]) {
+                this.drawer.Backends[0].removeAllChildrenFromContainer(this.container);
             }
             this.drawer.Backends.clear();
         }
+
         // Create the drawer
         this.drawer = new VexFlowMusicSheetDrawer(this.drawingParameters); // note that here the drawer.drawableBoundingBoxElement is lost. now saved in OSMD.
         this.drawer.drawableBoundingBoxElement = this.DrawBoundingBox;

+ 5 - 2
test/Util/generateDiffImagesPuppeteerLocalhost.js

@@ -94,6 +94,9 @@ async function init () {
     const browser = await puppeteer.launch({ headless: true })
     const page = await browser.newPage() // TODO set width/height
 
+    const defaultTimeoutInMs = 30000
+    page.setDefaultNavigationTimeout(defaultTimeoutInMs) // default setting for page navigationtimeout is 30000ms.
+
     // fix navigation error
     var responseEventOccurred = false
     var responseHandler = function (event) { responseEventOccurred = true }
@@ -103,10 +106,10 @@ async function init () {
             if (!responseEventOccurred) {
                 resolve(true)
             } else {
-                setTimeout(function () { resolve(true) }, 30000)
+                setTimeout(function () { resolve(true) }, defaultTimeoutInMs)
             }
             page.removeListener('response', responseHandler)
-        }, 500)
+        }, 1000)
     })
 
     page.on('response', responseHandler)