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

MaterialLoader support custom materials #21265

Open
nbilyk opened this issue Feb 12, 2021 · 4 comments
Open

MaterialLoader support custom materials #21265

nbilyk opened this issue Feb 12, 2021 · 4 comments

Comments

@nbilyk
Copy link

nbilyk commented Feb 12, 2021

The MaterialLoader as it is now isn't very extensible. I would like to be able to more easily create loaders for custom materials without duplicating the material loader.

The main problem is that while there's inversion of control for the textures, there isn't for Materials.

const material = new Materials[ json.type ]();

This is a factory pattern that isn't provided to the loader, nor is there a way to add a new material constructor / material type.

@mrdoob
Copy link
Owner

mrdoob commented Feb 12, 2021

The main problem is that while there's inversion of control for the textures, there isn't for Materials.

What is inversion of control?

And how would you extend MaterialLoader?

@nbilyk
Copy link
Author

nbilyk commented Feb 13, 2021

What is inversion of control?
IoC is a general term for a design pattern that decouples static logic and gives that control to the user or the API. A simple example is providing a callback. Dependency injection is a form of IoC. In the case of the material loader the textures are provided to the loader, which allows the caller to inject/set these textures.
I'd recommend the same principle for the material constructors.

My exact use case is that I have a custom material I'm loading from json, and I don't want to duplicate all of the MaterialLoader logic.

Easiest way:
If the Materials factory was provided instead of static, that would be one way to achieve what I want because I could set the constructor 'MyCustomMaterial': MyCustomMaterial. and then set the custom properties that weren't set from MaterialLoader by my own loader.

Ideal way:

MaterialLoader is a classic example of polymorphic serialization. (Same with Material#toJSON) The way it would look is you'd have a map of type enumerations to their serializers. Those serializers each are responsible only for their specific properties, and then call the serializer for material base.
If someone had their own materials, they'd add their concrete serializers to the material loader's map.

This same polymorphism applies to toJSON as well -- each material has a toJSON, serializes just what's specific to that material, then calls the base Material's toJSON.

@Mugen87
Copy link
Collaborator

Mugen87 commented Feb 13, 2021

Would an approach like in Yuka solve your issue? The idea is that users can register custom types for deserialization in certain classes via registerType(). This definition is then used in the default branch in fromJSON() (which means when an object type is not built-in, the logic checks user defined types).

and then set the custom properties that weren't set from MaterialLoader by my own loader.

Sidenote: If #11266 would be implement, it would not be necessary to have a custom loader. You could actually extend the fromJSON() method in your custom materials.

@nbilyk
Copy link
Author

nbilyk commented Feb 13, 2021

@Mugen87 Yes, this is exactly what I was picturing. The Yuka link is a good example of polymorphic unmarshalling.

I think this would also have the benefit of separating loading from the unmarshalling.

There are three separate steps here, and IHMO I believe they should be decoupled.

  • Loading - FileLoader is good, its single responsibility is handling a request and tying it to the loading manager, and spitting out a response. I haven't looked at it deeply, but I'm guessing this uses XMLHttpRequest under the hood? So it can result in blob, string, etc?
  • Deserialization - Turning the output of the loader into a plain JS object. Right now it looks like a lot of the loaders are hard-coded to JSON, but if this step was uncoupled it would be easy to have a JSON deserializer (just JSON.parse), and a binary deserializer (CBOR format or something), or whatever-format-your-data-is-stored-in
  • Unmarshalling - taking the plain JS Object and forming concrete instances. As discussed, this could be #fromJSON (I'd think about the name here, it's not a json string, and the object may not have parsed from json at all). In the case where the object type is sub-typed, polymorphic unmarshalling can be done via a registerType method like in Yuka.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants