Skip to content
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

Reduce the precision of numbers in path strings? #10

Closed
mindrones opened this issue Nov 18, 2016 · 24 comments · Fixed by #12
Closed

Reduce the precision of numbers in path strings? #10

mindrones opened this issue Nov 18, 2016 · 24 comments · Fixed by #12

Comments

@mindrones
Copy link

mindrones commented Nov 18, 2016

Hi,

while I'm aware that numbers in
M0.123456,0.123456L1.123456,1.123456L1.123456,0.123456Z
are used internally by the browser to calculate shapes, it seems weird to me the we really need 1e-6 pixel precision. I'm wondering if reducing numbers precision in path strings can enhance DOM performances because of shorter d strings to be parsed.

This function:

(pathString, precision) => pathString.replace(/\d+\.\d+/g, s => parseFloat(s).toFixed(precision))

works well, but parsing the path string seems a waste of CPU cycles: maybe it'd be better passing a precision option to path.toString() and process all floats with f => f.toFixed(precision) before creating the output string.
In that case it would be nice to have d3.geoPath.precision([digits]) too.

What do you think about it?
Thanks!

@mbostock
Copy link
Member

I doubt this would be a performance improvement, but it is nice if you want to generate smaller strings (for example if you are serializing to GeoJSON or SVG); see d3.geoQuantize for example.

I’d probably implement this by having a different implementation of Path that calls number.toFixed rather than relying on the default string-coercion of numbers. Three potential strategies:

  1. Duplicate the implementation entirely. Have one that calls number.toFixed and another that uses string-coercion. (Note that the implementation that calls number.toFixed should still coerce to a number before calling number.toFixed.)
  2. Have a shared implementation that calls a private method to convert an input number x to a string. The default implementation would be return x + "", while the fixed-precision version would be return x.toFixed(this._precision). This adds a bit of overhead to the default implementation since there’s now an additional function call per number, though.
  3. Have a wrapper implementation that coerces each input value to a number, calls number.toFixed to round the number, and then calls through to an underlying Path instance (which is the same as the current implementation). This has no overhead in the default case, but in the case where you want fixed precision, it requires two additional coercions (from the result of number.toFixed back to a number and then back to a string again).

@mindrones
Copy link
Author

mindrones commented Nov 20, 2016

What about using arrays to store in this._ and then do string manipulation/concatenation in .toString()?

A tentative version would be:

function Path(precision) {
  //...
  this._precision = precision
  this._ = []  // instead of this._ = "";
}

//...

moveTo: function(x, y) {
  this._.push(["M", [(this._x0 = this._x1 = +x), (this._y0 = this._y1 = +y)]]);
},
closePath: function() {
  if (this._x1 !== null) {
    this._x1 = this._x0, this._y1 = this._y0;
    this._.push(["Z"]);
  }
},
rect: function(x, y, w, h) {
  this._.push(["M", [(this._x0 = this._x1 = +x), (this._y0 = this._y1 = +y)]]);
  this._.push(["h", [+w]]);
  this._.push(["v", [+h]]);
  this._.push(["h", [-w]]);
  this._.push(["Z"]);
},

//...

toString: function() {
  // instead of return this._;
  return !this.precision
    ? this._.map(a => a[0] + (a[1] ? a[1].join(',') : '')).join('')
    : this._.map(a => a[0] + (a[1] ? a[1].map(n => n.toFixed(this._precision)).join(',') : '')).join('')
}

@mindrones
Copy link
Author

mindrones commented Nov 21, 2016

If you like this approach, in order to reduce calculations when we call toString, we might reset the list of commands (this._ = []) so that later calls of toString would only process latest commands, if any.

function Path(precision) {
  //...
  this._precision = precision;
  this._ = [];
  this._str = '';
}

toString: function() {
  if (this._.length === 0) { return this._str }
  if (!this._precision) {
      this._str += this._.map(a => a[0] + (a[1] ? a[1].join(',') : '')).join('')
  } else {
      this._str += this._.map(a => a[0] + (a[1] ? a[1].map(n => n.toFixed(this._precision)).join(',') : '')).join('')
  }
  this._ = []
  return this._str
}

@mbostock
Copy link
Member

