From f1d207eba89210e3bd7b5a57a56ba8d17d32f1dd Mon Sep 17 00:00:00 2001 From: Brad Date: Fri, 6 Nov 2015 13:17:20 -0500 Subject: [PATCH 1/5] Adds the 'selectable' member to the Entity class Entity now has a 'selectable' member which determines whether the given Entity is selectable by the user. 'selectable' defaults to true, and can be set via the 'selectable' field in the options parameter of the Entity constructor. Additionally, CzmlDataSource has been updated to handle the 'selectable' field and set an Entities 'selectable' value. --- Source/DataSources/CzmlDataSource.js | 5 +++++ Source/DataSources/Entity.js | 10 ++++++++++ Source/Renderer/Context.js | 2 +- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Source/DataSources/CzmlDataSource.js b/Source/DataSources/CzmlDataSource.js index 43cfb2ab268c..05b747323a59 100644 --- a/Source/DataSources/CzmlDataSource.js +++ b/Source/DataSources/CzmlDataSource.js @@ -856,6 +856,10 @@ define([ function processName(entity, packet, entityCollection, sourceUri) { entity.name = defaultValue(packet.name, entity.name); } + + function processSelectble(entity, packet, entityCollection, sourceUri) { + entity.selectable = devaultValue(packet.selectable, true); + } function processDescription(entity, packet, entityCollection, sourceUri) { var descriptionData = packet.description; @@ -1633,6 +1637,7 @@ define([ processLabel, // processModel, // processName, // + processSelectable, processDescription, // processPath, // processPoint, // diff --git a/Source/DataSources/Entity.js b/Source/DataSources/Entity.js index e86bb28c8c19..db7771f62da1 100644 --- a/Source/DataSources/Entity.js +++ b/Source/DataSources/Entity.js @@ -92,6 +92,7 @@ define([ * @param {String} [options.name] A human readable name to display to users. It does not have to be unique. * @param {TimeIntervalCollection} [options.availability] The availability, if any, associated with this object. * @param {Boolean} [options.show] A boolean value indicating if the entity and its children are displayed. + * @param {Boolean} [options.selectable] A boolean value indicating whether the entity is selectable. Default true. * @param {Property} [options.description] A string Property specifying an HTML description for this entity. * @param {PositionProperty} [options.position] A Property specifying the entity position. * @param {Property} [options.orientation] A Property specifying the entity orientation. @@ -128,6 +129,7 @@ define([ this._definitionChanged = new Event(); this._name = options.name; this._show = defaultValue(options.show, true); + this._selectable = defaultValue(options.selectable, true); this._parent = undefined; this._propertyNames = ['billboard', 'box', 'corridor', 'cylinder', 'description', 'ellipse', // 'ellipsoid', 'label', 'model', 'orientation', 'path', 'point', 'polygon', // @@ -269,6 +271,14 @@ define([ this._definitionChanged.raiseEvent(this, 'show', value, !value); } }, + + /** + * Gets or sets whether this entity is selectabled or not + * @memberof Entity.prototype + * @type {Boolean} + */ + selectable: createPropertyDescriptor('selectable'), + /** * Gets whether this entity is being displayed, taking into account * the visibility of any ancestor entities. diff --git a/Source/Renderer/Context.js b/Source/Renderer/Context.js index e261c4a1cb7d..55da4e41aec2 100644 --- a/Source/Renderer/Context.js +++ b/Source/Renderer/Context.js @@ -1112,7 +1112,7 @@ define([ } //>>includeEnd('debug'); - return this._pickObjects[pickColor.toRgba()]; + return ((pickColor = this._pickObjects[pickColor.toRgba()]) && pickColor.id.selectable) ? pickColor : undefined; }; function PickId(pickObjects, key, color) { From cf55ad53890ab7b70713644e0b59e514a35a91df Mon Sep 17 00:00:00 2001 From: Brad Date: Fri, 6 Nov 2015 14:18:34 -0500 Subject: [PATCH 2/5] Fixed Typos Fixed typos after testing --- Source/DataSources/CzmlDataSource.js | 4 ++-- Source/DataSources/Entity.js | 14 +++++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Source/DataSources/CzmlDataSource.js b/Source/DataSources/CzmlDataSource.js index 05b747323a59..de823c9fa562 100644 --- a/Source/DataSources/CzmlDataSource.js +++ b/Source/DataSources/CzmlDataSource.js @@ -857,8 +857,8 @@ define([ entity.name = defaultValue(packet.name, entity.name); } - function processSelectble(entity, packet, entityCollection, sourceUri) { - entity.selectable = devaultValue(packet.selectable, true); + function processSelectable(entity, packet, entityCollection, sourceUri) { + entity.selectable = defaultValue(packet.selectable, true); } function processDescription(entity, packet, entityCollection, sourceUri) { diff --git a/Source/DataSources/Entity.js b/Source/DataSources/Entity.js index db7771f62da1..e082f5d8bc09 100644 --- a/Source/DataSources/Entity.js +++ b/Source/DataSources/Entity.js @@ -277,7 +277,19 @@ define([ * @memberof Entity.prototype * @type {Boolean} */ - selectable: createPropertyDescriptor('selectable'), + selectable: { + get: function() { + return this._selectable; + }, + set: function(value) { + if (!defined(value)) + throw new DeveloperError('value is required'); + + + if (value === this._selectable) return; + this._selectable = value; + } + }, /** * Gets whether this entity is being displayed, taking into account From eb5ecd5033aea3a98d59dc18b7bdaaa763b1f316 Mon Sep 17 00:00:00 2001 From: Brad Date: Fri, 6 Nov 2015 14:37:24 -0500 Subject: [PATCH 3/5] jsHint fix jsHint doesn't like my bad coding conventions --- Source/DataSources/Entity.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Source/DataSources/Entity.js b/Source/DataSources/Entity.js index e082f5d8bc09..2a909efde9d2 100644 --- a/Source/DataSources/Entity.js +++ b/Source/DataSources/Entity.js @@ -282,11 +282,13 @@ define([ return this._selectable; }, set: function(value) { - if (!defined(value)) + if (!defined(value)) { throw new DeveloperError('value is required'); + } - - if (value === this._selectable) return; + if (value === this._selectable) { + return; + } this._selectable = value; } }, From aa5427071d9e72f8fc6d56fd8d9c46cc1c03cd9b Mon Sep 17 00:00:00 2001 From: Brad Date: Fri, 4 Dec 2015 11:56:13 -0500 Subject: [PATCH 4/5] Added Unit Tests for Selectable Entities Added unit tests and also fixed some bad assumptions which were exposed by existing unit tests --- Source/Renderer/Context.js | 2 +- Source/Renderer/PickFramebuffer.js | 3 +++ Specs/DataSources/CzmlDataSourceSpec.js | 14 ++++++++++++++ Specs/DataSources/EntitySpec.js | 3 +++ Specs/Widgets/Viewer/ViewerSpec.js | 20 ++++++++++++++++++++ 5 files changed, 41 insertions(+), 1 deletion(-) diff --git a/Source/Renderer/Context.js b/Source/Renderer/Context.js index 55da4e41aec2..e261c4a1cb7d 100644 --- a/Source/Renderer/Context.js +++ b/Source/Renderer/Context.js @@ -1112,7 +1112,7 @@ define([ } //>>includeEnd('debug'); - return ((pickColor = this._pickObjects[pickColor.toRgba()]) && pickColor.id.selectable) ? pickColor : undefined; + return this._pickObjects[pickColor.toRgba()]; }; function PickId(pickObjects, key, color) { diff --git a/Source/Renderer/PickFramebuffer.js b/Source/Renderer/PickFramebuffer.js index a81dcb738734..b3070ba9f70a 100644 --- a/Source/Renderer/PickFramebuffer.js +++ b/Source/Renderer/PickFramebuffer.js @@ -114,6 +114,9 @@ define([ var object = context.getObjectByPickColor(colorScratch); if (defined(object)) { + if (object.id && (typeof object.id === 'object')) { + return (object.id.selectable) ? object : undefined; + } return object; } } diff --git a/Specs/DataSources/CzmlDataSourceSpec.js b/Specs/DataSources/CzmlDataSourceSpec.js index cf44481f5ebd..4b24c0a5f426 100644 --- a/Specs/DataSources/CzmlDataSourceSpec.js +++ b/Specs/DataSources/CzmlDataSourceSpec.js @@ -2091,4 +2091,18 @@ defineSuite([ expect(error.message).toContain('CZML version information invalid. It is expected to be a property on the document object in the . version format.'); }); }); + + it('sets selectable according to the CZML field', function() { + var packet = { + position : { + cartesian : [1.0, 2.0, 3.0] + }, + selectable : false + }; + + var dataSource = new CzmlDataSource(); + dataSource.load(makePacket(packet)); + var entity = dataSource.entities.values[0]; + expect(entity.selectable).toBe(false); + }); }); diff --git a/Specs/DataSources/EntitySpec.js b/Specs/DataSources/EntitySpec.js index 35b34c13d103..ba2eeb4d99a2 100644 --- a/Specs/DataSources/EntitySpec.js +++ b/Specs/DataSources/EntitySpec.js @@ -80,11 +80,13 @@ defineSuite([ expect(entity.viewFrom).toBeUndefined(); expect(entity.wall).toBeUndefined(); expect(entity.entityCollection).toBeUndefined(); + expect(entity.selectable).toBe(true); var options = { id : 'someId', name : 'bob', show : false, + selectable: false, availability : new TimeIntervalCollection(), parent : new Entity(), customProperty : {}, @@ -113,6 +115,7 @@ defineSuite([ expect(entity.id).toEqual(options.id); expect(entity.name).toEqual(options.name); expect(entity.show).toBe(options.show); + expect(entity.selectable).toBe(options.selectable); expect(entity.availability).toBe(options.availability); expect(entity.parent).toBe(options.parent); expect(entity.customProperty).toBe(options.customProperty); diff --git a/Specs/Widgets/Viewer/ViewerSpec.js b/Specs/Widgets/Viewer/ViewerSpec.js index db5aef069274..465ebbe367ee 100644 --- a/Specs/Widgets/Viewer/ViewerSpec.js +++ b/Specs/Widgets/Viewer/ViewerSpec.js @@ -2,12 +2,14 @@ defineSuite([ 'Widgets/Viewer/Viewer', 'Core/Cartesian3', + 'Core/Cartesian2', 'Core/ClockRange', 'Core/ClockStep', 'Core/EllipsoidTerrainProvider', 'Core/JulianDate', 'Core/Matrix4', 'Core/WebMercatorProjection', + 'Core/Rectangle', 'DataSources/ConstantPositionProperty', 'DataSources/ConstantProperty', 'DataSources/DataSourceClock', @@ -36,12 +38,14 @@ defineSuite([ ], function( Viewer, Cartesian3, + Cartesian2, ClockRange, ClockStep, EllipsoidTerrainProvider, JulianDate, Matrix4, WebMercatorProjection, + Rectangle, ConstantPositionProperty, ConstantProperty, DataSourceClock, @@ -899,6 +903,22 @@ defineSuite([ }); }); }); + + it('cannot select an entity when selectable is false', function() { + var viewer = createViewer(container); + + var entity = viewer.entities.add({ + rectangle: { + coordinates: Rectangle.fromDegrees(-50.0, -50.0, 50.0, 50.0) + }, + selectable: false + }); + + var pickedObject = viewer.scene.pick(new Cartesian2(0, 0)); + expect(pickedObject).toBeUndefined(); + + viewer.destroy(); + }) it('does not crash when tracking an object with a position property whose value is undefined.', function() { viewer = createViewer(container); From e3fa7b0827273a151ae5e6487afaf5220421b61e Mon Sep 17 00:00:00 2001 From: Brad Date: Fri, 4 Dec 2015 12:10:11 -0500 Subject: [PATCH 5/5] jsHint fix --- Specs/Widgets/Viewer/ViewerSpec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Specs/Widgets/Viewer/ViewerSpec.js b/Specs/Widgets/Viewer/ViewerSpec.js index 465ebbe367ee..625e49ceb77c 100644 --- a/Specs/Widgets/Viewer/ViewerSpec.js +++ b/Specs/Widgets/Viewer/ViewerSpec.js @@ -907,7 +907,7 @@ defineSuite([ it('cannot select an entity when selectable is false', function() { var viewer = createViewer(container); - var entity = viewer.entities.add({ + viewer.entities.add({ rectangle: { coordinates: Rectangle.fromDegrees(-50.0, -50.0, 50.0, 50.0) }, @@ -918,7 +918,7 @@ defineSuite([ expect(pickedObject).toBeUndefined(); viewer.destroy(); - }) + }); it('does not crash when tracking an object with a position property whose value is undefined.', function() { viewer = createViewer(container);