observablehq / plot

A concise API for exploratory data visualization implementing a layered grammar of graphics
https://observablehq.com/plot/
ISC License
4.16k stars 171 forks source link

A transform that uses @mapbox/polylabel to derive “pois” #2098

Open Fil opened 3 weeks ago

Fil commented 3 weeks ago

The poi/polylabel transform guarantees that the selected point belongs to the interior of the polygon or multipolygon. For line and point geometries, it defers to the classic centroid.

This PR applies the transform by default, as an almost always superior alternative to Plot.centroid.

For most geometries the change is almost imperceptible. Some geometries get a much better treatment:

For the few specific projections that require clipping to sphere, make sure you use their versions from d3-geo-polygon rather than from d3-geo-projection.

TODO:

cc: @mourner

demo: https://observablehq.com/@observablehq/poi-polylabel-transform

Fil commented 3 weeks ago

How big is it?

The dependencies are:

If we copy and embed the code, the total size of grows by 1.8% (from 65715 to 66904 bytes, size of the gziped version of the minified script). There might be ways to reduce this a bit if we embed it, since we might not need all of it (though @mourner's code is usually quite minimalist).

mourner commented 3 weeks ago

@Fil let me know if there's anything I can do on the packages side to make including them as a dependency more viable and reduce the bundle size. Looking at the code, there doesn't seem to be anything superfluous there. Only need to bump tinyqueue to export ESM properly (edit: did so).

Fil commented 3 weeks ago

I was not clear in my remark. The cost of 1200 bytes is for the whole feature, including the two imports and my glue code. There is no difference between copying the source code or importing from the dependency, since it's part of the bundle (only d3 modules are external dependencies). We could cull a few bytes by copying the code and trimming it down to the absolute minimum (e.g. removing the debug log), and maybe there is a way to make Plot.centroid and this transform share more code.

Also, an alternative way of approaching this would be to make it part of d3-geo.

And I'll follow @mourner's suggestion to set the precision to 1 pixel.

mourner commented 3 weeks ago

I also just released polylabel 2.0.1 that saves some bytes, worth upgrading.

mbostock commented 3 weeks ago

I think if it’s a bundled dependency it’s okay.

Fil commented 3 weeks ago

Do you think peak could be a good name for this transform? (Instead of poi)

I've also considered wrapping this under the Plot.centroid transform, by adding a "derive" or "interpolate" option that would be similar to Plot.raster’s interpolate option. I have mixed feelings as this abuses the "centroid" word a little (not a center of mass anymore), but it could be seen as a generalization. The signature of that function might be (G, X, Y, projection) => void or (G, projection) => [X, Y] or (G, path) => [X, Y]

Also for more generality, the parameter describing the ellipse should be a vector (length = alpha ratio, angle = 0) — but this can be added later if requested (useful for people who would want to add labels at a 45° angle).