Merge pull request #2204 from wimrijnders/PR17

Some code cleanup and consolidation.

lib/graph3d/Graph3d.js

@@ -203,21 +203,23 @@ Graph3d.prototype._convert3Dto2D = function(point3d) {
  *                   camera
  */
 Graph3d.prototype._convertPointToTranslation = function(point3d) {
-  var ax = point3d.x * this.scale.x,
+  var cameraLocation = this.camera.getCameraLocation(),
+    cameraRotation = this.camera.getCameraRotation(),
+    ax = point3d.x * this.scale.x,
     ay = point3d.y * this.scale.y,
     az = point3d.z * this.scale.z,
 
-    cx = this.camera.getCameraLocation().x,
-    cy = this.camera.getCameraLocation().y,
-    cz = this.camera.getCameraLocation().z,
+    cx = cameraLocation.x,
+    cy = cameraLocation.y,
+    cz = cameraLocation.z,
 
   // calculate angles
-    sinTx = Math.sin(this.camera.getCameraRotation().x),
-    cosTx = Math.cos(this.camera.getCameraRotation().x),
-    sinTy = Math.sin(this.camera.getCameraRotation().y),
-    cosTy = Math.cos(this.camera.getCameraRotation().y),
-    sinTz = Math.sin(this.camera.getCameraRotation().z),
-    cosTz = Math.cos(this.camera.getCameraRotation().z),
+    sinTx = Math.sin(cameraRotation.x),
+    cosTx = Math.cos(cameraRotation.x),
+    sinTy = Math.sin(cameraRotation.y),
+    cosTy = Math.cos(cameraRotation.y),
+    sinTz = Math.sin(cameraRotation.z),
+    cosTz = Math.cos(cameraRotation.z),
 
   // calculate translation
     dx = cosTy * (sinTz * (ay - cy) + cosTz * (ax - cx)) - sinTy * (az - cz),
@@ -292,47 +294,6 @@ Graph3d.prototype._calcTranslations = function(points, sort) {
 };
 
 
-/**
- * Determine the indexes of the data columns, based on the given style and data
- * @param {DataSet} data
- * @param {Number}  style
- */
-Graph3d.prototype._determineColumnIndexes = function(data, style) {
-  if (this.style === Graph3d.STYLE.DOT ||
-    this.style === Graph3d.STYLE.DOTLINE ||
-    this.style === Graph3d.STYLE.LINE ||
-    this.style === Graph3d.STYLE.GRID ||
-    this.style === Graph3d.STYLE.SURFACE ||
-    this.style === Graph3d.STYLE.BAR) {
-    // 3 columns expected, and optionally a 4th with filter values
-    this.colX = 0;
-    this.colY = 1;
-    this.colZ = 2;
-    this.colValue = undefined;
-
-    if (data.getNumberOfColumns() > 3) {
-      this.colFilter = 3;
-    }
-  }
-  else if (this.style === Graph3d.STYLE.DOTCOLOR ||
-    this.style === Graph3d.STYLE.DOTSIZE ||
-    this.style === Graph3d.STYLE.BARCOLOR ||
-    this.style === Graph3d.STYLE.BARSIZE) {
-    // 4 columns expected, and optionally a 5th with filter values
-    this.colX = 0;
-    this.colY = 1;
-    this.colZ = 2;
-    this.colValue = 3;
-
-    if (data.getNumberOfColumns() > 4) {
-      this.colFilter = 4;
-    }
-  }
-  else {
-    throw new Error('Unknown style "' + this.style + '"');
-  }
-};
-
 Graph3d.prototype.getNumberOfRows = function(data) {
   return data.length;
 }
@@ -361,14 +322,57 @@ Graph3d.prototype.getDistinctValues = function(data, column) {
 
 
 Graph3d.prototype.getColumnRange = function(data,column) {
-  var minMax = {min:data[0][column],max:data[0][column]};
+  var minMax;
+
   for (var i = 0; i < data.length; i++) {
-    if (minMax.min > data[i][column]) { minMax.min = data[i][column]; }
-    if (minMax.max < data[i][column]) { minMax.max = data[i][column]; }
+    var item = data[i][column];
+
+    if (i === 0) {
+       minMax = { min: item, max: item};
+    } else {
+      if (minMax.min > item) { minMax.min = item; }
+      if (minMax.max < item) { minMax.max = item; }
+    }
   }
   return minMax;
 };
 
+
+/**
+ * Check if the state is consistent for the use of the value field.
+ *
+ * Throws if a problem is detected.
+ */
+Graph3d.prototype._checkValueField = function (data) {
+
+  var hasValueField = this.style === Graph3d.STYLE.BARCOLOR
+                   || this.style === Graph3d.STYLE.BARSIZE
+                   || this.style === Graph3d.STYLE.DOTCOLOR
+                   || this.style === Graph3d.STYLE.DOTSIZE;
+
+  if (!hasValueField) {
+    return;   // No need to check further
+  }
+
+  // Following field must be present for the current graph style
+  if (this.colValue === undefined) {
+    throw new Error('Expected data to have '
+      + ' field \'style\' '
+      + ' for graph style \'' + this.style + '\''
+    );
+  }
+
+  // The data must also contain this field.
+  // Note that only first data element is checked
+  if (data[0][this.colValue] === undefined) {
+    throw new Error('Expected data to have '
+      + ' field \'' + this.colValue + '\' '
+      + ' for graph style \'' + this.style + '\''
+    );
+  }
+};
+
+
 /**
  * Initialize the data from the data table. Calculate minimum and maximum values
  * and column index values
@@ -410,12 +414,6 @@ Graph3d.prototype._dataInitialize = function (rawData, style) {
   };
   this.dataSet.on('*', this._onChange);
 
-  // _determineColumnIndexes
-  // getNumberOfRows (points)
-  // getNumberOfColumns (x,y,z,v,t,t1,t2...)
-  // getDistinctValues (unique values?)
-  // getColumnRange
-
   // determine the location of x,y,z,value,filter columns
   this.colX = 'x';
   this.colY = 'y';
@@ -423,7 +421,8 @@ Graph3d.prototype._dataInitialize = function (rawData, style) {
 
   // check if a filter column is provided
   if (data[0].hasOwnProperty('filter')) {
-    this.colFilter = 'filter';  // Bugfix: only set this field if it's actually present!
+    // Only set this field if it's actually present
+    this.colFilter = 'filter';
 
     if (this.dataFilter === undefined) {
       this.dataFilter = new Filter(rawData, this.colFilter, this);
@@ -482,7 +481,6 @@ Graph3d.prototype._dataInitialize = function (rawData, style) {
   if (this.zMax <= this.zMin) this.zMax = this.zMin + 1;
   this.zStep = (this.defaultZStep !== undefined) ? this.defaultZStep : (this.zMax-this.zMin)/5;
 
-  // Bugfix: Only handle field 'style' if it's actually present
   if (data[0].hasOwnProperty('style')) {
     this.colValue = 'style';
     var valueRange = this.getColumnRange(data,this.colValue);
@@ -580,29 +578,7 @@ Graph3d.prototype._getDataPoints = function (data) {
     }
   }
   else {  // 'dot', 'dot-line', etc.
-
-    // Bugfix: ensure value field is present in data if expected
-    var hasValueField = this.style === Graph3d.STYLE.BARCOLOR
-                     || this.style === Graph3d.STYLE.BARSIZE
-                     || this.style === Graph3d.STYLE.DOTCOLOR
-                     || this.style === Graph3d.STYLE.DOTSIZE;
-
-    if (hasValueField) {
-      if (this.colValue === undefined) {
-        throw new Error('Expected data to have '
-          + ' field \'style\' '
-          + ' for graph style \'' + this.style + '\''
-        );
-      }
-
-      if (data[0][this.colValue] === undefined) {
-        throw new Error('Expected data to have '
-          + ' field \'' + this.colValue + '\' '
-          + ' for graph style \'' + this.style + '\''
-        );
-      }
-    }
-    
+    this._checkValueField(data);
 
     // copy all values from the google data table to a list with Point3d objects
     for (i = 0; i < data.length; i++) {
@@ -908,6 +884,7 @@ Graph3d.prototype._dotSize = function() {
  */
 Graph3d.prototype._getLegendWidth = function() {
   var width; 
+
   if (this.style === Graph3d.STYLE.DOTSIZE) {
     var dotSize = this._dotSize();
     width =  dotSize / 2 + dotSize * 2; 
@@ -926,12 +903,16 @@ Graph3d.prototype._getLegendWidth = function() {
 Graph3d.prototype._redrawLegend = function() {
  
   //Return without drawing anything, if no legend is specified 
-  if (this.showLegend !== true) {return;}     
+  if (this.showLegend !== true) {
+    return;
+  }     
 
   // Do not draw legend when graph style does not support
   if (this.style === Graph3d.STYLE.LINE
    || this.style === Graph3d.STYLE.BARSIZE //TODO add legend support for BARSIZE 
-  ){return;} 
+  ){
+    return;
+  } 
 
   // Legend types - size and color. Determine if size legend.  
   var isSizeLegend = (this.style === Graph3d.STYLE.BARSIZE 
@@ -1004,13 +985,13 @@ Graph3d.prototype._redrawLegend = function() {
   step.start(true);
 
   var y;
+  var from;
+  var to;
   while (!step.end()) {
     y = bottom - (step.getCurrent() - legendMin) / (legendMax - legendMin) * height;
-
-    ctx.beginPath();
-    ctx.moveTo(left - gridLineLen, y);
-    ctx.lineTo(left, y);
-    ctx.stroke();
+    from  = new Point2d(left - gridLineLen, y);
+    to    = new Point2d(left, y);
+    this._line(ctx, from, to);
 
     ctx.textAlign = 'right';
     ctx.textBaseline = 'middle';
@@ -1024,7 +1005,6 @@ Graph3d.prototype._redrawLegend = function() {
   ctx.textBaseline = 'top';
   var label = this.legendLabel;
   ctx.fillText(label, right, bottom + this.margin);
-  
 };
 
 /**
@@ -1390,6 +1370,21 @@ Graph3d.prototype._hsv2rgb = function(H, S, V) {
 };
 
 
+Graph3d.prototype._drawGridLine = function(ctx, from, to) {
+  if (from === undefined || to === undefined) {
+     return;
+  }
+
+  // calculate Hue from the current value. At zMin the hue is 240, at zMax the hue is 0
+  var zAvg = (from.point.z + to.point.z) / 2;
+  var h    = (1 - (zAvg - this.zMin) * this.scale.z  / this.verticalRatio) * 240;
+
+  ctx.lineWidth   = this._getStrokeWidth(from) * 2;
+  ctx.strokeStyle = this._hsv2rgb(h, 1, 1);
+  this._line(ctx, from.screen, to.screen);
+};
+
+
 /**
  * Draw all datapoints as a grid
  * This function can be used when the style is 'grid'
@@ -1412,9 +1407,9 @@ Graph3d.prototype._redrawDataGrid = function() {
   if (this.style === Graph3d.STYLE.SURFACE) {
     for (i = 0; i < this.dataPoints.length; i++) {
       point = this.dataPoints[i];
-      right = this.dataPoints[i].pointRight;
-      top   = this.dataPoints[i].pointTop;
-      cross = this.dataPoints[i].pointCross;
+      right = point.pointRight;
+      top   = point.pointTop;
+      cross = point.pointCross;
 
       if (point !== undefined && right !== undefined && top !== undefined && cross !== undefined) {
 
@@ -1474,28 +1469,8 @@ Graph3d.prototype._redrawDataGrid = function() {
   else { // grid style
     for (i = 0; i < this.dataPoints.length; i++) {
       point = this.dataPoints[i];
-      right = this.dataPoints[i].pointRight;
-      top   = this.dataPoints[i].pointTop;
-
-      if (point !== undefined && right !== undefined) {
-        // calculate Hue from the current value. At zMin the hue is 240, at zMax the hue is 0
-        zAvg = (point.point.z + right.point.z) / 2;
-        h = (1 - (zAvg - this.zMin) * this.scale.z  / this.verticalRatio) * 240;
-
-        ctx.lineWidth = this._getStrokeWidth(point) * 2;
-        ctx.strokeStyle = this._hsv2rgb(h, 1, 1);
-        this._line(ctx, point.screen, right.screen);
-      }
-
-      if (point !== undefined && top !== undefined) {
-        // calculate Hue from the current value. At zMin the hue is 240, at zMax the hue is 0
-        zAvg = (point.point.z + top.point.z) / 2;
-        h = (1 - (zAvg - this.zMin) * this.scale.z  / this.verticalRatio) * 240;
-
-        ctx.lineWidth = this._getStrokeWidth(point) * 2;
-        ctx.strokeStyle = this._hsv2rgb(h, 1, 1);
-        this._line(ctx, point.screen, top.screen);
-      }
+      this._drawGridLine(ctx, point, point.pointRight);
+      this._drawGridLine(ctx, point, point.pointTop);
     }
   }
 };

lib/graph3d/Settings.js

@@ -430,8 +430,6 @@ function setCameraPosition(cameraPosition, dst) {
 }
 
 
-
-
 module.exports.STYLE             = STYLE;
 module.exports.setDefaults       = setDefaults;
 module.exports.setOptions        = setOptions;