Smooth arcs (biocore#377)

* smooth * Default color is black * lighted default color * updated think lines * style * remove console statements * Apply suggestions from code review Co-authored-by: Marcus Fedarko <[email protected]> * fixed clade collapse * added dynamic scaling for wedges * fixed style * fixed test * Apply suggestions from code review Co-authored-by: Marcus Fedarko <[email protected]> Co-authored-by: Marcus Fedarko <[email protected]>
fedarko · Sep 17, 2020 · 22e2ae7 · 22e2ae7
1 parent 5d0f763
commit 22e2ae7
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 25 deletions.
diff --git a/empress/support_files/js/empress.js b/empress/support_files/js/empress.js
@@ -72,7 +72,7 @@ define([
          * @type {Array}
          * The default color of the tree
          */
-        this.DEFAULT_COLOR = [0.75, 0.75, 0.75];
+        this.DEFAULT_COLOR = [0.25, 0.25, 0.25];
 
         /**
          * @type {BPTree}
@@ -398,6 +398,7 @@ define([
 
         this.getLayoutInfo();
         this.centerLayoutAvgPoint();
+        // centerLayoutAvgPoint() calls drawTree(), so no need to call it here
     };
 
     /**
@@ -741,6 +742,19 @@ define([
         return new Float32Array(coords);
     };
 
+    /**
+     * Returns the number of lines/triangles to approximate an arc/wedge given
+     * the total angle of the arc/wedge.
+     *
+     * @param {Number} totalAngle The total angle of the arc/wedge
+     * @return {Number} The number of lines/triangles to approximate the arc
+     *                  or wedge.
+     */
+    Empress.prototype._numSampToApproximate = function (totalAngle) {
+        var numSamples = Math.floor(60 * Math.abs(totalAngle / Math.PI));
+        return numSamples >= 2 ? numSamples : 2;
+    };
+
     /**
      * Retrieves the coordinate info of the tree.
      *  format of coordinate info: [x, y, red, green, blue, ...]
@@ -853,14 +867,16 @@ define([
                     !this._tree.isleaf(this._tree.postorderselect(node)) &&
                     !this._collapsedClades.hasOwnProperty(node)
                 ) {
-                    // arcs are created by sampling 15 small lines along the
-                    // arc spanned by rotating (arcx0, arcy0), the line whose
-                    // origin is the root of the tree and endpoint is the start
-                    // of the arc, by arcendangle - arcstartangle radians.
-                    var numSamples = 15;
+                    // An arc will be created for all internal nodes.
+                    // arcs are created by sampling up to 60 small lines along
+                    // the arc spanned by rotating the line (arcx0, arcy0)
+                    // arcendangle - arcstartangle radians. This will create an
+                    // arc that starts at each internal node's rightmost child
+                    // and ends on the leftmost child.
                     var arcDeltaAngle =
                         this.getNodeInfo(node, "arcendangle") -
                         this.getNodeInfo(node, "arcstartangle");
+                    var numSamples = this._numSampToApproximate(arcDeltaAngle);
                     var sampleAngle = arcDeltaAngle / numSamples;
                     var sX = this.getNodeInfo(node, "arcx0");
                     var sY = this.getNodeInfo(node, "arcy0");
@@ -1245,14 +1261,12 @@ define([
                 // (TODO: this will need to be adapted when the arc is changed
                 // to be a bezier curve)
                 if (!this._tree.isleaf(this._tree.postorderselect(node))) {
-                    // arcs are created by sampling 15 small lines along the
-                    // arc spanned by rotating arcx0, the line whose origin
-                    // is the root of the tree and endpoint is the start of the
-                    // arc, by arcendangle - arcstartangle radians.
-                    var numSamples = 15;
+                    // An arc will be created for all internal nodes.
+                    // See getCoords() for details on how arcs are drawn.
                     var arcDeltaAngle =
                         this.getNodeInfo(node, "arcendangle") -
                         this.getNodeInfo(node, "arcstartangle");
+                    var numSamples = this._numSampToApproximate(arcDeltaAngle);
                     var sampleAngle = arcDeltaAngle / numSamples;
                     var sX = this.getNodeInfo(node, "arcx0");
                     var sY = this.getNodeInfo(node, "arcy0");
@@ -2249,7 +2263,7 @@ define([
         } else if (supported && this._barplotPanel.enabled) {
             this.drawBarplots(this._barplotPanel.layers);
         }
-        this.drawTree();
+        this.centerLayoutAvgPoint();
     };
 
     /**
@@ -2265,7 +2279,6 @@ define([
                 // NOTE: this function calls drawTree(), which is redundant
                 // since reLayout() already called it. Would be good to
                 // minimize redundant calls to that.
-                this.centerLayoutAvgPoint();
             } else {
                 // This should never happen under normal circumstances (the
                 // input to this function should always be an existing layout
@@ -2630,11 +2643,12 @@ define([
             cladeInfo.sY = sY;
             cladeInfo.totalAngle = totalAngle;
 
-            // create 15 triangles to approximate sector
-            var deltaAngle = totalAngle / 15;
-            cos = Math.cos(deltaAngle);
-            sin = Math.sin(deltaAngle);
-            for (var line = 0; line < 15; line++) {
+            // create triangles to approximate sector
+            var numSamples = this._numSampToApproximate(totalAngle);
+            var deltaAngle = totalAngle / numSamples;
+            cos = 1; // Math.cos(0)
+            sin = 0; // Math.sin(0)
+            for (var line = 0; line < numSamples; line++) {
                 addPoint(getCoords(rootNode));
 
                 x = sX * cos - sY * sin;

diff --git a/tests/test-circular-layout-computation.js b/tests/test-circular-layout-computation.js
@@ -226,8 +226,9 @@ require(["jquery", "BPTree", "BiomTable", "Empress"], function (
             ok(Math.abs(coords[30] - 2) < 1.0e-15); // start x arc position
             ok(Math.abs(coords[31 - 0]) < 1.0e-15); //start y arc position
             // prettier-ignore
-            ok(Math.abs(coords[175] - (-2)) < 1.0e-15); // end x arc position
-            ok(Math.abs(coords[176] - 0 < 1.0e-15)); // end y arc position
+            ok(Math.abs(coords[625] - (-2)) < 1.0e-15); // end x arc position
+            ok(Math.abs(coords[626] - 0 < 1.0e-15)); // end y arc position
+            console.log(coords);
         });
     });
 });
diff --git a/tests/test-empress.js b/tests/test-empress.js
@@ -795,11 +795,12 @@ require(["jquery", "UtilitiesForTesting", "util", "chroma"], function (
             sX = x * cos - y * sin;
             sY = x * Math.sin(langle - dangle) + y * Math.cos(langle - dangle);
 
-            // create 15 triangles to approximate sector
-            var deltaAngle = totalAngle / 15;
-            cos = Math.cos(deltaAngle);
-            sin = Math.sin(deltaAngle);
-            for (var line = 0; line < 15; line++) {
+            // create triangles to approximate sector
+            var numSamp = this.empress._numSampToApproximate(totalAngle);
+            var deltaAngle = totalAngle / numSamp;
+            cos = Math.cos(0);
+            sin = Math.sin(0);
+            for (var line = 0; line < numSamp; line++) {
                 // root of clade
                 exp.push(...[0, 1, 1, 1, 1]);