Skip to content

Commit

Permalink
Fix retina scale when display size is implicit
Browse files Browse the repository at this point in the history
The retinaScale helper now enforces the display size to the correct values because if no style has been set on the canvas, the render size is used as display size, making the chart bigger (or smaller) when deviceAspectRatio is different of 1.
  • Loading branch information
simonbrunel authored and etimberg committed Nov 11, 2016
1 parent efced47 commit 6b449a9
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 21 deletions.
5 changes: 2 additions & 3 deletions src/core/core.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,11 @@ module.exports = function(Chart) {

canvas.width = chart.width = newWidth;
canvas.height = chart.height = newHeight;

helpers.retinaScale(chart);

canvas.style.width = newWidth + 'px';
canvas.style.height = newHeight + 'px';

helpers.retinaScale(chart);

// Notify any plugins about the resize
var newSize = {width: newWidth, height: newHeight};
Chart.plugins.notify('resize', [me, newSize]);
Expand Down
25 changes: 16 additions & 9 deletions src/core/core.helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -832,17 +832,24 @@ module.exports = function(Chart) {
document.defaultView.getComputedStyle(el, null).getPropertyValue(property);
};
helpers.retinaScale = function(chart) {
var ctx = chart.ctx;
var canvas = chart.canvas;
var width = canvas.width;
var height = canvas.height;
var pixelRatio = chart.currentDevicePixelRatio = window.devicePixelRatio || 1;

if (pixelRatio !== 1) {
canvas.height = height * pixelRatio;
canvas.width = width * pixelRatio;
ctx.scale(pixelRatio, pixelRatio);
if (pixelRatio === 1) {
return;
}

var canvas = chart.canvas;
var height = chart.height;
var width = chart.width;

canvas.height = height * pixelRatio;
canvas.width = width * pixelRatio;
chart.ctx.scale(pixelRatio, pixelRatio);

// If no style has been set on the canvas, the render size is used as display size,
// making the chart visually bigger, so let's enforce it to the "correct" values.
// See https://github.com/chartjs/Chart.js/issues/3575
canvas.style.height = height + 'px';
canvas.style.width = width + 'px';
};
// -- Canvas methods
helpers.clear = function(chart) {
Expand Down
57 changes: 52 additions & 5 deletions test/core.controller.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe('Chart.Controller', function() {
expect(data.datasets.length).toBe(0);
});

it('should NOT alter config.data references', function() {
it('should not alter config.data references', function() {
var ds0 = {data: [10, 11, 12, 13]};
var ds1 = {data: [20, 21, 22, 23]};
var datasets = [ds0, ds1];
Expand Down Expand Up @@ -218,7 +218,7 @@ describe('Chart.Controller', function() {
});
});

it('should NOT apply aspect ratio when height specified', function() {
it('should not apply aspect ratio when height specified', function() {
var chart = acquireChart({
options: {
responsive: false,
Expand Down Expand Up @@ -310,7 +310,7 @@ describe('Chart.Controller', function() {
});
});

it('should NOT inject the resizer element', function() {
it('should not inject the resizer element', function() {
var chart = acquireChart({
options: {
responsive: false
Expand Down Expand Up @@ -425,7 +425,7 @@ describe('Chart.Controller', function() {
});
});

it('should NOT include parent padding when resizing the canvas', function(done) {
it('should not include parent padding when resizing the canvas', function(done) {
var chart = acquireChart({
type: 'line',
options: {
Expand Down Expand Up @@ -625,7 +625,7 @@ describe('Chart.Controller', function() {
});
});

it('should NOT resize the canvas when parent height changes', function(done) {
it('should not resize the canvas when parent height changes', function(done) {
var chart = acquireChart({
options: {
responsive: true,
Expand Down Expand Up @@ -666,6 +666,53 @@ describe('Chart.Controller', function() {
});
});

describe('Retina scale (a.k.a. device pixel ratio)', function() {
beforeEach(function() {
this.devicePixelRatio = window.devicePixelRatio;
window.devicePixelRatio = 3;
});

afterEach(function() {
window.devicePixelRatio = this.devicePixelRatio;
});

// see https://github.com/chartjs/Chart.js/issues/3575
it ('should scale the render size but not the "implicit" display size', function() {
var chart = acquireChart({
options: {
responsive: false
}
}, {
canvas: {
width: 320,
height: 240,
}
});

expect(chart).toBeChartOfSize({
dw: 320, dh: 240,
rw: 960, rh: 720,
});
});

it ('should scale the render size but not the "explicit" display size', function() {
var chart = acquireChart({
options: {
responsive: false
}
}, {
canvas: {
style: 'width: 320px; height: 240px'
}
});

expect(chart).toBeChartOfSize({
dw: 320, dh: 240,
rw: 960, rh: 720,
});
});
});

describe('controller.destroy', function() {
it('should reset context to default values', function() {
var chart = acquireChart({});
Expand Down
11 changes: 7 additions & 4 deletions test/mockContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,16 +214,19 @@
var chart = actual.chart;
var canvas = chart.ctx.canvas;
var style = getComputedStyle(canvas);
var pixelRatio = window.devicePixelRatio;
var dh = parseInt(style.height, 10);
var dw = parseInt(style.width, 10);
var rh = canvas.height;
var rw = canvas.width;
var orh = rh / pixelRatio;
var orw = rw / pixelRatio;

// sanity checks
if (chart.height !== rh) {
message = 'Expected chart height ' + chart.height + ' to be equal to render height ' + rh;
} else if (chart.width !== rw) {
message = 'Expected chart width ' + chart.width + ' to be equal to render width ' + rw;
if (chart.height !== orh) {
message = 'Expected chart height ' + chart.height + ' to be equal to original render height ' + orh;
} else if (chart.width !== orw) {
message = 'Expected chart width ' + chart.width + ' to be equal to original render width ' + orw;
}

// validity checks
Expand Down

0 comments on commit 6b449a9

Please sign in to comment.