From 9306d7fd49338f50e87f98e732b11a236ae31fd1 Mon Sep 17 00:00:00 2001 From: Guy B Date: Sat, 17 Dec 2022 09:41:21 -0500 Subject: [PATCH] fix: avoid resize loop when browser zoom is set to 90% (#10971) * test: new test to reproduce issue #10951 * test: validate the canvas style too * 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. * 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. * fix: Avoid the resize loop by fixing the rounding error in the retinaScale function. * fix: getMaximumSize was flooring non-integer height values unnecessarily. * Revert "fix: Avoid the resize loop by fixing the rounding error in the retinaScale function." This reverts commit 23525abc6aadc9880f841ff58dbd4a4ea0b14e88. * fix: Avoid the resize loop by fixing the rounding error in the retinaScale function. --- src/helpers/helpers.dom.ts | 6 ++--- test/specs/helpers.dom.tests.js | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/helpers/helpers.dom.ts b/src/helpers/helpers.dom.ts index ce1d1a0459d..ebd8d978d91 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) { @@ -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; diff --git a/test/specs/helpers.dom.tests.js b/test/specs/helpers.dom.tests.js index 545e153db33..4bf05da7568 100644 --- a/test/specs/helpers.dom.tests.js +++ b/test/specs/helpers.dom.tests.js @@ -254,6 +254,30 @@ describe('DOM helpers tests', function() { expect(canvas.style.width).toBe('400px'); }); + it ('should handle devicePixelRatio correctly', function() { + const chartWidth = 800; + const chartHeight = 400; + let devicePixelRatio = 0.8999999761581421; // 1.7999999523162842; + var chart = window.acquireChart({}, { + canvas: { + width: chartWidth, + height: chartHeight, + } + }); + + 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() { it('should use offsetX/Y when available', function() { const event = {offsetX: 50, offsetY: 100}; @@ -504,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); + }); });