Merge pull request #1101 from OpenGeoscience/remove-promise

feat: Improve when tile layers are idle.

src/object.js

@@ -1,6 +1,8 @@
 var inherit = require('./inherit');
 var timestamp = require('./timestamp');
 
+var lastInternalId = 0;
+
 /**
  * Create a new instance of class object.
  *
@@ -18,6 +20,7 @@ var object = function () {
   var util = require('./util');
 
   var m_this = this,
+      m_internalId = ++lastInternalId,
       m_eventHandlers = {},
       m_idleHandlers = [],
       m_promiseCount = 0;
@@ -77,15 +80,24 @@ var object = function () {
   this.addPromise = function (promise) {
     // called on any resolution of the promise
     function onDone() {
-      m_promiseCount -= 1;
+      if (promise._geojsTracked && promise._geojsTracked[m_internalId]) {
+        m_promiseCount -= 1;
+        delete promise._geojsTracked[m_internalId];
+      }
       if (!m_promiseCount) {
         m_idleHandlers.splice(0, m_idleHandlers.length)
           .forEach(function (handler) {
             handler();
           });
       }
     }
-    m_promiseCount += 1;
+    if (!promise._geojsTracked) {
+      promise._geojsTracked = {};
+    }
+    if (!promise._geojsTracked[m_internalId]) {
+      promise._geojsTracked[m_internalId] = true;
+      m_promiseCount += 1;
+    }
     if (promise.always) {
       promise.always(onDone);
     } else {
@@ -94,6 +106,27 @@ var object = function () {
     return m_this;
   };
 
+  /**
+   * Mark a promise as no longer required to resolve before the idle state is
+   * reached.
+   *
+   * @param {Promise} promise A promise object.
+   * @returns {this}
+   */
+  this.removePromise = function (promise) {
+    if (promise._geojsTracked && promise._geojsTracked[m_internalId]) {
+      m_promiseCount -= 1;
+      delete promise._geojsTracked[m_internalId];
+      if (!m_promiseCount) {
+        m_idleHandlers.splice(0, m_idleHandlers.length)
+          .forEach(function (handler) {
+            handler();
+          });
+      }
+    }
+    return m_this;
+  };
+
   /**
    * Bind an event handler to this object.
    *
@@ -217,6 +250,8 @@ var object = function () {
       m_eventHandlers = {};
       m_idleHandlers = [];
       m_promiseCount = 0;
+      // assign a new id so that adding and removing promises behave properly
+      m_internalId = ++lastInternalId;
     }
     if (Array.isArray(event)) {
       event.forEach(function (e) {

src/sceneObject.js

@@ -23,7 +23,8 @@ var sceneObject = function (arg) {
       m_children = [],
       s_exit = this._exit,
       s_trigger = this.geoTrigger,
-      s_addPromise = this.addPromise;
+      s_addPromise = this.addPromise,
+      s_removePromise = this.removePromise;
 
   /**
    * Add the promise here and also propagate up the scene tree.
@@ -39,6 +40,20 @@ var sceneObject = function (arg) {
     return m_this;
   };
 
+  /**
+   * Remove the promise here and also propagate up the scene tree.
+   *
+   * @param {Promise} promise A promise object.
+   * @returns {this}
+   */
+  this.removePromise = function (promise) {
+    if (m_parent) {
+      m_parent.removePromise(promise);
+    }
+    s_removePromise(promise);
+    return m_this;
+  };
+
   /**
    * Get/set parent of the object.
    *

src/tileLayer.js

@@ -72,6 +72,11 @@ var featureLayer = require('./featureLayer');
  *   bottom-up (the ingcs does not matter, only the gcs coordinate system).
  *   When falsy, this inverts the gcs y-coordinate when calculating local
  *   coordinates.
+ * @property {string} [idleAfter='view'] Consider the layer as idle once a
+ *   specific set of tiles is loaded.  'view' is when all tiles in view are
+ *   loaded.  'all' is when tiles in view and tiles that were once requested
+ *   have been loaded (this corresponds to having all network activity
+ *   finished).
  */
 
 /**
@@ -210,6 +215,7 @@ var tileLayer = function (arg) {
       m_queueSize = arg.queueSize || 6,
       m_initialQueueSize = arg.initialQueueSize || 0,
       m_lastTileSet = [],
+      m_promisedTiles = {},
       m_maxBounds = [],
       m_reference,
       m_exited,
@@ -1305,6 +1311,7 @@ var tileLayer = function (arg) {
 
     tiles.forEach(function (tile) {
       if (tile.fetched()) {
+        delete m_promisedTiles[tile.toString()];
         /* if we have already fetched the tile, we know we can just draw it,
          * as the bounds won't have changed since the call to _getTiles. */
         m_this.drawTile(tile);
