Skip to content

Commit 89d011d

Browse files
committed
keep domainpad a bit for bit no-op when it is not set
The three places that build the plot rect from `domain` were changed to read _offset and _length instead. That round trips through pixel space, and the result is not exactly the value it started from: splom cell domains came back about one ulp out, which moved rasterised markers enough to fail the splom_*-nodiag image baselines even though no padding was involved. Add the resolved padding to the original expressions instead, and expose it as ax._padStart / ax._padEnd for those callers. Adding a literal zero is exact, so an unpadded plot now produces the same numbers it always did. Also list domainpad in swapAxisGroup's noSwapAttrs, next to domain: its keys are named for screen edges, so swapping x and y would need them remapped.
1 parent a1d31d1 commit 89d011d

6 files changed

Lines changed: 83 additions & 28 deletions

File tree

src/plots/cartesian/axes.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4605,7 +4605,10 @@ function swapAxisGroup(gd, xIds, yIds) {
46054605
var allAxKeys = Object.keys(axAttrs);
46064606

46074607
var noSwapAttrs = [
4608-
'anchor', 'domain', 'overlaying', 'position', 'side', 'tickangle', 'editType'
4608+
// domainpad sits with domain here: its keys are named for screen edges, so
4609+
// swapping x and y would have to remap left/right onto top/bottom
4610+
'anchor', 'domain', 'domainpad', 'overlaying', 'position', 'side',
4611+
'tickangle', 'editType'
46094612
];
46104613
var numericTypes = ['linear', 'log'];
46114614

src/plots/cartesian/set_convert.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,12 @@ module.exports = function setConvert(ax, fullLayout) {
618618
}
619619

620620
ax._length = bandLength - padStart - padEnd;
621+
// the resolved padding, for the few places that build the plot rect from
622+
// `domain` themselves and would otherwise miss it. They add these instead of
623+
// rebuilding from _offset and _length, which would round trip through pixel
624+
// space and lose a bit or two even when there is no padding at all.
625+
ax._padStart = padStart;
626+
ax._padEnd = padEnd;
621627

622628
if(isY) {
623629
ax._offset = gs.t + (1 - ax.domain[1]) * gs.h + padStart;

src/traces/scattergl/plot.js

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,31 @@ var styleTextSelection = require('./edit_style').styleTextSelection;
1717
var reglPrecompiled = {};
1818

1919
function getViewport(fullLayout, xaxis, yaxis, plotGlPixelRatio) {
20-
// This is the same rectangle the svg side draws into, only measured up from the
21-
// bottom of the figure instead of down from the top. Read it off _offset and
22-
// _length rather than working it out from `domain` again, so that anything which
23-
// moves the plot area by a pixel amount - `domainpad`, say - lands here as well.
24-
var height = fullLayout.height;
25-
26-
var left = xaxis._offset;
27-
var right = xaxis._offset + xaxis._length;
28-
var bottom = height - (yaxis._offset + yaxis._length);
29-
var top = height - yaxis._offset;
20+
var gs = fullLayout._size;
21+
var width = fullLayout.width * plotGlPixelRatio;
22+
var height = fullLayout.height * plotGlPixelRatio;
23+
24+
var l = gs.l * plotGlPixelRatio;
25+
var b = gs.b * plotGlPixelRatio;
26+
var r = gs.r * plotGlPixelRatio;
27+
var t = gs.t * plotGlPixelRatio;
28+
var w = gs.w * plotGlPixelRatio;
29+
var h = gs.h * plotGlPixelRatio;
30+
31+
// `domainpad` takes pixels off the plot area that `domain` knows nothing about.
32+
// Add it to the expressions below rather than rebuilding the rect from _offset
33+
// and _length: that would round trip through pixel space and shift this rect by
34+
// a fraction of a pixel even on plots with no padding at all.
35+
var padL = (xaxis._padStart || 0) * plotGlPixelRatio;
36+
var padR = (xaxis._padEnd || 0) * plotGlPixelRatio;
37+
var padT = (yaxis._padStart || 0) * plotGlPixelRatio;
38+
var padB = (yaxis._padEnd || 0) * plotGlPixelRatio;
3039

3140
return [
32-
left * plotGlPixelRatio,
33-
bottom * plotGlPixelRatio,
34-
right * plotGlPixelRatio,
35-
top * plotGlPixelRatio
41+
l + xaxis.domain[0] * w + padL,
42+
b + yaxis.domain[0] * h + padB,
43+
(width - r) - (1 - xaxis.domain[1]) * w - padR,
44+
(height - t) - (1 - yaxis.domain[1]) * h - padT
3645
];
3746
}
3847

src/traces/splom/base_plot.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ function updateGrid(gd) {
9090
function makeGridData(gd) {
9191
var plotGlPixelRatio = gd._context.plotGlPixelRatio;
9292
var fullLayout = gd._fullLayout;
93+
var gs = fullLayout._size;
9394
var fullView = [
9495
0, 0,
9596
fullLayout.width * plotGlPixelRatio,
@@ -135,9 +136,9 @@ function makeGridData(gd) {
135136

136137
// ya.l2p assumes top-to-bottom coordinate system (a la SVG),
137138
// we need to compute bottom-to-top offsets and slopes.
138-
// Flip the axis' own bottom edge rather than rebuilding it from `domain`,
139-
// so pixel adjustments to the plot area such as `domainpad` come along:
140-
var yOffset = fullLayout.height - (ya._offset + ya._length);
139+
// `domainpad` is added on rather than folded in by reading _offset, so that
140+
// an unpadded axis lands on exactly the pixel it always did:
141+
var yOffset = gs.b + ya.domain[0] * gs.h + (ya._padEnd || 0);
141142
var ym = -ya._m;
142143
var yb = -ym * ya.r2l(ya.range[0], ya.calendar);
143144
var x, y;

src/traces/splom/plot.js

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,11 @@ function plotOne(gd, cd0) {
4242
viewOpts.domains = new Array(visibleLength);
4343

4444
// regl-splom places each cell as a fraction of the viewport below, which is the
45-
// whole plot area. Derive those fractions from where the axes actually ended up
46-
// rather than from `domain`, otherwise anything that shifts the plot area in
47-
// pixels - `domainpad` - would move the axes but leave the points behind.
48-
// regl counts y up from the bottom, so the y pair comes back reversed.
49-
function xFraction(px) { return (px - gs.l) / gs.w; }
50-
function yFraction(px) { return (fullLayout.height - px - gs.b) / gs.h; }
45+
// whole plot area, so `domainpad` has to be folded in as a fraction too or the
46+
// cells keep their unpadded size while the axes move. Added to `domain` rather
47+
// than recovered from _offset and _length, which would round trip through pixel
48+
// space and nudge every cell a little even with no padding set.
49+
function padFrac(px, total) { return (px || 0) / total; }
5150

5251
for(k = 0; k < visibleDims.length; k++) {
5352
i = visibleDims[k];
@@ -59,16 +58,16 @@ function plotOne(gd, cd0) {
5958
if(xa) {
6059
rng[0] = xa._rl[0];
6160
rng[2] = xa._rl[1];
62-
dmn[0] = xFraction(xa._offset);
63-
dmn[2] = xFraction(xa._offset + xa._length);
61+
dmn[0] = xa.domain[0] + padFrac(xa._padStart, gs.w);
62+
dmn[2] = xa.domain[1] - padFrac(xa._padEnd, gs.w);
6463
}
6564

6665
ya = AxisIDs.getFromId(gd, trace._diag[i][1]);
6766
if(ya) {
6867
rng[1] = ya._rl[0];
6968
rng[3] = ya._rl[1];
70-
dmn[1] = yFraction(ya._offset + ya._length);
71-
dmn[3] = yFraction(ya._offset);
69+
dmn[1] = ya.domain[0] + padFrac(ya._padEnd, gs.h);
70+
dmn[3] = ya.domain[1] - padFrac(ya._padStart, gs.h);
7271
}
7372
}
7473

test/jasmine/tests/axes_test.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8903,6 +8903,43 @@ describe('axis domainpad', function() {
89038903
.then(done, done.fail);
89048904
});
89058905

8906+
it('should leave every number exactly as it was when no padding is set', function(done) {
8907+
// The pad has to be a true no-op at its default. Rebuilding the plot rect
8908+
// from _offset and _length instead of adding the pad to `domain` shifted
8909+
// webgl output by a fraction of a pixel on unpadded plots, which is enough
8910+
// to move rasterised markers and fail an image baseline. Compare with ===,
8911+
// not toBeCloseTo, because that is the size of the mistake being guarded.
8912+
Plotly.newPlot(gd, [
8913+
{y: [1, 2, 3]},
8914+
{y: [2, 1, 3], xaxis: 'x2', yaxis: 'y2'}
8915+
], {
8916+
width: 600, height: 500, margin: {l: 80, r: 80, t: 100, b: 80},
8917+
xaxis: {domain: [0, 0.3103448275862069], anchor: 'y'},
8918+
yaxis: {domain: [0.6896551724137931, 1], anchor: 'x'},
8919+
xaxis2: {domain: [0.3448275862068966, 0.6551724137931034], anchor: 'y2'},
8920+
yaxis2: {domain: [0, 0.3103448275862069], anchor: 'x2'}
8921+
})
8922+
.then(function() {
8923+
var fl = gd._fullLayout;
8924+
var gs = fl._size;
8925+
8926+
['xaxis', 'yaxis', 'xaxis2', 'yaxis2'].forEach(function(name) {
8927+
var ax = fl[name];
8928+
var isY = name.charAt(0) === 'y';
8929+
var wantOffset = isY ?
8930+
gs.t + (1 - ax.domain[1]) * gs.h :
8931+
gs.l + ax.domain[0] * gs.w;
8932+
var wantLength = (isY ? gs.h : gs.w) * (ax.domain[1] - ax.domain[0]);
8933+
8934+
expect(ax._offset).withContext(name + ' offset').toBe(wantOffset);
8935+
expect(ax._length).withContext(name + ' length').toBe(wantLength);
8936+
expect(ax._padStart).withContext(name + ' padStart').toBe(0);
8937+
expect(ax._padEnd).withContext(name + ' padEnd').toBe(0);
8938+
});
8939+
})
8940+
.then(done, done.fail);
8941+
});
8942+
89068943
it('should not drift when the axis gets rescaled again and again', function(done) {
89078944
// setScale runs several times per draw, so the padding has to be read
89088945
// fresh each time rather than piled onto the previous result

0 commit comments

Comments
 (0)