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

Draco #10879

Merged
merged 6 commits into from Mar 13, 2017
Merged

Draco #10879

merged 6 commits into from Mar 13, 2017

Conversation

edsilv
Copy link
Contributor

@edsilv edsilv commented Feb 24, 2017

Added DRACOLoader.js and draco_decoder.js

this.materials = null;
};

const DracoModule = Module;
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm... how does this work? draco_decoder.js adds a variable called Module to the global scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes I spotted that. Not ideal... I copied this from Google's DRACO threejs renderer example: https://storage.googleapis.com/demos.webmproject.org/draco/draco_loader_throw.html
It looks like Module is an artifact of their emscripten process for draco_decoder.js

Copy link
Owner

Choose a reason for hiding this comment

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

Hmm... I could just merge this, but I know people are going to run into problems with this so I would prefer if we can fix it before merging...

Ideally the original authors would clean that up. So, maybe you can point this out at https://github.com/google/draco?

Or... they could also "move" the loader to this repo and maintain the loader here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this library is built with Emscripten, which is where the global Module variable is coming from. Emscripten supports a couple arguments at compile time which lets you provide proper namespaced modules for compiled code.

The arguments you want would be something like -s MODULARIZE=1 -s EXPORT_NAME="'DRACO'" which will create the module definition in window.DRACO instead of window.Module.

@jbrettle
Copy link

jbrettle commented Mar 7, 2017

We updated Draco with the requests from this ticket: google/draco#68

How should we proceed from here?

@mrdoob
Copy link
Owner

mrdoob commented Mar 7, 2017

@edsilv do you mind updating this PR with the updated draco?

@edsilv
Copy link
Contributor Author

edsilv commented Mar 8, 2017

@mrdoob That's done.

@mrdoob
Copy link
Owner

mrdoob commented Mar 8, 2017

Sweet! Do you mind adding an example too?

@edsilv
Copy link
Contributor Author

edsilv commented Mar 8, 2017

I have an example on the dev branch here: https://github.com/edsilv/virtex I could look at adding this to the three.js examples directory. Currently it loses the texture when compressing with DRACO. I also had to remove references to fileDisplayArea in DRACOLoader.js @jbrettle

@ondys
Copy link
Contributor

ondys commented Mar 9, 2017

@edsilv Draco does not store any material info, but textures or material can be still loaded separately, e.g., you can still load the obj .mtl file and use it with a mesh loaded with Draco. When .obj is encoded to .drc, Draco actually preserves material ids in a generic attribute that could be used to map multiple materials to an object.

We have plans to add support for metadata to our encoded file that would allow storing of arbitrary data inside the .drc file and this feature could be potentially used to store material info as well.

Personally I think it is ok to go ahead with an example without textures as it should be pretty straightforward to add material or texture loading separately from Draco decoding.

@yomotsu
Copy link
Contributor

yomotsu commented Mar 10, 2017

@edsilv
As another option, You can attach materials manually. Draco file still has UV info.
Here is a draco model with texture example
https://yomotsu.github.io/ZipLoader/examples/rameses.html

dracoDecoder.POSITION);
if (posAttId == -1) {
const errorMsg = "No position attribute found in the mesh.";
//fileDisplayArea.innerText = errorMsg;
Copy link
Owner

@mrdoob mrdoob Mar 10, 2017

Choose a reason for hiding this comment

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

Do you mind pulling again from draco?
They've removed the html element dependencies: google/draco#68

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mrdoob done.

@mrdoob mrdoob merged commit b9b1a53 into mrdoob:dev Mar 13, 2017
@mrdoob
Copy link
Owner

mrdoob commented Mar 13, 2017

Thanks!

@ghost
Copy link

ghost commented Apr 26, 2017

Since v85 now ships with the dracoloader, I've tried implementing it. My .obj model has subgroups that got traversed on load (with the old objloader), but when pushed through the .drc and the dracoloader this information seems to be lost and it can't be traversed. I've tried searching for a solution, but there's not much draco specific discussion out there yet, that is why i came here to ask for help. Can groups be stored in the .drc format or does the format not support that? My understanding was that the draco format merely compresses an underlying .obj, instead of altering it.

@ondys
Copy link
Contributor

ondys commented Apr 26, 2017

@stefan-kern Draco is actually converting the input .obj into an intermediate representation that is compressed, so some information can get lost. Subgroups (defined by the g tag) are actually currently not parsed by Draco. Only sub-objects and materials are stored as separate generic attributes, but they are currently not processed in the DRACOLoader.js (see for example this issue for processing sub-objects encoded with Draco).

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.

None yet

6 participants