mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
102.74k stars 35.38k forks source link

Cache clobbering when loading same file through `FileLoader`, `ImageBitmapLoader`, or `ImageLoader` #27301

Open sxxov opened 11 months ago

sxxov commented 11 months ago

Description

Currently, all these 3 classes use the THREE.Cache data structure to store their caches. However, whilst all of them return different values, they all key by URL & it means loading the same URL the second time through a different loader will produce an incorrect result.

Reproduction steps

  1. Load a file using FileLoader
  2. Load a file using ImageBitmapLoader
  3. The returned value from ImageBitmapLoader will be the value from FileLoader, instead of an ImageBitmap

Code

import * as THREE from 'three';

THREE.Cache.enabled = true;

const fileLoader = new THREE.FileLoader();
file = await (new Promise((resolve) => {
    fileLoader.load(url, (v) => {
        resolve(v);
    });
}));

const imageBitmapLoader = new THREE.ImageBitmapLoader();
imageBitmap = await (new Promise((resolve) => {
    imageBitmapLoader.load(url, (v) => {
        resolve(v);
    });
}));

if (file === imageBitmap) throw new Error('Incorrect cached data!');

Live example

When on the page, press Enable cache to see the incorrect result.

Sorry, I'm not familiar with JSFiddle!

Screenshots

No response

Version

r159

Device

No response

Browser

No response

OS

No response

Mugen87 commented 11 months ago

What is your use case for loading the same file with different loaders?

sxxov commented 11 months ago

No use-case! Just an edge-case that seemed like it could bite someone in the butt.

For example if I manually loaded a texture using ImageLoader to be displayed on the DOM, & then created a GLTF that pointed to that same texture, things would go kabooom.

Mugen87 commented 11 months ago

Okay, it's good to document this edge case but I'm also relieved that currently nothing breaks in your projects.

It seems doable to solve this issue for ImageBitmapLoader and FileLoader but I'm not sure this is true for ImageLoader. We can't use FileLoader (and fetch()) in ImageLoader which is documented in #10439.

sxxov commented 11 months ago

Thanks for the concern!!

We don't need to use FileLoader for ImageLoader though, perhaps just key the Cache entry differently, i.e. image:${url} & image-bitmap:${url}.

After this keyed cache change, using FileLoader would then be able to help consolidate the caching logic between ImageBitmapLoader & FileLoader, because you'd be able to use FileLoader for the initial blob, & then cache a new entry for the ImageBitmap.