fix: [#2359] work around image decode limits in chromium (#2361)

Closes #2359 This PR switches away from `Image.decode()` to use the older method of `Image.onload` this method is more reliable for large or numerous images due to a chromium bug around decode. @HxShard provided a great codesandbox illustrating the issue https://codesandbox.io/s/happy-gagarin-j1vjr ## Changes: - Creates a new Future type for working with native Promises' resolve/reject at any time - Creates a new Semaphore type that can limit async calls in between `enter` and `exit` - Fixes a small clock schedule bug as well
excaliburjs · Jun 25, 2022 · a374da8 · a374da8
1 parent b285ede
commit a374da8
Show file tree

Hide file tree

Showing 14 changed files with 451 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,6 +15,24 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 -
 
 ### Added
+- Added new `ex.Future` type which is a convenient way of wrapping a native browser promise and resolving/rejecting later
+  ```typescript
+  const future = new ex.Future();
+  const promise = future.promise; // returns promise
+  promise.then(() => {
+    console.log('Resolved!');
+  });
+  future.resolve(); // resolved promise
+  ```
+- Added new `ex.Semaphore` type to limit the number of concurrent cans in a section of code, this is used internally to work around a chrome browser limitation, but can be useful for throttling network calls or even async game events.
+  ```typescript
+  const semaphore = new ex.Semaphore(10); // Only allow 10 concurrent between enter() and exit()
+  ...
+
+  await semaphore.enter();
+  await methodToBeLimited();
+  semaphore.exit();
+  ```
 - Added new `ex.WatchVector` type that can observe changes to x/y more efficiently than `ex.watch()`
 - Added performance improvements 
    * `ex.Vector.distance` improvement
@@ -90,6 +108,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Add target element id to `ex.Screen.goFullScreen('some-element-id')` to influence the fullscreen element in the fullscreen browser API.
 
 ### Fixed
+- Fixed bug in `Clock.schedule` where callbacks would not fire at the correct time, this was because it was scheduling using browser time and not the clock's internal time.
+- Fixed issue in Chromium browsers where Excalibur crashes if more than 256 `Image.decode()` calls are happening in the same frame.
 - Fixed issue where `ex.EdgeCollider` were not working properly in `ex.CompositeCollider` for `ex.TileMap`'s
 - Fixed issue where `ex.BoundingBox` overlap return false due to floating point rounding error causing multiple collisions to be evaluated sometimes
 - Fixed issue with `ex.EventDispatcher` where removing a handler that didn't already exist would remove another handler by mistake

diff --git a/sandbox/index.html b/sandbox/index.html
@@ -11,6 +11,7 @@
       <li><a href="html/index.html">Sandbox Platformer</a></li>
       <li><a href="tests/parallel/">Parallel Actions</a></li>
       <li><a href="tests/isometric/">Isometric Map</a></li>
+      <li><a href="tests/decode-many/">Decode Many Images without failure (Chrome)</a></li>
       <li><a href="tests/side-collision/">Arcade: Sliding on Floor</a></li>
       <li><a href="tests/side-collision2/">Arcade: No clipping divergent collisions</a></li>
       <li><a href="tests/tilemap-custom-edge-collider/">Edge colliders work in a tilemap</a></li>

diff --git a/sandbox/tests/decode-many/index.html b/sandbox/tests/decode-many/index.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+  <meta http-equiv="X-UA-Compatible" content="IE=edge">
+  <meta name="viewport" content="width=device-width, initial-scale=1.0">
+  <title>Decode Images</title>
+</head>
+<body>
+  <p>There should be no errors in the console relating to loading images</p>
+  <script src="../../lib/excalibur.js"></script>
+  <script src="./index.js"></script>
+</body>
+</html>
diff --git a/sandbox/tests/decode-many/index.ts b/sandbox/tests/decode-many/index.ts
@@ -0,0 +1,63 @@
+var game = new ex.Engine({
+  width: 800,
+  height: 600
+});
+
+var loader = new ex.Loader();
+
+function generate() {
+  let srcs = [];
+  for (let i = 0; i < 800; i++) {
+    srcs.push(generateRandomImage());
+  }
+  let images = srcs.map(src => new ex.ImageSource(src));
+  loader.addResources(images);
+
+  let sprites = images.map(i => i.toSprite());
+
+  game.currentScene.onPostDraw = ctx => {
+    ctx.save();
+    ctx.scale(.25, .25);
+    for (let i = 0; i < sprites.length; i++) {
+      sprites[i].draw(ctx, (i * 100) % (800 * 4) + 10, Math.floor((i * 100) / (800 * 4)) * 100 + 10);
+    }
+    ctx.restore();
+  };
+}
+
+function drawRandomCircleOnContext(ctx) {
+  const x = Math.floor(Math.random() * 100);
+  const y = Math.floor(Math.random() * 100);
+  const radius = Math.floor(Math.random() * 20);
+
+  const r = Math.floor(Math.random() * 255);
+  const g = Math.floor(Math.random() * 255);
+  const b = Math.floor(Math.random() * 255);
+
+  ctx.beginPath();
+  ctx.arc(x, y, radius, Math.PI * 2, 0, false);
+  ctx.fillStyle = "rgba(" + r + "," + g + "," + b + ",1)";
+  ctx.fill();
+  ctx.closePath();
+}
+
+function generateRandomImage() {
+  const canvas = document.createElement("canvas");
+  canvas.width = 100;
+  canvas.height = 100;
+
+  const ctx = canvas.getContext("2d");
+  ctx.clearRect(0, 0, 100, 100);
+
+  for (let i = 0; i < 20; i++) {
+    drawRandomCircleOnContext(ctx);
+  }
+  return canvas.toDataURL("image/png");
+}
+
+
+generate();
+
+
+
+game.start(loader);
diff --git a/src/engine/Graphics/ImageSource.ts b/src/engine/Graphics/ImageSource.ts
@@ -4,6 +4,7 @@ import { Loadable } from '../Interfaces/Index';
 import { Logger } from '../Util/Log';
 import { TextureLoader } from '.';
 import { ImageFiltering } from './Filtering';
+import { Future } from '../Util/Future';
 
 export class ImageSource implements Loadable<HTMLImageElement> {
   private _logger = Logger.getInstance();
@@ -45,11 +46,11 @@ export class ImageSource implements Loadable<HTMLImageElement> {
     return this.data;
   }
 
+  private _readyFuture = new Future<HTMLImageElement>();
   /**
    * Promise the resolves when the image is loaded and ready for use, does not initiate loading
    */
-  public ready: Promise<HTMLImageElement>;
-  private _loadedResolve: (value?: HTMLImageElement | PromiseLike<HTMLImageElement>) => void;
+  public ready: Promise<HTMLImageElement> = this._readyFuture.promise;
 
   /**
    * The path to the image, can also be a data url like 'data:image/'
@@ -63,9 +64,6 @@ export class ImageSource implements Loadable<HTMLImageElement> {
     if (path.endsWith('.svg') || path.endsWith('.gif')) {
       this._logger.warn(`Image type is not fully supported, you may have mixed results ${path}. Fully supported: jpg, bmp, and png`);
     }
-    this.ready = new Promise<HTMLImageElement>((resolve) => {
-      this._loadedResolve = resolve;
-    });
   }
 
   /**
@@ -87,9 +85,15 @@ export class ImageSource implements Loadable<HTMLImageElement> {
 
       // Decode the image
       const image = new Image();
+      // Use Image.onload over Image.decode()
+      // https://bugs.chromium.org/p/chromium/issues/detail?id=1055828#c7
+      // Otherwise chrome will throw still Image.decode() failures for large textures
+      const loadedFuture = new Future<void>();
+      image.onload = () => loadedFuture.resolve();
       image.src = url;
       image.setAttribute('data-original-src', this.path);
-      await image.decode();
+
+      await loadedFuture.promise;
 
       // Set results
       this.data = image;
@@ -98,7 +102,7 @@ export class ImageSource implements Loadable<HTMLImageElement> {
     }
     TextureLoader.load(this.data, this._filtering);
     // todo emit complete
-    this._loadedResolve(this.data);
+    this._readyFuture.resolve(this.data);
     return this.data;
   }
 

diff --git a/src/engine/Loader.ts b/src/engine/Loader.ts
@@ -13,6 +13,7 @@ import { delay } from './Util/Util';
 import { ImageFiltering } from './Graphics/Filtering';
 import { clamp } from './Math/util';
 import { Sound } from './Resources/Sound/Sound';
+import { Future } from './Util/Future';
 
 /**
  * Pre-loading assets
@@ -310,12 +311,9 @@ export class Loader extends Class implements Loadable<Loadable<any>[]> {
 
   data: Loadable<any>[];
 
-  private _isLoadedResolve: () => any;
-  private _isLoadedPromise = new Promise<void>(resolve => {
-    this._isLoadedResolve = resolve;
-  });
+  private _loadingFuture = new Future<void>;
   public areResourcesLoaded() {
-    return this._isLoadedPromise;
+    return this._loadingFuture.promise;
   }
 
   /**
@@ -324,15 +322,16 @@ export class Loader extends Class implements Loadable<Loadable<any>[]> {
    */
   public async load(): Promise<Loadable<any>[]> {
     await this._image?.decode(); // decode logo if it exists
+    this.canvas.flagDirty();
 
     await Promise.all(
-      this._resourceList.map((r) =>
-        r.load().finally(() => {
+      this._resourceList.map(async (r) => {
+        await r.load().finally(() => {
           // capture progress
           this._numLoaded++;
           this.canvas.flagDirty();
-        })
-      )
+        });
+      })
     );
     // Wire all sound to the engine
     for (const resource of this._resourceList) {
@@ -341,7 +340,7 @@ export class Loader extends Class implements Loadable<Loadable<any>[]> {
       }
     }
 
-    this._isLoadedResolve();
+    this._loadingFuture.resolve();
 
     // short delay in showing the button for aesthetics
     await delay(200, this._engine?.clock);

diff --git a/src/engine/Util/Clock.ts b/src/engine/Util/Clock.ts
@@ -93,7 +93,8 @@ export abstract class Clock {
    * @param timeoutMs Optionally specify a timeout in milliseconds from now, default is 0ms which means the next possible tick
    */
   public schedule(cb: () => any, timeoutMs: number = 0) {
-    const scheduledTime = this.now() + timeoutMs;
+    // Scheduled based on internal elapsed time
+    const scheduledTime = this._totalElapsed + timeoutMs;
     this._scheduledCbs.push([cb, scheduledTime]);
   }
 

diff --git a/src/engine/Util/Future.ts b/src/engine/Util/Future.ts
@@ -0,0 +1,39 @@
+
+/**
+ * Future is a wrapper around a native browser Promise to allow resolving/rejecting at any time
+ */
+export class Future<T> {
+  // Code from StephenCleary https://gist.github.com/StephenCleary/ba50b2da419c03b9cba1d20cb4654d5e
+  private _resolver: (value: T) => void;
+  private _rejecter: (error: Error) => void;
+  private _isCompleted: boolean = false;
+
+  constructor() {
+    this.promise = new Promise((resolve, reject) => {
+      this._resolver = resolve;
+      this._rejecter = reject;
+    });
+  }
+
+  public readonly promise: Promise<T>;
+
+  public get isCompleted(): boolean {
+    return this._isCompleted;
+  }
+
+  public resolve(value: T): void {
+    if (this._isCompleted) {
+      return;
+    }
+    this._isCompleted = true;
+    this._resolver(value);
+  }
+
+  public reject(error: Error): void {
+    if (this._isCompleted) {
+      return;
+    }
+    this._isCompleted = true;
+    this._rejecter(error);
+  }
+}
diff --git a/src/engine/Util/Semaphore.ts b/src/engine/Util/Semaphore.ts
@@ -0,0 +1,59 @@
+import { Future } from './Future';
+
+class AsyncWaitQueue<T> {
+  // Code from StephenCleary https://gist.github.com/StephenCleary/ba50b2da419c03b9cba1d20cb4654d5e
+  private _queue: Future<T>[] = [];
+
+  public get length(): number {
+    return this._queue.length;
+  }
+
+  public enqueue(): Promise<T> {
+    const future = new Future<T>();
+    this._queue.push(future);
+    return future.promise;
+  }
+
+  public dequeue(value: T): void {
+    const future = this._queue.shift();
+    future.resolve(value);
+  }
+}
+
+/**
+ * Semaphore allows you to limit the amount of async calls happening between `enter()` and `exit()`
+ *
+ * This can be useful when limiting the number of http calls, browser api calls, etc either for performance or to work
+ * around browser limitations like max Image.decode() calls in chromium being 256.
+ */
+export class Semaphore {
+  private _waitQueue = new AsyncWaitQueue();
+  constructor(private _count: number) { }
+
+  public get count() {
+    return this._count;
+  }
+
+  public get waiting() {
+    return this._waitQueue.length;
+  }
+
+  public async enter() {
+    if (this._count !== 0) {
+      this._count--;
+      return Promise.resolve();
+    }
+    return this._waitQueue.enqueue();
+  }
+
+  public exit(count: number = 1) {
+    if (count === 0) {
+      return;
+    }
+    while (count !== 0 && this._waitQueue.length !== 0) {
+      this._waitQueue.dequeue(null);
+      count--;
+    }
+    this._count += count;
+  }
+}
diff --git a/src/engine/Util/Util.ts b/src/engine/Util/Util.ts
@@ -1,5 +1,6 @@
 import { Vector } from '../Math/vector';
 import { Clock } from './Clock';
+import { Future } from './Future';
 
 /**
  * Find the screen position of an HTML element
@@ -82,10 +83,10 @@ export function fail(message: never): never {
  * @param clock
  */
 export function delay(milliseconds: number, clock?: Clock): Promise<void> {
+  const future = new Future<void>();
   const schedule = clock?.schedule.bind(clock) ?? setTimeout;
-  return new Promise<void>(resolve => {
-    schedule(() => {
-      resolve();
-    }, milliseconds);
-  });
+  schedule(() => {
+    future.resolve();
+  }, milliseconds);
+  return future.promise;
 }
diff --git a/src/engine/index.ts b/src/engine/index.ts
@@ -72,6 +72,8 @@ export * from './Util/Clock';
 export * from './Util/WebAudio';
 export * from './Util/Toaster';
 export * from './Util/StateMachine';
+export * from './Util/Future';
+export * from './Util/Semaphore';
 
 // ex.Deprecated
 // import * as deprecated from './Deprecated';