From 1d491a07994ba5c5e3567e1364d1cd17e5a9c8d8 Mon Sep 17 00:00:00 2001 From: Tucker Tibbetts Date: Tue, 3 Nov 2015 12:58:19 -0500 Subject: [PATCH 1/7] Adds maximum scale property to models --- Source/DataSources/ModelGraphics.js | 12 ++++++++++++ Source/DataSources/ModelVisualizer.js | 1 + Source/Scene/Model.js | 16 +++++++++++++++- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/Source/DataSources/ModelGraphics.js b/Source/DataSources/ModelGraphics.js index 1533df353e0c..744e945b4fc5 100644 --- a/Source/DataSources/ModelGraphics.js +++ b/Source/DataSources/ModelGraphics.js @@ -31,6 +31,7 @@ define([ * @param {Property} [options.show=true] A boolean Property specifying the visibility of the model. * @param {Property} [options.scale=1.0] A numeric Property specifying a uniform linear scale. * @param {Property} [options.minimumPixelSize=0.0] A numeric Property specifying the approximate minimum pixel size of the model regardless of zoom. + * @param {Property} [options.maximumScale] The maximum scale size of a model. An upper limit for minimumPixelSize. * * @see {@link http://cesiumjs.org/2014/03/03/Cesium-3D-Models-Tutorial/|3D Models Tutorial} * @demo {@link http://cesiumjs.org/Cesium/Apps/Sandcastle/index.html?src=3D%20Models.html|Cesium Sandcastle 3D Models Demo} @@ -42,6 +43,8 @@ define([ this._scaleSubscription = undefined; this._minimumPixelSize = undefined; this._minimumPixelSizeSubscription = undefined; + this._maximumScale = undefined; + this._maximumScaleSubscription = undefined; this._uri = undefined; this._uriSubscription = undefined; this._definitionChanged = new Event(); @@ -91,6 +94,13 @@ define([ */ minimumPixelSize : createPropertyDescriptor('minimumPixelSize'), + /** + * Gets or sets the numeric Property specifying the maximum scale + * size of a model. This property is used as an upper limit for + * minimumPixelSize. + */ + maximumScale : createPropertyDescriptor('maximumScale'), + /** * Gets or sets the string Property specifying the URI of the glTF asset. * @memberof ModelGraphics.prototype @@ -112,6 +122,7 @@ define([ result.show = this.show; result.scale = this.scale; result.minimumPixelSize = this.minimumPixelSize; + result.maximumScale = this.maximumScale; result.uri = this.uri; return result; }; @@ -132,6 +143,7 @@ define([ this.show = defaultValue(this.show, source.show); this.scale = defaultValue(this.scale, source.scale); this.minimumPixelSize = defaultValue(this.minimumPixelSize, source.minimumPixelSize); + this.maximumScale = defaultValue(this.maximumScale, source.maximumScale); this.uri = defaultValue(this.uri, source.uri); }; diff --git a/Source/DataSources/ModelVisualizer.js b/Source/DataSources/ModelVisualizer.js index cd40d0761c8e..33fe9f258313 100644 --- a/Source/DataSources/ModelVisualizer.js +++ b/Source/DataSources/ModelVisualizer.js @@ -125,6 +125,7 @@ define([ model.show = true; model.scale = Property.getValueOrDefault(modelGraphics._scale, time, defaultScale); model.minimumPixelSize = Property.getValueOrDefault(modelGraphics._minimumPixelSize, time, defaultMinimumPixelSize); + model.maximumScale = Property.getValueOrDefault(modelGraphics._maximumScale, time, null); model.modelMatrix = Matrix4.clone(modelMatrix, model.modelMatrix); } return true; diff --git a/Source/Scene/Model.js b/Source/Scene/Model.js index ea72a7a31e33..b682495b1c2e 100644 --- a/Source/Scene/Model.js +++ b/Source/Scene/Model.js @@ -291,6 +291,7 @@ define([ * @param {Matrix4} [options.modelMatrix=Matrix4.IDENTITY] The 4x4 transformation matrix that transforms the model from model to world coordinates. * @param {Number} [options.scale=1.0] A uniform scale applied to this model. * @param {Number} [options.minimumPixelSize=0.0] The approximate minimum pixel size of the model regardless of zoom. + * @param {Number} [options.maximumScale] The maximum scale size of a model. An upper limit for minimumPixelSize. * @param {Object} [options.id] A user-defined object to return when the model is picked with {@link Scene#pick}. * @param {Boolean} [options.allowPicking=true] When true, each glTF mesh and primitive is pickable with {@link Scene#pick}. * @param {Boolean} [options.asynchronous=true] Determines if model WebGL resource creation will be spread out over several frames or block until completion once all glTF files are loaded. @@ -415,6 +416,17 @@ define([ this.minimumPixelSize = defaultValue(options.minimumPixelSize, 0.0); this._minimumPixelSize = this.minimumPixelSize; + /** + * The maximum scale size for a model. This can be used to give + * an upper limit to the minimumPixelSize, ensuring that the model + * is never an unreasonable scale. + + * + * @type {Number} + */ + this.maximumScale = defaultValue(options.maximumScale, null); + this._maximumScale = this.maximumScale; + /** * User-defined object returned when the model is picked. * @@ -823,6 +835,7 @@ define([ * @param {Matrix4} [options.modelMatrix=Matrix4.IDENTITY] The 4x4 transformation matrix that transforms the model from model to world coordinates. * @param {Number} [options.scale=1.0] A uniform scale applied to this model. * @param {Number} [options.minimumPixelSize=0.0] The approximate minimum pixel size of the model regardless of zoom. + * @param {Number} [options.maxiumumScale] The maximum scale for the model. * @param {Boolean} [options.allowPicking=true] When true, each glTF mesh and primitive is pickable with {@link Scene#pick}. * @param {Boolean} [options.asynchronous=true] Determines if model WebGL resource creation will be spread out over several frames or block until completion once all glTF files are loaded. * @param {Boolean} [options.debugShowBoundingVolume=false] For debugging only. Draws the bounding sphere for each {@link DrawCommand} in the model. @@ -849,6 +862,7 @@ define([ * modelMatrix : modelMatrix, * scale : 2.0, // double size * minimumPixelSize : 128, // never smaller than 128 pixels + * maximumScale: 20000, // never larger than 20000 * model size (overrides minimumPixelSize) * allowPicking : false, // not pickable * debugShowBoundingVolume : false, // default * debugWireframe : false @@ -2866,7 +2880,7 @@ define([ } } - return scale; + return (model.maximumScale && model.maximumScale < scale) ? model.maximumScale : scale; } function releaseCachedGltf(model) { From b9d441a1fa331e24204d3db88f5f3b4c9292414d Mon Sep 17 00:00:00 2001 From: Tucker Tibbetts Date: Tue, 3 Nov 2015 14:35:50 -0500 Subject: [PATCH 2/7] Add to contributors file --- CONTRIBUTORS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index dc5f96e9fb38..eb2fc573c319 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -73,5 +73,6 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for details on how to contribute to Cesiu * [Cole Murphy](https://github.com/fantasticole) * [Keat Tang](https://github.com/keattang) * [Denver Pierce](https://github.com/denverpierce) +* [Tucker Tibbetts](https://github.com/cttibbetts) Also see [our contributors page](http://cesiumjs.org/contributors.html) for more information. From ed60b7485045df87e9a375da484c4ff541e6e2da Mon Sep 17 00:00:00 2001 From: Tucker Tibbetts Date: Tue, 3 Nov 2015 15:49:31 -0500 Subject: [PATCH 3/7] Adds ModelGraphics unit tests --- Specs/DataSources/ModelGraphicsSpec.js | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Specs/DataSources/ModelGraphicsSpec.js b/Specs/DataSources/ModelGraphicsSpec.js index cce969078988..5aaef97cb833 100644 --- a/Specs/DataSources/ModelGraphicsSpec.js +++ b/Specs/DataSources/ModelGraphicsSpec.js @@ -13,7 +13,8 @@ defineSuite([ uri : '0', scale : 1, show : false, - minimumPixelSize : 2 + minimumPixelSize : 2, + maximumScale: 200 }; var model = new ModelGraphics(options); @@ -21,11 +22,13 @@ defineSuite([ expect(model.scale).toBeInstanceOf(ConstantProperty); expect(model.show).toBeInstanceOf(ConstantProperty); expect(model.minimumPixelSize).toBeInstanceOf(ConstantProperty); + expect(model.maximumScale).toBeInstanceOf(ConstantProperty); expect(model.uri.getValue()).toEqual(options.uri); expect(model.scale.getValue()).toEqual(options.scale); expect(model.show.getValue()).toEqual(options.show); expect(model.minimumPixelSize.getValue()).toEqual(options.minimumPixelSize); + expect(model.maximumScale.getValue()).toEqual(options.maximumScale); }); it('merge assigns unassigned properties', function() { @@ -34,6 +37,7 @@ defineSuite([ source.show = new ConstantProperty(true); source.scale = new ConstantProperty(1.0); source.minimumPixelSize = new ConstantProperty(2.0); + source.maximumScale = new ConstantProperty(200.0); var target = new ModelGraphics(); target.merge(source); @@ -42,6 +46,7 @@ defineSuite([ expect(target.show).toBe(source.show); expect(target.scale).toBe(source.scale); expect(target.minimumPixelSize).toBe(source.minimumPixelSize); + expect(target.maximumScale).toBe(source.maximumScale); }); it('merge does not assign assigned properties', function() { @@ -50,17 +55,20 @@ defineSuite([ source.show = new ConstantProperty(true); source.scale = new ConstantProperty(1.0); source.minimumPixelSize = new ConstantProperty(2.0); + source.maximumScale = new ConstantProperty(200.0); var uri = new ConstantProperty(''); var show = new ConstantProperty(true); var scale = new ConstantProperty(1.0); var minimumPixelSize = new ConstantProperty(2.0); + var maximumScale = new ConstantProperty(200.0); var target = new ModelGraphics(); target.uri = uri; target.show = show; target.scale = scale; target.minimumPixelSize = minimumPixelSize; + target.maximumScale = maximumScale; target.merge(source); @@ -68,6 +76,7 @@ defineSuite([ expect(target.show).toBe(show); expect(target.scale).toBe(scale); expect(target.minimumPixelSize).toBe(minimumPixelSize); + expect(target.maximumScale).toBe(maximumScale); }); it('clone works', function() { @@ -76,12 +85,14 @@ defineSuite([ source.show = new ConstantProperty(true); source.scale = new ConstantProperty(1.0); source.minimumPixelSize = new ConstantProperty(2.0); + source.maximumScale = new ConstantProperty(200.0); var result = source.clone(); expect(result.uri).toBe(source.uri); expect(result.show).toBe(source.show); expect(result.scale).toBe(source.scale); expect(result.minimumPixelSize).toBe(source.minimumPixelSize); + expect(result.maximumScale).toBe(source.maximumScale); }); it('merge throws if source undefined', function() { From cdbc2c138a9ad81fdcd741908952e96e4c4897de Mon Sep 17 00:00:00 2001 From: Tucker Tibbetts Date: Tue, 3 Nov 2015 15:52:00 -0500 Subject: [PATCH 4/7] Code review changes --- CHANGES.md | 1 + Source/DataSources/ModelGraphics.js | 4 +++- Source/Scene/Model.js | 7 +++---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 316b9cd9c74f..2eb4887c2dcb 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,7 @@ Change Log * Fixed an issue where the sun texture is not generated correctly on some mobile devices. [#3141](https://github.com/AnalyticalGraphicsInc/cesium/issues/3141) * Fixed a bug in the deprecated `jsonp` that prevented it from returning a promise. Its replacement, `loadJsonp`, was unaffected. * Fixed glTF implementation to read the version as a string as per the specification and to correctly handle backwards compatibility for axis-angle rotations in glTF 0.8 models. +* Added a maximumScale property to models, giving an upper limit for minimumPixelSize. ### 1.15 - 2015-11-02 diff --git a/Source/DataSources/ModelGraphics.js b/Source/DataSources/ModelGraphics.js index 744e945b4fc5..1e976495eb02 100644 --- a/Source/DataSources/ModelGraphics.js +++ b/Source/DataSources/ModelGraphics.js @@ -97,7 +97,9 @@ define([ /** * Gets or sets the numeric Property specifying the maximum scale * size of a model. This property is used as an upper limit for - * minimumPixelSize. + * {@link ModelGraphics#minimumPixelSize}. + * @memberof ModelGraphics.prototype + * @type {Property} */ maximumScale : createPropertyDescriptor('maximumScale'), diff --git a/Source/Scene/Model.js b/Source/Scene/Model.js index b682495b1c2e..1f4524fbb8d0 100644 --- a/Source/Scene/Model.js +++ b/Source/Scene/Model.js @@ -418,13 +418,12 @@ define([ /** * The maximum scale size for a model. This can be used to give - * an upper limit to the minimumPixelSize, ensuring that the model + * an upper limit to the {@link Model#minimumPixelSize}, ensuring that the model * is never an unreasonable scale. - * * @type {Number} */ - this.maximumScale = defaultValue(options.maximumScale, null); + this.maximumScale = options.maximumScale; this._maximumScale = this.maximumScale; /** @@ -2880,7 +2879,7 @@ define([ } } - return (model.maximumScale && model.maximumScale < scale) ? model.maximumScale : scale; + return defined(model.maximumScale) ? Math.min(model.maximumScale, scale) : scale; } function releaseCachedGltf(model) { From b4d11e8b03286a5e15ce329c183822c63a9467ca Mon Sep 17 00:00:00 2001 From: Tucker Tibbetts Date: Tue, 3 Nov 2015 16:07:03 -0500 Subject: [PATCH 5/7] Added Model unit tests, added new property to 3d model sandcastle --- Apps/Sandcastle/gallery/3D Models.html | 3 ++- CHANGES.md | 2 +- Specs/Scene/ModelSpec.js | 4 ++++ 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Apps/Sandcastle/gallery/3D Models.html b/Apps/Sandcastle/gallery/3D Models.html index c7d517302cd5..d02f6d65cc75 100644 --- a/Apps/Sandcastle/gallery/3D Models.html +++ b/Apps/Sandcastle/gallery/3D Models.html @@ -47,7 +47,8 @@ orientation : orientation, model : { uri : url, - minimumPixelSize : 128 + minimumPixelSize : 128, + maximumScale : 20000 } }); viewer.trackedEntity = entity; diff --git a/CHANGES.md b/CHANGES.md index 2eb4887c2dcb..304157ff1088 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,7 +6,7 @@ Change Log * Fixed an issue where the sun texture is not generated correctly on some mobile devices. [#3141](https://github.com/AnalyticalGraphicsInc/cesium/issues/3141) * Fixed a bug in the deprecated `jsonp` that prevented it from returning a promise. Its replacement, `loadJsonp`, was unaffected. * Fixed glTF implementation to read the version as a string as per the specification and to correctly handle backwards compatibility for axis-angle rotations in glTF 0.8 models. -* Added a maximumScale property to models, giving an upper limit for minimumPixelSize. +* Added `Model.maximumScale` and `ModelGraphics.maximumScale` properties, giving an upper limit for minimumPixelSize. ### 1.15 - 2015-11-02 diff --git a/Specs/Scene/ModelSpec.js b/Specs/Scene/ModelSpec.js index 753be25a5f26..5f41e8c82813 100644 --- a/Specs/Scene/ModelSpec.js +++ b/Specs/Scene/ModelSpec.js @@ -88,6 +88,7 @@ defineSuite([ })); modelPromises.push(loadModel(cesiumAirUrl, { minimumPixelSize : 1, + maximumScale : 200, asynchronous : false }).then(function(model) { cesiumAirModel = model; @@ -126,6 +127,7 @@ defineSuite([ show : false, scale : options.scale, minimumPixelSize : options.minimumPixelSize, + maximumScale : options.maximumScale, id : url, // for picking tests asynchronous : options.asynchronous, releaseGltfJson : options.releaseGltfJson, @@ -172,6 +174,7 @@ defineSuite([ expect(texturedBoxModel.modelMatrix).toEqual(modelMatrix); expect(texturedBoxModel.scale).toEqual(1.0); expect(texturedBoxModel.minimumPixelSize).toEqual(0.0); + expect(texturedBoxModel.maximumScale).toBeUndefined(); expect(texturedBoxModel.id).toEqual(texturedBoxUrl); expect(texturedBoxModel.allowPicking).toEqual(true); expect(texturedBoxModel.activeAnimations).toBeDefined(); @@ -672,6 +675,7 @@ defineSuite([ it('loads cesiumAir', function() { expect(cesiumAirModel.minimumPixelSize).toEqual(1); + expect(cesiumAirModel.maximumScale).toEqual(200); expect(cesiumAirModel.asynchronous).toEqual(false); }); From bdcf9916fc6d3faca5e93f075492d93bf0756940 Mon Sep 17 00:00:00 2001 From: Tucker Tibbetts Date: Wed, 4 Nov 2015 09:47:08 -0500 Subject: [PATCH 6/7] Switched to use getValueOrUndefined --- Source/DataSources/ModelVisualizer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/DataSources/ModelVisualizer.js b/Source/DataSources/ModelVisualizer.js index 33fe9f258313..f2824c89b2da 100644 --- a/Source/DataSources/ModelVisualizer.js +++ b/Source/DataSources/ModelVisualizer.js @@ -125,7 +125,7 @@ define([ model.show = true; model.scale = Property.getValueOrDefault(modelGraphics._scale, time, defaultScale); model.minimumPixelSize = Property.getValueOrDefault(modelGraphics._minimumPixelSize, time, defaultMinimumPixelSize); - model.maximumScale = Property.getValueOrDefault(modelGraphics._maximumScale, time, null); + model.maximumScale = Property.getValueOrUndefined(modelGraphics._maximumScale, time); model.modelMatrix = Matrix4.clone(modelMatrix, model.modelMatrix); } return true; From c9e402b0a055202d45ff2d42df5c64d48690abc0 Mon Sep 17 00:00:00 2001 From: Patrick Cozzi Date: Wed, 4 Nov 2015 17:15:08 -0500 Subject: [PATCH 7/7] Subtle interaction of Model.scale and maximumScale --- Source/Scene/Model.js | 9 ++++++--- Specs/Scene/ModelSpec.js | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Source/Scene/Model.js b/Source/Scene/Model.js index 1f4524fbb8d0..c277f27bf21d 100644 --- a/Source/Scene/Model.js +++ b/Source/Scene/Model.js @@ -616,7 +616,7 @@ define([ /** * The model's bounding sphere in its local coordinate system. This does not take into - * account glTF animations and skins. + * account glTF animations and skins nor does it take into account {@link Model#minimumPixelSize}. * * @memberof Model.prototype * @@ -640,7 +640,8 @@ define([ //>>includeEnd('debug'); var nonUniformScale = Matrix4.getScale(this.modelMatrix, boundingSphereCartesian3Scratch); - Cartesian3.multiplyByScalar(nonUniformScale, this.scale, nonUniformScale); + var scale = defined(this.maximumScale) ? Math.min(this.maximumScale, this.scale) : this.scale; + Cartesian3.multiplyByScalar(nonUniformScale, scale, nonUniformScale); var scaledBoundingSphere = this._scaledBoundingSphere; scaledBoundingSphere.center = Cartesian3.multiplyComponents(this._boundingSphere.center, nonUniformScale, scaledBoundingSphere.center); @@ -3065,12 +3066,14 @@ define([ // Model's model matrix needs to be updated var modelTransformChanged = !Matrix4.equals(this._modelMatrix, this.modelMatrix) || (this._scale !== this.scale) || - (this._minimumPixelSize !== this.minimumPixelSize) || (this.minimumPixelSize !== 0.0); // Minimum pixel size changed or is enabled + (this._minimumPixelSize !== this.minimumPixelSize) || (this.minimumPixelSize !== 0.0) || // Minimum pixel size changed or is enabled + (this._maximumScale !== this.maximumScale); if (modelTransformChanged || justLoaded) { Matrix4.clone(this.modelMatrix, this._modelMatrix); this._scale = this.scale; this._minimumPixelSize = this.minimumPixelSize; + this._maximumScale = this.maximumScale; var scale = getScale(this, context, frameState); var computedModelMatrix = this._computedModelMatrix; diff --git a/Specs/Scene/ModelSpec.js b/Specs/Scene/ModelSpec.js index 5f41e8c82813..b9ec15dda770 100644 --- a/Specs/Scene/ModelSpec.js +++ b/Specs/Scene/ModelSpec.js @@ -451,6 +451,20 @@ defineSuite([ texturedBoxModel.scale = originalScale; }); + it('boundingSphere returns the bounding sphere when maximumScale is reached', function() { + var originalScale = texturedBoxModel.scale; + var originalMaximumScale = texturedBoxModel.maximumScale; + texturedBoxModel.scale = 20; + texturedBoxModel.maximumScale = 10; + + var boundingSphere = texturedBoxModel.boundingSphere; + expect(boundingSphere.center).toEqualEpsilon(new Cartesian3(0.0, -2.5, 0.0), CesiumMath.EPSILON3); + expect(boundingSphere.radius).toEqualEpsilon(7.5, CesiumMath.EPSILON3); + + texturedBoxModel.scale = originalScale; + texturedBoxModel.maximumScale = originalMaximumScale; + }); + it('boundingSphere returns the bounding sphere when modelMatrix has non-uniform scale', function() { var originalMatrix = Matrix4.clone(texturedBoxModel.modelMatrix); Matrix4.multiplyByScale(texturedBoxModel.modelMatrix, new Cartesian3(2, 5, 10), texturedBoxModel.modelMatrix);