Skip to content
This repository was archived by the owner on Dec 22, 2023. It is now read-only.

ToyCar - PBR Next Phase1 Sample Model #278

Merged
merged 11 commits into from
Dec 1, 2020

Conversation

abwood
Copy link
Contributor

@abwood abwood commented Nov 26, 2020

This is the v4 sample model, with materials authored by @echadwick-wayfair. This model is featured in the PBR Next Phase 1 Press Release, showcasing KHR_materials_clearcoat, KHR_materials_transmission, and KHR_materials_sheen.

@cx20
Copy link
Contributor

cx20 commented Nov 27, 2020

Is this PR supposed to replace #268? If not, either or both of them are likely to have to change their names.

@abwood
Copy link
Contributor Author

abwood commented Nov 27, 2020

It is not intended to replace #268. I would be more than happy to rename this model, but I couldn't come up with a name that made sense in this repository. I had considered ToyCarPhase1, but I was concerned that the "Phase1" name only makes sense to the working group (these extensions represent phase 1 of PBR Next). I'm open to any suggestions on a rename here.

If this gets merged before #268, then in my mind it would make sense to rename that model (e.g. ToyCarSpecular).

CC @proog128 or @emackey

@echadwick-artist
Copy link
Contributor

echadwick-artist commented Nov 28, 2020 via email

@emackey
Copy link
Member

emackey commented Nov 28, 2020

I would prefer not to rename from simply "ToyCar", although the model may need to move to the extensions section of the readme.

As for the other ToyCar in #268, those extensions are still under review & discussion. I'm not sure that version of ToyCar demonstrates their effects in an intuitive way, like this version does with its extensions. We might not even end up using a car to demonstrate those extensions when complete, depending how things go.

So my vote would be, this version of the ToyCar should get merged first (pending a bit more review), and samples featuring not-yet-ratified extensions can be worked out later on. Sound good?

 - Fixed decal edges in the normal map
 - Added 8 cameras
@bghgary
Copy link
Contributor

bghgary commented Nov 30, 2020

Just a note: I tested this model in the Babylon.js sandbox with all of the extensions turned off and it still rendered properly. 👍 This model looks good to me barring some small comments.

image

Copy link
Member

@emackey emackey left a comment

Choose a reason for hiding this comment

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

I ran this ToyCar against the dev branch of the validator (2.0.0-dev.3.3 pre-release) which can validate the new PBR extensions, and it passed. Also the model is awesome. Merging.

@emackey emackey merged commit a35e94e into KhronosGroup:master Dec 1, 2020
@echadwick-artist
Copy link
Contributor

echadwick-artist commented Dec 1, 2020 via email

@emackey
Copy link
Member

emackey commented Dec 1, 2020

Ah, are you talking about the indents?

Alex fixed the indentation using a JSON pretty-formatter for VSCode. This is merged now, thanks!

Is Visual Studio Code the preferred editor for ascii glTF?

If you're asking me, the answer is so deep into "Yes" territory that I think it should probably be illegal to use other editing tools on raw glTF text.

@cx20
Copy link
Contributor

cx20 commented Dec 5, 2020

@bghgary BTW, I tried to insert this 3D model in Excel out of curiosity, and Excel crashed.
Probably because Excel does not support this glTF extension, but it should be modified so that it does not crash.

@bghgary
Copy link
Contributor

bghgary commented Dec 7, 2020

Thanks @cx20! I've forwarded your message to the team who hopefully can help. It might be a while due to the holiday season.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants