Update version -> 2.0 (#271) #272

lasalvavida · 2017-04-28T14:28:11Z

@lilleyse, @pjcozzi, this is the PR for merging 2.0 into master

* Added updateVersion stage as well as some refactoring * Split out into multiple functions * Cleaned up a few failing tests * Added new upgrades to specs * Application specific parameters are prefixed with underscores * Migrated changes back from cesium * Changed 1.1 to 2.0 * Lots of 1.0->2.0 changes in pipeline stages * Tweaks from model generation * Generate default material * Add updateVersion to buildForCesium * Select default scene * A few more small changes * Add removePipelineExtras to cesium build * kmc fixes * Build global cesium include * Update * Update CHANGES.md * Added two more update functions * Update CHANGES.md * Strip version numbers when guessing if invalid * WIP Array-based Traversal * More WIP changes * Removed combineMeshes -> nodes only have a singular mesh now * Operator -> operate * combineNodes, removeUnused and dagToTree traversal changes * More WIP * Bulk WIP changes for traversal * Fixed a few more failing tests * Removed findUsedIds * WIP, switching workspaces * WIP, pretty much just AO left * Fixed a few more tests * All tests pass * Removed riggedSimpleUnoptimized * Delete generateTangentsBitangents models * Don't look for slots in array, just append * Small fixes from testing * More fixes * A few test fixes and byteStride -> bufferView * A few more test fixes * Missed a function * Removed vestigial byteStride references * Some fixes from cesium changes * Fixed cesium dependency list * Fixed some plurality issues * Add KHR_technique_webgl extensions if there are techniques

coveralls · 2017-04-28T14:53:11Z

Coverage decreased (-1.9%) to 91.952% when pulling 1406b32 on 2.0 into ce48e5f on master.

pbr->materialsCommon generation

fanzhanggoogle · 2017-04-28T22:03:26Z

lib/uninterleaveAndPackBuffers.js

-            accessor.bufferView = accessor.extras._pipeline.bufferView;
+    // compute the total size for each arraybuffer type
+    ForEach.accessor(gltf, function (accessor) {
+        var bufferView = gltf.bufferViews[accessor.bufferView];


@lasalvavida I think in the most recent spec. Accessor doesn't have to have a bufferView property?

Thanks for the review, @fanzhanggoogle! You are correct, see KhronosGroup/glTF#863

@fanzhanggoogle you are correct, however the only situation where an accessor doesn't have a bufferView property is if it is a sparse accessor. That isn't something that we support here yet, however a guard to make sure it is defined may be a good idea.

Yes, I agree with using a guard here. Just a note, cctually in the most recently 2.0 spec, there is:
"When nor sparse, neither bufferView is defined, min and max properties could have any values. This is intended for use cases when binary data is supplied by external means (e.g., via extensions)."
Which means bufferView could just be omitted.

fanzhanggoogle · 2017-04-28T22:05:52Z

lib/uninterleaveAndPackBuffers.js

-                accessorIds.push(accessorId);
-            }
-        }
+    var bufferViews = [{


In this file, it packs all the bufferViews if they are belong to an accessor and write over the original array of bufferViews. However, if there are some bufferViews are not belong to any accessor, e.g. is a property of an extension, then it won't work.

@fanzhanggoogle, that is a good point, but I'm not really sure how to work around it, since you'll have the same problem with the removeUnused functions as well. This is an optimization pipeline after all. I think the best solution we have is to support known extensions. So whichever extension specifies a bufferView will also be checked and added here.

If your example use case comes from the mesh compression you guys have been working on, I would be happy to add support based on an extension spec draft.

Sounds good! Thanks. I'm actually working on a branch to add support for the mesh compression extension in glTF-pipeline, I'll make a PR once it's done. It would be great to have you review it.
I agree with you that supporting known extension is enough here.

pjcozzi · 2017-05-03T11:44:39Z

@lilleyse, @pjcozzi, this is the PR for merging 2.0 into master

@lilleyse I most likely will not have time to review this so please merge when ready without me.

lilleyse

This has a lot of much needed cleanup in it! I plan on testing more thoroughly later.

Some other differences in the spec that should be handled:

JOINT and WEIGHT are now JOINTS_0 and WEIGHTS_0.
byteStride is now required
Empty arrays are disallowed now. There are probably corner cases throughout the code related to this.

lilleyse · 2017-05-02T16:57:34Z

lib/getAccessorByteStride.js

-function getAccessorByteStride(accessor) {
-    if (accessor.byteStride > 0) {
-        return accessor.byteStride;
+function getAccessorByteStride(gltf, accessor) {


gltf is missing from the doc

lilleyse · 2017-05-03T15:02:27Z

lib/addCesiumRTC.js

@@ -44,6 +45,7 @@ function addCesiumRTC(gltf, options) {
    extensions.CESIUM_RTC = {


Hidden from the diff:

var extensions = gltf.extensions; if (!defined(extensions)) { extensions = {}; gltf.extensions = extensions; }

I noticed a few others areas like this and thought it could benefit from a smaller helper file.

lilleyse · 2017-05-03T15:02:59Z

lib/addCesiumRTC.js

@@ -6,6 +6,7 @@ var Ellipsoid = Cesium.Ellipsoid;
 var defaultValue = Cesium.defaultValue;


This file can use ForEach.technique and ForEach.techniqueParameters.

lilleyse · 2017-05-03T15:03:22Z

lib/addDefaults.js

+    ],
+    asset : {},
+    buffers : [
+        {
            byteLength: 0,
            type: 'arraybuffer'


This should be removed.

lilleyse · 2017-05-03T15:04:01Z

lib/addDefaults.js

+    ],
+    asset : {},
+    buffers : [
+        {
            byteLength: 0,


byteLength now has a minimum of 1.

lilleyse · 2017-05-03T15:35:32Z

lib/updateVersion.js

+function glTF10to20(gltf) {
+    if (!defined(gltf.asset)) {
+        gltf.asset = {};
+    }


The check here and in gltf08to10 aren't required since asset is always created initially.

lilleyse · 2017-05-03T15:36:03Z

lib/updateVersion.js

+            var skeletons = node.skeletons;
+            var skeletonsLength = skeletons.length;
+            if (skeletonsLength > 0) {
+                node.skeleton = globalMapping.nodes[skeletons[0]];


As noted before, skeleton is now part of skin.

lilleyse · 2017-05-03T15:40:02Z

lib/updateVersion.js

+                if (topLevelId === 'animations') {
+                    objectMapping = {};
+                    object.samplers = objectToArray(object.samplers, objectMapping);
+                    globalMapping[topLevelId].samplers = objectMapping;


Isn't this handled below?

lilleyse · 2017-05-03T15:41:02Z

lib/updateVersion.js

+                var bufferView = gltf.bufferViews[accessor.bufferView];
+                if (defined(bufferView.byteStride) && bufferView.byteStride !== byteStride) {
+                    // another accessor uses this with a different byte stride
+                    bufferView = clone(bufferView);


Bit of a nitpick, but in the case of the presence of an extras objects clone should be a deep clone.

lilleyse · 2017-05-03T15:42:53Z

lib/updateVersion.js

+            var yfov = perspective.yfov;
+            if (defined(yfov) && yfov === 0.0) {
+                perspective.yfov = 1.0;
+            }


coveralls · 2017-05-03T16:19:50Z

Coverage decreased (-1.9%) to 91.951% when pulling 8c909e8 on 2.0 into ce48e5f on master.

coveralls · 2017-05-03T17:28:28Z

Coverage decreased (-1.9%) to 91.951% when pulling 5b04132 on 2.0 into ce48e5f on master.

lilleyse · 2017-05-11T14:01:56Z

uninterleaveAndPackBuffers may need some further adjustments for byte alignment in 2.0:
#279 (comment)

…gets Infer missing bufferView targets

2.0 pbr

…t removes textures from gltf

…ngent count

fanzhanggoogle · 2017-07-27T17:54:02Z

Hi @lilleyse @lasalvavida , I was wondering if the PR is still active. Which is the active glTF-pipeline repo for glTF2.0? Thanks!

lilleyse · 2017-07-27T18:03:57Z

This PR hasn't been visibly active, but there is a separate pull request coming in here soon. A lot of the code here will also be used in Cesium for converting 1.0 models to 2.0.

Long term however, this PR will stay parked on a branch until a full cleanup is complete that puts gltf 2.0 first. The working branch for that is cleanup and will be ready a couple months from now.

fanzhanggoogle · 2017-07-27T18:56:04Z

Thanks. Good to know!

PBR Earth reflections

Added processPbrMetallicRoughness

lilleyse · 2017-08-07T15:16:42Z

Closing - but the keeping the branch alive since this is the basis for Cesium's implementation. Development for 2.0 will now be on the cleanup branch.

lasalvavida added 3 commits April 28, 2017 11:17

Added pbrToMaterialsCommon

06a0954

Moved out getJointCountForMaterials as a separate function and added

d373dc2

pbr->materialsCommon generation

Infer missing bufferView targets

c43c6d8

fanzhanggoogle reviewed Apr 28, 2017

View reviewed changes

lasalvavida and others added 2 commits May 3, 2017 11:31

Add option to break from object iterator

119d008

Style tweaks

8c909e8

lilleyse reviewed May 3, 2017

View reviewed changes

Mistake from previous commit

5b04132

lasalvavida and others added 2 commits May 14, 2017 20:57

Added some unit tests for ForEach iterator

9891c39

Merge pull request #275 from AnalyticalGraphicsInc/2.0-bufferView-tar…

3d107c2

…gets Infer missing bufferView targets

lilleyse mentioned this pull request Jun 5, 2017

PBR roadmap CesiumGS/cesium#5423

Closed

14 tasks

lasalvavida and others added 5 commits June 9, 2017 10:52

Added spec for getJointCountForMaterials and some cleanup

d9842a0

Clean up jsHint

5c722f9

fit -> it

d7d9391

Fix failing test

8f83cc2

Merge pull request #274 from AnalyticalGraphicsInc/2.0-pbr

4308188

2.0 pbr

pjcozzi mentioned this pull request Jun 14, 2017

Move general glTF helpers to their own libraries CesiumGS/cesium#3103

Closed

lilleyse mentioned this pull request Jun 27, 2017

Upgrade tileset stage CesiumGS/3d-tiles-validator#86

Merged

1 task

moneimne added 6 commits July 11, 2017 15:23

Added processPbrMetallicRoughness

715ebc2

Updated processPbrMetallicRoughness per PR comments and fixed bug tha…

40a7600

…t removes textures from gltf

Split glsl type and webGL conversions into separate files

7785d21

Added default byteOffset to accessors

6c9cc69

Added default byteOffset to bufferViews

8f7cf87

Took uninterleaveAndPackBuffers from master and upgraded to 2.0

357aaa7

moneimne added 14 commits July 17, 2017 17:22

Fixed jsHint issues and removeUnusedProperties tests

401691d

Fixed removeUnusedVertices tests by changing according to master

58ca4b6

Fixed quantizeAttributes tests by changing according to master

81b8b32

Fixed mergeDuplicateVertices tests by changing according to master

ff7af4a

Fixed generateTangentsBitangents test to account for tangent and bita…

abcc18a

…ngent count

Updated gulpfile and added skinning upgrade to 2.0

e2097f9

Keep extensions as object instead of array

ccdf46b

Should not change materials to arrays for 1.0

a67e5e7

Only try to switch jointNames over if defined

cae4279

Updated tests to support changes to diffuse texture changes

8a444f7

Updated per changes in Cesium

59f4478

Another update per Cesium changes

f24ef8a

Removed print statement

b10c842

Updated per Cesium cmpt tiles fix

1ae7781

emackey and others added 2 commits July 27, 2017 15:09

Emulate the Earth with a set of gradients.

bacada9

Merge pull request #1 from emackey/pbr-earth-reflections

4350ab8

PBR Earth reflections

donmccurdy mentioned this pull request Aug 4, 2017

consider binary glTF using KHR-MC extension KhronosGroup/glTF-Sample-Models#109

Closed

moneimne and others added 3 commits August 7, 2017 09:32

Updated per changes to Cesium for gltf-2.0 PR

bf83211

Removed a couple files from cesium-combine gul task

bdbcdc9

Merge pull request #301 from moneimne/processPbrMetallicRoughness

6965cf0

Added processPbrMetallicRoughness

lilleyse closed this Aug 7, 2017

fanzhanggoogle mentioned this pull request Sep 2, 2017

2.0 mesh compression fanzhanggoogle/gltf-pipeline#1

Open

fanzhanggoogle mentioned this pull request Sep 27, 2017

KHR_draco_mesh_compression KhronosGroup/glTF#874

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update version -> 2.0 (#271) #272

Update version -> 2.0 (#271) #272

lasalvavida commented Apr 28, 2017

coveralls commented Apr 28, 2017

fanzhanggoogle Apr 28, 2017

pjcozzi May 3, 2017

lasalvavida May 3, 2017

fanzhanggoogle May 3, 2017

fanzhanggoogle Apr 28, 2017

lasalvavida May 3, 2017

fanzhanggoogle May 3, 2017

pjcozzi commented May 3, 2017

lilleyse left a comment •

edited

Loading

lilleyse May 2, 2017

lilleyse May 3, 2017

lilleyse May 3, 2017

lilleyse May 3, 2017

lilleyse May 3, 2017

lilleyse May 3, 2017

lilleyse May 3, 2017

lilleyse May 3, 2017

lilleyse May 3, 2017

lilleyse May 3, 2017

coveralls commented May 3, 2017

coveralls commented May 3, 2017

lilleyse commented May 11, 2017

fanzhanggoogle commented Jul 27, 2017

lilleyse commented Jul 27, 2017

fanzhanggoogle commented Jul 27, 2017

lilleyse commented Aug 7, 2017

		@@ -44,6 +45,7 @@ function addCesiumRTC(gltf, options) {
		extensions.CESIUM_RTC = {

		@@ -6,6 +6,7 @@ var Ellipsoid = Cesium.Ellipsoid;
		var defaultValue = Cesium.defaultValue;

Update version -> 2.0 (#271) #272

Update version -> 2.0 (#271) #272

Conversation

lasalvavida commented Apr 28, 2017

coveralls commented Apr 28, 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

pjcozzi commented May 3, 2017

lilleyse left a comment • edited Loading

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

coveralls commented May 3, 2017

coveralls commented May 3, 2017

lilleyse commented May 11, 2017

fanzhanggoogle commented Jul 27, 2017

lilleyse commented Jul 27, 2017

fanzhanggoogle commented Jul 27, 2017

lilleyse commented Aug 7, 2017

lilleyse left a comment •

edited

Loading