Bug 1457206 - Round values for all shape coordinates. r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Thu, 26 Apr 2018 18:17:37 +0200
changeset 472110 4421b373b86a8cb14b4f92dc5c7735f92925f9b1
parent 472109 1d4d0038e10aed3211da445f680f1957108b4efa
child 472111 0e1f60508407f11848916a8b881cad85da70b414
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1457206
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 1457206 - Round values for all shape coordinates. r=pbro MozReview-Commit-ID: H2fspzF5mYz
devtools/server/actors/highlighters/shapes.js
--- a/devtools/server/actors/highlighters/shapes.js
+++ b/devtools/server/actors/highlighters/shapes.js
@@ -874,20 +874,18 @@ class ShapesHighlighter extends AutoRefr
   _transformPolygon() {
     let { pointsInfo } = this[_dragging];
 
     let polygonDef = (this.fillRule) ? `${this.fillRule}, ` : "";
     polygonDef += pointsInfo.map(point => {
       let { unitX, unitY, valueX, valueY, ratioX, ratioY } = point;
       let vector = [valueX / ratioX, valueY / ratioY];
       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);
+      newX = round(newX * ratioX, unitX);
+      newY = round(newY * ratioY, unitY);
 
       return `${newX}${unitX} ${newY}${unitY}`;
     }).join(", ");
     polygonDef = `polygon(${polygonDef}) ${this.geometryBox}`.trim();
 
     this.emit("highlighter-event", { type: "shape-change", value: polygonDef });
   }
 
