From 9548410c3bbf5a64a857d791bb84864a6309ccea Mon Sep 17 00:00:00 2001 From: WofWca Date: Mon, 6 Sep 2021 15:08:08 +0300 Subject: [PATCH 1/6] fix: chart constantly jumping in 1-2 pixel steps The issue is noticeable at `millisPerPixel` being between ~ 0.5 & 2.0 the size of `requestAnimationFrame` period. Canvas image update period would be inconsistent - some would last longer, some less, some would get skipped altogether. --- smoothie.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/smoothie.js b/smoothie.js index ad40ed5..2ea7e73 100644 --- a/smoothie.js +++ b/smoothie.js @@ -784,31 +784,31 @@ if (this.options.limitFPS > 0 && nowMillis - this.lastRenderTimeMillis < (1000/this.options.limitFPS)) return; + time = time || nowMillis - (this.delay || 0); + + // Round time down to pixel granularity, so motion appears smoother. + time -= time % this.options.millisPerPixel; + if (!this.isAnimatingScale) { // We're not animating. We can use the last render time and the scroll speed to work out whether // we actually need to paint anything yet. If not, we can return immediately. - - // Render at least every 1/6th of a second. The canvas may be resized, which there is - // no reliable way to detect. - var maxIdleMillis = Math.min(1000/6, this.options.millisPerPixel); - - if (nowMillis - this.lastRenderTimeMillis < maxIdleMillis) { - return; + var sameTime = this.lastChartTimestamp === time; + if (sameTime) { + // Render at least every 1/6th of a second. The canvas may be resized, which there is + // no reliable way to detect. + var needToRenderInCaseCanvasResized = nowMillis - this.lastRenderTimeMillis > 1000/6; + if (!needToRenderInCaseCanvasResized) { + return; + } } } - this.resize(); - this.lastRenderTimeMillis = nowMillis; - - canvas = canvas || this.canvas; - time = time || nowMillis - (this.delay || 0); - - // Round time down to pixel granularity, so motion appears smoother. - time -= time % this.options.millisPerPixel; - this.lastChartTimestamp = time; + this.resize(); + + canvas = canvas || this.canvas; var context = canvas.getContext('2d'), chartOptions = this.options, dimensions = { top: 0, left: 0, width: canvas.clientWidth, height: canvas.clientHeight }, From 1fffdd8506cdfa2a7ff20105cf6a81ef1a0dce6d Mon Sep 17 00:00:00 2001 From: WofWca Date: Mon, 6 Sep 2021 15:17:52 +0300 Subject: [PATCH 2/6] docs: update changelog --- smoothie.js | 1 + 1 file changed, 1 insertion(+) diff --git a/smoothie.js b/smoothie.js index 2ea7e73..8e2387a 100644 --- a/smoothie.js +++ b/smoothie.js @@ -95,6 +95,7 @@ * Add title option, by @mesca * Fix data drop stoppage by rejecting NaNs in append(), by @timdrysdale * Allow setting interpolation per time series, by @WofWca (#123) + * Fix chart constantly jumping in 1-2 pixel steps, by @WofWca (#131) */ ;(function(exports) { From a0648dc76a05eca8d673a70dc451e645121fad1c Mon Sep 17 00:00:00 2001 From: WofWca Date: Tue, 21 Sep 2021 23:19:45 +0800 Subject: [PATCH 3/6] remove the `grid.sharpLines` option This reverts f1a0a8882 --- builder/index.html | 1 - smoothie.d.ts | 2 -- smoothie.js | 11 ----------- 3 files changed, 14 deletions(-) diff --git a/builder/index.html b/builder/index.html index 6eb48a0..0bc8f61 100755 --- a/builder/index.html +++ b/builder/index.html @@ -430,7 +430,6 @@ bindColor({target: chart.options.grid, name: 'Grid line color', propertyName: 'strokeStyle', opacity: 1}); bindRange({target: chart.options.grid, name: 'Vertical sections', propertyName: 'verticalSections', min: 0, max: 20}); bindRange({target: chart.options.grid, name: 'Time line spacing', propertyName: 'millisPerLine', min: 1000, max: 10000, step: 1000}); - bindCheckBox({target: chart.options.grid, name: 'Sharp grid lines', propertyName: 'sharpLines'}); bindCheckBox({target: chart.options.grid, name: 'Draw border', propertyName: 'borderVisible'}); // Labels diff --git a/smoothie.d.ts b/smoothie.d.ts index cd452bd..ff572ad 100644 --- a/smoothie.d.ts +++ b/smoothie.d.ts @@ -86,8 +86,6 @@ export interface IGridOptions { strokeStyle?: string; /** Distance between vertical grid lines. */ millisPerLine?: number; - /** Controls whether grid lines are 1px sharp, or softened. */ - sharpLines?: boolean; /** Number of vertical sections marked out by horizontal grid lines. */ verticalSections?: number; /** Whether the grid lines trace the border of the chart or not. */ diff --git a/smoothie.js b/smoothie.js index 8e2387a..fe9a123 100644 --- a/smoothie.js +++ b/smoothie.js @@ -295,7 +295,6 @@ * lineWidth: 1, // the pixel width of grid lines * strokeStyle: '#777777', // colour of grid lines * millisPerLine: 1000, // distance between vertical grid lines - * sharpLines: false, // controls whether grid lines are 1px sharp, or softened * verticalSections: 2, // number of vertical sections marked out by horizontal grid lines * borderVisible: true // whether the grid lines trace the border of the chart or not * }, @@ -388,7 +387,6 @@ fillStyle: '#000000', strokeStyle: '#777777', lineWidth: 1, - sharpLines: false, millisPerLine: 1000, verticalSections: 2, borderVisible: true @@ -864,9 +862,6 @@ t >= oldestValidTime; t -= chartOptions.grid.millisPerLine) { var gx = timeToXPixel(t); - if (chartOptions.grid.sharpLines) { - gx -= 0.5; - } context.moveTo(gx, 0); context.lineTo(gx, dimensions.height); } @@ -877,9 +872,6 @@ // Horizontal (value) dividers. for (var v = 1; v < chartOptions.grid.verticalSections; v++) { var gy = Math.round(v * dimensions.height / chartOptions.grid.verticalSections); - if (chartOptions.grid.sharpLines) { - gy -= 0.5; - } context.beginPath(); context.moveTo(0, gy); context.lineTo(dimensions.width, gy); @@ -1034,9 +1026,6 @@ var stepPixels = dimensions.height / chartOptions.grid.verticalSections; for (var v = 1; v < chartOptions.grid.verticalSections; v++) { var gy = dimensions.height - Math.round(v * stepPixels); - if (chartOptions.grid.sharpLines) { - gy -= 0.5; - } var yValue = chartOptions.yIntermediateFormatter(this.valueRange.min + (v * step), chartOptions.labels.precision); //left of right axis? intermediateLabelPos = From 8f6c10b45abc5003e729a8e1f05efd71e3e65f88 Mon Sep 17 00:00:00 2001 From: WofWca Date: Wed, 22 Sep 2021 20:07:36 +0800 Subject: [PATCH 4/6] make all lines sharp Fixes #57 --- smoothie.js | 48 ++++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/smoothie.js b/smoothie.js index fe9a123..77c8d92 100644 --- a/smoothie.js +++ b/smoothie.js @@ -137,7 +137,17 @@ low = mid + 1; } return low; - } + }, + // So lines (especially vertical and horizontal) look a) consistent along their length and b) sharp. + pixelSnap: function(position, lineWidth) { + if (lineWidth % 2 === 0) { + // Closest pixel edge. + return Math.round(position); + } else { + // Closest pixel center. + return Math.floor(position) + 0.5; + } + }, }; /** @@ -813,17 +823,18 @@ dimensions = { top: 0, left: 0, width: canvas.clientWidth, height: canvas.clientHeight }, // Calculate the threshold time for the oldest data points. oldestValidTime = time - (dimensions.width * chartOptions.millisPerPixel), - valueToYPixel = function(value) { - var offset = value - this.currentVisMinValue; - return this.currentValueRange === 0 - ? dimensions.height - : dimensions.height - (Math.round((offset / this.currentValueRange) * dimensions.height)); + valueToYPosition = function(value, lineWidth) { + var offset = value - this.currentVisMinValue, + unsnapped = this.currentValueRange === 0 + ? dimensions.height + : dimensions.height * (1 - offset / this.currentValueRange); + return Util.pixelSnap(unsnapped, lineWidth); }.bind(this), - timeToXPixel = function(t) { - if(chartOptions.scrollBackwards) { - return Math.round((time - t) / chartOptions.millisPerPixel); - } - return Math.round(dimensions.width - ((time - t) / chartOptions.millisPerPixel)); + timeToXPosition = function(t, lineWidth) { + var unsnapped = chartOptions.scrollBackwards + ? (time - t) / chartOptions.millisPerPixel + : dimensions.width - ((time - t) / chartOptions.millisPerPixel); + return Util.pixelSnap(unsnapped, lineWidth); }; this.updateValueRange(); @@ -861,7 +872,7 @@ for (var t = time - (time % chartOptions.grid.millisPerLine); t >= oldestValidTime; t -= chartOptions.grid.millisPerLine) { - var gx = timeToXPixel(t); + var gx = timeToXPosition(t, chartOptions.grid.lineWidth); context.moveTo(gx, 0); context.lineTo(gx, dimensions.height); } @@ -871,7 +882,7 @@ // Horizontal (value) dividers. for (var v = 1; v < chartOptions.grid.verticalSections; v++) { - var gy = Math.round(v * dimensions.height / chartOptions.grid.verticalSections); + var gy = Util.pixelSnap(v * dimensions.height / chartOptions.grid.verticalSections, chartOptions.grid.lineWidth); context.beginPath(); context.moveTo(0, gy); context.lineTo(dimensions.width, gy); @@ -890,9 +901,10 @@ if (chartOptions.horizontalLines && chartOptions.horizontalLines.length) { for (var hl = 0; hl < chartOptions.horizontalLines.length; hl++) { var line = chartOptions.horizontalLines[hl], - hly = Math.round(valueToYPixel(line.value)) - 0.5; + lineWidth = line.lineWidth || 1, + hly = valueToYPosition(line.value, lineWidth); context.strokeStyle = line.color || '#ffffff'; - context.lineWidth = line.lineWidth || 1; + context.lineWidth = lineWidth; context.beginPath(); context.moveTo(0, hly); context.lineTo(dimensions.width, hly); @@ -923,8 +935,8 @@ // Retain lastX, lastY for calculating the control points of bezier curves. var firstX = 0, firstY = 0, lastX = 0, lastY = 0; for (var i = 0; i < dataSet.length && dataSet.length !== 1; i++) { - var x = timeToXPixel(dataSet[i][0]), - y = valueToYPixel(dataSet[i][1]); + var x = timeToXPosition(dataSet[i][0], seriesOptions.lineWidth), + y = valueToYPosition(dataSet[i][1], seriesOptions.lineWidth); if (i === 0) { firstX = x; @@ -1045,7 +1057,7 @@ for (var t = time - (time % chartOptions.grid.millisPerLine); t >= oldestValidTime; t -= chartOptions.grid.millisPerLine) { - var gx = timeToXPixel(t); + var gx = timeToXPosition(t, 0); // Only draw the timestamp if it won't overlap with the previously drawn one. if ((!chartOptions.scrollBackwards && gx < textUntilX) || (chartOptions.scrollBackwards && gx > textUntilX)) { // Formats the timestamp based on user specified formatting function From e22ba7e02f2deb6207af7eb4b019153cd3979b43 Mon Sep 17 00:00:00 2001 From: WofWca Date: Wed, 22 Sep 2021 20:08:31 +0800 Subject: [PATCH 5/6] docs: update changelog --- smoothie.js | 1 + 1 file changed, 1 insertion(+) diff --git a/smoothie.js b/smoothie.js index 77c8d92..6d5d512 100644 --- a/smoothie.js +++ b/smoothie.js @@ -96,6 +96,7 @@ * Fix data drop stoppage by rejecting NaNs in append(), by @timdrysdale * Allow setting interpolation per time series, by @WofWca (#123) * Fix chart constantly jumping in 1-2 pixel steps, by @WofWca (#131) + * Fix: make all lines sharp, by @WofWca (#134) */ ;(function(exports) { From b6a6b0a90a703fe6e253ba74188196ae79f0d778 Mon Sep 17 00:00:00 2001 From: WofWca Date: Thu, 7 Oct 2021 12:44:56 +0800 Subject: [PATCH 6/6] WIP: fix: chart jumping and wobbling on `window.devicePixelRatio !== 1` --- examples/example1.html | 9 +++++++-- smoothie.js | 28 ++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/examples/example1.html b/examples/example1.html index b05cba5..72c25fa 100644 --- a/examples/example1.html +++ b/examples/example1.html @@ -11,8 +11,13 @@ }, 500); function createTimeline() { - var chart = new SmoothieChart(); - chart.addTimeSeries(random, { strokeStyle: 'rgba(0, 255, 0, 1)', fillStyle: 'rgba(0, 255, 0, 0.2)', lineWidth: 4 }); + var chart = new SmoothieChart({ + millisPerPixel: 1000 / (74.6 / 3), + interpolation: 'step', + maxValue: 10000, + minValue: 0, + }); + chart.addTimeSeries(random, { strokeStyle: 'rgba(0, 255, 0, 1)', fillStyle: 'rgba(0, 255, 0, 0.2)', lineWidth: 2 * 1 / window.devicePixelRatio }); chart.streamTo(document.getElementById("chart"), 500); } diff --git a/smoothie.js b/smoothie.js index 6d5d512..c769ee5 100644 --- a/smoothie.js +++ b/smoothie.js @@ -141,12 +141,30 @@ }, // So lines (especially vertical and horizontal) look a) consistent along their length and b) sharp. pixelSnap: function(position, lineWidth) { - if (lineWidth % 2 === 0) { + // TODO grid lines and bezier lines still (occasionally) wobble. But it's still better than it was. + + var dpr = window.devicePixelRatio, + coordinatesPerPixel = 1 / dpr; + + // return position - position % window.devicePixelRatio; + // return position - position % coordinatesPerPixel; + + // if (lineWidth % (2 * dpr) === 0) { + + // TODO may need to replace the strict comparison with `<= coordinatesPerPixel / 2` (or something + // like this), that will minimize smudging instead of only removing it when it's strictly divisible. + // Not only because of truncation error that comes with `dpr !== 1` but because it also makes sense for + // `dpr === 1`. + if (lineWidth % (2 * coordinatesPerPixel) === 0) { // Closest pixel edge. - return Math.round(position); + // return Math.round(position); + + // TODO It's not the closest, it's round down. + return position - position % coordinatesPerPixel; } else { // Closest pixel center. - return Math.floor(position) + 0.5; + // return Math.floor(position) + 0.5; + return position - position % coordinatesPerPixel + coordinatesPerPixel / 2; } }, }; @@ -797,7 +815,9 @@ time = time || nowMillis - (this.delay || 0); // Round time down to pixel granularity, so motion appears smoother. - time -= time % this.options.millisPerPixel; + // time -= time % this.options.millisPerPixel; + // time -= time % (this.options.millisPerPixel / window.devicePixelRatio); + time -= time % (this.options.millisPerPixel / window.devicePixelRatio); if (!this.isAnimatingScale) { // We're not animating. We can use the last render time and the scroll speed to work out whether