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

Add defaultMaterialLoader function to URDFLoader.js #298

Closed
wants to merge 1 commit into from

Conversation

sunjiang0018
Copy link

Currently URDFLoader can only load MeshPhongMaterial, and cannot load other materials (MeshStandardMaterial, etc.). Add defaultMeshLoader method to load more materials

@sunjiang0018 sunjiang0018 changed the title Add defaultMaterialLoader function to URDFLoader Add defaultMaterialLoader function to URDFLoader.js Nov 5, 2024
@gkjohnson
Copy link
Owner

gkjohnson commented Nov 6, 2024

Hello! Thanks for the contribution. Is there a reason you need a replacement function like this rather than postprocessing the model once it has been loaded? Something like so:

const loader = new URDFLoader();
loader.loadAsync( url ).then( robot => {

  robot.traverse( c => {

    if ( c.material ) {

      const mat = c.material;
      c.material = new MeshStandardMaterial( {
        color: mat.color, 
        opacity: mat.opacity, 
        depthWrite: mat.depthWrite, 
        transparent: mat.transparent, 
      } );

    }

  } );

} );

edit: heh - closed right as a commented 😅

@sunjiang0018
Copy link
Author

Hello! Thanks for the contribution. Is there a reason you need a replacement function like this rather than postprocessing the model once it has been loaded? Something like so:

const loader = new URDFLoader();
loader.loadAsync( url ).then( robot => {

  robot.traverse( c => {

    if ( c.material ) {

      const mat = c.material;
      c.material = new MeshStandardMaterial( {
        color: mat.color, 
        opacity: mat.opacity, 
        depthWrite: mat.depthWrite, 
        transparent: mat.transparent, 
      } );

    }

  } );

} );

edit: heh - closed right as a commented 😅

oh, just saw😅. it seems to solve my problem, although it feels a bit inelegant. Thanks anyway

@gkjohnson
Copy link
Owner

it feels a bit inelegant. Thanks anyway

If there are simple solutions that don't require internal modifications to that class I think it's best. It's easy to wrap this into a custom loader wrapper, as well.

If there's more demand for this or some functionality that's not possible then I'm happy to discuss and reconsider. Thanks again for the contribution!

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.

2 participants