Your approaches likely have significantly more overhead than the ones I mentioned due to the large number of temporary objects and the resulting garbage collection cost.

@mindrones
Copy link
Author

Thanks, I suspected that too. If you are planning to do this soon, I'll wait and see the solution you'll come up with (always good reading d3 code), otherwise time permitting I'll try a patch following one of your approaches.

mbostock added a commit that referenced this issue Nov 22, 2016
@mbostock
Copy link
Member

My take…

Option 1 is perhaps the fastest, but also the least maintainable since the code is wholly duplicated. So I’m not wild about that.

Option 3 can’t fix the precision of derived coordinates in path.arcTo and path.arc. (Only the input coordinates are rounded, so for example, x1 + t21 * x21 may not be rounded.)

That leaves option 2, which introduces a small amount of overhead in the default case, but if we can measure that and it’s not significant then I’d be okay with this change. However, I would ask you to implement tests.

@mindrones
Copy link
Author

Ah fantastic! OK will do.

@mindrones
Copy link
Author

mindrones commented Nov 23, 2016

I did a first quick test just measuring the time (not memory consumption for example), using lodash for convenience:

_.each(_.range(7), function(exp) {
    var cycles = Math.pow(10, exp)

    var start = new Date;
    _.times(cycles, function(n) {
        d3path.path().moveTo(10.1234567890, 10.1234567890);
    })
    var curpath = (new Date - start) / cycles;

    start = new Date;
    _.times(cycles, function(n) {
        d3pathToFixed.path().moveTo(10.1234567890, 10.1234567890);
    })
    var newpath = (new Date - start) / cycles;

    start = new Date;
    _.times(cycles, function(n) {
        d3pathToFixed.pathFixed(2).moveTo(10.1234567890, 10.1234567890);
    })
    var newpathfixed = (new Date - start) / cycles;

    console.log({exp: '1e' + exp, curpath: curpath, newpath: newpath, newpathfixed: newpathfixed})
})

with these results; I did it just 3 times as they seem to be consistent. I'll add all the other path functions (lineTo, etc) asap.

$ node index.js 
{ exp: '1e0', curpath: 1, newpath: 0, newpathfixed: 0 }
{ exp: '1e1', curpath: 0, newpath: 0, newpathfixed: 0 }
{ exp: '1e2', curpath: 0, newpath: 0, newpathfixed: 0.01 }
{ exp: '1e3',
  curpath: 0.001,
  newpath: 0.001,
  newpathfixed: 0.003 }
{ exp: '1e4',
  curpath: 0.0001,
  newpath: 0.0002,
  newpathfixed: 0.002 }
{ exp: '1e5',
  curpath: 0.00008,
  newpath: 0.00017,
  newpathfixed: 0.00218 }
{ exp: '1e6',
  curpath: 0.000067,
  newpath: 0.00008,
  newpathfixed: 0.002227 }

$ node index.js 
{ exp: '1e0', curpath: 1, newpath: 0, newpathfixed: 0 }
{ exp: '1e1', curpath: 0, newpath: 0, newpathfixed: 0 }
{ exp: '1e2', curpath: 0, newpath: 0, newpathfixed: 0.01 }
{ exp: '1e3',
  curpath: 0.001,
  newpath: 0.002,
  newpathfixed: 0.002 }
{ exp: '1e4',
  curpath: 0.0001,
  newpath: 0.0002,
  newpathfixed: 0.0021 }
{ exp: '1e5',
  curpath: 0.0001,
  newpath: 0.00018,
  newpathfixed: 0.00235 }
{ exp: '1e6',
  curpath: 0.000057,
  newpath: 0.000099,
  newpathfixed: 0.002258 }

$ node index.js 
{ exp: '1e0', curpath: 1, newpath: 0, newpathfixed: 0 }
{ exp: '1e1', curpath: 0, newpath: 0, newpathfixed: 0 }
{ exp: '1e2', curpath: 0, newpath: 0, newpathfixed: 0.01 }
{ exp: '1e3',
  curpath: 0.001,
  newpath: 0.002,
  newpathfixed: 0.002 }
{ exp: '1e4', curpath: 0, newpath: 0.0003, newpathfixed: 0.0025 }
{ exp: '1e5',
  curpath: 0.00007,
  newpath: 0.00015,
  newpathfixed: 0.00205 }
{ exp: '1e6',
  curpath: 0.000063,
  newpath: 0.000074,
  newpathfixed: 0.002155 }

