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

Change Draco modules to be created asynchronously #561

Closed

Conversation

FrankGalligan
Copy link
Contributor

In general Draco npm modules have always been created asynchronously. But in gltf-pipeline they worked usually with getting created synchronously.

We are planning to release Draco 1.4.0, which uses a new version of emscripten and causes gltf-pipline to fail as the timing has changed.

The change in this PR makes it so the current version of the Draco npm modules work and it will work when 1.4.0 is released.

The question is is there a better way to create the Draco modules asynchronously in gltf-pipeline?

@FrankGalligan
Copy link
Contributor Author

@lilleyse Any issue with this PR? We want to release a new version of Draco as soon as we can and we really don't want to break gltf-pipeline.

@lilleyse
Copy link
Contributor

@FrankGalligan I tried out another approach that's more self-contained in compressDracoMeshes but I'm constantly getting Maximum call stack size exceeded not matter what I try. Could you take a look if I'm doing anything wrong? https://github.com/CesiumGS/gltf-pipeline/compare/draco-async?expand=1

@FrankGalligan
Copy link
Contributor Author

@lilleyse Your branch worked for me using Draco 1.3.6 (the current version). Also I think I created a PR by mistake #562

@FrankGalligan
Copy link
Contributor Author

@lilleyse I can reproduce the error you are seeing with Draco 1.3.6.

I had the newer modules when I was testing before. So your code works with the next release of Draco npm modules but doesn't work with Draco npm 1.3.6

I will see if I can debug what is going on with 1.3.6

@FrankGalligan
Copy link
Contributor Author

@lilleyse Unfortunately I really only program in JavaScript to make changes to gltf-pipeline, so my experience is very minimal.

I can see it gets into this function and returns:
https://github.com/CesiumGS/gltf-pipeline/blob/draco-async/lib/compressDracoMeshes.js#L58

Then it tries to run compress stage and then it breaks. From the error message it looks like you might have an infinite recursion.

Can the promise just keep creating another promise?

@lilleyse
Copy link
Contributor

The infinite recursion seems to be some strange interaction between the promise and the Draco module internals, and I can't seem to figure out why.

But if it's working in 1.4.0 then I can publish a gltf-pipeline release locked to 1.3.6 and then publish a new release once 1.4.0 is out. If that sounds good I'll go ahead with the first release.

@lilleyse
Copy link
Contributor

@FrankGalligan now that #563 is merged you can go ahead and release Draco 1.4.0

@lilleyse
Copy link
Contributor

lilleyse commented Dec 7, 2020

Fixed in #562

@lilleyse lilleyse closed this Dec 7, 2020
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