Skip to content

Heatmap Padding #868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1620,6 +1620,7 @@ Plotly.restyle = function restyle(gd, astr, val, traces) {
// objects need to be made) but not a recalc
var replotAttrs = [
'zmin', 'zmax', 'zauto',
'xgap', 'ygap',
'marker.cmin', 'marker.cmax', 'marker.cauto',
'line.cmin', 'line.cmax',
'marker.line.cmin', 'marker.line.cmax',
Expand Down
15 changes: 14 additions & 1 deletion src/traces/heatmap/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,20 @@ module.exports = extendFlat({},
'in the `z` data are filled in.'
].join(' ')
},

xgap: {
valType: 'number',
dflt: 0,
min: 0,
role: 'style',
description: 'Sets the horizontal gap (in pixels) between bricks.'
},
ygap: {
valType: 'number',
dflt: 0,
min: 0,
role: 'style',
description: 'Sets the vertical gap (in pixels) between bricks.'
},
_nestedModules: {
'colorbar': 'Colorbar'
}
Expand Down
9 changes: 8 additions & 1 deletion src/traces/heatmap/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,14 @@ module.exports = function supplyDefaults(traceIn, traceOut, defaultColor, layout
}

coerce('text');
coerce('zsmooth');

var zsmooth = coerce('zsmooth');
if(zsmooth === false) {
// ensure that xgap and ygap are coerced only when zsmooth allows them to have an effect.
coerce('xgap');
coerce('ygap');
}

coerce('connectgaps', hasColumns(traceOut) && (traceOut.zsmooth !== false));

