From 1bddf8511845fb330854bb3aaae614b7e619767a Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Wed, 17 Dec 2025 12:03:11 +0100 Subject: [PATCH 1/7] Return null from loadTexture when image fails to load intead of wrongly creating a VideoTexture --- src/systems/material.js | 18 ++++++++++++------ src/utils/src-loader.js | 30 ++++++++++++++++++------------ tests/systems/material.test.js | 21 +++++++++++++++++++++ 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/systems/material.js b/src/systems/material.js index dcb22b616c4..93f23ad6ae5 100755 --- a/src/systems/material.js +++ b/src/systems/material.js @@ -30,10 +30,14 @@ export var System = registerSystem('material', { * * @param {string|Element} src - URL or element * @param {object} data - Relevant texture properties - * @param {function} cb - Callback to pass texture to + * @param {function} cb - Callback that receives the texture, or null if loading failed */ loadTexture: function (src, data, cb) { this.loadTextureSource(src, function sourceLoaded (source) { + if (source === null) { + cb(null); + return; + } var texture = createCompatibleTexture(source); setTextureProperties(texture, data); cb(texture); @@ -44,7 +48,7 @@ export var System = registerSystem('material', { * Determine whether `src` is an image or video. Then try to load the asset, then call back. * * @param {string|Element} src - URL or element. - * @param {function} cb - Callback to pass texture source to. + * @param {function} cb - Callback that receives the texture source, or null if loading failed. */ loadTextureSource: function (src, cb) { var self = this; @@ -52,7 +56,7 @@ export var System = registerSystem('material', { var hash = this.hash(src); if (sourceCache[hash]) { - sourceCache[hash].then(cb); + sourceCache[hash].then(cb, function () { cb(null); }); return; } @@ -64,14 +68,15 @@ export var System = registerSystem('material', { sourceLoaded(new Promise(doSourceLoad)); function doSourceLoad (resolve, reject) { - utils.srcLoader.validateSrc(src, loadImageCb, loadVideoCb); + utils.srcLoader.validateSrc(src, loadImageCb, loadVideoCb, notFoundCb); function loadImageCb (src) { self.loadImage(src, resolve); } function loadVideoCb (src) { self.loadVideo(src, resolve); } + function notFoundCb () { reject(new Error('Resource not found: ' + src)); } } function sourceLoaded (sourcePromise) { sourceCache[hash] = Promise.resolve(sourcePromise); - sourceCache[hash].then(cb); + sourceCache[hash].then(cb, function () { cb(null); }); } }, @@ -199,8 +204,9 @@ function loadImageUrl (src) { resolveSource, function () { /* no-op */ }, function (xhr) { - error('`$s` could not be fetched (Error code: %s; Response: %s)', xhr.status, + error('`%s` could not be fetched (Error code: %s; Response: %s)', src, xhr.status, xhr.statusText); + reject(new Error('Failed to load image: ' + src)); } ); diff --git a/src/utils/src-loader.js b/src/utils/src-loader.js index ba1304d4731..2de40cb5584 100644 --- a/src/utils/src-loader.js +++ b/src/utils/src-loader.js @@ -13,14 +13,17 @@ var warn = debug('utils:src-loader:warn'); * @param {string|Element} src - URL or media element. * @param {function} isImageCb - callback if texture is an image. * @param {function} isVideoCb - callback if texture is a video. + * @param {function} isNotFoundCb - optional callback if texture is not found. */ -export function validateSrc (src, isImageCb, isVideoCb) { - checkIsImage(src, function isAnImageUrl (isImage) { - if (isImage) { +export function validateSrc (src, isImageCb, isVideoCb, isNotFoundCb) { + checkIsImage(src, function isAnImageUrl (result) { + if (result === true) { isImageCb(src); - return; + } else if (result === false) { + isVideoCb(src); + } else if (isNotFoundCb) { + isNotFoundCb(src); } - isVideoCb(src); }); } @@ -120,7 +123,7 @@ export function parseUrl (src) { * Call back whether `src` is an image. * * @param {string|Element} src - URL or element that will be tested. - * @param {function} onResult - Callback with whether `src` is an image. + * @param {function} onResult - Callback with true (image), false (video), or null (not found). */ function checkIsImage (src, onResult) { var request; @@ -139,18 +142,21 @@ function checkIsImage (src, onResult) { contentType = request.getResponseHeader('Content-Type'); if (contentType == null) { checkIsImageFallback(src, onResult); + } else if (contentType.startsWith('image')) { + onResult(true); } else { - if (contentType.startsWith('image')) { - onResult(true); - } else { - onResult(false); - } + onResult(false); } } else { - checkIsImageFallback(src, onResult); + // Non-success status (404, etc.) - resource not found. + onResult(null); } request.abort(); }); + request.addEventListener('error', function () { + // Network error - resource not found. + onResult(null); + }); request.send(); } diff --git a/tests/systems/material.test.js b/tests/systems/material.test.js index 2617d22183e..69015fde291 100644 --- a/tests/systems/material.test.js +++ b/tests/systems/material.test.js @@ -3,6 +3,7 @@ import { entityFactory } from '../helpers.js'; var IMAGE1 = 'base/tests/assets/test.png'; var IMAGE2 = 'base/tests/assets/test2.png'; +var IMAGE_FAIL = 'base/tests/assets/nonexistent.png'; var VIDEO1 = 'base/tests/assets/test.mp4'; var VIDEO2 = 'base/tests/assets/test2.mp4'; @@ -122,6 +123,26 @@ suite('material system', function () { done(); }); }); + + test('returns null when image fails to load', function (done) { + var system = this.system; + + system.loadTextureSource(IMAGE_FAIL, function (source) { + assert.equal(source, null); + done(); + }); + }); + }); + + suite('loadTexture', function () { + test('returns null when image fails to load', function (done) { + var system = this.system; + + system.loadTexture(IMAGE_FAIL, {}, function (texture) { + assert.equal(texture, null); + done(); + }); + }); }); suite('loadVideo', function () { From b7867d02c75a6bb341fa4d2e6f2eef63308248ed Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Wed, 17 Dec 2025 14:21:37 +0100 Subject: [PATCH 2/7] Also add tests for video not found --- tests/systems/material.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/systems/material.test.js b/tests/systems/material.test.js index 69015fde291..9e6aed27293 100644 --- a/tests/systems/material.test.js +++ b/tests/systems/material.test.js @@ -6,6 +6,7 @@ var IMAGE2 = 'base/tests/assets/test2.png'; var IMAGE_FAIL = 'base/tests/assets/nonexistent.png'; var VIDEO1 = 'base/tests/assets/test.mp4'; var VIDEO2 = 'base/tests/assets/test2.mp4'; +var VIDEO_FAIL = 'base/tests/assets/nonexistent.mp4'; suite('material system', function () { setup(function (done) { @@ -143,6 +144,15 @@ suite('material system', function () { done(); }); }); + + test('returns null when video fails to load', function (done) { + var system = this.system; + + system.loadTexture(VIDEO_FAIL, {}, function (texture) { + assert.equal(texture, null); + done(); + }); + }); }); suite('loadVideo', function () { @@ -217,6 +227,15 @@ suite('material system', function () { done(); }); }); + + test('returns null when video fails to load', function (done) { + var system = this.system; + + system.loadTextureSource(VIDEO_FAIL, function (source) { + assert.equal(source, null); + done(); + }); + }); }); }); }); From b5683544c1172d624f79133c06ba1b40d68d0ae2 Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Wed, 17 Dec 2025 16:41:00 +0100 Subject: [PATCH 3/7] Still use checkIsImageFallback in case of 3xx redirects or server not supporting HEAD requests --- src/utils/src-loader.js | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/utils/src-loader.js b/src/utils/src-loader.js index 2de40cb5584..f6d30d19f82 100644 --- a/src/utils/src-loader.js +++ b/src/utils/src-loader.js @@ -141,31 +141,43 @@ function checkIsImage (src, onResult) { if (request.status >= 200 && request.status < 300) { contentType = request.getResponseHeader('Content-Type'); if (contentType == null) { - checkIsImageFallback(src, onResult); + // No content-type, try Image. If fails, assume video. + checkIsImageFallback(src, onResult, false); } else if (contentType.startsWith('image')) { onResult(true); } else { onResult(false); } } else { - // Non-success status (404, etc.) - resource not found. - onResult(null); + // Non-success status (3xx redirects, 404, 405, etc.) - try loading via Image tag + // as it handles redirects and the server might not support HEAD requests. + // If Image also fails, resource is not found. + checkIsImageFallback(src, onResult, null); } request.abort(); }); request.addEventListener('error', function () { - // Network error - resource not found. - onResult(null); + // Network error (CORS, etc.) - try loading via Image tag. + // If Image also fails, resource is not found. + checkIsImageFallback(src, onResult, null); }); request.send(); } -function checkIsImageFallback (src, onResult) { +/** + * Try loading src as an image to determine if it's an image. + * + * @param {string} src - URL to test. + * @param {function} onResult - Callback with result. + * @param {boolean|null} onErrorResult - Value to pass to onResult if image fails to load. + * false = assume video, null = resource not found. + */ +function checkIsImageFallback (src, onResult, onErrorResult) { var tester = new Image(); tester.addEventListener('load', onLoad); function onLoad () { onResult(true); } tester.addEventListener('error', onError); - function onError () { onResult(false); } + function onError () { onResult(onErrorResult); } tester.src = src; } From d508abab71630c6527a834e6359d6bbdab666e61 Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Wed, 17 Dec 2025 17:26:56 +0100 Subject: [PATCH 4/7] We can assume server not supporting HEAD request that it's not a video, revert the previous logic --- src/systems/material.js | 3 +-- src/utils/src-loader.js | 48 +++++++++++++++++++++++++---------------- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/systems/material.js b/src/systems/material.js index 93f23ad6ae5..c696b60d796 100755 --- a/src/systems/material.js +++ b/src/systems/material.js @@ -68,10 +68,9 @@ export var System = registerSystem('material', { sourceLoaded(new Promise(doSourceLoad)); function doSourceLoad (resolve, reject) { - utils.srcLoader.validateSrc(src, loadImageCb, loadVideoCb, notFoundCb); + utils.srcLoader.validateSrc(src, loadImageCb, loadVideoCb); function loadImageCb (src) { self.loadImage(src, resolve); } function loadVideoCb (src) { self.loadVideo(src, resolve); } - function notFoundCb () { reject(new Error('Resource not found: ' + src)); } } function sourceLoaded (sourcePromise) { diff --git a/src/utils/src-loader.js b/src/utils/src-loader.js index f6d30d19f82..609d413f5f3 100644 --- a/src/utils/src-loader.js +++ b/src/utils/src-loader.js @@ -13,17 +13,14 @@ var warn = debug('utils:src-loader:warn'); * @param {string|Element} src - URL or media element. * @param {function} isImageCb - callback if texture is an image. * @param {function} isVideoCb - callback if texture is a video. - * @param {function} isNotFoundCb - optional callback if texture is not found. */ -export function validateSrc (src, isImageCb, isVideoCb, isNotFoundCb) { - checkIsImage(src, function isAnImageUrl (result) { - if (result === true) { +export function validateSrc (src, isImageCb, isVideoCb) { + checkIsImage(src, function isAnImageUrl (isImage) { + if (isImage) { isImageCb(src); - } else if (result === false) { - isVideoCb(src); - } else if (isNotFoundCb) { - isNotFoundCb(src); + return; } + isVideoCb(src); }); } @@ -119,19 +116,39 @@ export function parseUrl (src) { return parsedSrc[1]; } +var IMAGE_EXTENSIONS = ['jpg', 'jpeg', 'png', 'webp', 'avif']; + +/** + * Get file extension from URL (without query string or hash). + */ +function getExtension (src) { + var pathname = src.split('?')[0].split('#')[0]; + var ext = pathname.split('.').pop().toLowerCase(); + return ext; +} + /** * Call back whether `src` is an image. * * @param {string|Element} src - URL or element that will be tested. - * @param {function} onResult - Callback with true (image), false (video), or null (not found). + * @param {function} onResult - Callback with whether `src` is an image. */ function checkIsImage (src, onResult) { var request; + var ext; if (src.tagName) { onResult(src.tagName === 'IMG'); return; } + + // Check file extension first to avoid HEAD request for images. + ext = getExtension(src); + if (IMAGE_EXTENSIONS.indexOf(ext) !== -1) { + onResult(true); + return; + } + request = new XMLHttpRequest(); // Try to send HEAD request to check if image first. @@ -141,8 +158,7 @@ function checkIsImage (src, onResult) { if (request.status >= 200 && request.status < 300) { contentType = request.getResponseHeader('Content-Type'); if (contentType == null) { - // No content-type, try Image. If fails, assume video. - checkIsImageFallback(src, onResult, false); + checkIsImageFallback(src, onResult); } else if (contentType.startsWith('image')) { onResult(true); } else { @@ -151,14 +167,12 @@ function checkIsImage (src, onResult) { } else { // Non-success status (3xx redirects, 404, 405, etc.) - try loading via Image tag // as it handles redirects and the server might not support HEAD requests. - // If Image also fails, resource is not found. - checkIsImageFallback(src, onResult, null); + checkIsImageFallback(src, onResult); } request.abort(); }); request.addEventListener('error', function () { // Network error (CORS, etc.) - try loading via Image tag. - // If Image also fails, resource is not found. checkIsImageFallback(src, onResult, null); }); request.send(); @@ -169,15 +183,13 @@ function checkIsImage (src, onResult) { * * @param {string} src - URL to test. * @param {function} onResult - Callback with result. - * @param {boolean|null} onErrorResult - Value to pass to onResult if image fails to load. - * false = assume video, null = resource not found. */ -function checkIsImageFallback (src, onResult, onErrorResult) { +function checkIsImageFallback (src, onResult) { var tester = new Image(); tester.addEventListener('load', onLoad); function onLoad () { onResult(true); } tester.addEventListener('error', onError); - function onError () { onResult(onErrorResult); } + function onError () { onResult(false); } tester.src = src; } From 4c2ea27576436a53be48445deb482e4425d9dec6 Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Wed, 17 Dec 2025 17:27:05 +0100 Subject: [PATCH 5/7] Revert "Also add tests for video not found" This reverts commit b7867d02c75a6bb341fa4d2e6f2eef63308248ed. --- tests/systems/material.test.js | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/tests/systems/material.test.js b/tests/systems/material.test.js index 9e6aed27293..69015fde291 100644 --- a/tests/systems/material.test.js +++ b/tests/systems/material.test.js @@ -6,7 +6,6 @@ var IMAGE2 = 'base/tests/assets/test2.png'; var IMAGE_FAIL = 'base/tests/assets/nonexistent.png'; var VIDEO1 = 'base/tests/assets/test.mp4'; var VIDEO2 = 'base/tests/assets/test2.mp4'; -var VIDEO_FAIL = 'base/tests/assets/nonexistent.mp4'; suite('material system', function () { setup(function (done) { @@ -144,15 +143,6 @@ suite('material system', function () { done(); }); }); - - test('returns null when video fails to load', function (done) { - var system = this.system; - - system.loadTexture(VIDEO_FAIL, {}, function (texture) { - assert.equal(texture, null); - done(); - }); - }); }); suite('loadVideo', function () { @@ -227,15 +217,6 @@ suite('material system', function () { done(); }); }); - - test('returns null when video fails to load', function (done) { - var system = this.system; - - system.loadTextureSource(VIDEO_FAIL, function (source) { - assert.equal(source, null); - done(); - }); - }); }); }); }); From 46164adca419b42b87f579712c9f1668e422ad4d Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Wed, 17 Dec 2025 17:40:38 +0100 Subject: [PATCH 6/7] Remove third param --- src/utils/src-loader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/src-loader.js b/src/utils/src-loader.js index 609d413f5f3..9ee297cb878 100644 --- a/src/utils/src-loader.js +++ b/src/utils/src-loader.js @@ -173,7 +173,7 @@ function checkIsImage (src, onResult) { }); request.addEventListener('error', function () { // Network error (CORS, etc.) - try loading via Image tag. - checkIsImageFallback(src, onResult, null); + checkIsImageFallback(src, onResult); }); request.send(); } From ea4ff84d2154f42b5a05833ab2591b25abbc2542 Mon Sep 17 00:00:00 2001 From: Vincent Fretin Date: Wed, 17 Dec 2025 17:43:25 +0100 Subject: [PATCH 7/7] Update comments --- src/systems/material.js | 4 ++-- src/utils/src-loader.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/systems/material.js b/src/systems/material.js index c696b60d796..3adbb9ced35 100755 --- a/src/systems/material.js +++ b/src/systems/material.js @@ -30,7 +30,7 @@ export var System = registerSystem('material', { * * @param {string|Element} src - URL or element * @param {object} data - Relevant texture properties - * @param {function} cb - Callback that receives the texture, or null if loading failed + * @param {function} cb - Callback that receives the texture, or null if image loading failed */ loadTexture: function (src, data, cb) { this.loadTextureSource(src, function sourceLoaded (source) { @@ -48,7 +48,7 @@ export var System = registerSystem('material', { * Determine whether `src` is an image or video. Then try to load the asset, then call back. * * @param {string|Element} src - URL or element. - * @param {function} cb - Callback that receives the texture source, or null if loading failed. + * @param {function} cb - Callback that receives the texture source, or null if image loading failed. */ loadTextureSource: function (src, cb) { var self = this; diff --git a/src/utils/src-loader.js b/src/utils/src-loader.js index 9ee297cb878..99da51906e5 100644 --- a/src/utils/src-loader.js +++ b/src/utils/src-loader.js @@ -142,7 +142,7 @@ function checkIsImage (src, onResult) { return; } - // Check file extension first to avoid HEAD request for images. + // Check file extension first to avoid HEAD request for common image extensions. ext = getExtension(src); if (IMAGE_EXTENSIONS.indexOf(ext) !== -1) { onResult(true);