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

Material.fromMaterial #22179

Open
mjurczyk opened this issue Jul 23, 2021 · 6 comments
Open

Material.fromMaterial #22179

mjurczyk opened this issue Jul 23, 2021 · 6 comments

Comments

@mjurczyk
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Having mobile / XR development in mind (ie. cases when hardware tends to not be the most powerful), it is sometimes useful to convert MeshStandardMaterial to something comparable, but a bit lighter (ex. MeshLambertMaterial)

Doing it this way:

const cheaperMaterial = new MeshPhongMaterial({ ...originalStandardMaterial });

may likely lead to 100k logs per second, since some properties of complex materials are not there in phong/lambert.

Describe the solution you'd like

On the Material prototype, we may consider adding a Material.fromMaterial(m) helper (similar to Array.from) - that converts one material to another, taking into consideration props / optimisation / unnecessary texture disposal (ex. roughness maps when going from PBR materials.)

Having it on the prototype as a static method would allow to easily create material alternatives:

const cheaperMaterial = MeshPhongMaterial.fromMaterial(originalPBRMaterial);

Describe alternatives you've considered

Usually you'd just do it by hand (example) and hope the materials in three don't change over time and you didn't forget to dispose / remove something that should have been disposed / removed. Since materials and their props are strictly bound to three, it may be nice to have an util built-in three to convert between material types.

@donmccurdy
Copy link
Collaborator

I'm doing this in another project. Doesn't catch all the overlap between MeshStandardMaterial and others, but does a lot, and works similarly for switching between point and line materials as well.

Material.prototype.copy.call(outputMaterial, inputMaterial);
outputMaterial.color.copy(inputMaterial.color);
outputMaterial.map = inputMaterial.map;

An alternative to a new method might be to improve .copy(...) so that it avoids overwriting properties of this that are missing from source. Could make copying a little slower, on the other hand. Not sure if there's another way to approach this without adding another place in the code that needs to contain an exhaustive list of a material's properties.

@gkjohnson
Copy link
Collaborator

For some more context updating "copy" was proposed in #14401 and #17565 but didn't gain traction. I agree it would be a nice feature, though, because I'm often having to convert standard materials to phong, as well.

@mjurczyk
Copy link
Contributor Author

❤️ Since there's no significant backlash, I'll try to compose a PR next week for it (also taking into account the linked issues @gkjohnson 🙌 )

@LeviPesin
Copy link
Contributor

Related: #25004

@donmccurdy
Copy link
Collaborator

donmccurdy commented Dec 31, 2022

Kind of an aside, but maybe related — I wish that we had a fromJSON method generally on classes, per #11266. Without it ObjectLoader needs to know about all possible object types to deserialize them, but with fromJSON we could potentially register objects with ObjectLoader at runtime. This might offer an alternative to fromMaterial ...

a.fromJSON(b.toJSON());

... because I'd tend to assume most/all material properties are optional in JSON (can't we skip "opacity": 1 if that's the default?) whereas they're often more strictly non-nullable in the class itself.

@looeee
Copy link
Collaborator

looeee commented Jan 3, 2023

There is prior art on getting somewhat visually equivalent values for spec/gloss from a metal/roughness material, by the way (and vice versa).

I did some research on this for the FBXLoader. I didn't apply it in the end so I'm not sure what the best choice of formula is, but if you research this you should find plenty of info.

We should also ensure that of the available formulas for doing this we only use one throughout the repo. Is the conversion already done in any of the loaders?

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

6 participants