@@ -901,21 +899,24 @@ class ShapesHighlighter extends AutoRefr
           ratioX, ratioY, ratioRad } = this[_dragging];
     let { radius } = this.coordUnits;
 
     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}`;
+      radius = round(Math.abs((newCx - transX) * ratioRad), unitRad);
+      radius = `${radius}${unitRad}`;
     }
 
-    let circleDef = `circle(${radius} at ${newCx * ratioX}${unitX} ` +
-        `${newCy * ratioY}${unitY}) ${this.geometryBox}`.trim();
+    newCx = round(newCx * ratioX, unitX);
+    newCy = round(newCy * ratioY, unitY);
+    let circleDef = `circle(${radius} at ${newCx}${unitX} ${newCy}${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
@@ -926,49 +927,58 @@ class ShapesHighlighter extends AutoRefr
           ratioX, ratioY, ratioRX, ratioRY } = this[_dragging];
     let { rx, ry } = this.coordUnits;
 
     let [newCx, newCy] = apply(this.transformMatrix, [valueX / ratioX, valueY / ratioY]);
     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}`;
+      rx = round(Math.abs((newCx - transX) * ratioRX), unitRX);
+      rx = `${rx}${unitRX}`;
+      ry = round(Math.abs((newCy - transY) * ratioRY), unitRY);
+      ry = `${ry}${unitRY}`;
     }
 
-    let ellipseDef = `ellipse(${rx} ${ry} at ${newCx * ratioX}${unitX} ` +
-          `${newCy * ratioY}${unitY}) ${this.geometryBox}`.trim();
+    newCx = round(newCx * ratioX, unitX);
+    newCy = round(newCy * ratioY, unitY);
+
+    let centerStr = `${newCx}${unitX} ${newCy}${unitY}`;
+    let ellipseDef = `ellipse(${rx} ${ry} at ${centerStr}) ${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;
 
     let topLeft = [ left.value / left.ratio, top.value / top.ratio ];
     let [newLeft, newTop] = apply(this.transformMatrix, topLeft);
-    newLeft = `${newLeft * left.ratio}${left.unit}`;
-    newTop = `${newTop * top.ratio}${top.unit}`;
+    newLeft = round(newLeft * left.ratio, left.unit);
+    newLeft = `${newLeft}${left.unit}`;
+    newTop = round(newTop * top.ratio, top.unit);
+    newTop = `${newTop}${top.unit}`;
 
     // Right and bottom values are relative to the right and bottom edges of the
     // element, so convert to the value relative to the left/top edges before scaling
     // and convert back.
     let bottomRight = [ width - right.value / right.ratio,
                         height - bottom.value / bottom.ratio ];
     let [newRight, newBottom] = apply(this.transformMatrix, bottomRight);
-    newRight = `${(width - newRight) * right.ratio}${right.unit}`;
-    newBottom = `${(height - newBottom) * bottom.ratio}${bottom.unit}`;
+    newRight = round((width - newRight) * right.ratio, right.unit);
+    newRight = `${newRight}${right.unit}`;
+    newBottom = round((height - newBottom) * bottom.ratio, bottom.unit);
+    newBottom = `${newBottom}${bottom.unit}`;
 
-    let round = this.insetRound;
-    let insetDef = (round) ?
-          `inset(${newTop} ${newRight} ${newBottom} ${newLeft} round ${round})` :
+    let insetDef = (this.insetRound) ?
+          `inset(${newTop} ${newRight} ${newBottom} ${newLeft} round ${this.insetRound})`
+          :
           `inset(${newTop} ${newRight} ${newBottom} ${newLeft})`;
     insetDef += (this.geometryBox) ? this.geometryBox : "";
 
     this.emit("highlighter-event", { type: "shape-change", value: insetDef });
   }
 
   /**
    * Handle a click when highlighting a polygon.
@@ -1004,20 +1014,18 @@ class ShapesHighlighter extends AutoRefr
    * the element style.
    * @param {Number} pageX the new x coordinate of the point
    * @param {Number} pageY the new y coordinate of the point
    */
   _handlePolygonMove(pageX, pageY) {
     let { point, unitX, unitY, valueX, valueY, ratioX, ratioY, x, y } = this[_dragging];
     let deltaX = (pageX - x) * ratioX;
     let deltaY = (pageY - y) * ratioY;
-    let precisionX = getDecimalPrecision(unitX);
-    let precisionY = getDecimalPrecision(unitY);
-    let newX = (valueX + deltaX).toFixed(precisionX);
-    let newY = (valueY + deltaY).toFixed(precisionY);
+    let newX = round(valueX + deltaX, unitX);
+    let newY = round(valueY + deltaY, unitY);
 
     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 = `polygon(${polygonDef}) ${this.geometryBox}`.trim();
 
@@ -1115,31 +1123,31 @@ class ShapesHighlighter extends AutoRefr
    */
   _handleCircleMove(point, pageX, pageY) {
     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 newCx = `${round(valueX + deltaX, unitX)}${unitX}`;
+      let newCy = `${round(valueY + deltaY, unitY)}${unitY}`;
       // 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.emit("highlighter-event", { type: "shape-change", value: circleDef });
     } 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 newRadius = `${round(value + delta, unit)}${unit}`;
 
       let circleDef = `circle(${newRadius} at ${cx} ${cy}) ${this.geometryBox}`.trim();
 
       this.emit("highlighter-event", { type: "shape-change", value: circleDef });
     }
   }
 
   /**
@@ -1203,39 +1211,39 @@ class ShapesHighlighter extends AutoRefr
   _handleEllipseMove(point, pageX, pageY) {
     let { percentX, percentY } = this.convertPageCoordsToPercent(pageX, pageY);
     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 newCx = `${round(valueX + deltaX, unitX)}${unitX}`;
+      let newCy = `${round(valueY + deltaY, unitY)}${unitY}`;
       let ellipseDef =
         `ellipse(${rx} ${ry} at ${newCx} ${newCy}) ${this.geometryBox}`.trim();
 
       this.emit("highlighter-event", { type: "shape-change", value: ellipseDef });
     } 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 newRadius = `${round(value + delta, unit)}${unit}`;
 
       let ellipseDef =
         `ellipse(${newRadius} ${ry} at ${cx} ${cy}) ${this.geometryBox}`.trim();
 
       this.emit("highlighter-event", { type: "shape-change", value: ellipseDef });
     } 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 newRadius = `${round(value + delta, unit)}${unit}`;
 
       let ellipseDef =
         `ellipse(${rx} ${newRadius} at ${cx} ${cy}) ${this.geometryBox}`.trim();
 
       this.emit("highlighter-event", { type: "shape-change", value: ellipseDef });
     }
   }
 
@@ -1271,34 +1279,34 @@ class ShapesHighlighter extends AutoRefr
    * @param {Number} pageX the x coordinate of the mouse position, in terms of %
    *        relative to the element
    * @param {Number} pageY the y coordinate of the mouse position, in terms of %
    *        relative to the element
    * @memberof ShapesHighlighter
    */
   _handleInsetMove(point, pageX, pageY) {
     let { top, left, right, bottom } = this.coordUnits;
-    let round = this.insetRound;
     let { value, origValue, unit, ratio } = this[_dragging];
 
     if (point === "left") {
       let delta = (pageX - origValue) * ratio;
-      left = `${value + delta}${unit}`;
+      left = `${round(value + delta, unit)}${unit}`;
     } else if (point === "right") {
       let delta = (pageX - origValue) * ratio;
-      right = `${value - delta}${unit}`;
+      right = `${round(value - delta, unit)}${unit}`;
     } else if (point === "top") {
       let delta = (pageY - origValue) * ratio;
-      top = `${value + delta}${unit}`;
+      top = `${round(value + delta, unit)}${unit}`;
     } else if (point === "bottom") {
       let delta = (pageY - origValue) * ratio;
-      bottom = `${value - delta}${unit}`;
+      bottom = `${round(value - delta, unit)}${unit}`;
     }
-    let insetDef = (round) ?
-      `inset(${top} ${right} ${bottom} ${left} round ${round})` :
+
+    let insetDef = (this.insetRound) ?
+      `inset(${top} ${right} ${bottom} ${left} round ${this.insetRound})` :
       `inset(${top} ${right} ${bottom} ${left})`;
 
     insetDef += (this.geometryBox) ? this.geometryBox : "";
 
     this.emit("highlighter-event", { type: "shape-change", value: insetDef });
   }
 
   _handleMouseMoveNotDragging(pageX, pageY) {
@@ -1604,18 +1612,17 @@ class ShapesHighlighter extends AutoRefr
       if (distance <= clickWidth &&
           Math.min(x1, x2) - clickWidth <= pageX &&
           pageX <= Math.max(x1, x2) + clickWidth &&
           Math.min(y1, y2) - clickWidth <= pageY &&
           pageY <= Math.max(y1, y2) + clickWidth) {
         // Get the point on the line closest to the clicked point.
         let [newX, newY] = projection(x1, y1, x2, y2, pageX, pageY);
         // Default unit for new points is percentages
-        let precision = getDecimalPrecision("%");
-        this._addPolygonPoint(i, newX.toFixed(precision), newY.toFixed(precision));
+        this._addPolygonPoint(i, round(newX, "%"), round(newY, "%"));
         return;
       }
     }
   }
 
   /**
    * Check if the center point or radius of the circle highlighter is at given coords
    * @param {Number} pageX the x coordinate on the page, in % relative to the element
@@ -2815,19 +2822,16 @@ const getAnchorPoint = (type) => {
     anchor = anchor + "w";
   }
 
   return anchor;
 };
 
 /**
 * Get the decimal point precision for values depending on unit type.
-* Used as argument for `toFixed()` on coordinate values when:
-* - transforming shapes
-* - inserting new points on a polygon.
 * Only handle pixels and falsy values for now. Round them to the nearest integer value.
 * All other unit types round to two decimal points.
 *
 * @param {String|undefined} unitType any one of the accepted CSS unit types for position.
 * @return {Number} decimal precision when rounding a value
 */
 function getDecimalPrecision(unitType) {
   switch (unitType) {
@@ -2836,9 +2840,26 @@ function getDecimalPrecision(unitType) {
     case undefined:
       return 0;
     default:
       return 2;
   }
 }
 exports.getDecimalPrecision = getDecimalPrecision;
 
+/**
+ * Round up a numeric value to a fixed number of decimals depending on CSS unit type.
+ * Used when generating output shape values when:
+ * - transforming shapes
+ * - inserting new points on a polygon.
+ *
+ * @param {Number} number
+ *        Value to round up.
+ * @param {String} unitType
+ *        CSS unit type, like "px", "%", "em", "vh", etc.
+ * @return {Number}
+ *         Rounded value
+ */
+function round(number, unitType) {
+  return number.toFixed(getDecimalPrecision(unitType));
+}
+
 exports.ShapesHighlighter = ShapesHighlighter;