From 8d84a198b9bb88ab9864b6d659f3cedeae981779 Mon Sep 17 00:00:00 2001 From: Nicolas Chevobbe Date: Fri, 15 Jan 2016 22:45:05 +0100 Subject: [PATCH] Bug 1227477 - Polish the way the timeline time graduations are calculated; r=pbro --- .../components/animation-timeline.js | 15 ++-- .../test/browser_animation_timeline_header.js | 15 ++-- .../test/unit/test_findOptimalTimeInterval.js | 88 +++++++++---------- devtools/client/animationinspector/utils.js | 69 +++++++-------- 4 files changed, 96 insertions(+), 91 deletions(-) diff --git a/devtools/client/animationinspector/components/animation-timeline.js b/devtools/client/animationinspector/components/animation-timeline.js index 785cb018256..d58ce2f9e78 100644 --- a/devtools/client/animationinspector/components/animation-timeline.js +++ b/devtools/client/animationinspector/components/animation-timeline.js @@ -410,15 +410,20 @@ AnimationsTimeline.prototype = { 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", diff --git a/devtools/client/animationinspector/test/browser_animation_timeline_header.js b/devtools/client/animationinspector/test/browser_animation_timeline_header.js index e3d861ce4ad..384da7feda6 100644 --- a/devtools/client/animationinspector/test/browser_animation_timeline_header.js +++ b/devtools/client/animationinspector/test/browser_animation_timeline_header.js @@ -24,11 +24,14 @@ add_task(function*() { 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"); @@ -36,9 +39,9 @@ add_task(function*() { 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 @@ -46,6 +49,6 @@ add_task(function*() { 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`); }); }); diff --git a/devtools/client/animationinspector/test/unit/test_findOptimalTimeInterval.js b/devtools/client/animationinspector/test/unit/test_findOptimalTimeInterval.js index 5fab2b43a3b..64451bfdf38 100644 --- a/devtools/client/animationinspector/test/unit/test_findOptimalTimeInterval.js +++ b/devtools/client/animationinspector/test/unit/test_findOptimalTimeInterval.js @@ -14,66 +14,64 @@ const {findOptimalTimeInterval} = require("devtools/client/animationinspector/ut // 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, + minTimeInterval: 33, 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, - expectedInterval: 50 + minTimeInterval: 987, + expectedInterval: 1000 }, { - 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 + minTimeInterval: 1234, + expectedInterval: 2500 }, { - 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 -}, { - 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 -}, { - 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 { diff --git a/devtools/client/animationinspector/utils.js b/devtools/client/animationinspector/utils.js index 12c4d486a63..efd744e71bc 100644 --- a/devtools/client/animationinspector/utils.js +++ b/devtools/client/animationinspector/utils.js @@ -18,17 +18,15 @@ 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; @@ -72,9 +70,9 @@ exports.createNode = createNode; * @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"); @@ -93,17 +91,19 @@ function drawGraphElementBackground(document, id, graphWidth, timeScale) { // 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. @@ -116,31 +116,30 @@ 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; + for (let i = 0; i < OPTIMAL_TIME_INTERVAL_MULTIPLES.length; i++) { + interval = OPTIMAL_TIME_INTERVAL_MULTIPLES[i] * multiplier; + if (minTimeInterval <= interval) { + return interval; + } + } if (++numIters > OPTIMAL_TIME_INTERVAL_MAX_ITERS) { - return scaledStep; + return interval; } - if (scaledStep < minSpacing) { - timingStep *= 2; - continue; - } - return scaledStep; + multiplier *= 10; } }