Also, using .toFixed() "the number is rounded if necessary, and the fractional part is padded with zeros if necessary so that it has the specified length": to avoid the string conversion and the padded zeroes maybe it would be faster to just round the number with a custom round() similar to this one.

@mbostock
Copy link
Member

I’m not sure that such the proposed rounding method is guaranteed to limit the number of decimal digits. Though certainly it does seem to be a widely-used technique. So, maybe? That was essentially how d3.round was implemented in D3 3.x; see round.js.

function round(x, n) {
  return n ? Math.round(x * (n = Math.pow(10, n))) / n : Math.round(x);
}

I removed d3.round from D3 4.0 since I didn’t think it was reliable, but perhaps it is reliable? It’s possible my judgment was affected by the earlier implementation of d3.round, which was not reliable; see d3/d3#210 (comment).

The other exponential notation technique is also interesting (as packaged in round10.)

function round(x, n) {
  x = +x, n = +n;
  x = (x + "").split("e");
  x = Math.round(+(x[0] + "e" + (x[1] ? (+x[1] + n) : n)));
  x = (x + "").split("e");
  return +(x[0] + "e" + (x[1] ? (+x[1] - n) : -n));
}

This technique is arguably more intuitive, for example:

round(1.275, 2) // 1.28

But the first technique’s answer is arguably more correct, since a more exact representation of 1.275 is 1.27499999999999991118:

(1.275).toFixed(20) // "1.27499999999999991118"

So really the question is whether you want to interpret the input value 1.275 as its binary floating point value in IEEE754, or as the shortest-equivalent decimal value, i.e., as the exact decimal number represented by the result of number.toString.

Certainly the first technique is a lot faster, and if your input values aren’t hand-authored decimal values anyway (e.g., they are the result of trigonometric computations), then I think it’s perfectly reasonable to treat them as the language-native binary floating point values.

So all that suggests it might be reasonable to re-introduce d3.round, with the caveat that we have a better understanding now of how it should behave.

@mindrones
Copy link
Author

I've tried adding the linked round function:

function round(number, precision) {
    var factor = Math.pow(10, precision);
    var tempNumber = number * factor;
    var roundedTempNumber = Math.round(tempNumber);
    return roundedTempNumber / factor;
}

export function pathRounded(digits) {
  var path = new Path;
  (digits = +digits).toFixed(digits); // Validate digits.
  path._format = function(x) { return round(+x, digits); };
  return path;
} 

and it seems it's ~2x faster than toFixed():

$ node index.js 
{ exp: '1e0',
  curpath: 1,
  newpath: 0,
  newpathfixed: 0,
  newpathrounded: 1 }
{ exp: '1e1',
  curpath: 0,
  newpath: 0,
  newpathfixed: 0,
  newpathrounded: 0 }
{ exp: '1e2',
  curpath: 0,
  newpath: 0,
  newpathfixed: 0,
  newpathrounded: 0.01 }
{ exp: '1e3',
  curpath: 0.001,
  newpath: 0.002,
  newpathfixed: 0.002,
  newpathrounded: 0.002 }
{ exp: '1e4',
  curpath: 0.0001,
  newpath: 0.0002,
  newpathfixed: 0.0019,
  newpathrounded: 0.0009 }
{ exp: '1e5',
  curpath: 0.00007,
  newpath: 0.00016,
  newpathfixed: 0.00206,
  newpathrounded: 0.00114 }
{ exp: '1e6',
  curpath: 0.000054,
  newpath: 0.000085,
  newpathfixed: 0.002005,
  newpathrounded: 0.001053 }
--- curpath --- M10.123456789,10.123456789
--- newpath --- M10.123456789,10.123456789
--- newpathfixed --- M10.12,10.12
--- newpathrounded --- M10.12,10.12

so I agree it would be nice to have a proper round() function to be used for such cases.
We should do some more in-depth testing to we get a better idea.

Also maybe input values should be considered numbers, so to avoid using +x?

@mindrones
Copy link
Author

