From 676aaf118fddcfa89af918cb5ea8c304b9d569fc Mon Sep 17 00:00:00 2001 From: Ed Mackey Date: Fri, 25 May 2018 16:28:29 -0400 Subject: [PATCH 1/7] Detect glTF 2.0 and change forward to match Cesium's convention. --- .../DracoCompressed/CesiumMilkTruck.gltf | 18 +++++++++++ .../gallery/Physically-Based Materials.html | 2 +- Source/Scene/Model.js | 32 +++++++++++++++++++ .../ThirdParty/GltfPipeline/updateVersion.js | 4 +++ .../DracoCompression/CesiumMan/CesiumMan.gltf | 4 +-- .../CesiumMilkTruck/CesiumMilkTruck.gltf | 18 +++++++++++ 6 files changed, 75 insertions(+), 3 deletions(-) diff --git a/Apps/SampleData/models/DracoCompressed/CesiumMilkTruck.gltf b/Apps/SampleData/models/DracoCompressed/CesiumMilkTruck.gltf index 2132e20303b6..f9edc433dec6 100644 --- a/Apps/SampleData/models/DracoCompressed/CesiumMilkTruck.gltf +++ b/Apps/SampleData/models/DracoCompressed/CesiumMilkTruck.gltf @@ -17,6 +17,24 @@ "children": [ 3, 1 + ], + "matrix": [ + 0, + 0, + 1, + 0, + 0, + 1, + 0, + 0, + -1, + 0, + 0, + 0, + 0, + 0, + 0, + 1 ] }, { diff --git a/Apps/Sandcastle/gallery/Physically-Based Materials.html b/Apps/Sandcastle/gallery/Physically-Based Materials.html index 960d6d5dc5cd..279e730f51cc 100644 --- a/Apps/Sandcastle/gallery/Physically-Based Materials.html +++ b/Apps/Sandcastle/gallery/Physically-Based Materials.html @@ -53,7 +53,7 @@ viewer.scene.globe.depthTestAgainstTerrain = true; var position = new Cesium.Cartesian3(-1371108.6511167218, -5508684.080096612, 2901825.449865087); -var heading = Cesium.Math.toRadians(90); +var heading = Cesium.Math.toRadians(180); var pitch = Cesium.Math.toRadians(2); var roll = Cesium.Math.toRadians(-6); var hpr = new Cesium.HeadingPitchRoll(heading, pitch, roll); diff --git a/Source/Scene/Model.js b/Source/Scene/Model.js index 64132b2e194e..3a8cd7dae38e 100644 --- a/Source/Scene/Model.js +++ b/Source/Scene/Model.js @@ -579,6 +579,7 @@ define([ this._ignoreCommands = defaultValue(options.ignoreCommands, false); this._requestType = options.requestType; this._upAxis = defaultValue(options.upAxis, Axis.Y); + this._forwardAxis = defaultValue(options.upAxis, Axis.Z); /** * @private @@ -973,6 +974,25 @@ define([ } }, + /** + * Gets the model's forward axis. + * By default, glTF 2.0 models are z-forward according to the glTF spec, however older + * glTF (1.0, 0.8) models used x-forward. Note that only Axis.X and Axis.Z are supported. + * + * @memberof Model.prototype + * + * @type {Number} + * @default Axis.Z + * @readonly + * + * @private + */ + forwardAxis : { + get : function() { + return this._forwardAxis; + } + }, + /** * Gets the model's triangle count. * @@ -1361,6 +1381,10 @@ define([ } else if (model._upAxis === Axis.X) { BoundingSphere.transformWithoutScale(boundingSphere, Axis.X_UP_TO_Z_UP, boundingSphere); } + if (model._forwardAxis === Axis.Z) { + // glTF 2.0 has a Z-forward convention that must be adapted here to X-forward. + BoundingSphere.transformWithoutScale(boundingSphere, Axis.Z_UP_TO_X_UP, boundingSphere); + } return boundingSphere; } @@ -4303,6 +4327,10 @@ define([ }; frameState.brdfLutGenerator.update(frameState); updateVersion(this.gltf); + if (defined(this.gltf.asset) && defined(this.gltf.asset.extras) && + this.gltf.asset.extras.gltf_pipeline_upgrade_10to20) { + this._forwardAxis = Axis.X; + } ModelUtility.checkSupportedExtensions(this.extensionsRequired); addPipelineExtras(this.gltf); addDefaults(this.gltf); @@ -4437,6 +4465,10 @@ define([ } else if (this._upAxis === Axis.X) { Matrix4.multiplyTransformation(computedModelMatrix, Axis.X_UP_TO_Z_UP, computedModelMatrix); } + if (this._forwardAxis === Axis.Z) { + // glTF 2.0 has a Z-forward convention that must be adapted here to X-forward. + Matrix4.multiplyTransformation(computedModelMatrix, Axis.Z_UP_TO_X_UP, computedModelMatrix); + } } // Update modelMatrix throughout the graph as needed diff --git a/Source/ThirdParty/GltfPipeline/updateVersion.js b/Source/ThirdParty/GltfPipeline/updateVersion.js index 2292007d5fdb..637af6603081 100644 --- a/Source/ThirdParty/GltfPipeline/updateVersion.js +++ b/Source/ThirdParty/GltfPipeline/updateVersion.js @@ -883,8 +883,12 @@ define([ if (!defined(gltf.asset)) { gltf.asset = {}; } + if (!defined(gltf.asset.extras)) { + gltf.asset.extras = {}; + } var asset = gltf.asset; asset.version = '2.0'; + asset.extras.gltf_pipeline_upgrade_10to20 = true; // material.instanceTechnique properties should be directly on the material. instanceTechnique is a gltf 0.8 property but is seen in some 1.0 models. updateInstanceTechniques(gltf); // animation.samplers now refers directly to accessors and animation.parameters should be removed diff --git a/Specs/Data/Models/DracoCompression/CesiumMan/CesiumMan.gltf b/Specs/Data/Models/DracoCompression/CesiumMan/CesiumMan.gltf index 440d4adc3857..9d07ce45ff71 100644 --- a/Specs/Data/Models/DracoCompression/CesiumMan/CesiumMan.gltf +++ b/Specs/Data/Models/DracoCompression/CesiumMan/CesiumMan.gltf @@ -18,13 +18,13 @@ 1 ], "matrix": [ - 1, 0, 0, + 1, 0, + 1, 0, 0, - -1, 0, 0, 1, diff --git a/Specs/Data/Models/DracoCompression/CesiumMilkTruck/CesiumMilkTruck.gltf b/Specs/Data/Models/DracoCompression/CesiumMilkTruck/CesiumMilkTruck.gltf index 2132e20303b6..f9edc433dec6 100644 --- a/Specs/Data/Models/DracoCompression/CesiumMilkTruck/CesiumMilkTruck.gltf +++ b/Specs/Data/Models/DracoCompression/CesiumMilkTruck/CesiumMilkTruck.gltf @@ -17,6 +17,24 @@ "children": [ 3, 1 + ], + "matrix": [ + 0, + 0, + 1, + 0, + 0, + 1, + 0, + 0, + -1, + 0, + 0, + 0, + 0, + 0, + 0, + 1 ] }, { From 2569796b8933d732ee3caecfbef4e498efd2a3b0 Mon Sep 17 00:00:00 2001 From: Ed Mackey Date: Tue, 29 May 2018 11:55:10 -0400 Subject: [PATCH 2/7] Fix name of option. --- Source/Scene/Model.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Scene/Model.js b/Source/Scene/Model.js index 3a8cd7dae38e..bf8fcd158845 100644 --- a/Source/Scene/Model.js +++ b/Source/Scene/Model.js @@ -579,7 +579,7 @@ define([ this._ignoreCommands = defaultValue(options.ignoreCommands, false); this._requestType = options.requestType; this._upAxis = defaultValue(options.upAxis, Axis.Y); - this._forwardAxis = defaultValue(options.upAxis, Axis.Z); + this._forwardAxis = defaultValue(options.forwardAxis, Axis.Z); /** * @private From 30d95f1bfc9553c7fa27ded23f8e09e8c4a77b79 Mon Sep 17 00:00:00 2001 From: Ed Mackey Date: Tue, 29 May 2018 13:41:49 -0400 Subject: [PATCH 3/7] Fix tests --- Specs/Scene/ModelSpec.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Specs/Scene/ModelSpec.js b/Specs/Scene/ModelSpec.js index 50e4b24ddf65..29cc9fc043d5 100644 --- a/Specs/Scene/ModelSpec.js +++ b/Specs/Scene/ModelSpec.js @@ -2229,6 +2229,7 @@ defineSuite([ function checkVertexColors(model) { model.zoomTo(); + scene.camera.rotateRight(CesiumMath.PI_OVER_TWO); scene.camera.moveUp(0.1); // Red scene.camera.moveLeft(0.5); @@ -2485,7 +2486,8 @@ defineSuite([ it('loads a glTF with KHR_draco_mesh_compression extension with integer attributes', function() { return loadModel(dracoCompressedModelWithAnimationUrl, { - dequantizeInShader : false + dequantizeInShader : false, + forwardAxis : Axis.X }).then(function(m) { verifyRender(m); primitives.remove(m); @@ -2533,7 +2535,8 @@ defineSuite([ it('loads a draco compressed glTF and dequantizes in the shader, skipping generic attributes', function() { return loadModel(dracoCompressedModelWithAnimationUrl, { - dequantizeInShader : true + dequantizeInShader : true, + forwardAxis : Axis.X }).then(function(m) { verifyRender(m); From 897c01cf55b9b6514db41a82dccba9953966b92a Mon Sep 17 00:00:00 2001 From: Ed Mackey Date: Tue, 29 May 2018 14:35:00 -0400 Subject: [PATCH 4/7] Add new tests for forwardAxis. --- Specs/Scene/ModelSpec.js | 50 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/Specs/Scene/ModelSpec.js b/Specs/Scene/ModelSpec.js index 29cc9fc043d5..7eb89fcd40cc 100644 --- a/Specs/Scene/ModelSpec.js +++ b/Specs/Scene/ModelSpec.js @@ -396,7 +396,7 @@ defineSuite([ model.show = false; }); - it('Renders x-up model', function() { + it('renders x-up model', function() { return Resource.fetchJson(boxEcefUrl).then(function(gltf) { // Model data is z-up. Edit the transform to be z-up to x-up. gltf.nodes.node_transform.matrix = Matrix4.pack(Axis.Z_UP_TO_X_UP, new Array(16)); @@ -412,7 +412,7 @@ defineSuite([ }); }); - it('Renders y-up model', function() { + it('renders y-up model', function() { return Resource.fetchJson(boxEcefUrl).then(function(gltf) { // Model data is z-up. Edit the transform to be z-up to y-up. gltf.nodes.node_transform.matrix = Matrix4.pack(Axis.Z_UP_TO_Y_UP, new Array(16)); @@ -428,7 +428,7 @@ defineSuite([ }); }); - it('Renders z-up model', function() { + it('renders z-up model', function() { return Resource.fetchJson(boxEcefUrl).then(function(gltf) { // Model data is z-up. Edit the transform to be the identity. gltf.nodes.node_transform.matrix = Matrix4.pack(Matrix4.IDENTITY, new Array(16)); @@ -444,6 +444,50 @@ defineSuite([ }); }); + it('renders x-forward model', function() { + return Resource.fetchJson(boxEcefUrl).then(function(gltf) { + return loadModelJson(gltf, { + forwardAxis : Axis.X + }).then(function(m) { + verifyRender(m); + expect(m.forwardAxis).toBe(Axis.X); + primitives.remove(m); + }); + }); + }); + + it('renders z-forward model', function() { + return Resource.fetchJson(boxPbrUrl).then(function(gltf) { + return loadModelJson(gltf, { + forwardAxis : Axis.Z + }).then(function(m) { + verifyRender(m); + expect(m.forwardAxis).toBe(Axis.Z); + primitives.remove(m); + }); + }); + }); + + it('detects glTF 1.0 models as x-forward', function() { + return Resource.fetchJson(boxEcefUrl).then(function(gltf) { + return loadModelJson(gltf).then(function(m) { + verifyRender(m); + expect(m.forwardAxis).toBe(Axis.X); + primitives.remove(m); + }); + }); + }); + + it('detects glTF 2.0 models as z-forward', function() { + return Resource.fetchJson(boxPbrUrl).then(function(gltf) { + return loadModelJson(gltf).then(function(m) { + verifyRender(m); + expect(m.forwardAxis).toBe(Axis.Z); + primitives.remove(m); + }); + }); + }); + it('resolves readyPromise', function() { return texturedBoxModel.readyPromise.then(function(model) { verifyRender(model); From 8a58c11312bfd6a9ef7de13682efaf458edafaa6 Mon Sep 17 00:00:00 2001 From: Ed Mackey Date: Tue, 29 May 2018 14:41:36 -0400 Subject: [PATCH 5/7] CHANGES.md --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 3ffb7cd10ddd..9ee69b5e8bab 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,6 +9,7 @@ Change Log * Removed `Scene.copyGlobeDepth`. Globe depth will now be copied by default when supported. [#6393](https://github.com/AnalyticalGraphicsInc/cesium/pull/6393) * The default `classificationType` for `GroundPrimitive`, `CorridorGraphics`, `EllipseGraphics`, `PolygonGraphics` and `RectangleGraphics` is now `ClassificationType.TERRAIN`. If you wish the geometry to color both terrain and 3D tiles, pass in the option `classificationType: Cesium.ClassificationType.BOTH`. * Removed support for the `options` argument for `Credit` [#6373](https://github.com/AnalyticalGraphicsInc/cesium/issues/6373). Pass in an html string instead. +* glTF 2.0 models corrected to face +Z forwards per specification. Internally Cesium uses +X as forward, so a new +Z to +X rotation was added for 2.0 models only. [#6632](https://github.com/AnalyticalGraphicsInc/cesium/pull/6632) ##### Deprecated :hourglass_flowing_sand: * The `Scene.fxaa` property has been deprecated and will be removed in Cesium 1.47. Use `Scene.postProcessStages.fxaa.enabled`. From 9809e31520a6e4474f3d1225cb0b3a3c5c7e495f Mon Sep 17 00:00:00 2001 From: Ed Mackey Date: Tue, 29 May 2018 15:15:59 -0400 Subject: [PATCH 6/7] Add option to suppress autodetect of forward axis. --- Source/Scene/Batched3DModel3DTileContent.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Source/Scene/Batched3DModel3DTileContent.js b/Source/Scene/Batched3DModel3DTileContent.js index 1f2403897761..0c2dae59c09f 100644 --- a/Source/Scene/Batched3DModel3DTileContent.js +++ b/Source/Scene/Batched3DModel3DTileContent.js @@ -12,6 +12,7 @@ define([ '../Core/RequestType', '../Core/RuntimeError', '../Renderer/Pass', + './Axis', './Cesium3DTileBatchTable', './Cesium3DTileFeature', './Cesium3DTileFeatureTable', @@ -32,6 +33,7 @@ define([ RequestType, RuntimeError, Pass, + Axis, Cesium3DTileBatchTable, Cesium3DTileFeature, Cesium3DTileFeatureTable, @@ -364,6 +366,7 @@ define([ requestType : RequestType.TILES3D, modelMatrix : tile.computedTransform, upAxis : tileset._gltfUpAxis, + forwardAxis : Axis.X, shadows: tileset.shadows, debugWireframe: tileset.debugWireframe, incrementallyLoadTextures : false, @@ -386,6 +389,7 @@ define([ requestType : RequestType.TILES3D, modelMatrix : tile.computedTransform, upAxis : tileset._gltfUpAxis, + forwardAxis : Axis.X, debugWireframe : tileset.debugWireframe, vertexShaderLoaded : getVertexShaderCallback(content), classificationShaderLoaded : getClassificationFragmentShaderCallback(content), From 56f94d93e9535129c5d305b1a219acdf245cba18 Mon Sep 17 00:00:00 2001 From: Sean Lilley Date: Mon, 11 Jun 2018 10:10:37 -0400 Subject: [PATCH 7/7] Moved breaking change to 1.47 section --- CHANGES.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 6bdb55d8b3e7..153b0138a7db 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -3,6 +3,9 @@ Change Log ### 1.47 - 2018-07-02 +##### Breaking Changes :mega: +* glTF 2.0 models corrected to face +Z forwards per specification. Internally Cesium uses +X as forward, so a new +Z to +X rotation was added for 2.0 models only. [#6632](https://github.com/AnalyticalGraphicsInc/cesium/pull/6632) + ##### Additions :tada: * `PostProcessStage` has a `selectedFeatures` property which is an array of primitives used for selectively applying a post-process stage. In the fragment shader, use the function `bool czm_selected(vec2 textureCoordinates` to determine whether or not the stage should be applied at that fragment. * The black-and-white and silhouette stages have per-feature support.