Skip to content

Commit

Permalink
fix: avoid resize loop when browser zoom is set to 90% (#10971)
Browse files Browse the repository at this point in the history
* 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 ed7a348.

* 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 23525ab.

* fix: Avoid the resize loop by fixing the rounding error in the retinaScale function.
  • Loading branch information
gbaron committed Dec 17, 2022
1 parent 64a0278 commit 9306d7f
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 3 deletions.
6 changes: 3 additions & 3 deletions src/helpers/helpers.dom.ts
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand Down
41 changes: 41 additions & 0 deletions test/specs/helpers.dom.tests.js
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
});
});

0 comments on commit 9306d7f

Please sign in to comment.