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 defines won't be set in constructor options #24771

Closed
alexpineda opened this issue Oct 10, 2022 · 3 comments
Closed

Material defines won't be set in constructor options #24771

alexpineda opened this issue Oct 10, 2022 · 3 comments

Comments

@alexpineda
Copy link

I ran into this issue while attempting to add the built in vUv varyings to a customized MeshBasicMaterial.

new MeshBasicMaterial({
                defines: {
                    USE_UV: "",
                },
                onBeforeCompile: (shader: Shader) => {
                    ...
                    );
                },
            })

In this case it will not work since the defines is not defined in the material. This would work in something like MeshStandardMaterial (albeit it would override STANDARD unless merged or also included).

Expected behavior

It should always set defines (eg. defaulted in Material), or maybe even merge defines on constructor.

Platform:

  • Three.js version: 145
@WestLangley
Copy link
Collaborator

Does the defines pattern in #10764 (comment) solve your issue?

@alexpineda
Copy link
Author

Yes of course. Just making note as the api seems inconsistent (depending on which material is used). I'm happy to move on.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 10, 2022

Defines are not something that the user is supposed to set when creating built-in materials (I'm not referring to custom materials via ShaderMaterial). I understand that defines might be relevant when using onBeforeCompile() but apart from that there should be no need to work with them.

Hence, the approach outlined in #24771 (comment) is more appropriate.

@Mugen87 Mugen87 closed this as completed Oct 10, 2022
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

No branches or pull requests

3 participants