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
☠☠ backed out by 41df1e7a38d5 ☠ ☠
authorRazvan Caliman <rcaliman@mozilla.com>
Mon, 19 Feb 2018 17:51:10 +0100
changeset 408160 14a22276dc5342bfeea0662a8f82a4c2daddd7ee
parent 408159 6be00f9f1cd8ec1b33524c4418fac736d8ec4dc7
child 408161 195379cf14f053d716d904b75ce4c00d8046e32a
push id100878
push userarchaeopteryx@coole-files.de
push dateWed, 14 Mar 2018 20:52:55 +0000
treeherdermozilla-inbound@195379cf14f0 [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