Bug 1227477 - Polish the way the timeline time graduations are calculated; r=pbro
authorNicolas Chevobbe <chevobbe.nicolas@gmail.com>
Fri, 15 Jan 2016 22:45:05 +0100
changeset 282345 0ae6c7e277af
parent 282279 103a122789a3
child 282346 df5126ec7873
push id29956
push userkwierso@gmail.com
push dateFri, 29 Jan 2016 21:40:32 +0000
treeherdermozilla-central@54eea211e234 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1227477
milestone47.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 1227477 - Polish the way the timeline time graduations are calculated; r=pbro
devtools/client/animationinspector/components/animation-timeline.js
devtools/client/animationinspector/test/browser_animation_timeline_header.js
devtools/client/animationinspector/test/unit/test_findOptimalTimeInterval.js
devtools/client/animationinspector/utils.js
--- a/devtools/client/animationinspector/components/animation-timeline.js
+++ b/devtools/client/animationinspector/components/animation-timeline.js
@@ -405,25 +405,30 @@ AnimationsTimeline.prototype = {
   onAnimationStateChanged: function() {
     // For now, simply re-render the component. The animation front's state has
     // already been updated.
     this.render(this.animations);
   },
 
   drawHeaderAndBackground: function() {
     let width = this.timeHeaderEl.offsetWidth;
-    let scale = width / (TimeScale.maxEndTime - TimeScale.minStartTime);
+    let animationDuration = TimeScale.maxEndTime - TimeScale.minStartTime;
+    let minTimeInterval = TIME_GRADUATION_MIN_SPACING * animationDuration / width;
+    let intervalLength = findOptimalTimeInterval(minTimeInterval);
+    let intervalWidth = intervalLength * width / animationDuration;
+
     drawGraphElementBackground(this.win.document, "time-graduations",
-                               width, scale);
+                               width, intervalWidth);
 
     // And the time graduation header.
     this.timeHeaderEl.innerHTML = "";
-    let interval = findOptimalTimeInterval(scale, TIME_GRADUATION_MIN_SPACING);
-    for (let i = 0; i < width; i += interval) {
-      let pos = 100 * i / width;
+
+    for (let i = 0; i <= width / intervalWidth; i++) {
+      let pos = 100 * i * intervalWidth / width;
+
       createNode({
         parent: this.timeHeaderEl,
         nodeType: "span",
         attributes: {
           "class": "time-tick",
           "style": `left:${pos}%`
         },
         textContent: TimeScale.formatTime(TimeScale.distanceToRelativeTime(pos))
--- a/devtools/client/animationinspector/test/browser_animation_timeline_header.js
+++ b/devtools/client/animationinspector/test/browser_animation_timeline_header.js
@@ -19,33 +19,36 @@ add_task(function*() {
   yield addTab(TEST_URL_ROOT + "doc_simple_animation.html");
   let {panel} = yield openAnimationInspector();
 
   let timeline = panel.animationsTimelineComponent;
   let headerEl = timeline.timeHeaderEl;
 
   info("Find out how many time graduations should there be");
   let width = headerEl.offsetWidth;
-  let scale = width / (TimeScale.maxEndTime - TimeScale.minStartTime);
+
+  let animationDuration = TimeScale.maxEndTime - TimeScale.minStartTime;
+  let minTimeInterval = TIME_GRADUATION_MIN_SPACING * animationDuration / width;
+
   // Note that findOptimalTimeInterval is tested separately in xpcshell test
   // test_findOptimalTimeInterval.js, so we assume that it works here.
-  let interval = findOptimalTimeInterval(scale, TIME_GRADUATION_MIN_SPACING);
-  let nb = Math.ceil(width / interval);
+  let interval = findOptimalTimeInterval(minTimeInterval);
+  let nb = Math.ceil(animationDuration / interval);
 
   is(headerEl.querySelectorAll(".time-tick").length, nb,
      "The expected number of time ticks were found");
 
   info("Make sure graduations are evenly distributed and show the right times");
   [...headerEl.querySelectorAll(".time-tick")].forEach((tick, i) => {
     let left = parseFloat(tick.style.left);
-    let expectedPos = i * interval * 100 / width;
+    let expectedPos = i * interval * 100 / animationDuration;
     is(Math.round(left), Math.round(expectedPos),
-      "Graduation " + i + " is positioned correctly");
+      `Graduation ${i} is positioned correctly`);
 
     // Note that the distancetoRelativeTime and formatTime functions are tested
     // separately in xpcshell test test_timeScale.js, so we assume that they
     // work here.
     let formattedTime = TimeScale.formatTime(
       TimeScale.distanceToRelativeTime(expectedPos, width));
     is(tick.textContent, formattedTime,
-      "Graduation " + i + " has the right text content");
+      `Graduation ${i} has the right text content`);
   });
 });
--- a/devtools/client/animationinspector/test/unit/test_findOptimalTimeInterval.js
+++ b/devtools/client/animationinspector/test/unit/test_findOptimalTimeInterval.js
@@ -9,75 +9,73 @@
 var Cu = Components.utils;
 const {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
 const {findOptimalTimeInterval} = require("devtools/client/animationinspector/utils");
 
 // This test array contains objects that are used to test the
 // findOptimalTimeInterval function. Each object should have the following
 // properties:
 // - desc: an optional string that will be printed out
-// - timeScale: a number that represents how many pixels is 1ms
-// - minSpacing: an optional number that represents the minim space between 2
-//   time graduations
+// - minTimeInterval: a number that represents the minimum time in ms
+//   that should be displayed in one interval
 // - expectedInterval: a number that you expect the findOptimalTimeInterval
 //   function to return as a result.
 //   Optionally you can pass a string where `interval` is the calculated
 //   interval, this string will be eval'd and tested to be truthy.
 const TEST_DATA = [{
-  desc: "With 1px being 1ms and no minSpacing, expect the interval to be the " +
-        "interval multiple",
-  timeScale: 1,
-  minSpacing: undefined,
+  desc: "With no minTimeInterval, expect the interval to be 0",
+  minTimeInterval: null,
+  expectedInterval: 0
+}, {
+  desc: "With a minTimeInterval of 0 ms, expect the interval to be 0",
+  minTimeInterval: 0,
+  expectedInterval: 0
+}, {
+  desc: "With a minInterval of 1ms, expect the interval to be the 1ms too",
+  minTimeInterval: 1,
+  expectedInterval: 1
+}, {
+  desc: "With a very small minTimeInterval, expect the interval to be 1ms",
+  minTimeInterval: 1e-31,
+  expectedInterval: 1
+}, {
+  desc: "With a minInterval of 2.5ms, expect the interval to be 2.5ms too",
+  minTimeInterval: 2.5,
+  expectedInterval: 2.5
+}, {
+  desc: "With a minInterval of 5ms, expect the interval to be 5ms too",
+  minTimeInterval: 5,
+  expectedInterval: 5
+}, {
+  desc: "With a minInterval of 7ms, expect the interval to be the next " +
+        "multiple of 5",
+  minTimeInterval: 7,
+  expectedInterval: 10
+}, {
+  minTimeInterval: 20,
   expectedInterval: 25
 }, {
-  desc: "With 1px being 1ms and a custom minSpacing being a multiple of 25 " +
-        "expect the interval to be the custom min spacing",
-  timeScale: 1,
-  minSpacing: 50,
-  expectedInterval: 50
-}, {
-  desc: "With 1px being 1ms and a custom minSpacing not being multiple of 25 " +
-        "expect the interval to be the next multiple of 10",
-  timeScale: 1,
-  minSpacing: 26,
+  minTimeInterval: 33,
   expectedInterval: 50
 }, {
-  desc: "If 1ms corresponds to a distance that is greater than the min " +
-        "spacing then, expect the interval to be this distance",
-  timeScale: 20,
-  minSpacing: undefined,
-  expectedInterval: 20
-}, {
-  desc: "If 1ms corresponds to a distance that is greater than the min " +
-        "spacing then, expect the interval to be this distance, even if it " +
-        "isn't a multiple of 25",
-  timeScale: 33,
-  minSpacing: undefined,
-  expectedInterval: 33
+  minTimeInterval: 987,
+  expectedInterval: 1000
 }, {
-  desc: "If 1ms is a very small distance, then expect this distance to be " +
-        "multiplied by 25, 50, 100, 200, etc... until it goes over the min " +
-        "spacing",
-  timeScale: 0.001,
-  minSpacing: undefined,
-  expectedInterval: 12.8
+  minTimeInterval: 1234,
+  expectedInterval: 2500
 }, {
-  desc: "If the time scale is such that we need to iterate more than the " +
-        "maximum allowed number of iterations, then expect an interval lower " +
-        "than the minimum one",
-  timeScale: 1e-31,
-  minSpacing: undefined,
-  expectedInterval: "interval < 10"
+  minTimeInterval: 9800,
+  expectedInterval: 10000
 }];
 
 function run_test() {
-  for (let {timeScale, desc, minSpacing, expectedInterval} of TEST_DATA) {
-    do_print("Testing timeScale: " + timeScale + " and minSpacing: " +
-              minSpacing + ". Expecting " + expectedInterval + ".");
+  for (let {minTimeInterval, desc, expectedInterval} of TEST_DATA) {
+    do_print(`Testing minTimeInterval: ${minTimeInterval}.
+              Expecting ${expectedInterval}.`);
 
-    let interval = findOptimalTimeInterval(timeScale, minSpacing);
+    let interval = findOptimalTimeInterval(minTimeInterval);
     if (typeof expectedInterval == "string") {
       ok(eval(expectedInterval), desc);
     } else {
       equal(interval, expectedInterval, desc);
     }
   }
 }
--- a/devtools/client/animationinspector/utils.js
+++ b/devtools/client/animationinspector/utils.js
@@ -13,27 +13,25 @@ var {loader} = Cu.import("resource://dev
 loader.lazyRequireGetter(this, "EventEmitter",
                                "devtools/shared/event-emitter");
 
 const STRINGS_URI = "chrome://devtools/locale/animationinspector.properties";
 const L10N = new ViewHelpers.L10N(STRINGS_URI);
 // How many times, maximum, can we loop before we find the optimal time
 // interval in the timeline graph.
 const OPTIMAL_TIME_INTERVAL_MAX_ITERS = 100;
-// Background time graduations should be multiple of this number of millis.
-const TIME_INTERVAL_MULTIPLE = 25;
-const TIME_INTERVAL_SCALES = 3;
-// The default minimum spacing between time graduations in px.
-const TIME_GRADUATION_MIN_SPACING = 10;
+// Time graduations should be multiple of one of these number.
+const OPTIMAL_TIME_INTERVAL_MULTIPLES = [1, 2.5, 5];
+
 // RGB color for the time interval background.
 const TIME_INTERVAL_COLOR = [128, 136, 144];
 // byte
-const TIME_INTERVAL_OPACITY_MIN = 32;
+const TIME_INTERVAL_OPACITY_MIN = 64;
 // byte
-const TIME_INTERVAL_OPACITY_ADD = 32;
+const TIME_INTERVAL_OPACITY_MAX = 96;
 
 const MILLIS_TIME_FORMAT_MAX_DURATION = 4000;
 
 /**
  * DOM node creation helper function.
  * @param {Object} Options to customize the node to be created.
  * - nodeType {String} Optional, defaults to "div",
  * - attributes {Object} Optional attributes object like
@@ -67,19 +65,19 @@ exports.createNode = createNode;
 
 /**
  * Given a data-scale, draw the background for a graph (vertical lines) into a
  * canvas and set that canvas as an image-element with an ID that can be used
  * from CSS.
  * @param {Document} document The document where the image-element should be set.
  * @param {String} id The ID for the image-element.
  * @param {Number} graphWidth The width of the graph.
- * @param {Number} timeScale How many px is 1ms in the graph.
+ * @param {Number} intervalWidth The width of one interval
  */
-function drawGraphElementBackground(document, id, graphWidth, timeScale) {
+function drawGraphElementBackground(document, id, graphWidth, intervalWidth) {
   let canvas = document.createElement("canvas");
   let ctx = canvas.getContext("2d");
 
   // Set the canvas width (as requested) and height (1px, repeated along the Y
   // axis).
   canvas.width = graphWidth;
   canvas.height = 1;
 
@@ -88,64 +86,65 @@ function drawGraphElementBackground(docu
   let pixelArray = imageData.data;
 
   let buf = new ArrayBuffer(pixelArray.length);
   let view8bit = new Uint8ClampedArray(buf);
   let view32bit = new Uint32Array(buf);
 
   // Build new millisecond tick lines...
   let [r, g, b] = TIME_INTERVAL_COLOR;
-  let alphaComponent = TIME_INTERVAL_OPACITY_MIN;
-  let interval = findOptimalTimeInterval(timeScale);
+  let opacities = [TIME_INTERVAL_OPACITY_MAX, TIME_INTERVAL_OPACITY_MIN];
 
-  // Insert one pixel for each division on each scale.
-  for (let i = 1; i <= TIME_INTERVAL_SCALES; i++) {
-    let increment = interval * Math.pow(2, i);
-    for (let x = 0; x < canvas.width; x += increment) {
-      let position = x | 0;
-      view32bit[position] = (alphaComponent << 24) | (b << 16) | (g << 8) | r;
+  // Insert one tick line on each interval
+  for (let i = 0; i <= graphWidth / intervalWidth; i++) {
+    let x = i * intervalWidth;
+    // Ensure the last line is drawn on canvas
+    if (x >= graphWidth) {
+      x = graphWidth - 0.5;
     }
-    alphaComponent += TIME_INTERVAL_OPACITY_ADD;
+    let position = x | 0;
+    let alphaComponent = opacities[i % opacities.length];
+
+    view32bit[position] = (alphaComponent << 24) | (b << 16) | (g << 8) | r;
   }
 
   // Flush the image data and cache the waterfall background.
   pixelArray.set(view8bit);
   ctx.putImageData(imageData, 0, 0);
   document.mozSetImageElement(id, canvas);
 }
 
 exports.drawGraphElementBackground = drawGraphElementBackground;
 
 /**
  * Find the optimal interval between time graduations in the animation timeline
- * graph based on a time scale and a minimum spacing.
- * @param {Number} timeScale How many px is 1ms in the graph.
- * @param {Number} minSpacing The minimum spacing between 2 graduations,
- * defaults to TIME_GRADUATION_MIN_SPACING.
- * @return {Number} The optimal interval, in pixels.
+ * graph based on a minimum time interval
+ * @param {Number} minTimeInterval Minimum time in ms in one interval
+ * @return {Number} The optimal interval time in ms
  */
-function findOptimalTimeInterval(timeScale,
-                                 minSpacing=TIME_GRADUATION_MIN_SPACING) {
-  let timingStep = TIME_INTERVAL_MULTIPLE;
+function findOptimalTimeInterval(minTimeInterval) {
   let numIters = 0;
+  let multiplier = 1;
 
-  if (timeScale > minSpacing) {
-    return timeScale;
+  if (!minTimeInterval) {
+    return 0;
   }
 
+  let interval;
   while (true) {
-    let scaledStep = timeScale * timingStep;
-    if (++numIters > OPTIMAL_TIME_INTERVAL_MAX_ITERS) {
-      return scaledStep;
+    for (let i = 0; i < OPTIMAL_TIME_INTERVAL_MULTIPLES.length; i++) {
+      interval = OPTIMAL_TIME_INTERVAL_MULTIPLES[i] * multiplier;
+      if (minTimeInterval <= interval) {
+        return interval;
+      }
     }
-    if (scaledStep < minSpacing) {
-      timingStep *= 2;
-      continue;
+    if (++numIters > OPTIMAL_TIME_INTERVAL_MAX_ITERS) {
+      return interval;
     }
-    return scaledStep;
+    multiplier *= 10;
   }
 }
 
 exports.findOptimalTimeInterval = findOptimalTimeInterval;
 
 /**
  * Format a timestamp (in ms) as a mm:ss.mmm string.
  * @param {Number} time