diff --git a/src/bundles/rune/src/__tests__/rune.test.ts b/src/bundles/rune/src/__tests__/rune.test.ts new file mode 100644 index 0000000000..9c468def8a --- /dev/null +++ b/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'); + }); +}); diff --git a/src/bundles/rune/src/rune.ts b/src/bundles/rune/src/rune.ts index 42cac5ccb7..e0d642565a 100644 --- a/src/bundles/rune/src/rune.ts +++ b/src/bundles/rune/src/rune.ts @@ -146,6 +146,46 @@ export class Rune { public toReplString = () => ''; } +/** + * 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 { + 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((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 => { if (rune.texture === null) return null; const imageSource = rune.texture; - let image: HTMLImageElement; - if (typeof imageSource !== 'string') { - image = imageSource; - } else { - image = await new Promise((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);