-
Notifications
You must be signed in to change notification settings - Fork 34
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 GLTF Scene Loader #425
Conversation
b4d9d6f
to
ca3321b
Compare
Just realized I never clicked "send review" |
This code now being orphaned, and quite self contained (we don't break bigwheels by bringing it in), I would be fine to merge this as-is (minus the small fixes), and iterate from there, as long as this compiled on all 3 platforms. |
0afb2ee
to
c685cdc
Compare
@Keenuts ehh - I don't mind trying to address the comments directly in this PR, that way nothing slips through unaddressed. I don't think this PR is needed too immediately |
c685cdc
to
a178020
Compare
@Keenuts can you re-review discussions above? |
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.
I have to admit I mostly skimmed the code at the end. I left some notes (some mainly for the record, feel free to ignore or add your thoughts).
Sure, we can address the open comments. But since this is a 4K Loc PR adding a new API with 0 tests, I think we'll miss many bugs anyway. So as long as nothing relies on this before it's proven to work, I'm OK merging this. |
@Keenuts @anishmgoyal any objections to merging this as is and addressing the outstanding comments in a follow up? |
Co-authored-by: Hai Nguyen <chaoticbob@users.noreply.github.com> Co-authored-by: Aliya Pazylbekova <apazylbe@users.noreply.github.com>
Also: add a default destructor to PrimitiveBatch
Sorry - I've been distracted with other things. Merging this sounds good to me |
Using the library
cgltf
, implements a GLTF scene laoder for BigWheels's scene graph.Also, defines an interface for generic scene loaders, in case we come up with other ways of storing and loading scenes.