Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GLTFLoader: Fix use of setPath with external resources #27106

Merged
merged 6 commits into from
Nov 4, 2023

Conversation

Archimagus
Copy link
Contributor

Description

GLTF Files can load external assets that are referenced with a relative path.
Currently this breaks if you use setPath on the GLTFLoader

If a base path is set, resources will be relative paths from that
Example  path = 'https://my-cnd-server.com/', url = 'assets/models/model.gltf'
resourcePath = 'https://my-cnd-server.com/
referenced resource 'model.bin' will attempt to be  loaded from 'https://my-cnd-server.com/model.bin'
and fail.

With this change

If a base path is set, resources will be relative paths from that plus the relative path of the gltf file
Example  path = 'https://my-cnd-server.com/', url = 'assets/models/model.gltf'
resourcePath = 'https://my-cnd-server.com/assets/models/'
referenced resource 'model.bin' will be loaded from 'https://my-cnd-server.com/assets/models/model.bin'
referenced resource '../textures/texture.png' will be loaded from 'https://my-cnd-server.com/assets/textures/texture.png'

sampleRenderTarget is being deleted and set to null by super.dispose();
the check for undefined was incorrect causing an attempt to call dispose on a null object.
@donmccurdy donmccurdy changed the title Fix for setting path not affecting GLTF Sub Assets correctly. GLTFLoader: Fix use of setPath with external resources Nov 3, 2023
Fix code style.
@Mugen87 Mugen87 added this to the r159 milestone Nov 3, 2023
@Mugen87 Mugen87 merged commit 15d3a1d into mrdoob:dev Nov 4, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants