Currently, GLTFParser and constants in GLTFLoader are scoped and cannot be used in user-land.
However, sometimes it is needed.
Usecases:
Also, WEBGL_CONSTANTS
and other constants would be useful if available under THREE
or somewhere
What I would like to have is:
THREE.GLTFParser
and maybe:
gltfLoaderInstance.gltfParser
THREE.WEBGL_CONSTANTS.FLOAT = 5126
and so onOne option available now is the loader.parse()
method — it accepts JSON or an ArrayBuffer you've already downloaded and parses it, which is helpful for cases like the JSONP API you mention.
We've also discussed (https://github.com/mrdoob/three.js/pull/14492) adding a getParser(...)
method to GLTFLoader, so you can access a parser instance before content is downloaded. The loader creates a new parser for each asset it loads, to contain caching information etc.
Between the two, I think that would cover everything you suggest except the constants? Exposing the constants could be OK, and has been suggested before (https://github.com/mrdoob/three.js/pull/11978). Another option would be to put these constants on NPM separately, similar to https://github.com/DefinitelyTyped/DefinitelyTyped/pull/29801.
Thanks for your suggestions and comments.
I didn't know that the loader can call parse()
and It could cover most.
(Sorry, I should have read the doc and code more)
What I wanted to do is inject some function in loader.parse()
.
getParser()
and modifying the parser before parse()
could do that as you said. (related to https://github.com/mrdoob/three.js/issues/15418)
But using GLTFParser
directly without GLTFLoader
can also cover that and helpful. I was wondering if you could consider this too.
Anyway looking forward to getParser()
👍
Regarding the constants, I think ideally it should be in one place, rather than in both GLTFLoader.js and NPM, but it still sounds good!
Wouldn't be opposed to move GLTFParser
outside of the closure and add it to the THREE
namespace.
The module version of the file could eventually export GLTFLoader
and GLTFParser
.
Rather than another top-level object, maybe just GLTFLoader.Parser
? Even when using the parser directly users should generally not construct it themselves, but use loader.getParser()
when that is implemented. Constants could be something like GLTFLoader.WebGLConstants.FLOAT
.
... ideally it should be in one place, rather than in both GLTFLoader.js and NPM...
The idea here would be that there should be _official_ glTF constants on NPM (e.g. maintained by Khronos, not just for three.js), so users can install them in the rare case that they need custom parsing. But threejs examples cannot have dependencies, so the constants will have to be duplicated for GLTFLoader.
^Feel free to open a PR for moving the parser and constants out of the closure if this is still helpful. I would wait on other PRs for the rest.
glTF-ish file may need special parsing using modified(extended) GLTFParser.
Just curious to know what "glTF-ish file" means by.
Rather than another top-level object, maybe just GLTFLoader.Parser?
Voting for non top-level object.
I think putting under THREE
may require better maintenance, for example writing document and keeping compatibility as much as possible. It may be too early for the parser yet. I guess we still optimize/refactor/update the parser.
@takahirox GLTFLoader.Parser
could be okay for that?
Just curious to know what "glTF-ish file" means by.
I meant VRM and maybe eVRM (can be seen in ニコニ立体), or othrer Web App oriented models.
Making .getParser()
PR. I think I can share soon. We can start with that.
@yomotsu My understanding is VRM consists of glTF 2.0 core spec with some limitation + VRM custom extension. And parser
in user side is necessary for handling the custom extension, correct? "glTF-ish" sounded a bit vague to me so I just want it to be more specific.
That's right. VRM is a glTF based extended format, and the Parser may need to handle HumanBones, Facial expression blendings, MToon Materials, and others.
In addition, some Web App like ニコニ立体 may use encrypted VRM like "eVRM" in order to protect user upload models, and injecting user-land function to the parser may be needed as well.
To be clear, we can't guarantee that the parser's API will remain the same over time. Internal methods might be renamed or removed, and extending them may be fragile. If you're really customizing the loader for new formats and extensions, it's very possible you'll want to just fork the loader.
But if you're OK with all that, a PR is still fine with me.
Thanks for your suggestions. I understand and getParser
would be safer to use.
Even though, having the official Parser class as GLTFLoader.Parser
would be great for me.
@takahirox Would it be okay?
I wanna suggest that we first go with .getParser()
. And thinking of the option exposing GLTFLoader.Parser
when we realize we really need.
Okay, I will wait for it 👍
Re: https://github.com/mrdoob/three.js/issues/15477#issuecomment-449906411
The glTF 2.0 specs allow for embedded data URIs.
For that scenario, what would be the value for the path parameter in with the loader.parse() method ? Would it be an empty string ("")?
@MarioDelgadoSr yes, an empty string is fine in that case. You can also use LoadingManager's setURLModifier() method to affect this.
@donmccurdy (re: https://github.com/mrdoob/three.js/issues/15477#issuecomment-503241742)
Including link to documentation on LoadingManager's setURLModifier() method for anyone else researching use case: https://threejs.org/docs/index.html#api/en/loaders/managers/LoadingManager.setURLModifier
Do we still need this, with access to the gltf.parser
object and the newer extension API?
It would be better for me.
@FMS-Cat What do you think?
Yes, exposed parser and the plugin system is working well.
The plugin system provides the extensibility of the loader. How do you want to use the exposed parser class? In other words, What are the use cases the plugin system still doesn't cover?
If I understand @FMS-Cat correctly, he's saying that this issue can be closed.
I mean, the constructor of GLTFParser is okay to not be exposed I think. my bad.
OK, thanks for clarifying.
Then, let's close this issue for now. And let's reopen or create a new one if you folks encounter a use case the plugin system doesn't cover.
Most helpful comment
OK, thanks for clarifying.
Then, let's close this issue for now. And let's reopen or create a new one if you folks encounter a use case the plugin system doesn't cover.