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 fc0d5bbaed8b ☠ ☠
authorRazvan Caliman <rcaliman@mozilla.com>
Mon, 19 Feb 2018 17:51:10 +0100
changeset 412497 c3b0f6497bb73401f0847bb8e7d8f331673f45e1
parent 412496 d3c654a2185f86142b0df54c9279906e94ff9ed5
child 412498 0ea578dacf239fbb31c3d6e3f6b616d6f61da263
push id62411
push usercbrindusan@mozilla.com
push dateTue, 10 Apr 2018 01:35:46 +0000
treeherderautoland@0ea578dacf23 [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