Add gltf-model component and system. #2333

donmccurdy · 2017-01-31T04:23:21Z

Description:

Adds gltf-model component and system, based on @xirvr's aframe-gltf implementation.

Changes proposed:

Include THREE.GLTFLoader.
Currently excludes animations, but for Three.js r83+ an implementation might look very much like aframe-extras.animation-mixer.
~~Adds example model (which is 11MB, so we can pick another from gltf-test if we prefer).~~

dmarcos · 2017-01-31T22:01:57Z

src/components/gltf-model.js

+
+    this.remove();
+
+    this.loader.load(src, function (gltfModel) {


Let's name the anonymous function for easier debugging

ngokevin · 2017-02-01T01:26:19Z

We can host the sample models at https://github.com/aframevr/assets. Just commit it there, and it'll be available via cdn.aframe.io

ngokevin

Got an error for r83 three.js loader using your model, do I update to r84? Code looks good though, we can start test/docs.

GLTFLoader.js:762 Uncaught (in promise) RangeError: Invalid typed array length
    at new Float32Array (native)
    at http://localhost:9000/dist/aframe-master.js:54669:14
    at _each (http://localhost:9000/dist/aframe-master.js:54310:27)
    at http://localhost:9000/dist/aframe-master.js:54646:11

ngokevin · 2017-02-01T01:38:52Z

src/systems/gltf-model.js

+   * Registers a glTF asset.
+   * @param {object} gltf Asset containing a scene and (optional) animations and cameras.
+   */
+  addModel: function (gltf) {


I'd do registerModel/unregisterModel

donmccurdy · 2017-02-01T05:22:18Z

Updated.

ngokevin · 2017-02-01T05:36:35Z

docs/primitives/a-gltf-model.md

@@ -0,0 +1,33 @@
+---
+title: <a-collada-model>


ngokevin · 2017-02-01T05:37:35Z

src/components/gltf-model.js

+ * glTF model loader.
+ */
+module.exports.Component = registerComponent('gltf-model', {
+  schema: {type: 'model'},


Should this be multiprop to allow room for future props?

I slightly prefer multiprop, but can't actually think of any others we'd need. Maybe crossorigin. I'm expecting animation to be a separate component, but if not that would definitely need some properties. I've used src: {type: 'asset'} in most of my loaders so far i think.

Alright, can just leave it as is

ngokevin · 2017-02-01T05:38:19Z

src/extras/primitives/primitives/a-gltf-model.js

+var registerPrimitive = require('../primitives').registerPrimitive;
+var utils = require('../../../utils/');
+
+registerPrimitive('a-gltf-model', utils.extendDeep({}, getMeshMixin(), {


Does the material component have an effect on gltf mesh? If not, we can remove getMeshMixin.

Done (for both gltf and collada, for consistency)

ngokevin · 2017-02-01T05:38:39Z

src/systems/gltf-model.js

+   * Updates shaders for all glTF models in the system.
+   */
+  tick: function () {
+    var sceneEl = this.sceneEl;


we could test this code too

ngokevin · 2017-02-01T05:40:53Z

tests/components/gltf-model.test.js

+
+var SRC = '/base/tests/assets/box/Box.gltf';
+
+suite.only('gltf-model', function () {


got an only stuck on here (need to lint that, i do this a lot).

I've been following this template for tests now (https://github.com/aframevr/angle/blob/master/templates/component/tests/index.test.js#L6) ... closured variables for el and store the `component instance. saves a lot of proxy variables and grabbing the component.

ngokevin · 2017-02-01T05:41:43Z

docs/components/gltf-model.md

+    <a-asset-item id="tree" src="/path/to/tree.gltf"></a-asset-item>
+  </a-assets>
+
+  <a-entity collada-model="#tree"></a-entity>


ngokevin · 2017-02-01T05:43:33Z

docs/components/gltf-model.md

+<a-entity gltf-model="url(/path/to/tree.gltf)"></a-entity>
+```
+
+## More Resources


Can we add sections on playing animations until we get an API?

Yes, but the answer is "write a component that looks very much like animation-mixer", so IMO it would be better to leave that for another PR.

ngokevin · 2017-02-01T05:44:32Z

docs/components/gltf-model.md

+parent_section: components
+---
+
+[glTF][about-gltf] is an open project by Khronos providing a common, extensible format for 3D assets that is both efficient and highly interoperable with modern web technologies.


Could describe give the advantages and purpose of glTF over other common formats (obj, collada). The .psd vs .jpg analogy is a good one

ngokevin · 2017-02-01T05:45:04Z

docs/components/gltf-model.md

+
+See the [official glTF specification][spec] for available features, community resources, and ways to contribute.
+
+[fbx-converter]: http://gltf.autodesk.io/


I like putting link definitions above the paragraph its used to easier keep track

ngokevin · 2017-02-01T05:49:21Z

examples/test/model/index.html

@@ -11,13 +11,16 @@
      <a-assets>


Would be cool to add text labels under each model for the model format. And the scene could use a better background <a-sky color="#FAFAFA">

Could also add the different format types in the page title & meta description Models (glTF, COLLADA, OBJ)

scope creep alert 😉

Added the meta title/description, but I'm getting errors trying to do text.

<a-text value="foo"></a-text>

THREE.WebGLShader: gl.getShaderInfoLog() fragment WARNING: 0:102: ' ' : extension directive should occur before any non-preprocessor tokens � 1: precision highp float; 2: precision highp int; 3: #define SHADER_NAME ShaderMaterial 4: #define GAMMA_FACTOR 2 5: #define NUM_CLIPPING_PLANES 0 6: #define UNION_CLIPPING_PLANES 0 7: uniform mat4 viewMatrix; ...

Is that on latest? We pushed some text patches switching to raw shader material.

In the latest and greatest master, that warning has been eliminated by using RawShaderMaterial for the sdf and msdf text shaders. so... maybe needs rebase?

Rebase did the trick. ✅

ngokevin

Nice

christimiller · 2017-02-01T06:41:58Z

I don't have any code comments, but wanted to say thanks for this PR and glTF FTW :)

Fixes #2261.

donmccurdy · 2017-02-02T03:57:27Z

I believe this is caught up now.

ngokevin · 2017-02-02T05:55:46Z

tests/systems/gltf-model.test.js

+/* global process, setup, suite, test, sinon, THREE */
+var entityFactory = require('../helpers').entityFactory;
+
+suite.only('glTF system', function () {


ngokevin · 2017-02-02T05:56:43Z

looks good to me!

ngokevin · 2017-02-02T11:24:47Z

great looks totally fantastic

* Add gltf-model component and system. Fixes aframevr#2261. * Set *.bin extension as binary. * Remove model; reference assets repo. * Rename system API to (un)registerModel. * Add component docs. * Add primitive wrapper, and docs. * Revert .gitattributes changes. * Add registration for <a-gltf-model/>. * Add tests. * what other tests * Fix errors in docs. * Add formats to meta title/desc. * Remove material extensions from collada and gltf primitives. * Add tests for gltf-model system. * Add labels to models demo. * Add 'Why use glTF' section. * Wording tweak. * sigh

xirvr's gltf component has been merged in aframevr#2333

xirvr's gltf component has been merged in #2333

dmarcos reviewed Jan 31, 2017

View reviewed changes

ngokevin added the needs review label Jan 31, 2017

ngokevin added this to the 0.5.0 milestone Feb 1, 2017

dmarcos added needs changes and removed needs review labels Feb 1, 2017

ngokevin requested changes Feb 1, 2017

View reviewed changes

ngokevin reviewed Feb 1, 2017

View reviewed changes

ngokevin requested changes Feb 1, 2017

View reviewed changes

donmccurdy added 10 commits February 1, 2017 22:00

Add gltf-model component and system.

c2b5462

Fixes #2261.

Set *.bin extension as binary.

eb75294

Remove model; reference assets repo.

e2ba0d3

Rename system API to (un)registerModel.

4604c6f

Add component docs.

7feb873

Add primitive wrapper, and docs.

8ba7ebd

Revert .gitattributes changes.

e4f239e

Add registration for <a-gltf-model/>.

7b7f501

Add tests.

6beb60d

what other tests

b2fc6de

donmccurdy added 6 commits February 1, 2017 22:02

Fix errors in docs.

53515cb

Add formats to meta title/desc.

79ddddb

Remove material extensions from collada and gltf primitives.

1979fed

Add tests for gltf-model system.

3c3a3bd

Add labels to models demo.

1b16020

Add 'Why use glTF' section.

456cdaa

Wording tweak.

05a0f03

ngokevin reviewed Feb 2, 2017

View reviewed changes

ngokevin approved these changes Feb 2, 2017

View reviewed changes

sigh

892a854

ngokevin merged commit 6306459 into aframevr:master Feb 2, 2017

andyli added a commit to andyli/aframe that referenced this pull request Jun 7, 2017

doc: reference A-Frame's own glft primitive

33b945d

xirvr's gltf component has been merged in aframevr#2333

andyli mentioned this pull request Jun 7, 2017

doc: reference A-Frame's own glft primitive #2735

Merged

dmarcos pushed a commit that referenced this pull request Jun 7, 2017

doc: reference A-Frame's own glft primitive (#2735)

6836b71

xirvr's gltf component has been merged in #2333

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gltf-model component and system. #2333

Add gltf-model component and system. #2333

donmccurdy commented Jan 31, 2017 •

edited

dmarcos Jan 31, 2017

ngokevin commented Feb 1, 2017

ngokevin left a comment

ngokevin Feb 1, 2017

donmccurdy commented Feb 1, 2017

ngokevin Feb 1, 2017

ngokevin Feb 1, 2017

donmccurdy Feb 1, 2017

ngokevin Feb 1, 2017

ngokevin Feb 1, 2017

donmccurdy Feb 2, 2017

ngokevin Feb 1, 2017

donmccurdy Feb 2, 2017

ngokevin Feb 1, 2017

ngokevin Feb 1, 2017

ngokevin Feb 1, 2017

donmccurdy Feb 2, 2017 •

edited

ngokevin Feb 1, 2017

donmccurdy Feb 2, 2017

ngokevin Feb 1, 2017

donmccurdy Feb 2, 2017

ngokevin Feb 1, 2017

ngokevin Feb 1, 2017

donmccurdy Feb 2, 2017 •

edited

ngokevin Feb 2, 2017

machenmusik Feb 2, 2017 •

edited

donmccurdy Feb 2, 2017

ngokevin left a comment

christimiller commented Feb 1, 2017

donmccurdy commented Feb 2, 2017

ngokevin Feb 2, 2017

ngokevin commented Feb 2, 2017

ngokevin commented Feb 2, 2017


		var SRC = '/base/tests/assets/box/Box.gltf';

		suite.only('gltf-model', function () {


		See the [official glTF specification][spec] for available features, community resources, and ways to contribute.

		[fbx-converter]: http://gltf.autodesk.io/

Add gltf-model component and system. #2333

Add gltf-model component and system. #2333

Conversation

donmccurdy commented Jan 31, 2017 • edited

Choose a reason for hiding this comment

ngokevin commented Feb 1, 2017

ngokevin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donmccurdy commented Feb 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donmccurdy Feb 2, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donmccurdy Feb 2, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

machenmusik Feb 2, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ngokevin left a comment

Choose a reason for hiding this comment

christimiller commented Feb 1, 2017

donmccurdy commented Feb 2, 2017

Choose a reason for hiding this comment

ngokevin commented Feb 2, 2017

ngokevin commented Feb 2, 2017

donmccurdy commented Jan 31, 2017 •

edited

donmccurdy Feb 2, 2017 •

edited

donmccurdy Feb 2, 2017 •

edited

machenmusik Feb 2, 2017 •

edited