colorscaleDefaults(traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'z'});
Expand Down
58 changes: 57 additions & 1 deletion src/traces/heatmap/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,55 @@ function plotOne(gd, plotinfo, cd) {
rcount = 0,
gcount = 0,
bcount = 0,
brickWithPadding,
xb,
j,
xi,
v,
row,
c;

function applyBrickPadding(trace, x0, x1, y0, y1, xIndex, xLength, yIndex, yLength) {
var padding = {
x0: x0,
x1: x1,
y0: y0,
y1: y1
},
xEdgeGap = trace.xgap * 2 / 3,
yEdgeGap = trace.ygap * 2 / 3,
xCenterGap = trace.xgap / 3,
yCenterGap = trace.ygap / 3;

if(yIndex === yLength - 1) { // top edge brick
padding.y1 = y1 - yEdgeGap;
}

if(xIndex === xLength - 1) { // right edge brick
padding.x0 = x0 + xEdgeGap;
}

if(yIndex === 0) { // bottom edge brick
padding.y0 = y0 + yEdgeGap;
}

if(xIndex === 0) { // left edge brick
padding.x1 = x1 - xEdgeGap;
}

if(xIndex > 0 && xIndex < xLength - 1) { // brick in the center along x
padding.x0 = x0 + xCenterGap;
padding.x1 = x1 - xCenterGap;
}

if(yIndex > 0 && yIndex < yLength - 1) { // brick in the center along y
padding.y0 = y0 + yCenterGap;
padding.y1 = y1 - yCenterGap;
}

return padding;
}

function setColor(v, pixsize) {
if(v !== undefined) {
var c = s((v - min) / (max - min));
Expand Down Expand Up @@ -364,7 +406,21 @@ function plotOne(gd, plotinfo, cd) {
v = row[i];
c = setColor(v, (xb[1] - xb[0]) * (yb[1] - yb[0]));
context.fillStyle = 'rgba(' + c.join(',') + ')';
context.fillRect(xb[0], yb[0], (xb[1] - xb[0]), (yb[1] - yb[0]));

brickWithPadding = applyBrickPadding(trace,
xb[0],
xb[1],
yb[0],
yb[1],
i,
n,
j,
m);

context.fillRect(brickWithPadding.x0,
brickWithPadding.y0,
(brickWithPadding.x1 - brickWithPadding.x0),
(brickWithPadding.y1 - brickWithPadding.y0));
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/traces/histogram2d/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ module.exports = extendFlat({},
nbinsy: histogramAttrs.nbinsy,
ybins: histogramAttrs.ybins,

xgap: heatmapAttrs.xgap,
ygap: heatmapAttrs.ygap,
zsmooth: heatmapAttrs.zsmooth,

_nestedModules: {
Expand Down
7 changes: 6 additions & 1 deletion src/traces/histogram2d/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ module.exports = function supplyDefaults(traceIn, traceOut, layout) {

handleSampleDefaults(traceIn, traceOut, coerce);

coerce('zsmooth');
var zsmooth = coerce('zsmooth');
if(zsmooth === false) {
// ensure that xgap and ygap are coerced only when zsmooth allows them to have an effect.
coerce('xgap');
coerce('ygap');
}

colorscaleDefaults(
traceIn, traceOut, layout, coerce, {prefix: '', cLetter: 'z'}
Expand Down
Binary file added test/image/baselines/heatmap_brick_padding.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
26 changes: 26 additions & 0 deletions test/image/mocks/heatmap_brick_padding.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"data": [
{
"z": [
[
1,
20,
30
],
[
20,
1,
60
],
[
30,
60,
1
]
],
"xgap": 9,
"ygap": 6,
"type": "heatmap"
}
]
}
109 changes: 109 additions & 0 deletions test/jasmine/tests/heatmap_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,42 @@ describe('heatmap supplyDefaults', function() {
expect(traceOut.visible).toBe(false);
});

it('should set paddings to 0 when not defined', function() {
traceIn = {
type: 'heatmap',
z: [[1, 2], [3, 4]]
};

supplyDefaults(traceIn, traceOut, defaultColor, layout);
expect(traceOut.xgap).toBe(0);
expect(traceOut.ygap).toBe(0);
});

it('should not step on defined paddings', function() {
traceIn = {
xgap: 10,
type: 'heatmap',
z: [[1, 2], [3, 4]]
};

supplyDefaults(traceIn, traceOut, defaultColor, layout);
expect(traceOut.xgap).toBe(10);
expect(traceOut.ygap).toBe(0);
});

it('should not coerce gap if zsmooth is set', function() {
traceIn = {
xgap: 10,
zsmooth: 'best',
type: 'heatmap',
z: [[1, 2], [3, 4]]
};

supplyDefaults(traceIn, traceOut, defaultColor, layout);
expect(traceOut.xgap).toBe(undefined);
expect(traceOut.ygap).toBe(undefined);
});

});

describe('heatmap convertColumnXYZ', function() {
Expand Down Expand Up @@ -381,7 +417,80 @@ describe('heatmap plot', function() {

done();
});
});

it('draws canvas with correct margins', function(done) {
var mockWithPadding = require('@mocks/heatmap_brick_padding.json'),
mockWithoutPadding = Lib.extendDeep({}, mockWithPadding),
gd = createGraphDiv(),
getContextStub = {
fillRect: jasmine.createSpy()
},
originalCreateElement = document.createElement;

mockWithoutPadding.data[0].xgap = 0;
mockWithoutPadding.data[0].ygap = 0;

spyOn(document, 'createElement').and.callFake(function(elementType) {
var element = originalCreateElement.call(document, elementType);
if(elementType === 'canvas') {
spyOn(element, 'getContext').and.returnValue(getContextStub);
}
return element;
});

var argumentsWithoutPadding = [],
argumentsWithPadding = [];
Plotly.plot(gd, mockWithoutPadding.data, mockWithoutPadding.layout).then(function() {
argumentsWithoutPadding = getContextStub.fillRect.calls.allArgs().slice(0);
return Plotly.plot(gd, mockWithPadding.data, mockWithPadding.layout);
}).then(function() {
var centerXGap = mockWithPadding.data[0].xgap / 3;
var centerYGap = mockWithPadding.data[0].ygap / 3;
var edgeXGap = mockWithPadding.data[0].xgap * 2 / 3;
var edgeYGap = mockWithPadding.data[0].ygap * 2 / 3;

argumentsWithPadding = getContextStub.fillRect.calls.allArgs().slice(getContextStub.fillRect.calls.allArgs().length - 9);
expect(argumentsWithPadding).toEqual([
[argumentsWithoutPadding[0][0],
argumentsWithoutPadding[0][1] + edgeYGap,
argumentsWithoutPadding[0][2] - edgeXGap,
argumentsWithoutPadding[0][3] - edgeYGap],
[argumentsWithoutPadding[1][0] + centerXGap,
argumentsWithoutPadding[1][1] + edgeYGap,
argumentsWithoutPadding[1][2] - edgeXGap,
argumentsWithoutPadding[1][3] - edgeYGap],
[argumentsWithoutPadding[2][0] + edgeXGap,
argumentsWithoutPadding[2][1] + edgeYGap,
argumentsWithoutPadding[2][2] - edgeXGap,
argumentsWithoutPadding[2][3] - edgeYGap],
[argumentsWithoutPadding[3][0],
argumentsWithoutPadding[3][1] + centerYGap,
argumentsWithoutPadding[3][2] - edgeXGap,
argumentsWithoutPadding[3][3] - edgeYGap],
[argumentsWithoutPadding[4][0] + centerXGap,
argumentsWithoutPadding[4][1] + centerYGap,
argumentsWithoutPadding[4][2] - edgeXGap,
argumentsWithoutPadding[4][3] - edgeYGap],
[argumentsWithoutPadding[5][0] + edgeXGap,
argumentsWithoutPadding[5][1] + centerYGap,
argumentsWithoutPadding[5][2] - edgeXGap,
argumentsWithoutPadding[5][3] - edgeYGap],
[argumentsWithoutPadding[6][0],
argumentsWithoutPadding[6][1],
argumentsWithoutPadding[6][2] - edgeXGap,
argumentsWithoutPadding[6][3] - edgeYGap],
[argumentsWithoutPadding[7][0] + centerXGap,
argumentsWithoutPadding[7][1],
argumentsWithoutPadding[7][2] - edgeXGap,
argumentsWithoutPadding[7][3] - edgeYGap],
[argumentsWithoutPadding[8][0] + edgeXGap,
argumentsWithoutPadding[8][1],
argumentsWithoutPadding[8][2] - edgeXGap,
argumentsWithoutPadding[8][3] - edgeYGap
]]);
done();
});
});
});

Expand Down
59 changes: 59 additions & 0 deletions test/jasmine/tests/histogram2d_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
var supplyDefaults = require('@src/traces/histogram2d/defaults');


describe('Test histogram2d', function() {
'use strict';

describe('supplyDefaults', function() {
var traceIn,
traceOut;

beforeEach(function() {
traceOut = {};
});

it('should set zsmooth to false when zsmooth is empty', function() {
traceIn = {};
supplyDefaults(traceIn, traceOut, {});
expect(traceOut.zsmooth).toBe(false);
});

it('doesnt step on zsmooth when zsmooth is set', function() {
traceIn = {
zsmooth: 'fast'
};
supplyDefaults(traceIn, traceOut, {});
expect(traceOut.zsmooth).toBe('fast');
});

it('should set xgap and ygap to 0 when xgap and ygap are empty', function() {
traceIn = {};
supplyDefaults(traceIn, traceOut, {});
expect(traceOut.xgap).toBe(0);
expect(traceOut.ygap).toBe(0);
});

it('shouldnt step on xgap and ygap when xgap and ygap are set', function() {
traceIn = {
xgap: 10,
ygap: 5
};
supplyDefaults(traceIn, traceOut, {});
expect(traceOut.xgap).toBe(10);
expect(traceOut.ygap).toBe(5);
});

it('shouldnt coerce gap when zsmooth is set', function() {
traceIn = {
xgap: 10,
ygap: 5,
zsmooth: 'best'
};
supplyDefaults(traceIn, traceOut, {});
expect(traceOut.xgap).toBe(undefined);
expect(traceOut.ygap).toBe(undefined);
});

});

});
Loading