Merge pull request #2754 from AnalyticalGraphicsInc/ancestor-show

Make Entity.show work properly for all descendants.

CHANGES.md

@@ -19,6 +19,7 @@ Change Log
 * `TileProviderError` now optionally takes an `error` parameter with more details of the error or exception that occurred.  `ImageryLayer` passes that information through when tiles fail to load.  This allows tile provider error handling to take a different action when a tile returns a 404 versus a 500, for example.
 * `ArcGisMapServerImageryProvider` now has a `maximumLevel` constructor parameter.
 * `ArcGisMapServerImageryProvider` picking now works correctly when the `layers` parameter is specified.  Previously, it would pick from all layers even if only displaying a subset.
+* Setting `Entity.show` now properly toggles the display of all descendant entities, previously it was only affecting its direct children.
 * Fixed a crash caused by `ImageryLayer` attempting to generate mipmaps for textures that are not a power-of-two size.
 * Fixed a bug where `ImageryLayerCollection.pickImageryLayerFeatures` would return incorrect results when picking from a terrain tile that was partially covered by correct-level imagery and partially covered by imagery from an ancestor level.
 * Fixed incorrect counting of `debug.tilesWaitingForChildren` in `QuadtreePrimitive`.

Source/DataSources/Entity.js

@@ -176,16 +176,15 @@ define([
         this.merge(options);
     };
 
-    function updateShow(entity, isShowing) {
-        var children = entity._children;
+    function updateShow(entity, children, isShowing) {
         var length = children.length;
         for (var i = 0; i < length; i++) {
             var child = children[i];
             var childShow = child._show;
             var oldValue = !isShowing && childShow;
             var newValue = isShowing && childShow;
             if (oldValue !== newValue) {
-                child._definitionChanged.raiseEvent(child, 'isShowing', newValue, oldValue);
+                updateShow(child, child._children, isShowing);
             }
         }
         entity._definitionChanged.raiseEvent(entity, 'isShowing', isShowing, !isShowing);
@@ -257,7 +256,7 @@ define([
                 var isShowing = this.isShowing;
 
                 if (wasShowing !== isShowing) {
-                    updateShow(this, isShowing);
+                    updateShow(this, this._children, isShowing);
                 }
 
                 this._definitionChanged.raiseEvent(this, 'show', value, !value);
@@ -271,7 +270,7 @@ define([
          */
         isShowing : {
             get : function() {
-                return this._show && (!defined(this._parent) || this._parent._show);
+                return this._show && (!defined(this._parent) || this._parent.isShowing);
             }
         },
         /**
@@ -302,7 +301,7 @@ define([
                 var isShowing = this.isShowing;
 
                 if (wasShowing !== isShowing) {
-                    updateShow(this, isShowing);
+                    updateShow(this, this._children, isShowing);
                 }
 
                 this._definitionChanged.raiseEvent(this, 'parent', value, oldValue);

Specs/DataSources/EntitySpec.js

@@ -384,16 +384,13 @@ defineSuite([
         expect(entity.isShowing).toBe(false);
     });
 
-    it('isShowing works with parent.', function() {
-        var entity = new Entity();
-        entity.parent = new Entity();
-
+    function ancestorShowTest(entity, ancestor) {
         var listener = jasmine.createSpy('listener');
         entity.definitionChanged.addEventListener(listener);
 
-        entity.parent.show = false;
+        ancestor.show = false;
 
-        //Setting entity.parent show to false causes entity to raise
+        //Setting ancestor show to false causes entity to raise
         //its own isShowing event, but not the show event.
         expect(listener.calls.count()).toBe(1);
         expect(listener.calls.argsFor(0)).toEqual([entity, 'isShowing', false, true]);
@@ -411,23 +408,39 @@ defineSuite([
 
         listener.calls.reset();
 
-        //Setting parent show to true does not trigger the entity.isShowing event
+        //Setting ancestor show to true does not trigger the entity.isShowing event
         //because entity.show is false;
-        entity.parent.show = true;
+        ancestor.show = true;
         expect(entity.show).toBe(false);
         expect(entity.isShowing).toBe(false);
         expect(listener.calls.count()).toBe(0);
 
         listener.calls.reset();
 
         //Setting entity.show to try now causes both events to be raised
-        //because the parent is also showing.
+        //because the ancestor is also showing.
         entity.show = true;
         expect(listener.calls.count()).toBe(2);
         expect(listener.calls.argsFor(0)).toEqual([entity, 'isShowing', true, false]);
         expect(listener.calls.argsFor(1)).toEqual([entity, 'show', true, false]);
         expect(entity.show).toBe(true);
         expect(entity.isShowing).toBe(true);
+    }
+
+    it('isShowing works with parent.', function() {
+        var parent = new Entity();
+        var entity = new Entity();
+        entity.parent = parent;
+        ancestorShowTest(entity, parent);
+    });
+
+    it('isShowing works with grandparent.', function() {
+        var grandparent = new Entity();
+        var parent = new Entity();
+        parent.parent = grandparent;
+        var entity = new Entity();
+        entity.parent = parent;
+        ancestorShowTest(entity, grandparent);
     });
 
     it('isShowing works when replacing parent.', function() {