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

Support KHR_technique_webgl extension in GLTF2Loader #11344

Merged
merged 2 commits into from
May 16, 2017

Conversation

takahirox
Copy link
Collaborator

@takahirox takahirox commented May 16, 2017

This PR lets GLTF2Loader support KHR_technique_webgl extension

Update:
I've left the existing json.techniques code as is so far though
techniques is moved to extension in glTF 2.0 because
it seems there're many models which have techniques under corejson.

I'm gonna remove it when glTF 2.0 release is officially announced.

@takahirox
Copy link
Collaborator Author

/cc @donmccurdy

Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

Nice! 👍

@@ -1642,11 +1668,16 @@ THREE.GLTF2Loader = ( function () {

materialType = DeferredShaderMaterial;

var technique = json.techniques[ material.technique ];
var extension = extensions[ EXTENSIONS.KHR_TECHNIQUE_WEBGL ];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to spec-gloss, could we do something like this to factor most of this code out of this large loadMaterials method?

materialType = DeferredShaderMaterial;
extensions[ EXTENSIONS.KHR_TECHNIQUE_WEBGL ].extendParams( materialParams, material, dependencies );

Copy link
Collaborator Author

@takahirox takahirox May 16, 2017

Choose a reason for hiding this comment

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

I'm thinking to do that when we remove the existing json.target code (see the Update in the PR comment)
So, adding some comments so far?

@mrdoob mrdoob merged commit 64e0f25 into mrdoob:dev May 16, 2017
@mrdoob
Copy link
Owner

mrdoob commented May 16, 2017

Thanks!

@takahirox takahirox deleted the GLTF2TECHEX branch May 17, 2017 00:14
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.

3 participants