Bug 1435373 - Minor refactor for shape output string guarding against empty this.geometryBox. Round values in getDistance() util function to avoid verbose precision.r=gl
authorRazvan Caliman <rcaliman@mozilla.com>
Tue, 10 Apr 2018 13:58:48 +0200
changeset 412585 4dac79a5a4a5a4c97f8ef176793b3cb811ea204b
parent 412584 de33fb39fa4878714120d06ce6a0c72179016be9
child 412586 bc52ed730824a5cdc45169d27e187a7e90fbac72
push id33809
push userrgurzau@mozilla.com
push dateTue, 10 Apr 2018 16:54:59 +0000
treeherdermozilla-central@0a2dae2d8cf9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgl
bugs1435373
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1435373 - Minor refactor for shape output string guarding against empty this.geometryBox. Round values in getDistance() util function to avoid verbose precision.r=gl MozReview-Commit-ID: IBB4mkvAu6h
devtools/server/actors/highlighters/shapes.js
devtools/server/actors/utils/shapes-utils.js
--- a/devtools/server/actors/highlighters/shapes.js
+++ b/devtools/server/actors/highlighters/shapes.js
@@ -836,18 +836,17 @@ class ShapesHighlighter extends AutoRefr
       let [newX, newY] = apply(this.transformMatrix, vector);
       let precisionX = getDecimalPrecision(unitX);
       let precisionY = getDecimalPrecision(unitY);
       newX = (newX * ratioX).toFixed(precisionX);
       newY = (newY * ratioY).toFixed(precisionY);
 
       return `${newX}${unitX} ${newY}${unitY}`;
     }).join(", ");
