From 1ae1e15a5009d81cb27e1f7297cedc8caf5bd57a Mon Sep 17 00:00:00 2001
From: wimrijnders <wrijnders@gmail.com>
Date: Fri, 14 Apr 2017 11:00:58 +0200
Subject: [PATCH] =?UTF-8?q?Network:=20use=20separate=20refresh=20indicator?=
 =?UTF-8?q?=20in=20NodeBase,=20instead=20of=20width=E2=80=A6=20(#2885)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Network: use separate refresh indicator in NodeBase, instead of width parameter

* Added usage of iBaseNode.doRefresh to image shapes.

* Rearranged code in NodeBase.needRefresh()

* Fixes for review

* Removed strange link

* Added 's' in needsRefresh(); typo fix

* Fixes for review, code cleanup

* Modified ratio calc to original working.

* Made code in CircularImage.resize() more sensible.
---
 .../network/events/interactionEvents.html     |  2 +
 lib/network/modules/NodesHandler.js           |  6 +-
 lib/network/modules/components/Node.js        | 12 ++-
 .../modules/components/nodes/shapes/Box.js    |  2 +-
 .../modules/components/nodes/shapes/Circle.js |  7 +-
 .../components/nodes/shapes/CircularImage.js  | 31 ++++----
 .../components/nodes/shapes/Database.js       |  2 +-
 .../components/nodes/shapes/Ellipse.js        |  2 +-
 .../modules/components/nodes/shapes/Icon.js   |  2 +-
 .../modules/components/nodes/shapes/Image.js  |  8 +-
 .../modules/components/nodes/shapes/Text.js   |  2 +-
 .../components/nodes/util/CircleImageBase.js  | 78 +++++++++----------
 .../modules/components/nodes/util/NodeBase.js | 18 +++++
 .../components/nodes/util/ShapeBase.js        |  2 +-
 14 files changed, 96 insertions(+), 78 deletions(-)

diff --git a/examples/network/events/interactionEvents.html b/examples/network/events/interactionEvents.html
index f9ba9c1e..538fbfad 100644
--- a/examples/network/events/interactionEvents.html
+++ b/examples/network/events/interactionEvents.html
@@ -24,6 +24,7 @@
 <pre id="eventSpan"></pre>
 
 <script type="text/javascript">
+
     // create an array with nodes
     var nodes = new vis.DataSet([
         {id: 1, label: 'Node 1', title: 'I have a popup!'},
@@ -53,6 +54,7 @@
     network.on("click", function (params) {
         params.event = "[original event]";
         document.getElementById('eventSpan').innerHTML = '<h2>Click event:</h2>' + JSON.stringify(params, null, 4);
+        console.log('click event, getNodeAt returns: ' + this.getNodeAt(params.pointer.DOM));
     });
     network.on("doubleClick", function (params) {
         params.event = "[original event]";
diff --git a/lib/network/modules/NodesHandler.js b/lib/network/modules/NodesHandler.js
index 2241d5b3..5ed50e24 100644
--- a/lib/network/modules/NodesHandler.js
+++ b/lib/network/modules/NodesHandler.js
@@ -171,7 +171,7 @@ class NodesHandler {
         for (let nodeId in this.body.nodes) {
           if (this.body.nodes.hasOwnProperty(nodeId)) {
             this.body.nodes[nodeId].updateLabelModule();
-            this.body.nodes[nodeId]._reset();
+            this.body.nodes[nodeId].needsRefresh();
           }
         }
       }
@@ -180,12 +180,12 @@ class NodesHandler {
       if (options.size !== undefined) {
         for (let nodeId in this.body.nodes) {
           if (this.body.nodes.hasOwnProperty(nodeId)) {
-            this.body.nodes[nodeId]._reset();
+            this.body.nodes[nodeId].needsRefresh();
           }
         }
       }
 
-      // update the state of the letiables if needed
+      // update the state of the variables if needed
       if (options.hidden !== undefined || options.physics !== undefined) {
         this.body.emitter.emit('_dataChanged');
       }
diff --git a/lib/network/modules/components/Node.js b/lib/network/modules/components/Node.js
index 996d3229..be4000ff 100644
--- a/lib/network/modules/components/Node.js
+++ b/lib/network/modules/components/Node.js
@@ -352,7 +352,7 @@ class Node {
           break;
       }
     }
-    this._reset();
+    this.needsRefresh();
   }
 
 
@@ -361,7 +361,7 @@ class Node {
    */
   select() {
     this.selected = true;
-    this._reset();
+    this.needsRefresh();
   }
 
 
@@ -370,18 +370,16 @@ class Node {
    */
   unselect() {
     this.selected = false;
-    this._reset();
+    this.needsRefresh();
   }
 
 
 
   /**
    * Reset the calculated size of the node, forces it to recalculate its size
-   * @private
    */
-  _reset() {
-    this.shape.width = undefined;
-    this.shape.height = undefined;
+  needsRefresh() {
+    this.shape.refreshNeeded = true;
   }
 
 
diff --git a/lib/network/modules/components/nodes/shapes/Box.js b/lib/network/modules/components/nodes/shapes/Box.js
index 7f92b66c..5b3eb7a3 100644
--- a/lib/network/modules/components/nodes/shapes/Box.js
+++ b/lib/network/modules/components/nodes/shapes/Box.js
@@ -9,7 +9,7 @@ class Box extends NodeBase {
   }
 
   resize(ctx, selected = this.selected, hover = this.hover) {
-    if ((this.width === undefined) || this.labelModule.differentState(selected, hover)) {
+    if (this.needsRefresh(selected, hover)) {
       this.textSize = this.labelModule.getTextSize(ctx, selected, hover);
       this.width = this.textSize.width + this.margin.right + this.margin.left;
       this.height = this.textSize.height + this.margin.top + this.margin.bottom;
diff --git a/lib/network/modules/components/nodes/shapes/Circle.js b/lib/network/modules/components/nodes/shapes/Circle.js
index a8f6d3bc..764d7340 100644
--- a/lib/network/modules/components/nodes/shapes/Circle.js
+++ b/lib/network/modules/components/nodes/shapes/Circle.js
@@ -8,8 +8,8 @@ class Circle extends CircleImageBase {
     this._setMargins(labelModule);
   }
 
-  resize(ctx, selected = this.selected, hover = this.hover, values = { size: this.options.size}) {
-    if ((this.width === undefined) || (this.labelModule.differentState(selected, hover))) {
+  resize(ctx, selected = this.selected, hover = this.hover) {
+    if (this.needsRefresh(selected, hover)) {
       this.textSize = this.labelModule.getTextSize(ctx, selected, hover);
       var diameter = Math.max(this.textSize.width + this.margin.right + this.margin.left,
                               this.textSize.height + this.margin.top + this.margin.bottom);
@@ -26,8 +26,9 @@ class Circle extends CircleImageBase {
     this.left = x - this.width / 2;
     this.top = y - this.height / 2;
 
-    this._drawRawCircle(ctx, x, y, selected, hover, values);
+    this._drawRawCircle(ctx, x, y, values);
 
+    // TODO: values overwritten by updateBoundingBox(); is this bit necessary?
     this.boundingBox.top = y - values.size;
     this.boundingBox.left = x - values.size;
     this.boundingBox.right = x + values.size;
diff --git a/lib/network/modules/components/nodes/shapes/CircularImage.js b/lib/network/modules/components/nodes/shapes/CircularImage.js
index 0eefe7bc..95c0b5ba 100644
--- a/lib/network/modules/components/nodes/shapes/CircularImage.js
+++ b/lib/network/modules/components/nodes/shapes/CircularImage.js
@@ -7,26 +7,23 @@ class CircularImage extends CircleImageBase {
     super(options, body, labelModule);
 
     this.setImages(imageObj, imageObjAlt);
-
-    this._swapToImageResizeWhenImageLoaded = true;
   }
 
   resize(ctx, selected = this.selected, hover = this.hover) {
-    if ((this.imageObj.src === undefined) ||
+    var imageAbsent = (this.imageObj.src === undefined) ||
         (this.imageObj.width === undefined) ||
-        (this.imageObj.height === undefined) ||
-        (this.labelModule.differentState(selected, hover))) {
+        (this.imageObj.height === undefined);
+
+    if (imageAbsent) {
       var diameter = this.options.size * 2;
       this.width = diameter;
       this.height = diameter;
-      this._swapToImageResizeWhenImageLoaded = true;
       this.radius = 0.5*this.width;
-    } else {
-      if (this._swapToImageResizeWhenImageLoaded) {
-        this.width = undefined;
-        this.height = undefined;
-        this._swapToImageResizeWhenImageLoaded = false;
-      }
+			return;
+    }
+
+    // At this point, an image is present, i.e. this.imageObj is valid.
+    if (this.needsRefresh(selected, hover)) {
       this._resizeImage();
     }
   }
@@ -37,15 +34,14 @@ class CircularImage extends CircleImageBase {
       this.switchImages(selected);
     }
 
-    this.resize();
+    this.selected = selected;
 
+    this.resize(ctx, selected, hover);
     this.left = x - this.width / 2;
     this.top = y - this.height / 2;
 
-    let size = Math.min(0.5*this.height, 0.5*this.width);
-
     // draw the background circle. IMPORTANT: the stroke in this method is used by the clip method below.
-    this._drawRawCircle(ctx, x, y, selected, hover, values);
+    this._drawRawCircle(ctx, x, y, values);
 
     // now we draw in the circle, we save so we can revert the clip operation after drawing.
     ctx.save();
@@ -61,11 +57,14 @@ class CircularImage extends CircleImageBase {
     this.updateBoundingBox(x,y);
   }
 
+  // TODO: compare with Circle.updateBoundingBox(), consolidate? More stuff is happening here
   updateBoundingBox(x,y) {
     this.boundingBox.top = y - this.options.size;
     this.boundingBox.left = x - this.options.size;
     this.boundingBox.right = x + this.options.size;
     this.boundingBox.bottom = y + this.options.size;
+
+    // TODO: compare with Image.updateBoundingBox(), consolidate?
     this.boundingBox.left = Math.min(this.boundingBox.left, this.labelModule.size.left);
     this.boundingBox.right = Math.max(this.boundingBox.right, this.labelModule.size.left + this.labelModule.size.width);
     this.boundingBox.bottom = Math.max(this.boundingBox.bottom, this.boundingBox.bottom + this.labelOffset);
diff --git a/lib/network/modules/components/nodes/shapes/Database.js b/lib/network/modules/components/nodes/shapes/Database.js
index 3c7490a1..efc6493c 100644
--- a/lib/network/modules/components/nodes/shapes/Database.js
+++ b/lib/network/modules/components/nodes/shapes/Database.js
@@ -9,7 +9,7 @@ class Database extends NodeBase {
   }
 
   resize(ctx, selected, hover) {
-    if ((this.width === undefined) || (this.labelModule.differentState(selected, hover))) {
+    if (this.needsRefresh(selected, hover)) {
       this.textSize = this.labelModule.getTextSize(ctx, selected, hover);
       var size = this.textSize.width + this.margin.right + this.margin.left;
       this.width = size;
diff --git a/lib/network/modules/components/nodes/shapes/Ellipse.js b/lib/network/modules/components/nodes/shapes/Ellipse.js
index 9669a1c7..f10f0a40 100644
--- a/lib/network/modules/components/nodes/shapes/Ellipse.js
+++ b/lib/network/modules/components/nodes/shapes/Ellipse.js
@@ -8,7 +8,7 @@ class Ellipse extends NodeBase {
   }
 
   resize(ctx, selected = this.selected, hover = this.hover) {
-    if ((this.width === undefined) || (this.labelModule.differentState(selected, hover))) {
+    if (this.needsRefresh(selected, hover)) {
       var textSize = this.labelModule.getTextSize(ctx, selected, hover);
 
       this.height = textSize.height * 2;
diff --git a/lib/network/modules/components/nodes/shapes/Icon.js b/lib/network/modules/components/nodes/shapes/Icon.js
index 2264fda9..f55080cc 100644
--- a/lib/network/modules/components/nodes/shapes/Icon.js
+++ b/lib/network/modules/components/nodes/shapes/Icon.js
@@ -9,7 +9,7 @@ class Icon extends NodeBase {
   }
 
   resize(ctx, selected, hover) {
-    if ((this.width === undefined) || (this.labelModule.differentState(selected, hover))) {
+    if (this.needsRefresh(selected, hover)) {
       this.iconSize = {
         width: Number(this.options.icon.size),
         height: Number(this.options.icon.size)
diff --git a/lib/network/modules/components/nodes/shapes/Image.js b/lib/network/modules/components/nodes/shapes/Image.js
index 5b22f9f2..a95e5cd4 100644
--- a/lib/network/modules/components/nodes/shapes/Image.js
+++ b/lib/network/modules/components/nodes/shapes/Image.js
@@ -9,8 +9,10 @@ class Image extends CircleImageBase {
     this.setImages(imageObj, imageObjAlt);
   }
 
-  resize() {
-    this._resizeImage();
+  resize(ctx, selected = this.selected, hover = this.hover) {
+    if (this.needsRefresh(selected, hover)) {
+      this._resizeImage();
+    }
   }
 
   draw(ctx, x, y, selected, hover, values) {
@@ -21,7 +23,7 @@ class Image extends CircleImageBase {
 
     this.selected = selected;
 
-    this.resize();
+    this.resize(ctx, selected, hover);
     this.left = x - this.width / 2;
     this.top = y - this.height / 2;
 
diff --git a/lib/network/modules/components/nodes/shapes/Text.js b/lib/network/modules/components/nodes/shapes/Text.js
index e1b7a44f..f2b1ae40 100644
--- a/lib/network/modules/components/nodes/shapes/Text.js
+++ b/lib/network/modules/components/nodes/shapes/Text.js
@@ -9,7 +9,7 @@ class Text extends NodeBase {
   }
 
   resize(ctx, selected, hover) {
-    if ((this.width === undefined) || this.labelModule.differentState(selected, hover)) {
+    if (this.needsRefresh(selected, hover)) {
       this.textSize = this.labelModule.getTextSize(ctx, selected, hover);
       this.width = this.textSize.width + this.margin.right + this.margin.left;
       this.height = this.textSize.height + this.margin.top + this.margin.bottom;
diff --git a/lib/network/modules/components/nodes/util/CircleImageBase.js b/lib/network/modules/components/nodes/util/CircleImageBase.js
index 7630236f..9d90268b 100644
--- a/lib/network/modules/components/nodes/util/CircleImageBase.js
+++ b/lib/network/modules/components/nodes/util/CircleImageBase.js
@@ -1,10 +1,22 @@
 import NodeBase from '../util/NodeBase'
 
+/**
+ * NOTE: This is a bad base class
+ *
+ * Child classes are:
+ *
+ *   Image       - uses *only* image methods
+ *   Circle      - uses *only* _drawRawCircle
+ *   CircleImage - uses all
+ *
+ * TODO: Refactor, move _drawRawCircle to different module, derive Circle from NodeBase
+ *       Rename this to ImageBase
+ *       Consolidate common code in Image and CircleImage to base class
+ */
 class CircleImageBase extends NodeBase {
   constructor(options, body, labelModule) {
     super(options, body, labelModule);
     this.labelOffset = 0;
-    this.imageLoaded = false;
     this.selected = false;
   }
 
@@ -38,57 +50,43 @@ class CircleImageBase extends NodeBase {
   }
 
   /**
-   * This function resizes the image by the options size when the image has not yet loaded. If the image has loaded, we
-   * force the update of the size again.
+   * Adjust the node dimensions for a loaded image.
    *
-   * @private
+   * Pre: this.imageObj is valid
    */
   _resizeImage() {
-    let force = false;
-    if (!this.imageObj.width || !this.imageObj.height) { // undefined or 0
-      this.imageLoaded = false;
-    }
-    else if (this.imageLoaded === false) {
-      this.imageLoaded = true;
-      force = true;
-    }
+    var width, height;
 
-    if (!this.width || !this.height || force === true) {  // undefined or 0
-      var width, height, ratio;
-      if (this.imageObj.width && this.imageObj.height) { // not undefined or 0
-        width = 0;
-        height = 0;
-      }
-      if (this.options.shapeProperties.useImageSize === false) {
-        if (this.imageObj.width > this.imageObj.height) {
-          ratio = this.imageObj.width / this.imageObj.height;
-          width = this.options.size * 2 * ratio || this.imageObj.width;
-          height = this.options.size * 2 || this.imageObj.height;
+    if (this.options.shapeProperties.useImageSize === false) {
+      // Use the size property
+      var ratio_width  = 1;
+      var ratio_height = 1;
+
+      // Only calculate the proper ratio if both width and height not zero
+      if (this.imageObj.width && this.imageObj.height) {
+      	if (this.imageObj.width > this.imageObj.height) {
+        	ratio_width = this.imageObj.width / this.imageObj.height;
         }
         else {
-          if (this.imageObj.width && this.imageObj.height) { // not undefined or 0
-            ratio = this.imageObj.height / this.imageObj.width;
-          }
-          else {
-            ratio = 1;
-          }
-          width = this.options.size * 2;
-          height = this.options.size * 2 * ratio;
+        	ratio_height = this.imageObj.height / this.imageObj.width;
         }
       }
-      else {
-        // when not using the size property, we use the image size
-        width = this.imageObj.width;
-        height = this.imageObj.height;
-      }
-      this.width = width;
-      this.height = height;
-      this.radius = 0.5 * this.width;
+
+      width  = this.options.size * 2 * ratio_width;
+      height = this.options.size * 2 * ratio_height;
+    }
+    else {
+      // Use the image size
+      width  = this.imageObj.width;
+      height = this.imageObj.height;
     }
 
+    this.width = width;
+    this.height = height;
+    this.radius = 0.5 * this.width;
   }
 
-  _drawRawCircle(ctx, x, y, selected, hover, values) {
+  _drawRawCircle(ctx, x, y, values) {
     var borderWidth = values.borderWidth / this.body.view.scale;
     ctx.lineWidth = Math.min(this.width, borderWidth);
 
diff --git a/lib/network/modules/components/nodes/util/NodeBase.js b/lib/network/modules/components/nodes/util/NodeBase.js
index d6a6e693..1f7aaf03 100644
--- a/lib/network/modules/components/nodes/util/NodeBase.js
+++ b/lib/network/modules/components/nodes/util/NodeBase.js
@@ -9,6 +9,7 @@ class NodeBase {
     this.width = undefined;
     this.radius = undefined;
     this.margin = undefined;
+    this.refreshNeeded = true;
     this.boundingBox = {top: 0, left: 0, right: 0, bottom: 0};
   }
 
@@ -89,6 +90,23 @@ class NodeBase {
       }
     }
   }
+
+
+  /**
+   * Determine if the shape of a node needs to be recalculated.
+   *
+   * @protected
+   */
+  needsRefresh(selected, hover) {
+    if (this.refreshNeeded === true) {
+      // This is probably not the best location to reset this member.
+      // However, in the current logic, it is the most convenient one.
+      this.refreshNeeded = false;
+      return true;
+    }
+
+    return  (this.width === undefined) || (this.labelModule.differentState(selected, hover));
+  }
 }
 
 export default NodeBase;
diff --git a/lib/network/modules/components/nodes/util/ShapeBase.js b/lib/network/modules/components/nodes/util/ShapeBase.js
index e430e74c..c11dd4c8 100644
--- a/lib/network/modules/components/nodes/util/ShapeBase.js
+++ b/lib/network/modules/components/nodes/util/ShapeBase.js
@@ -6,7 +6,7 @@ class ShapeBase extends NodeBase {
   }
 
   _resizeShape(selected = this.selected, hover = this.hover, values = { size: this.options.size }) {
-    if ((this.width === undefined) || (this.labelModule.differentState(selected, hover))) {
+    if (this.needsRefresh(selected, hover)) {
       var size = 2 * values.size;
       this.width = size;
       this.height = size;