Skip to content

Conversation

@clintonman
Copy link

ColladaExporter material bugs:
was using groups to determine material count, in my tests the group count was always zero resulting in no material export
changed to take the largest value between the group and material counts

removed incompatible tags on collada shaders - https://www.khronos.org/files/collada_spec_1_4.pdf
constant shader does not have diffuse, shininess or specular tags
lambert shader does not have shininess or specular tags

added web page to demo the changes

clintonman added 2 commits December 22, 2018 21:55
…d, changed material to get number of materials from the material array instead of the empty geometry.groups array and added an example for the changes
<!DOCTYPE html>
<html lang="en">
<head>
<title>three.js webgl - teapot buffer geometry</title>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<title>three.js webgl - collada exporter</title>

and probably rename file to misc_exporter_collada.html

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the 2 changes above and just noticed the files.js
Should I add this to the "misc" section of the list?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, otherwise the example won't be visible on the overview page: https://threejs.org/examples/

…he file from webgl_geometry_teapot_collada.html to misc_exporter_collada.html
@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 26, 2018

For testing: https://raw.githack.com/clintonman/three.js/colladaexport/examples/misc_exporter_collada.html

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 27, 2018

/cc @gkjohnson

@Mugen87 Mugen87 changed the title Colladaexport ColladaExporter: Optimize material export and added example Dec 27, 2018
@gkjohnson
Copy link
Collaborator

This looks good to me -- my only comment is that it seems like the constant shader type apparently doesn't actually map very well to the MeshBasicMaterial, but it doesn't seem that there's a much better option in the spec. Maybe it's worth adding a console warning like in the GLTFExporter stating that the material textures aren't going to be exported as expected?

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 28, 2018

Maybe it's worth adding a console warning like in the GLTFExporter stating that the material textures aren't going to be exported as expected?

I think this can still be done in a separate PR.

@mrdoob mrdoob added this to the r100 milestone Dec 28, 2018
@mrdoob mrdoob merged commit 7e5f396 into mrdoob:dev Dec 28, 2018
@mrdoob
Copy link
Owner

mrdoob commented Dec 28, 2018

Thanks!

@clintonman clintonman deleted the colladaexport branch December 28, 2018 20:47
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.

5 participants