Skip to content

Conversation

@Fil
Copy link
Contributor

@Fil Fil commented Dec 6, 2022

closes #1136

@Fil Fil requested a review from mbostock December 6, 2022 13:16
@Fil Fil marked this pull request as draft December 6, 2022 13:58
@Fil Fil force-pushed the fil/projection-autoheight branch from e8825f7 to e87ebe5 Compare December 6, 2022 19:09
@Fil
Copy link
Contributor Author

Fil commented Dec 6, 2022

it works! Can uses the projection's ratio (e.g. 1 for orthographic, 0.5 for equirectangular); measures the path if a domain is given. Defaults to the golden ratio for any other projection.

@Fil Fil marked this pull request as ready for review December 6, 2022 19:10
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

This extends the public interface of what we’re calling a projection to include projection.ratio (rather than just the existing projection.stream). Do we intend to support user-supplied projections with auto-height by having them supply projection.ratio? Or do we only want to support this for the built-in projections? I think I probably wouldn’t use this mechanism as the public interface… especially since we could potentially infer the ratio by projecting the outline via projection.stream and d3.geoPath.bounds. But I also think it’s not a high priority to support this for custom projections. Given the time pressure I’d prefer to only do this for our internal projections to start.

@Fil Fil added the geo Maps and projections label Dec 6, 2022
@Fil
Copy link
Contributor Author

Fil commented Dec 6, 2022

I've tried to hide the .ratio property in #1163, with a Symbol. (In the future we might consider opening it, but adding domain: sphere should be fine in most (all?) cases. Just a weeny bit slower.)

@mbostock
Copy link
Member

mbostock commented Dec 6, 2022

I need to think about this one more.

@Fil
Copy link
Contributor Author

Fil commented Dec 7, 2022

I made the same mistake to have autoHeight depend on the actual user-given margins and width, when it should depend on the defaults (otherwise changing the width will also change the default height).

If we want to make everything fit exactly with user specified dimensions (and domain), this starts to look much more like the dataAspectRatio option (discussed in #837). It could be done as a follow-up in a comprehensive manner, maybe as an opt-in height: "fit" option.

In the meantime I'll put up a much simpler alternative.

@Fil Fil closed this Dec 7, 2022
Fil added a commit that referenced this pull request Dec 7, 2022
the height for projections (including the null projection when Plot.geo is used) is computed for a target frame with an aspect ratio that defaults to the golden ratio. When using facets, the computation takes into account the number of rows and columns, with similar limits to what total size is acceptable (1260px).

The projection's preferred aspect ratio is adapted for a few named projections. For example "equal-earth" and "equirectangular" are wider than tall, "mercator" and a few azimuthal projections default to a square.

closes #1136
supersedes #1162
mbostock added a commit that referenced this pull request Dec 8, 2022
* autoHeight

the height for projections (including the null projection when Plot.geo is used) is computed for a target frame with an aspect ratio that defaults to the golden ratio. When using facets, the computation takes into account the number of rows and columns, with similar limits to what total size is acceptable (1260px).

The projection's preferred aspect ratio is adapted for a few named projections. For example "equal-earth" and "equirectangular" are wider than tall, "mercator" and a few azimuthal projections default to a square.

closes #1136
supersedes #1162

* determine the width of a facet from the (default) value of paddingInner=0.1, paddingOuter=0, instead of relying on the fx scale still being in the intermediate state where its range is [0,1]. There's a 2px discrepancy in the tests, which is probably coming from some rounding.

* refactor: use the ratio implied by tx and ty

* aspectRatio

* auto height based on width

* Update src/dimensions.js

Co-authored-by: Philippe Rivière <fil@rezo.net>

* fix auto height when geometry-less projection

* fix default height for custom projections

Co-authored-by: Mike Bostock <mbostock@gmail.com>
chaichontat pushed a commit to chaichontat/plot that referenced this pull request Jan 14, 2024
* autoHeight

the height for projections (including the null projection when Plot.geo is used) is computed for a target frame with an aspect ratio that defaults to the golden ratio. When using facets, the computation takes into account the number of rows and columns, with similar limits to what total size is acceptable (1260px).

The projection's preferred aspect ratio is adapted for a few named projections. For example "equal-earth" and "equirectangular" are wider than tall, "mercator" and a few azimuthal projections default to a square.

closes observablehq#1136
supersedes observablehq#1162

* determine the width of a facet from the (default) value of paddingInner=0.1, paddingOuter=0, instead of relying on the fx scale still being in the intermediate state where its range is [0,1]. There's a 2px discrepancy in the tests, which is probably coming from some rounding.

* refactor: use the ratio implied by tx and ty

* aspectRatio

* auto height based on width

* Update src/dimensions.js

Co-authored-by: Philippe Rivière <fil@rezo.net>

* fix auto height when geometry-less projection

* fix default height for custom projections

Co-authored-by: Mike Bostock <mbostock@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

geo Maps and projections

Projects

None yet

Development

Successfully merging this pull request may close these issues.

autoHeight + projections

3 participants