mindrones commented Nov 23, 2016

I did some further quick-testing. Given these functions:

// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/round#PHP-Like_rounding_Method
function roundMDN(number, precision) {
    var factor = Math.pow(10, precision);
    var tempNumber = number * factor;
    var roundedTempNumber = Math.round(tempNumber);
    return roundedTempNumber / factor;
}

export function pathRoundedMDN(digits) {
  var path = new Path;
  (digits = +digits).toFixed(digits); // Validate digits.
  path._format = function(x) { return roundMDN(+x, digits); };
  return path;
}

export function pathRoundedMDNinputIsNumber(digits) {
  var path = new Path;
  (digits = +digits).toFixed(digits); // Validate digits.
  path._format = function(x) { return roundMDN(x, digits); };
  return path;
}

// https://github.com/d3/d3-format/issues/32
function round(x, n) {
  return n == null ? Math.round(x) : Math.round(x * (n = Math.pow(10, n))) / n;
}

export function pathRoundedD3(digits) {
  var path = new Path;
  (digits = +digits).toFixed(digits); // Validate digits.
  path._format = function(x) { return round(+x, digits); };
  return path;
}

export function pathRoundedD3InputIsNumber(digits) {
  var path = new Path;
  (digits = +digits).toFixed(digits); // Validate digits.
  path._format = function(x) { return round(x, digits); };
  return path;
}

we get this:

$ node index.js 
{ exp: '1e0',
  curpath: 1,
  newpath: 0,
  newpathfixed: 0,
  rounded_MDN: 0,
  rounded_MDN_inputAsNumber: 0,
  rounded_D3: 1,
  rounded_D3_inputAsNumber: 0 }
{ exp: '1e1',
  curpath: 0,
  newpath: 0,
  newpathfixed: 0,
  rounded_MDN: 0,
  rounded_MDN_inputAsNumber: 0,
  rounded_D3: 0,
  rounded_D3_inputAsNumber: 0 }
{ exp: '1e2',
  curpath: 0,
  newpath: 0,
  newpathfixed: 0.01,
  rounded_MDN: 0,
  rounded_MDN_inputAsNumber: 0.01,
  rounded_D3: 0,
  rounded_D3_inputAsNumber: 0 }
{ exp: '1e3',
  curpath: 0.001,
  newpath: 0.001,
  newpathfixed: 0.002,
  rounded_MDN: 0.001,
  rounded_MDN_inputAsNumber: 0.002,
  rounded_D3: 0.001,
  rounded_D3_inputAsNumber: 0.003 }
{ exp: '1e4',
  curpath: 0.0001,
  newpath: 0.0001,
  newpathfixed: 0.0021,
  rounded_MDN: 0.0011,
  rounded_MDN_inputAsNumber: 0.0007,
  rounded_D3: 0.001,
  rounded_D3_inputAsNumber: 0.0012 }
{ exp: '1e5',
  curpath: 0.00019,
  newpath: 0.00012,
  newpathfixed: 0.00184,
  rounded_MDN: 0.001,
  rounded_MDN_inputAsNumber: 0.00115,
  rounded_D3: 0.00116,
  rounded_D3_inputAsNumber: 0.00097 }
{ exp: '1e6',
  curpath: 0.000063,
  newpath: 0.000082,
  newpathfixed: 0.002072,
  rounded_MDN: 0.001002,
  rounded_MDN_inputAsNumber: 0.001043,
  rounded_D3: 0.001006,
  rounded_D3_inputAsNumber: 0.00097 }
--- curpath --- M10.123456789,10.123456789
--- newpath --- M10.123456789,10.123456789
--- newpathfixed --- M10.12,10.12
--- rounded_MDN --- M10.12,10.12
--- rounded_D3 --- M10.12,10.12

so it would seem that not using +x doesn't affect calculation time that much, but it helps, and that your rounding function is a little bit faster.

@mindrones
Copy link
Author

After doing some testing I've moved some code in this repo so that we can play with ideas eventually. I've added some versions of moveTo using ifs trying to see if it's possible not to repeat code too much: it seems better than using _format at least :)

Now that I have a rough structure, I'll add more tests for the other commands so that we can discuss on numbers.

Meanwhile here are some results:

