Skip to content

Commit 93c78c6

Browse files
mbostockmbostock
committed
Fix drift in brush extent.
Previously, if you set the brush extent externally, the extent could drift slightly because it was internally stored in pixel space rather than in data space. To avoid drift, the brush now preserves the extent exactly as-set, only nullifying the externally-set extent when the brush is moved.
1 parent e46b161 commit 93c78c6

4 files changed

Lines changed: 116 additions & 24 deletions

File tree

‎d3.v2.js‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4235,10 +4235,11 @@ function d3_svg_axisSubdivide(scale, ticks, m) {
42354235
}
42364236
d3.svg.brush = function() {
42374237
var event = d3_eventDispatch(brush, "brushstart", "brush", "brushend"),
4238-
x, // x-scale, optional
4239-
y, // y-scale, optional
4238+
x = null, // x-scale, optional
4239+
y = null, // y-scale, optional
42404240
resizes = d3_svg_brushResizes[0],
4241-
extent = [[0, 0], [0, 0]]; // [x0, y0], [x1, y1]
4241+
extent = [[0, 0], [0, 0]], // [x0, y0], [x1, y1], in pixels (integers)
4242+
extentDomain; // the extent in data space, lazily created
42424243

42434244
function brush(g) {
42444245
g.each(function() {
@@ -4469,6 +4470,7 @@ d3.svg.brush = function() {
44694470

44704471
// Update the stored bounds.
44714472
if (extent[0][i] !== min || extent[1][i] !== max) {
4473+
extentDomain = null;
44724474
extent[0][i] = min;
44734475
extent[1][i] = max;
44744476
return true;
@@ -4513,39 +4515,50 @@ d3.svg.brush = function() {
45134515

45144516
// Invert the pixel extent to data-space.
45154517
if (!arguments.length) {
4518+
z = extentDomain || extent;
45164519
if (x) {
4517-
x0 = extent[0][0], x1 = extent[1][0];
4518-
if (x.invert) x0 = x.invert(x0), x1 = x.invert(x1);
4519-
if (x1 < x0) t = x0, x0 = x1, x1 = t;
4520+
x0 = z[0][0], x1 = z[1][0];
4521+
if (!extentDomain) {
4522+
x0 = extent[0][0], x1 = extent[1][0];
4523+
if (x.invert) x0 = x.invert(x0), x1 = x.invert(x1);
4524+
if (x1 < x0) t = x0, x0 = x1, x1 = t;
4525+
}
45204526
}
45214527
if (y) {
4522-
y0 = extent[0][1], y1 = extent[1][1];
4523-
if (y.invert) y0 = y.invert(y0), y1 = y.invert(y1);
4524-
if (y1 < y0) t = y0, y0 = y1, y1 = t;
4528+
y0 = z[0][1], y1 = z[1][1];
4529+
if (!extentDomain) {
4530+
y0 = extent[0][1], y1 = extent[1][1];
4531+
if (y.invert) y0 = y.invert(y0), y1 = y.invert(y1);
4532+
if (y1 < y0) t = y0, y0 = y1, y1 = t;
4533+
}
45254534
}
45264535
return x && y ? [[x0, y0], [x1, y1]] : x ? [x0, x1] : y && [y0, y1];
45274536
}
45284537

45294538
// Scale the data-space extent to pixels.
4539+
extentDomain = [[0, 0], [0, 0]];
45304540
if (x) {
45314541
x0 = z[0], x1 = z[1];
45324542
if (y) x0 = x0[0], x1 = x1[0];
4543+
extentDomain[0][0] = x0, extentDomain[1][0] = x1;
45334544
if (x.invert) x0 = x(x0), x1 = x(x1);
45344545
if (x1 < x0) t = x0, x0 = x1, x1 = t;
4535-
extent[0][0] = x0, extent[1][0] = x1;
4546+
extent[0][0] = x0 | 0, extent[1][0] = x1 | 0;
45364547
}
45374548
if (y) {
45384549
y0 = z[0], y1 = z[1];
45394550
if (x) y0 = y0[1], y1 = y1[1];
4551+
extentDomain[0][1] = y0, extentDomain[1][1] = y1;
45404552
if (y.invert) y0 = y(y0), y1 = y(y1);
45414553
if (y1 < y0) t = y0, y0 = y1, y1 = t;
4542-
extent[0][1] = y0, extent[1][1] = y1;
4554+
extent[0][1] = y0 | 0, extent[1][1] = y1 | 0;
45434555
}
45444556

45454557
return brush;
45464558
};
45474559

45484560
brush.clear = function() {
4561+
extentDomain = null;
45494562
extent[0][0] =
45504563
extent[0][1] =
45514564
extent[1][0] =

‎d3.v2.min.js‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/svg/brush.js‎

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
d3.svg.brush = function() {
22
var event = d3_eventDispatch(brush, "brushstart", "brush", "brushend"),
3-
x, // x-scale, optional
4-
y, // y-scale, optional
3+
x = null, // x-scale, optional
4+
y = null, // y-scale, optional
55
resizes = d3_svg_brushResizes[0],
6-
extent = [[0, 0], [0, 0]]; // [x0, y0], [x1, y1]
6+
extent = [[0, 0], [0, 0]], // [x0, y0], [x1, y1], in pixels (integers)
7+
extentDomain; // the extent in data space, lazily created
78

89
function brush(g) {
910
g.each(function() {
@@ -234,6 +235,7 @@ d3.svg.brush = function() {
234235

235236
// Update the stored bounds.
236237
if (extent[0][i] !== min || extent[1][i] !== max) {
238+
extentDomain = null;
237239
extent[0][i] = min;
238240
extent[1][i] = max;
239241
return true;
@@ -278,39 +280,50 @@ d3.svg.brush = function() {
278280

279281
// Invert the pixel extent to data-space.
280282
if (!arguments.length) {
283+
z = extentDomain || extent;
281284
if (x) {
282-
x0 = extent[0][0], x1 = extent[1][0];
283-
if (x.invert) x0 = x.invert(x0), x1 = x.invert(x1);
284-
if (x1 < x0) t = x0, x0 = x1, x1 = t;
285+
x0 = z[0][0], x1 = z[1][0];
286+
if (!extentDomain) {
287+
x0 = extent[0][0], x1 = extent[1][0];
288+
if (x.invert) x0 = x.invert(x0), x1 = x.invert(x1);
289+
if (x1 < x0) t = x0, x0 = x1, x1 = t;
290+
}
285291
}
286292
if (y) {
287-
y0 = extent[0][1], y1 = extent[1][1];
288-
if (y.invert) y0 = y.invert(y0), y1 = y.invert(y1);
289-
if (y1 < y0) t = y0, y0 = y1, y1 = t;
293+
y0 = z[0][1], y1 = z[1][1];
294+
if (!extentDomain) {
295+
y0 = extent[0][1], y1 = extent[1][1];
296+
if (y.invert) y0 = y.invert(y0), y1 = y.invert(y1);
297+
if (y1 < y0) t = y0, y0 = y1, y1 = t;
298+
}
290299
}
291300
return x && y ? [[x0, y0], [x1, y1]] : x ? [x0, x1] : y && [y0, y1];
292301
}
293302

294303
// Scale the data-space extent to pixels.
304+
extentDomain = [[0, 0], [0, 0]];
295305
if (x) {
296306
x0 = z[0], x1 = z[1];
297307
if (y) x0 = x0[0], x1 = x1[0];
308+
extentDomain[0][0] = x0, extentDomain[1][0] = x1;
298309
if (x.invert) x0 = x(x0), x1 = x(x1);
299310
if (x1 < x0) t = x0, x0 = x1, x1 = t;
300-
extent[0][0] = x0, extent[1][0] = x1;
311+
extent[0][0] = x0 | 0, extent[1][0] = x1 | 0;
301312
}
302313
if (y) {
303314
y0 = z[0], y1 = z[1];
304315
if (x) y0 = y0[1], y1 = y1[1];
316+
extentDomain[0][1] = y0, extentDomain[1][1] = y1;
305317
if (y.invert) y0 = y(y0), y1 = y(y1);
306318
if (y1 < y0) t = y0, y0 = y1, y1 = t;
307-
extent[0][1] = y0, extent[1][1] = y1;
319+
extent[0][1] = y0 | 0, extent[1][1] = y1 | 0;
308320
}
309321

310322
return brush;
311323
};
312324

313325
brush.clear = function() {
326+
extentDomain = null;
314327
extent[0][0] =
315328
extent[0][1] =
316329
extent[1][0] =

‎test/svg/brush-test.js‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
require("../env");
2+
3+
var vows = require("vows"),
4+
assert = require("assert");
5+
6+
var suite = vows.describe("d3.svg.brush");
7+
8+
suite.addBatch({
9+
"brush": {
10+
topic: function() {
11+
return d3.svg.brush;
12+
},
13+
14+
"x": {
15+
"defaults to null": function(brush) {
16+
assert.isNull(brush().x());
17+
}
18+
},
19+
20+
"y": {
21+
"defaults to null": function(brush) {
22+
assert.isNull(brush().y());
23+
}
24+
},
25+
26+
"extent": {
27+
"returns null when no scales are attached": function(brush) {
28+
assert.isNull(brush().extent());
29+
},
30+
"returns a one-dimensional array if only x is defined": function(brush) {
31+
var b = brush().x(d3.scale.linear());
32+
assert.deepEqual(b.extent(), [0, 0]);
33+
},
34+
"takes a one-dimensional array if only x is defined": function(brush) {
35+
var b = brush().x(d3.scale.linear()).extent([0.1, 0.4]);
36+
assert.deepEqual(b.extent(), [0.1, 0.4]);
37+
},
38+
"returns a one-dimensional array if only y is defined": function(brush) {
39+
var b = brush().y(d3.scale.linear());
40+
assert.deepEqual(b.extent(), [0, 0]);
41+
},
42+
"takes a one-dimensional array if only y is defined": function(brush) {
43+
var b = brush().y(d3.scale.linear()).extent([0.1, 0.4]);
44+
assert.deepEqual(b.extent(), [0.1, 0.4]);
45+
},
46+
"returns a two-dimensional array if x and y are defined": function(brush) {
47+
var b = brush().x(d3.scale.linear()).y(d3.scale.linear());
48+
assert.deepEqual(b.extent(), [[0, 0], [0, 0]]);
49+
},
50+
"takes a two-dimensional array if x and y are defined": function(brush) {
51+
var b = brush().x(d3.scale.linear()).y(d3.scale.linear()).extent([[0.1, 0.2], [0.3, 0.4]]);
52+
assert.deepEqual(b.extent(), [[0.1, 0.2], [0.3, 0.4]]);
53+
},
54+
"preserves the set extent exactly": function(brush) {
55+
var lo = new Number(0.1),
56+
hi = new Number(0.3),
57+
b = brush().x(d3.scale.linear()).extent([lo, hi]),
58+
extent = b.extent();
59+
assert.strictEqual(extent[0], lo);
60+
assert.strictEqual(extent[1], hi);
61+
}
62+
}
63+
}
64+
});
65+
66+
suite.export(module);

0 commit comments

Comments
 (0)