search

d3 / d3-path

Serialize Canvas path commands to SVG.
https://d3js.org/d3-path
ISC License
196 stars 37 forks source link

Add d3.pathRound. #12

Closed mbostock closed 1 year ago

mbostock commented 7 years ago

Fixes #10, taking option 2.

mbostock commented 5 years ago

I’m pretty sure we don’t want to introduce the overhead of calling the identity function in the common case, which means a lot of code duplication.

Fil commented 4 years ago

Now trying to call this._format as parsimoniously as possible; in the standard case _format return +x. I hope this limits the overhead.

(I'll review the git pseudo-conflicts situation if the approach seems acceptable.)

mbostock commented 4 years ago

I think of format as something that takes a number and returns a string. Relying on format to return a number (and taking arbitrary input) suggests that “format” is the wrong name.

Did you overwrite the original commits?

Fil commented 4 years ago

Hmm it looks like I force-pushed over your original commit ( https://github.com/d3/d3-path/commit/7b1c4104034c008eb0c8b60b08c7da7b55331351 ) in June 2019, after a rebase.

Fil commented 4 years ago

should we name this function this._number?

Fil commented 4 years ago

yet another rebase, sorry—this has me struggling

Reference commits are: https://github.com/d3/d3-path/commit/7b1c4104034c008eb0c8b60b08c7da7b55331351 (original) and https://github.com/d3/d3-path/pull/12/commits/2897aa62351747b14b7e77bd651d7e93555cedf7 (rebase against branch two)

mbostock commented 1 year ago

I’ve updated this to take a cleaner approach using an internal tagged template literal, and changed d3.pathFixed so that it defaults digits to three (which matches number.toLocaleString and seems like a very reasonable default). I think this is ready to go.

mbostock commented 1 year ago

The latest commit switches to rounding per performance recommendations in #10 and https://github.com/d3/d3-geo/pull/272#issuecomment-1357067796.