-    polygonDef = (this.geometryBox) ? `polygon(${polygonDef}) ${this.geometryBox}` :
-                                      `polygon(${polygonDef})`;
+    polygonDef = `polygon(${polygonDef}) ${this.geometryBox}`.trim();
 
     this.currentNode.style.setProperty(this.property, polygonDef, "important");
   }
 
   /**
    * Transform a circle depending on the current transformation matrix.
    * @param {Number} transX the number of pixels the shape is translated on the x axis
    *                 before scaling
@@ -860,21 +859,19 @@ class ShapesHighlighter extends AutoRefr
     let [newCx, newCy] = apply(this.transformMatrix, [valueX / ratioX, valueY / ratioY]);
     if (transX !== null) {
       // As part of scaling, the shape is translated to be tangent to the line y=0.
       // To get the new radius, we translate the new cx back to that point and get
       // the distance to the line y=0.
       radius = `${Math.abs((newCx - transX) * ratioRad)}${unitRad}`;
     }
 
-    let circleDef = (this.geometryBox) ?
-      `circle(${radius} at ${newCx * ratioX}${unitX} ` +
-        `${newCy * ratioY}${unitY} ${this.geometryBox}` :
-      `circle(${radius} at ${newCx * ratioX}${unitX} ${newCy * ratioY}${unitY}`;
-    this.currentNode.style.setProperty(this.property, circleDef, "important");
+    let circleDef = `circle(${radius} at ${newCx * ratioX}${unitX} ` +
+        `${newCy * ratioY}${unitY}) ${this.geometryBox}`.trim();
+    this.emit("highlighter-event", { type: "shape-change", value: circleDef });
   }
 
   /**
    * Transform an ellipse depending on the current transformation matrix.
    * @param {Number} transX the number of pixels the shape is translated on the x axis
    *                 before scaling
    * @param {Number} transY the number of pixels the shape is translated on the y axis
    *                 before scaling
@@ -888,22 +885,19 @@ class ShapesHighlighter extends AutoRefr
     if (transX !== null && transY !== null) {
       // As part of scaling, the shape is translated to be tangent to the lines y=0 & x=0.
       // To get the new radii, we translate the new center back to that point and get the
       // distances to the line x=0 and y=0.
       rx = `${Math.abs((newCx - transX) * ratioRX)}${unitRX}`;
       ry = `${Math.abs((newCy - transY) * ratioRY)}${unitRY}`;
     }
 
-    let ellipseDef = (this.geometryBox) ?
-        `ellipse(${rx} ${ry} at ${newCx * ratioX}${unitX} ` +
-          `${newCy * ratioY}${unitY}) ${this.geometryBox}` :
-        `ellipse(${rx} ${ry} at ${newCx * ratioX}${unitX} ` +
-          `${newCy * ratioY}${unitY})`;
-    this.currentNode.style.setProperty(this.property, ellipseDef, "important");
+    let ellipseDef = `ellipse(${rx} ${ry} at ${newCx * ratioX}${unitX} ` +
+          `${newCy * ratioY}${unitY}) ${this.geometryBox}`.trim();
+    this.emit("highlighter-event", { type: "shape-change", value: ellipseDef });
   }
 
   /**
    * Transform an inset depending on the current transformation matrix.
    */
   _transformInset() {
     let { top, left, right, bottom } = this[_dragging].pointsInfo;
     let { width, height } = this.currentDimensions;
@@ -975,18 +969,17 @@ class ShapesHighlighter extends AutoRefr
     let newX = (valueX + deltaX).toFixed(precisionX);
     let newY = (valueY + deltaY).toFixed(precisionY);
 
     let polygonDef = (this.fillRule) ? `${this.fillRule}, ` : "";
     polygonDef += this.coordUnits.map((coords, i) => {
       return (i === point) ?
         `${newX}${unitX} ${newY}${unitY}` : `${coords[0]} ${coords[1]}`;
     }).join(", ");
-    polygonDef = (this.geometryBox) ? `polygon(${polygonDef}) ${this.geometryBox}` :
-                                      `polygon(${polygonDef})`;
+    polygonDef = `polygon(${polygonDef}) ${this.geometryBox}`.trim();
 
     this.currentNode.style.setProperty(this.property, polygonDef, "important");
   }
 
   /**
    * Set the inline style of the polygon, adding a new point.
    * TODO: Bug 1436054 - Do not default to percentage unit when inserting new point.
    * https://bugzilla.mozilla.org/show_bug.cgi?id=1436054
@@ -996,18 +989,17 @@ class ShapesHighlighter extends AutoRefr
    * @param {Number} y the y coordinate of the new point
    */
   _addPolygonPoint(after, x, y) {
     let polygonDef = (this.fillRule) ? `${this.fillRule}, ` : "";
     polygonDef += this.coordUnits.map((coords, i) => {
       return (i === after) ? `${coords[0]} ${coords[1]}, ${x}% ${y}%` :
                              `${coords[0]} ${coords[1]}`;
     }).join(", ");
-    polygonDef = (this.geometryBox) ? `polygon(${polygonDef}) ${this.geometryBox}` :
-                                      `polygon(${polygonDef})`;
+    polygonDef = `polygon(${polygonDef}) ${this.geometryBox}`.trim();
 
     this.hoveredPoint = after + 1;
     this._emitHoverEvent(this.hoveredPoint);
     this.currentNode.style.setProperty(this.property, polygonDef, "important");
   }
 
   /**
    * Set the inline style of the polygon, deleting the given point.
@@ -1015,18 +1007,17 @@ class ShapesHighlighter extends AutoRefr
    */
   _deletePolygonPoint(point) {
     let coordinates = this.coordUnits.slice();
     coordinates.splice(point, 1);
     let polygonDef = (this.fillRule) ? `${this.fillRule}, ` : "";
     polygonDef += coordinates.map((coords, i) => {
       return `${coords[0]} ${coords[1]}`;
     }).join(", ");
-    polygonDef = (this.geometryBox) ? `polygon(${polygonDef}) ${this.geometryBox}` :
-                                      `polygon(${polygonDef})`;
+    polygonDef = `polygon(${polygonDef}) ${this.geometryBox}`.trim();
 
     this.hoveredPoint = null;
     this._emitHoverEvent(this.hoveredPoint);
     this.currentNode.style.setProperty(this.property, polygonDef, "important");
   }
   /**
    * Handle a click when highlighting a circle.
    * @param {Number} pageX the x coordinate of the click
@@ -1081,34 +1072,31 @@ class ShapesHighlighter extends AutoRefr
     let { radius, cx, cy } = this.coordUnits;
 
     if (point === "center") {
       let { unitX, unitY, valueX, valueY, ratioX, ratioY, x, y} = this[_dragging];
       let deltaX = (pageX - x) * ratioX;
       let deltaY = (pageY - y) * ratioY;
       let newCx = `${valueX + deltaX}${unitX}`;
       let newCy = `${valueY + deltaY}${unitY}`;
-      let circleDef = (this.geometryBox) ?
-            `circle(${radius} at ${newCx} ${newCy}) ${this.geometryBox}` :
-            `circle(${radius} at ${newCx} ${newCy})`;
+      // if not defined by the user, geometryBox will be an empty string; trim() cleans up
+      let circleDef = `circle(${radius} at ${newCx} ${newCy}) ${this.geometryBox}`.trim();
 
       this.currentNode.style.setProperty(this.property, circleDef, "important");
     } else if (point === "radius") {
       let { value, unit, origRadius, ratio } = this[_dragging];
       // convert center point to px, then get distance between center and mouse.
       let { x: pageCx, y: pageCy } = this.convertPercentToPageCoords(this.coordinates.cx,
                                                                      this.coordinates.cy);
       let newRadiusPx = getDistance(pageCx, pageCy, pageX, pageY);
 
       let delta = (newRadiusPx - origRadius) * ratio;
       let newRadius = `${value + delta}${unit}`;
 
-      let circleDef = (this.geometryBox) ?
-                      `circle(${newRadius} at ${cx} ${cy} ${this.geometryBox}` :
-                      `circle(${newRadius} at ${cx} ${cy}`;
+      let circleDef = `circle(${newRadius} at ${cx} ${cy}) ${this.geometryBox}`.trim();
 
       this.currentNode.style.setProperty(this.property, circleDef, "important");
     }
   }
 
   /**
    * Handle a click when highlighting an ellipse.
    * @param {Number} pageX the x coordinate of the click
@@ -1172,43 +1160,40 @@ class ShapesHighlighter extends AutoRefr
     let { rx, ry, cx, cy } = this.coordUnits;
 
     if (point === "center") {
       let { unitX, unitY, valueX, valueY, ratioX, ratioY, x, y} = this[_dragging];
       let deltaX = (pageX - x) * ratioX;
       let deltaY = (pageY - y) * ratioY;
       let newCx = `${valueX + deltaX}${unitX}`;
       let newCy = `${valueY + deltaY}${unitY}`;
-      let ellipseDef = (this.geometryBox) ?
-        `ellipse(${rx} ${ry} at ${newCx} ${newCy}) ${this.geometryBox}` :
-        `ellipse(${rx} ${ry} at ${newCx} ${newCy})`;
+      let ellipseDef =
+        `ellipse(${rx} ${ry} at ${newCx} ${newCy}) ${this.geometryBox}`.trim();
 
       this.currentNode.style.setProperty(this.property, ellipseDef, "important");
     } else if (point === "rx") {
       let { value, unit, origRadius, ratio } = this[_dragging];
       let newRadiusPercent = Math.abs(percentX - this.coordinates.cx);
       let { width } = this.currentDimensions;
       let delta = ((newRadiusPercent / 100 * width) - origRadius) * ratio;
       let newRadius = `${value + delta}${unit}`;
 
-      let ellipseDef = (this.geometryBox) ?
-        `ellipse(${newRadius} ${ry} at ${cx} ${cy}) ${this.geometryBox}` :
-        `ellipse(${newRadius} ${ry} at ${cx} ${cy})`;
+      let ellipseDef =
+        `ellipse(${newRadius} ${ry} at ${cx} ${cy}) ${this.geometryBox}`.trim();
 
       this.currentNode.style.setProperty(this.property, ellipseDef, "important");
     } else if (point === "ry") {
       let { value, unit, origRadius, ratio } = this[_dragging];
       let newRadiusPercent = Math.abs(percentY - this.coordinates.cy);
       let { height } = this.currentDimensions;
       let delta = ((newRadiusPercent / 100 * height) - origRadius) * ratio;
       let newRadius = `${value + delta}${unit}`;
 
-      let ellipseDef = (this.geometryBox) ?
-        `ellipse(${rx} ${newRadius} at ${cx} ${cy}) ${this.geometryBox}` :
-        `ellipse(${rx} ${newRadius} at ${cx} ${cy})`;
+      let ellipseDef =
+        `ellipse(${rx} ${newRadius} at ${cx} ${cy}) ${this.geometryBox}`.trim();
 
       this.currentNode.style.setProperty(this.property, ellipseDef, "important");
     }
   }
 
   /**
    * Handle a click when highlighting an inset.
    * @param {Number} pageX the x coordinate of the click
--- a/devtools/server/actors/utils/shapes-utils.js
+++ b/devtools/server/actors/utils/shapes-utils.js
@@ -4,17 +4,17 @@
  * Get the distance between two points on a plane.
  * @param {Number} x1 the x coord of the first point
  * @param {Number} y1 the y coord of the first point
  * @param {Number} x2 the x coord of the second point
  * @param {Number} y2 the y coord of the second point
  * @returns {Number} the distance between the two points
  */
 const getDistance = (x1, y1, x2, y2) => {
-  return Math.hypot(x2 - x1, y2 - y1);
+  return Math.round(Math.hypot(x2 - x1, y2 - y1));
 };
 
 /**
  * Determine if the given x/y coords are along the edge of the given ellipse.
  * We allow for a small area around the edge that still counts as being on the edge.
  * @param {Number} x the x coordinate of the click
  * @param {Number} y the y coordinate of the click
  * @param {Number} cx the x coordinate of the center of the ellipse