updated gltf importer #10
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added gltf subdirectory
should have all the files needed to run a gltf importer. Only thing would be the Cmake in the conv if its not updated
brlcad
requested changes
Dec 15, 2021
There was a problem hiding this comment.
Hi @volivarez60 this commit looks like a mistake or change from earlier. Can you undo it? Doesn't look like you submitted a src/libgcv/plugins/gltf plugin so this commit will break the build.
brlcad
requested changes
Dec 15, 2021
There was a problem hiding this comment.
@volivarez60 It's probably because you pulled tinygltf from git already. Try deleting the tinygltf/.git folder, then git add src/conv/gltf/tinygltf. If you do that, you should be able to commit and the PR can be updated with the necessary files.
brlcad
approved these changes
Dec 15, 2021
|
A thought - can the tinygltf contents be stripped down to a minimum? (I.e., something like LICENSE, tiny_gltf.* and stb_image* files?) |
|
The examples folder in particular is 23M, and models and tests together add another 1.4M |
|
Yeah I could get rid of them most likely, I can fix that probably Saturday because im out of town until then.
Get Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: Clifford Yapp ***@***.***>
Sent: Thursday, December 16, 2021 12:21:00 PM
To: BRL-CAD/brlcad ***@***.***>
Cc: volivarez60 ***@***.***>; Mention ***@***.***>
Subject: Re: [BRL-CAD/brlcad] updated gltf importer (PR #10)
The examples folder in particular is 23M, and models and tests together add another 1.4M
—
Reply to this email directly, view it on GitHub<#10 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANYNKTKBZ7GWPZHBT57WXDDURIUYZANCNFSM5J2R4JMQ>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
This change doesn't belong here.. Testing edit to PR.
unnecessary file
revert back to traversing into the empty reader stub
we don't need or want to be tracking license files for deps that are not in use, nor redistributing binary compilations we also don't use.
…me num lines also avoids needing to quote filepaths with spaces or wrapping the whole thing in a set() call.
brlcad
added a commit
that referenced
this pull request
Feb 24, 2022
he was on a team with 4 others, but his work was incorporated separately with the recent PR #10 merge.
zhuodannychen
pushed a commit
to zhuodannychen/brlcad
that referenced
this pull request
May 9, 2023
Started working with top and bottom headers
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.