Executing 1 command call per test...
path.current.path().moveTo x 5,390,351 ops/sec ±1.49% (83 runs sampled)
path.withFormat.path().moveTo x 4,964,609 ops/sec ±1.62% (82 runs sampled)
path.withFormat.pathCoerceFixed(2).moveTo x 500,920 ops/sec ±2.95% (75 runs sampled)
path.withFormat.pathFixed(2).moveTo x 751,370 ops/sec ±2.77% (73 runs sampled)
path.withFormat.pathCoerceRound(2).moveTo x 1,391,397 ops/sec ±2.69% (72 runs sampled)
path.withFormat.pathRound(2).moveTo x 1,436,686 ops/sec ±2.95% (70 runs sampled)
path.withIf.path().moveTo x 4,541,117 ops/sec ±2.07% (83 runs sampled)
path.withIf.path(2).moveTo x 3,711,988 ops/sec ±1.22% (86 runs sampled)
Executing 10 command calls per test...
path.current.path().moveTo x 1,179,496 ops/sec ±1.68% (84 runs sampled)
path.withFormat.path().moveTo x 1,217,988 ops/sec ±1.15% (83 runs sampled)
path.withFormat.pathCoerceFixed(2).moveTo x 71,614 ops/sec ±1.67% (81 runs sampled)
path.withFormat.pathFixed(2).moveTo x 106,151 ops/sec ±1.81% (81 runs sampled)
path.withFormat.pathCoerceRound(2).moveTo x 418,692 ops/sec ±1.83% (78 runs sampled)
path.withFormat.pathRound(2).moveTo x 413,930 ops/sec ±2.28% (78 runs sampled)
path.withIf.path().moveTo x 1,261,124 ops/sec ±1.21% (86 runs sampled)
path.withIf.path(2).moveTo x 741,122 ops/sec ±1.42% (86 runs sampled)
Executing 100 command calls per test...
path.current.path().moveTo x 158,843 ops/sec ±3.43% (78 runs sampled)
path.withFormat.path().moveTo x 165,261 ops/sec ±2.01% (81 runs sampled)
path.withFormat.pathCoerceFixed(2).moveTo x 6,840 ops/sec ±2.83% (78 runs sampled)
path.withFormat.pathFixed(2).moveTo x 9,616 ops/sec ±2.17% (81 runs sampled)
path.withFormat.pathCoerceRound(2).moveTo x 53,534 ops/sec ±2.24% (80 runs sampled)
path.withFormat.pathRound(2).moveTo x 53,603 ops/sec ±3.87% (74 runs sampled)
path.withIf.path().moveTo x 156,071 ops/sec ±2.09% (82 runs sampled)
path.withIf.path(2).moveTo x 81,802 ops/sec ±2.55% (79 runs sampled)
Executing 1000 command calls per test...
path.current.path().moveTo x 14,793 ops/sec ±2.08% (82 runs sampled)
path.withFormat.path().moveTo x 14,506 ops/sec ±3.15% (81 runs sampled)
path.withFormat.pathCoerceFixed(2).moveTo x 663 ops/sec ±2.71% (77 runs sampled)
path.withFormat.pathFixed(2).moveTo x 1,055 ops/sec ±3.30% (78 runs sampled)
path.withFormat.pathCoerceRound(2).moveTo x 5,157 ops/sec ±2.73% (80 runs sampled)
path.withFormat.pathRound(2).moveTo x 5,880 ops/sec ±2.03% (81 runs sampled)
path.withIf.path().moveTo x 14,044 ops/sec ±4.20% (80 runs sampled)
path.withIf.path(2).moveTo x 7,836 ops/sec ±2.94% (82 runs sampled)
Executing 10000 command calls per test...
path.current.path().moveTo x 1,039 ops/sec ±17.22% (77 runs sampled)
path.withFormat.path().moveTo x 906 ops/sec ±18.15% (70 runs sampled)
path.withFormat.pathCoerceFixed(2).moveTo x 62.34 ops/sec ±2.52% (62 runs sampled)
path.withFormat.pathFixed(2).moveTo x 81.83 ops/sec ±25.11% (58 runs sampled)
path.withFormat.pathCoerceRound(2).moveTo x 313 ops/sec ±20.34% (60 runs sampled)
path.withFormat.pathRound(2).moveTo x 282 ops/sec ±19.74% (59 runs sampled)
path.withIf.path().moveTo x 350 ops/sec ±5.77% (70 runs sampled)
path.withIf.path(2).moveTo x 318 ops/sec ±8.97% (51 runs sampled)