@@ -1341,7 +1348,6 @@ var tileLayer = function (arg) {
             m_this._setTileTree(tile);
           });
 
-          m_this.addPromise(tile);
           tile._queued = true;
         } else {
           /* If we are using a fetch queue, tell the queue so this tile can
@@ -1351,6 +1357,8 @@ var tileLayer = function (arg) {
             m_this._queue.add(tile);
           }
         }
+        m_this.addPromise(tile);
+        m_promisedTiles[tile.toString()] = tile;
       }
     });
     // purge all old tiles when the new tiles are loaded (successfully or not)
@@ -1363,6 +1371,17 @@ var tileLayer = function (arg) {
           m_this._purge(zoom, true);
         }
       );
+    // for tiles that aren't in view, remove them from the list of tiles that
+    // are needed to be loaded to be considered idle.
+    if (m_this._options.idleAfter !== 'all') {
+      for (const hash in m_promisedTiles) {
+        const tile = m_promisedTiles[hash];
+        if (tiles.indexOf(tile) < 0) {
+          m_this.removePromise(tile);
+          delete m_promisedTiles[hash];
+        }
+      }
+    }
     return m_this;
   };
 
@@ -1506,7 +1525,7 @@ var tileLayer = function (arg) {
     } else {
       /* For tile layers that should only keep one layer, if loading is
        * finished, purge all but the current layer.  This is important for
-       * semi-transparanet layers. */
+       * semi-transparent layers. */
       if ((doneLoading || m_this._isCovered(tile)) &&
           zoom !== tile.index.level) {
         return true;
@@ -1699,6 +1718,7 @@ tileLayer.defaults = {
   tilesMaxBounds: null,
   topDown: false,
   keepLower: true,
+  idleAfter: 'view',
   // cacheSize: 400,  // set depending on keepLower
   tileRounding: Math.round,
   attribution: '',

tests/cases/sceneObject.js

@@ -311,5 +311,32 @@ describe('geo.sceneObject', function () {
       }, 50);
 
     });
+    it('removePromise', function () {
+      var defer = $.Deferred(),
+          handlerRoot = new CallCounter(null),
+          handler1 = new CallCounter(null),
+          handler2 = new CallCounter(null),
+          handler3 = new CallCounter(null);
+
+      function checkCallCount(countr, count1, count2, count3) {
+        expect(handlerRoot.ncalls).toBe(countr);
+        expect(handler1.ncalls).toBe(count1);
+        expect(handler2.ncalls).toBe(count2);
+        expect(handler3.ncalls).toBe(count3);
+      }
+
+      child3.addPromise(defer);
+      root.onIdle(handlerRoot.call);
+      child1.onIdle(handler1.call);
+      child2.onIdle(handler2.call);
+      child3.onIdle(handler3.call);
+
+      checkCallCount(0, 1, 0, 0);
+
+      child3.removePromise(defer);
+      checkCallCount(1, 1, 1, 1);
+
+      defer.resolve();
+    });
   });
 });

tests/cases/tileLayer.js

@@ -342,6 +342,7 @@ describe('geo.tileLayer', function () {
       tileHeight: 1024,
       cacheSize: 100,
       keepLower: true,
+      idleAfter: 'view',
       wrapX: false,
       wrapY: true,
       url: function () {},