fix(rune): wait for texture images to finish loading before drawing

src/bundles/rune/src/__tests__/rune.test.ts

@@ -0,0 +1,152 @@
+import { mat4 } from 'gl-matrix';
+import { describe, expect, test, vi } from 'vitest';
+import { Rune, drawRunesToFrameBuffer } from '../rune';
+
+/**
+ * Minimal WebGLRenderingContext stand-in covering just the calls made by
+ * `initShaderProgram` and `drawRunesToFrameBuffer`. `texImage2D` is the call
+ * under test: the 9-arg overload uploads the 1x1 placeholder pixel, the
+ * 6-arg overload uploads the actual image.
+ */
+function createMockGl(events: string[]) {
+  const gl = {
+    FRAMEBUFFER: 'FRAMEBUFFER',
+    VERTEX_SHADER: 'VERTEX_SHADER',
+    FRAGMENT_SHADER: 'FRAGMENT_SHADER',
+    COMPILE_STATUS: 'COMPILE_STATUS',
+    TEXTURE_2D: 'TEXTURE_2D',
+    RGBA: 'RGBA',
+    UNSIGNED_BYTE: 'UNSIGNED_BYTE',
+    TEXTURE_WRAP_S: 'TEXTURE_WRAP_S',
+    TEXTURE_WRAP_T: 'TEXTURE_WRAP_T',
+    CLAMP_TO_EDGE: 'CLAMP_TO_EDGE',
+    TEXTURE_MIN_FILTER: 'TEXTURE_MIN_FILTER',
+    LINEAR: 'LINEAR',
+    TEXTURE0: 'TEXTURE0',
+    ARRAY_BUFFER: 'ARRAY_BUFFER',
+    STATIC_DRAW: 'STATIC_DRAW',
+    FLOAT: 'FLOAT',
+    TRIANGLES: 'TRIANGLES',
+
+    bindFramebuffer: vi.fn(),
+    createShader: vi.fn(() => ({})),
+    shaderSource: vi.fn(),
+    compileShader: vi.fn(),
+    getShaderParameter: vi.fn(() => true),
+    createProgram: vi.fn(() => ({})),
+    attachShader: vi.fn(),
+    linkProgram: vi.fn(),
+    useProgram: vi.fn(),
+    getAttribLocation: vi.fn(() => 0),
+    getUniformLocation: vi.fn(() => ({})),
+    uniform1i: vi.fn(),
+    uniform4fv: vi.fn(),
+    uniformMatrix4fv: vi.fn(),
+    createBuffer: vi.fn(() => ({})),
+    bindBuffer: vi.fn(),
+    bufferData: vi.fn(),
+    vertexAttribPointer: vi.fn(),
+    enableVertexAttribArray: vi.fn(),
+    createTexture: vi.fn(() => ({})),
+    bindTexture: vi.fn(),
+    texImage2D: vi.fn((...args: unknown[]) => {
+      if (args.length === 6) events.push('texImage2D-with-image');
+    }),
+    activeTexture: vi.fn(),
+    generateMipmap: vi.fn(),
+    texParameteri: vi.fn(),
+    drawArrays: vi.fn(() => events.push('drawArrays'))
+  };
+  return gl as unknown as WebGLRenderingContext;
+}
+
+function fakeTriangle(texture: HTMLImageElement) {
+  return Rune.of({
+    vertices: new Float32Array([
+      -1, -1, 0, 1,
+      1, -1, 0, 1,
+      0, 1, 0, 1
+    ]),
+    texture
+  });
+}
+
+describe(drawRunesToFrameBuffer, () => {
+  test('waits for an already in-flight texture image to finish loading before uploading it (issue #891)', async () => {
+    // Mirrors what `deserializeRune` (protocol.ts) hands to the draw path:
+    // an `HTMLImageElement` whose fetch/decode may still be in progress.
+    let loadListener: (() => void) | undefined;
+    const fakeImage = {
+      complete: false,
+      naturalWidth: 0,
+      width: 2,
+      height: 2,
+      src: 'https://example.com/paw.png',
+      addEventListener: vi.fn((event: string, listener: () => void) => {
+        if (event === 'load') loadListener = listener;
+      }),
+      removeEventListener: vi.fn()
+    } as unknown as HTMLImageElement;
+
+    const events: string[] = [];
+    const gl = createMockGl(events);
+    const rune = fakeTriangle(fakeImage);
+
+    const drawPromise = drawRunesToFrameBuffer(
+      gl,
+      [rune],
+      mat4.create(),
+      new Float32Array([1, 1, 1, 1])
+    );
+
+    // The image hasn't loaded yet, so the real texture upload must not have
+    // happened - flushing microtasks can't unblock it, only firing `onload` can.
+    await Promise.resolve();
+    await Promise.resolve();
+    expect(events).not.toContain('texImage2D-with-image');
+
+    (fakeImage as { complete: boolean }).complete = true;
+    (fakeImage as { naturalWidth: number }).naturalWidth = 2;
+    loadListener!();
+
+    await drawPromise;
+
+    expect(events.indexOf('texImage2D-with-image')).toBeGreaterThanOrEqual(0);
+    expect(events.indexOf('texImage2D-with-image')).toBeLessThan(events.indexOf('drawArrays'));
+  });
+
+  test('uploads an already-loaded texture image without waiting', async () => {
+    const fakeImage = {
+      complete: true,
+      naturalWidth: 2,
+      width: 2,
+      height: 2,
+      src: 'https://example.com/paw.png'
+    } as unknown as HTMLImageElement;
+
+    const events: string[] = [];
+    const gl = createMockGl(events);
+    const rune = fakeTriangle(fakeImage);
+
+    await drawRunesToFrameBuffer(gl, [rune], mat4.create(), new Float32Array([1, 1, 1, 1]));
+
+    expect(events).toEqual(['texImage2D-with-image', 'drawArrays']);
+  });
+
+  test('rejects when a texture image already failed to load', async () => {
+    const fakeImage = {
+      complete: true,
+      naturalWidth: 0,
+      width: 0,
+      height: 0,
+      src: 'https://example.com/missing.png'
+    } as unknown as HTMLImageElement;
+
+    const gl = createMockGl([]);
+    const rune = fakeTriangle(fakeImage);
+
+    await expect(
+      drawRunesToFrameBuffer(gl, [rune], mat4.create(), new Float32Array([1, 1, 1, 1]))
+    ).rejects.toThrow('failed to load texture image');
+  });
+});

