-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Model caching #2340
Model caching #2340
Conversation
@bagnell can you review? |
Conflicts: CHANGES.md
return destroyObject(this); | ||
}; | ||
|
||
return Model; | ||
}); | ||
|
||
//TODO: ref count cached animations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bagnell we should not have let these TODOs get into master (they are on the roadmap). I'll remove them shortly in my next pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I wasn't sure because we have other TODO, PERFORMANCE_IDEA, PERFORMANCE_TODO comments. Some of which I implemented optimizing some of the geometries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sometimes I leave a PERFORMANCE_IDEA
(and they even come in handy once in a while), but call me on it if I leave a TODO
in.
Would like to get this in for 1.5.
Open the Chrome task manager, and note the memory usage and load time for the following Sandcastle example using both master and this branch.
Master:
This branch:
This adds caching to glTF models. This is a replacement for #2177. Although this doesn't include any commits from that pull request, I have still credited @Relfos in CONTRIBUTORS.md for his significant work here.
Also added a few ideas for improvements to the model roadmap: #927