@mbostock
Copy link
Member

Can you summarize those results? It’s hard to read those numbers at a glance. If only there were some way to, I dunno, visualize it… 😉

@mindrones
Copy link
Author

mindrones commented Jan 24, 2017

I've worked a bit more on these benchmarks, adding the ability to measure the used heap memory: here you can find a detailed explanation of what I've done.

Also, I've added an interactive chart to explore the results: you can compare results for different implementations, number of digits and commands (moveTo, lineTo, etc..).

For example, here's the case for .moveTo() not rounding input values (digits = null):

d3_benchmarks_moveto_null

while here's the case for rounding with _format() and ifs, with digits equal to 5 and 15:

d3_benchmarks_moveto_rounding_5_15

Here we compare not rounding versus rounding with 5 digits:

d3_benchmarks_moveto_rounding_5_vs_not_rounding


EDIT: The implementation using _format() (withFormat.path.*) has curves named pathCoerceRound, pathCoerceFixed, pathFixed: I added them to check the impact of input values coercion and of toFixed when invoking the command a lot of times: you can't compare these results with the "iffed" implementation so ignore them if you're not interested. The interesting part is that coercion slowdown should be neglectable; here's an example:

screen shot 2017-01-31 at 17 29 15


I still have to investigate why we get higher memory usage (and sometimes higher execution times, as in the picture above) when we call the command just once (pathInstance.moveTo(10.1234567890123456,10.1234567890123456)): it might be that garbage collection happens during the test (I doubt it being a very quick test) or something related to how I measure the heap (explicitly garbage collect before and after releasing the path instance p).

All in all, my interpretation is that using _format() would be OK as the execution times increase ~2x when we call for example .moveTo() 10 or more times.
Looking forward to read yours :)

@mindrones
Copy link
Author

mindrones commented Jan 25, 2017

Re: greater-than-expected execution time when invoking the command just once, I'm now thinking it might be because we're hitting microsecond scale, hence those points' x-coordinate doesn't represent the actual duration. Still mumbling about memory values.

EDIT I've installed microtime (which benchmark detects and uses, if present), but seems it won't help when invoking the command just once, so probably it's just that we loose accuracy at that scale.

screen shot 2017-01-26 at 12 45 23

@mindrones
Copy link
Author

mindrones commented Jan 26, 2017

By the way I noticed a couple of things in Chrome.