src/bundles/rune/src/rune.ts

@@ -146,6 +146,46 @@ export class Rune {
   public toReplString = () => '<Rune>';
 }
 
+/**
+ * Resolves once `image` has actually finished loading. `rune.texture` can
+ * arrive as an `HTMLImageElement` whose fetch/decode is still in flight -
+ * `deserializeRune` (protocol.ts) constructs one and hands it over without
+ * waiting, since deserialization itself is synchronous. Using such an image
+ * for `texImage2D` before it's ready silently leaves the 1x1 placeholder
+ * pixel as the visible texture (see
+ * https://github.com/source-academy/modules/issues/891) - hence this wait,
+ * regardless of whether the image was just-created or handed in already
+ * loading.
+ */
+function waitForImageToLoad(image: HTMLImageElement): Promise<HTMLImageElement> {
+  if (image.complete) {
+    return image.naturalWidth === 0
+      ? Promise.reject(new EvaluatorRuntimeError(`Rune: failed to load texture image at ${image.src}`))
+      : Promise.resolve(image);
+  }
+  return new Promise<HTMLImageElement>((resolve, reject) => {
+    // Uses addEventListener/removeEventListener rather than the onload/
+    // onerror/onabort properties so a caller-supplied image (e.g. via
+    // `Rune.of`) keeps whatever handlers it already had.
+    function cleanup() {
+      image.removeEventListener('load', onLoad);
+      image.removeEventListener('error', onError);
+      image.removeEventListener('abort', onError);
+    }
+    function onLoad() {
+      cleanup();
+      resolve(image);
+    }
+    function onError() {
+      cleanup();
+      reject(new EvaluatorRuntimeError(`Rune: failed to load texture image at ${image.src}`));
+    }
+    image.addEventListener('load', onLoad);
+    image.addEventListener('error', onError);
+    image.addEventListener('abort', onError);
+  });
+}
+
 /**
  * Draws the list of runes with the prepared WebGLRenderingContext, with each rune overlapping each other onto a given framebuffer. if the framebuffer is null, draw to the default canvas.
  *
@@ -229,23 +269,11 @@ export async function drawRunesToFrameBuffer(
   const loadTexture = async (rune: Rune): Promise<WebGLTexture | null> => {
     if (rune.texture === null) return null;
     const imageSource = rune.texture;
-    let image: HTMLImageElement;
-    if (typeof imageSource !== 'string') {
-      image = imageSource;
-    } else {
-      image = await new Promise<HTMLImageElement>((resolve, reject) => {
-        const image = Object.assign(new Image(), {
-          crossOrigin: 'anonymous',
-          src: imageSource
-        });
-        image.onload = () => {
-          rune.texture = image;
-          resolve(image);
-        };
-        image.onabort = reject;
-        image.onerror = reject;
-      });
-    }
+    const image = typeof imageSource === 'string'
+      ? Object.assign(new Image(), { crossOrigin: 'anonymous', src: imageSource })
+      : imageSource;
+    await waitForImageToLoad(image);
+    rune.texture = image;
 
     const texture = gl.createTexture();
     gl.bindTexture(gl.TEXTURE_2D, texture);