From 10ea729733c0f73f114a348ea16534929863be3d Mon Sep 17 00:00:00 2001 From: Guy Baron Date: Tue, 13 Dec 2022 21:31:05 -0500 Subject: [PATCH 1/8] test: new test to reproduce issue #10951 --- test/specs/helpers.dom.tests.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/specs/helpers.dom.tests.js b/test/specs/helpers.dom.tests.js index 545e153db33..1d0fa6ed8da 100644 --- a/test/specs/helpers.dom.tests.js +++ b/test/specs/helpers.dom.tests.js @@ -254,6 +254,29 @@ describe('DOM helpers tests', function() { expect(canvas.style.width).toBe('400px'); }); + it ('retinaScale should not change chart size', function() { + const chartWidth = 800; + const chartHeight = 400; + let devicePixelRatio = 0.8999999761581421; // 1.7999999523162842; + var chart = window.acquireChart({}, { + canvas: { + width: chartWidth, + height: chartHeight, + style: `width: ${chartWidth}px; height: ${chartHeight}px;` + } + }); + + helpers.retinaScale(chart, devicePixelRatio, false); + + var canvas = chart.canvas; + + expect(canvas.width).toBe(Math.floor(chartWidth * devicePixelRatio)); + expect(canvas.height).toBe(Math.floor(chartHeight * devicePixelRatio)); + + expect(chart.width).toBe(chartWidth); + expect(chart.height).toBe(chartHeight); + }); + describe('getRelativePosition', function() { it('should use offsetX/Y when available', function() { const event = {offsetX: 50, offsetY: 100}; From bf4fdcff86d11ef78c35c823732f6f8d8e06f288 Mon Sep 17 00:00:00 2001 From: Guy Baron Date: Thu, 15 Dec 2022 14:04:48 -0500 Subject: [PATCH 2/8] test: validate the canvas style too --- test/specs/helpers.dom.tests.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/specs/helpers.dom.tests.js b/test/specs/helpers.dom.tests.js index 1d0fa6ed8da..06db1c4daa5 100644 --- a/test/specs/helpers.dom.tests.js +++ b/test/specs/helpers.dom.tests.js @@ -254,7 +254,7 @@ describe('DOM helpers tests', function() { expect(canvas.style.width).toBe('400px'); }); - it ('retinaScale should not change chart size', function() { + it ('should handle devicePixelRatio correctly', function() { const chartWidth = 800; const chartHeight = 400; let devicePixelRatio = 0.8999999761581421; // 1.7999999523162842; @@ -262,19 +262,20 @@ describe('DOM helpers tests', function() { canvas: { width: chartWidth, height: chartHeight, - style: `width: ${chartWidth}px; height: ${chartHeight}px;` } }); - helpers.retinaScale(chart, devicePixelRatio, false); + helpers.retinaScale(chart, devicePixelRatio, true); var canvas = chart.canvas; - expect(canvas.width).toBe(Math.floor(chartWidth * devicePixelRatio)); expect(canvas.height).toBe(Math.floor(chartHeight * devicePixelRatio)); expect(chart.width).toBe(chartWidth); expect(chart.height).toBe(chartHeight); + + expect(canvas.style.width).toBe(`${chartWidth}px`); + expect(canvas.style.height).toBe(`${chartHeight}px`); }); describe('getRelativePosition', function() { From ed7a34814dd01f57eabc2379fc7187b9a41c8732 Mon Sep 17 00:00:00 2001 From: Guy Baron Date: Thu, 15 Dec 2022 14:21:21 -0500 Subject: [PATCH 3/8] fix: Avoid reassigning the the chart size. For specific values of pixelRatio the assignment would cause the size to reduce by 1px. Since it's called from the ResizeObserver it will be stuck in a loop that constantly reduce the size of the chart and canvas. --- src/helpers/helpers.dom.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/helpers/helpers.dom.ts b/src/helpers/helpers.dom.ts index 60b0aefb0e1..f9b1af7f772 100644 --- a/src/helpers/helpers.dom.ts +++ b/src/helpers/helpers.dom.ts @@ -222,9 +222,6 @@ export function retinaScale( const deviceHeight = Math.floor(chart.height * pixelRatio); const deviceWidth = Math.floor(chart.width * pixelRatio); - chart.height = deviceHeight / pixelRatio; - chart.width = deviceWidth / pixelRatio; - const canvas = chart.canvas; // If no style has been set on the canvas, the render size is used as display size, From f70d6afcc2690c1b1151e87a17977a742962a934 Mon Sep 17 00:00:00 2001 From: Guy Baron Date: Fri, 16 Dec 2022 16:38:05 -0500 Subject: [PATCH 4/8] Revert "fix: Avoid reassigning the the chart size. For specific values of pixelRatio the assignment would cause the size to reduce by 1px. Since it's called from the ResizeObserver it will be stuck in a loop that constantly reduce the size of the chart and canvas." This reverts commit ed7a34814dd01f57eabc2379fc7187b9a41c8732. --- src/helpers/helpers.dom.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/helpers/helpers.dom.ts b/src/helpers/helpers.dom.ts index b4d976864f2..ce1d1a0459d 100644 --- a/src/helpers/helpers.dom.ts +++ b/src/helpers/helpers.dom.ts @@ -222,6 +222,9 @@ export function retinaScale( const deviceHeight = Math.floor(chart.height * pixelRatio); const deviceWidth = Math.floor(chart.width * pixelRatio); + chart.height = deviceHeight / pixelRatio; + chart.width = deviceWidth / pixelRatio; + const canvas = chart.canvas; // If no style has been set on the canvas, the render size is used as display size, From 23525abc6aadc9880f841ff58dbd4a4ea0b14e88 Mon Sep 17 00:00:00 2001 From: Guy Baron Date: Fri, 16 Dec 2022 22:01:05 -0500 Subject: [PATCH 5/8] fix: Avoid the resize loop by fixing the rounding error in the retinaScale function. --- src/core/core.controller.js | 4 ++-- src/helpers/helpers.dom.ts | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 2ba26c9f3ee..7d7bc125bf2 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -295,8 +295,8 @@ class Chart { const newRatio = options.devicePixelRatio || this.platform.getDevicePixelRatio(); const mode = this.width ? 'resize' : 'attach'; - this.width = newSize.width; - this.height = newSize.height; + this.width = Math.floor(newSize.width); + this.height = Math.floor(newSize.height); this._aspectRatio = this.aspectRatio; if (!retinaScale(this, newRatio, true)) { return; diff --git a/src/helpers/helpers.dom.ts b/src/helpers/helpers.dom.ts index ce1d1a0459d..b4d976864f2 100644 --- a/src/helpers/helpers.dom.ts +++ b/src/helpers/helpers.dom.ts @@ -222,9 +222,6 @@ export function retinaScale( const deviceHeight = Math.floor(chart.height * pixelRatio); const deviceWidth = Math.floor(chart.width * pixelRatio); - chart.height = deviceHeight / pixelRatio; - chart.width = deviceWidth / pixelRatio; - const canvas = chart.canvas; // If no style has been set on the canvas, the render size is used as display size, From 20b247593bea4218eb9784b072a4ebc6e0b6905f Mon Sep 17 00:00:00 2001 From: Guy Baron Date: Fri, 16 Dec 2022 22:04:10 -0500 Subject: [PATCH 6/8] fix: getMaximumSize was flooring non-integer height values unnecessarily. --- src/helpers/helpers.dom.ts | 2 +- test/specs/helpers.dom.tests.js | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/helpers/helpers.dom.ts b/src/helpers/helpers.dom.ts index b4d976864f2..cd2d7c07b36 100644 --- a/src/helpers/helpers.dom.ts +++ b/src/helpers/helpers.dom.ts @@ -188,7 +188,7 @@ export function getMaximumSize( height -= paddings.height + borders.height; } width = Math.max(0, width - margins.width); - height = Math.max(0, aspectRatio ? Math.floor(width / aspectRatio) : height - margins.height); + height = Math.max(0, aspectRatio ? width / aspectRatio : height - margins.height); width = round1(Math.min(width, maxWidth, containerSize.maxWidth)); height = round1(Math.min(height, maxHeight, containerSize.maxHeight)); if (width && !height) { diff --git a/test/specs/helpers.dom.tests.js b/test/specs/helpers.dom.tests.js index 06db1c4daa5..4bf05da7568 100644 --- a/test/specs/helpers.dom.tests.js +++ b/test/specs/helpers.dom.tests.js @@ -528,4 +528,21 @@ describe('DOM helpers tests', function() { document.body.removeChild(container); }); + + it('should round non-integer container dimensions', () => { + const container = document.createElement('div'); + container.style.width = '799.999px'; + container.style.height = '299.999px'; + + document.body.appendChild(container); + + const target = document.createElement('div'); + target.style.width = '200px'; + target.style.height = '100px'; + container.appendChild(target); + + expect(helpers.getMaximumSize(target, undefined, undefined, 2)).toEqual(jasmine.objectContaining({width: 800, height: 400})); + + document.body.removeChild(container); + }); }); From 853ddb3073d74ce3127796250a0da5c0df23b924 Mon Sep 17 00:00:00 2001 From: Guy Baron Date: Sat, 17 Dec 2022 07:17:53 -0500 Subject: [PATCH 7/8] Revert "fix: Avoid the resize loop by fixing the rounding error in the retinaScale function." This reverts commit 23525abc6aadc9880f841ff58dbd4a4ea0b14e88. --- src/core/core.controller.js | 4 ++-- src/helpers/helpers.dom.ts | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 7d7bc125bf2..2ba26c9f3ee 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -295,8 +295,8 @@ class Chart { const newRatio = options.devicePixelRatio || this.platform.getDevicePixelRatio(); const mode = this.width ? 'resize' : 'attach'; - this.width = Math.floor(newSize.width); - this.height = Math.floor(newSize.height); + this.width = newSize.width; + this.height = newSize.height; this._aspectRatio = this.aspectRatio; if (!retinaScale(this, newRatio, true)) { return; diff --git a/src/helpers/helpers.dom.ts b/src/helpers/helpers.dom.ts index cd2d7c07b36..fe1f53600ac 100644 --- a/src/helpers/helpers.dom.ts +++ b/src/helpers/helpers.dom.ts @@ -222,6 +222,9 @@ export function retinaScale( const deviceHeight = Math.floor(chart.height * pixelRatio); const deviceWidth = Math.floor(chart.width * pixelRatio); + chart.height = deviceHeight / pixelRatio; + chart.width = deviceWidth / pixelRatio; + const canvas = chart.canvas; // If no style has been set on the canvas, the render size is used as display size, From 1f21c679eeef8ff5bd059746b59cebf8f0d51878 Mon Sep 17 00:00:00 2001 From: Guy Baron Date: Sat, 17 Dec 2022 07:21:09 -0500 Subject: [PATCH 8/8] fix: Avoid the resize loop by fixing the rounding error in the retinaScale function. --- src/helpers/helpers.dom.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/helpers/helpers.dom.ts b/src/helpers/helpers.dom.ts index fe1f53600ac..ebd8d978d91 100644 --- a/src/helpers/helpers.dom.ts +++ b/src/helpers/helpers.dom.ts @@ -222,8 +222,8 @@ export function retinaScale( const deviceHeight = Math.floor(chart.height * pixelRatio); const deviceWidth = Math.floor(chart.width * pixelRatio); - chart.height = deviceHeight / pixelRatio; - chart.width = deviceWidth / pixelRatio; + chart.height = Math.floor(chart.height); + chart.width = Math.floor(chart.width); const canvas = chart.canvas;