Skip to content

Updating circle, ellipse, line-arc, and sector to use smarter approximations #2910

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.

Sign up for GitHub

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

Open
lemmingapex wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Updating circle, ellipse, line-arc, and sector to use smarter approximations #2910

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
554fadd
#2897 Updating circle, ellipse, line-arc, and sector to use a more ac…
lemmingapex Jun 23, 2025
5a81a92
#2897 Updaing readme
lemmingapex Jun 23, 2025
6ca84c9
#2897 Contributors update.
lemmingapex Jun 23, 2025
60c4ab5
Merge branch 'master' into 2897-smarter-circle-approximation
lemmingapex Jul 28, 2026
664f3dd
#2897 Fixing merge conflicts with master.
lemmingapex Jul 29, 2026
3a5da78
fixing turf-line-arc -- reaches bearing2 despite floating-point drift…
lemmingapex Jul 29, 2026
3673822
#2897 Micro-optimizing performance and addressing extreme edge case.
lemmingapex Jul 29, 2026
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
2 changes: 2 additions & 0 deletions documentation.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ toc:
- shortestPath
- unkinkPolygon
Helper:
- calculateNumberOfRegularPolygonSidesToBestApproximateEqualAreaCircle
- calculatePolygonCircumRadiusToBestApproximateEqualAreaCircle
- featureCollection
- feature
- geometryCollection
Expand Down
1 change: 1 addition & 0 deletions packages/turf-circle/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Takes a [Point][1] and calculates the circle polygon given a radius in [Units][2
* `options` **[Object][6]** Optional parameters (optional, default `{}`)

* `options.steps` **[number][5]** number of steps (optional, default `64`)
* `options.maximumRimDeviation` **[number][5]?** if provided, will ignore steps and use a number of steps such that the rim of returned approximate regular polygon is at most this far away from the true circle
* `options.units` **Units** Supports all valid Turf [Units][2] (optional, default `'kilometers'`)
* `options.properties` **[Object][6]** properties (optional, default `{}`)

Expand Down
23 changes: 20 additions & 3 deletions packages/turf-circle/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { GeoJsonProperties, Feature, Point, Polygon } from "geojson";
import { destination } from "@turf/destination";
import { polygon, Units } from "@turf/helpers";
import {
calculateNumberOfRegularPolygonSidesToBestApproximateEqualAreaCircle,
calculatePolygonCircumRadiusToBestApproximateEqualAreaCircle,
Comment on lines +4 to +5

@mfedderly mfedderly Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These feel too internal to be exposed through @turf/helpers, but so far we haven't built anything to share code across packages and keep it all in sync. I'm not sure if I want to try some sort of code duplication mechanism in the monorepo or start adding very small support packages for code like this so that it will still be deduplicated in bundlers.

I think my first instinct would be to make some @turf/internal-* packages which we don't publish API docs for, but we treat as public API from a versioning standpoint. I still don't like it, but I think that's a better outcome than trying to share code and having it wind up duplicated across packages, even if the implementation itself isn't very large. I considered simlinks, but that sounds like it requires some config on Windows to make Git work, and another config in TypeScript to make that work. I don't really want to add a file-copy step to the repo initialization on a fresh checkout.

@lemmingapex lemmingapex Jul 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree about the visibility of these methods. I wasn't sure where else they should go with the current architecture. Whatever you decide here is good with me, just let me know how you want to proceed and where an appropriate place for this code might be.

polygon,
Units,
} from "@turf/helpers";

/**
* Takes a {@link Point} and calculates the circle polygon given a radius in {@link https://turfjs.org/docs/api/types/Units Units}; and steps for precision.
Expand All @@ -10,6 +15,7 @@ import { polygon, Units } from "@turf/helpers";
* @param {number} radius radius of the circle
* @param {Object} [options={}] Optional parameters
* @param {number} [options.steps=64] number of steps
* @param {number} [options.maximumRimDeviation] if provided, will ignore steps and use a number of steps such that the rim of returned approximate regular polygon is at most this far away from the true circle

@mfedderly mfedderly Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its not clear what units the rim deviation is measured in, and having this just override the steps argument seems like a bit of a force to maintain back compatability.

I'm trying to release as many fixes from the PR backlog as 7.4 in the near term, but then I'm going to move to work on v8 and merge a bunch of the breaking changes that have been sitting around. Would you make this interface differently if you could make a breaking change?

I'm on the fence on whether I want to add options.method (one of "inscribed" | "circumscribed" | "equiareal"), or just yoink everyone onto the newer more accurate version. Even in the world where we just have this only this implementation, there may still be a desire to bound the number of steps so you don't wind up with a polygon with way more points than expected and something downstream becomes less performant. So steps and maximumRimDeviation may both be here to stay in some form?

There's a similar break I'm going to be making in @turf/buffer when I move us to the new clipper2 implementation in v8. It currently uses steps but clipper2 under the hood exposes arcTolerance instead.

@lemmingapex lemmingapex Jul 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maximumRimDeviation should have the same units as radius. Do you think it makes sense to have a comment or note about the units in the @param to make this clear?

Regarding the options.method, we could certainly make this happen alongside steps and maximumRimDeviation. This has pros and cons. It gives the caller the power to pick the kind of circle they want, but also introduces some complexity, especially if we expose this method option on the related methods/functions like ellipse, line-arc, and sector.

Regarding the steps and maximumRimDeviation parameters, I think a discriminated union using TypeScript would make steps and maximumRimDeviation mutually exclusive. The caller can provide one or the other, but not both: preventing an invalid state entirely. It would look something like this:

/** Precision controlled by fixed step count */
type StepsPrecision = {
  /** Number of steps/sides for the generated polygon (default: 64) */
  steps?: number;
  maximumRimDeviation?: never;
};

/** Precision controlled by maximum deviation/tolerance threshold */
type DeviationPrecision = {
  steps?: never;
  /** Maximum allowable distance error between the polygon rim and the true circle */
  maximumRimDeviation: number;
};

/** Optional configuration for generating circle polygons */
export type CircleOptions<P extends GeoJsonProperties = GeoJsonProperties> = {
  /** Turf distance unit for radius and tolerance calculations */
  units?: Units;
  /** Custom GeoJSON properties to attach to the resulting Feature */
  properties?: P;
} & (StepsPrecision | DeviationPrecision);

Please let me know what you would like to do.

* @param {Units} [options.units='kilometers'] Supports all valid Turf {@link https://turfjs.org/docs/api/types/Units Units}
* @param {Object} [options.properties={}] properties
* @returns {Feature<Polygon>} circle polygon
Expand All @@ -27,12 +33,23 @@ function circle<P extends GeoJsonProperties = GeoJsonProperties>(
radius: number,
options: {
steps?: number;
maximumRimDeviation?: number;
units?: Units;
properties?: P;
} = {}
): Feature<Polygon, P> {
// default params
const steps = options.steps || 64;
let steps = options.steps || 64;
if (options.maximumRimDeviation && options.maximumRimDeviation > 0) {
steps =
calculateNumberOfRegularPolygonSidesToBestApproximateEqualAreaCircle(
radius,
options.maximumRimDeviation
);
}

const circumRadius =
calculatePolygonCircumRadiusToBestApproximateEqualAreaCircle(radius, steps);

@mfedderly mfedderly Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd have to do a quick benchmark to reason about any performance changes here. Since its just in the initialization I expect this to be mostly in the same ballpark speed wise.

@lemmingapex lemmingapex Jul 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calculatePolygonCircumRadiusToBestApproximateEqualAreaCircle is fast. It executes in constant time $\mathcal{O}(1)$ and runs in under 10–20 nanoseconds per call in modern JS engines. I can make one or two micro-optimizations that may shave off 1 or 2 nanoseconds potentially. I didn't notice any timing changes in the current tests with this addition. Does this make sense?

@mfedderly mfedderly Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm not suggesting optimizing now, but before merging I just want to make sure we don't introduce some large performance regression that anyone will actually notice. We transitioned union and a few other packages to polyclip-ts for correctness, but it uses BigDecimal underneath for that correctness which turned out to be very impactful performance wise. So its something I try to keep an eye on as we make changes.

(I'm willing to introduce performance hits when it meaningfully impacts correctness, but I try to keep the impact to a minimum where possible)

@lemmingapex lemmingapex Jul 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes perfect sense! I agree with that.

@lemmingapex lemmingapex Jul 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have optimized the calculatePolygonCircumRadiusToBestApproximateEqualAreaCircle code. This method has a duration in the single digit nanosecond range at this point. There's only one other optimization I can think of, which would be to precompute and store a scaling factor for the default number of steps. This type of optimization is getting into sub-nanosecond territory.

const properties: any = options.properties
? options.properties
: !Array.isArray(center) && center.type === "Feature" && center.properties
Expand All @@ -43,7 +60,7 @@ function circle<P extends GeoJsonProperties = GeoJsonProperties>(
const coordinates = [];
for (let i = 0; i < steps; i++) {
coordinates.push(
destination(center, radius, (i * -360) / steps, options).geometry
destination(center, circumRadius, (i * -360) / steps, options).geometry
.coordinates
);
}
Expand Down
3 changes: 3 additions & 0 deletions packages/turf-circle/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
"version": "7.3.5",
"description": "Takes a point and calculates the circle polygon given a radius.",
"author": "Turf Authors",
"contributors": [
"Scott Wiedemann <@lemmingapex>"
],
"license": "MIT",
"bugs": {
"url": "https://github.com/Turfjs/turf/issues"
Expand Down
130 changes: 65 additions & 65 deletions packages/turf-circle/test/out/circle1.geojson
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,71 +20,71 @@
"type": "Polygon",
"coordinates": [
[
[-75.343, 40.028966],
[-75.348756, 40.028749],
[-75.354456, 40.028101],
[-75.360046, 40.027029],
[-75.365472, 40.025541],
[-75.37068, 40.023653],
[-75.375622, 40.021383],
[-75.380248, 40.018753],
[-75.384516, 40.015788],
[-75.388383, 40.012517],
[-75.391813, 40.008972],
[-75.394772, 40.005185],
[-75.397232, 40.001195],
[-75.399169, 39.997039],
[-75.400565, 39.992758],
[-75.401406, 39.988393],
[-75.401685, 39.983985],
[-75.401399, 39.979578],
[-75.40055, 39.975213],
[-75.399148, 39.970934],
[-75.397204, 39.96678],
[-75.39474, 39.962792],
[-75.391777, 39.959008],
[-75.388345, 39.955465],
[-75.384477, 39.952197],
[-75.380211, 39.949235],
[-75.375586, 39.946608],
[-75.370648, 39.94434],
[-75.365444, 39.942455],
[-75.360025, 39.940969],
[-75.354442, 39.939897],
[-75.348748, 39.93925],
[-75.343, 39.939034],
[-75.337252, 39.93925],
[-75.331558, 39.939897],
[-75.325975, 39.940969],
[-75.320556, 39.942455],
[-75.315352, 39.94434],
[-75.310414, 39.946608],
[-75.305789, 39.949235],
[-75.301523, 39.952197],
[-75.297655, 39.955465],
[-75.294223, 39.959008],
[-75.29126, 39.962792],
[-75.288796, 39.96678],
[-75.286852, 39.970934],
[-75.28545, 39.975213],
[-75.284601, 39.979578],
[-75.284315, 39.983985],
[-75.284594, 39.988393],
[-75.285435, 39.992758],
[-75.286831, 39.997039],
[-75.288768, 40.001195],
[-75.291228, 40.005185],
[-75.294187, 40.008972],
[-75.297617, 40.012517],
[-75.301484, 40.015788],
[-75.305752, 40.018753],
[-75.310378, 40.021383],
[-75.31532, 40.023653],
[-75.320528, 40.025541],
[-75.325954, 40.027029],
[-75.331544, 40.028101],
[-75.337244, 40.028749],
[-75.343, 40.028966]
[-75.343, 40.029002],
[-75.348761, 40.028785],
[-75.354466, 40.028137],
[-75.36006, 40.027063],
[-75.36549, 40.025574],
[-75.370702, 40.023685],
[-75.375648, 40.021413],
[-75.380278, 40.018781],
[-75.384549, 40.015814],
[-75.38842, 40.01254],
[-75.391852, 40.008992],
[-75.394813, 40.005202],
[-75.397275, 40.001209],
[-75.399214, 39.99705],
[-75.400611, 39.992765],
[-75.401453, 39.988396],
[-75.401732, 39.983985],
[-75.401446, 39.979574],
[-75.400596, 39.975206],
[-75.399193, 39.970923],
[-75.397248, 39.966766],
[-75.394781, 39.962775],
[-75.391816, 39.958988],
[-75.388382, 39.955442],
[-75.384511, 39.952171],
[-75.38024, 39.949207],
[-75.375612, 39.946578],
[-75.37067, 39.944308],
[-75.365462, 39.942421],
[-75.360038, 39.940934],
[-75.354451, 39.939862],
[-75.348753, 39.939214],
[-75.343, 39.938998],
[-75.337247, 39.939214],
[-75.331549, 39.939862],
[-75.325962, 39.940934],
[-75.320538, 39.942421],
[-75.31533, 39.944308],
[-75.310388, 39.946578],
[-75.30576, 39.949207],
[-75.301489, 39.952171],
[-75.297618, 39.955442],
[-75.294184, 39.958988],
[-75.291219, 39.962775],
[-75.288752, 39.966766],
[-75.286807, 39.970923],
[-75.285404, 39.975206],
[-75.284554, 39.979574],
[-75.284268, 39.983985],
[-75.284547, 39.988396],
[-75.285389, 39.992765],
[-75.286786, 39.99705],
[-75.288725, 40.001209],
[-75.291187, 40.005202],
[-75.294148, 40.008992],
[-75.29758, 40.01254],
[-75.301451, 40.015814],
[-75.305722, 40.018781],
[-75.310352, 40.021413],
[-75.315298, 40.023685],
[-75.32051, 40.025574],
[-75.32594, 40.027063],
[-75.331534, 40.028137],
[-75.337239, 40.028785],
[-75.343, 40.029002]
]
]
}
Expand Down
Loading
Loading