Large d strings make debugging DOM elements pretty slow (plus, Chrome won't let you edit them), so hopefully shorter strings will improve performances, at least while debugging.

screen shot 2016-12-02 at 14 47 16

By inspecting the d attribute, it would seem that internally Chrome rounds numbers to the 3rd digit, or it just format numbers in that panel for readability.

screen shot 2016-12-02 at 14 48 04

@mindrones
Copy link
Author

mindrones commented Jan 30, 2017

I've added another interactive to see hands-on if we get some performance benefits in the browser: please have a try at https://mindrones.github.io/d3-benchmarks/path_client/ (I've explained what I've done here).

It's a path with all its points on a circle and you can control how many points you want to draw: these points gets updated with a timer, so they continuously change at each iteration, making the circle rotate indefinitely.
I patched the d3.line() implementation so that it uses path.withFormat.path() and path.withFormat.pathRound() and added line.precision() to control the number of digits.

Basically:

  • using the path implementation with format() doesn't seem to impact performances;
  • if you use enough points (100k on my machine), you'll see that not rounding is ~2x slower than rounding with no digits. Rounding with 3 digits (1/1000th of a pixel seems enough to me), gives benefits too.

Using the implementation with format() and changing the number of digits we get what we see below (let's focus on the mean value in the stats widget):

  • not rounding:

    path_client_withformat_null

  • rounding with 3 digits

    path_client_withformat_3

  • rounding with zero digits:

    path_client_withformat_0

Note that if you're interested to see these numbers with more precision than comparing stats widgets :), we could do an interactive that measures and plots these numbers using window.performance.now()

To summarize, I think rounding numbers in d3-path functions helps charts using <path> with a lot of points, while using format() doesn't seem to slow down things too much even if set to the identity function. Of course if this goes upstream, functions depending on d3-path methods should be patched with functions/methods like .precision() or similar to use the new implementation.

@MrOrz
Copy link

MrOrz commented Sep 30, 2017

Hi all,

I just come across this post when seeking for solutions for this kind of DOM snapshot error:

           <path
    -        d="M4.440892098500626e-15,-58.48076606885378A1.5,1.5,0,0,1,1.538461538461543,-59.98027289113208A60,60,0,1,1,-49.818670465862,33.43800342445478A1.5,1.5,0,0,1,-49.37694389668395,31.335561450590387L-49.37694389668395,31.335561450590387A1.5,1.5,0,0,1,-47.327736942568905,31.766103253232043A57,57,0,1,0,1.461538461538468,-56.98125924657548A1.5,1.5,0,0,1,4.440892098500626e-15,-58.48076606885378Z"
    +        d="M4.440892098500626e-15,-58.48076606885378A1.5,1.5,0,0,1,1.538461538461543,-59.98027289113208A60,60,0,1,1,-49.818670465862006,33.43800342445478A1.5,1.5,0,0,1,-49.37694389668395,31.335561450590387L-49.37694389668395,31.335561450590387A1.5,1.5,0,0,1,-47.327736942568905,31.766103253232043A57,57,0,1,0,1.461538461538468,-56.98125924657548A1.5,1.5,0,0,1,4.440892098500626e-15,-58.48076606885378Z"
             fill="#F9CF54"
           />

The snapshot was generated on Mac but the unit tests were ran on Linux systems, and the floating point error makes the entire snapshot testing fail.

I found @mindrones's snippet really useful in this scenario. When doing tests, we have the luxury to trade CPU cycles for a passing unit test.

The following mock works for me:

// <jest root>/__mocks__/d3-path.js
const pathModule = require.requireActual('d3-path');
const PRECISION = 2;

pathModule.path.prototype.toString = function() {
  // Ref: https://github.com/d3/d3-path/issues/10
  //
  return this._.replace(/\d+\.\d+/g, s => parseFloat(s).toFixed(PRECISION));
};

module.exports = pathModule;

@mbostock
Copy link
Member

For comparing paths in tests, see also test.pathEqual from d3-shape.

@tobia
Copy link

tobia commented Nov 24, 2017

I came upon this issue because the paths generated by geoPath have the coordinates in full floating point precision, which results in huge SVG attributes, slow/crashing developer tools, and the kind of issues mentioned in this thread.

In my case, I just made the SVG virtual size (viewBox) large enough for the visual precision I need (about 1.5 SVG units per screen pixel, for normal resolution screens) and then truncated the decimals entirely:

const path = d3.geoPath().projection(...)
const path_trunc = d => path(d).replace(/\.\d+/g, '')

But I was wondering if geoPath had this functionality already built-in, instead of hacking away at the generated string!

I'm not sure how to use geoQuantize to achieve the same result, or if it's even the right tool for the job. This is strictly a SVG serialization issue.

@jackrusher
Copy link

Hello everything, thanks for all your hard work on D3!

I'm just dropping round to mention that I'd love to see this issued fixed. I'm currently consuming D3 by way of Vega, and reducing path decimals from 13 digits of mantissa down to 4 reduces the file size of emitted SVGs by ~75% and makes downstream testing much easier.

@fouronnes
Copy link

Just dropping by to say looking forward to this patch as well! My use case is also optimizing output file size, and the potential savings are huge. Thanks again for the great work!

@jkjustjoshing
Copy link

I'm seeing a similar issue to @MrOrz, but in the context of React server side rendering hydration. The path string that is generated in Nodejs on the server has slight floating point differences to what's generated in the browser, resulting in this error.

image

@mbostock
Copy link
Member

Apologies for taking a few years, but I’m working on this now in #12 and it should be available soon. Note that d3-path isn’t used by d3-geo, but we’ll take a similar approach in d3.geoPath to limit the precision of generated SVG path data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

7 participants