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

Fix and additional options for Peirce Quincuncial projections #2978

Merged
merged 26 commits into from
Dec 20, 2021

Conversation

tcwilkinson
Copy link
Contributor

@tcwilkinson tcwilkinson commented Dec 9, 2021

This fixes the current forward implementation of Peirce Quincuncial proj to correctly flip/reflect out the southern hemisphere to four triangles, and rotate entire result to a square or diamond. (It there resolves the issues identified with pull request #2230 , where southern hemisphere was wrongly projected over northern, and reverses the restriction to northern hemisphere introduced there). It also adds additional lateral projection of the hemispheres.

  • This PR adds an optional parameter +type which allows selection of projection. The +type=square and +type=diamond types match in principle ESRI's twin implementations of square and diamond PQ projs. The default if not specified is +type=diamond.

  • The previous behaviour restricted to the northern hemisphere can be reproduced using the +type=nhemisphere, though this is an edge case only.

  • An additional +type=horizontal and +type=vertical rectangular lateral versions have been added that place each hemisphere side-by-side. This is primarily to allow creation of projections such as Greiger Triptychial, which also require the additional optional params o_scrollx or o_scrolly in order to shift parts of the projection from one side of the map to the other.

  • Additional documentation has been added to proj description, including quoting the usual meridian used in common usage of projection, and images showing the different types.

Would be grateful for review and any suggestions especially from @kbevers @rouault who are familier with these implementations.

  • Tests added
  • Added clear title that can be used to generate release notes
  • Fully documented, including updating docs/source/*.rst for new API

…ern hemisphere; addition of diamond version of projection peirce_q_d
@tcwilkinson tcwilkinson marked this pull request as draft December 10, 2021 00:37
Copy link
Member

@rouault rouault left a comment

Choose a reason for hiding this comment

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

Tests in ./test/gie/peirce_q.gie (they are run by ctest typically) will need to be adjusted for the southern hemisphere. Note: IMHO there are far more test points than needed. You could likely reduce to just what is needed to test each code branch.

src/projections/adams.cpp Outdated Show resolved Hide resolved
src/projections/adams.cpp Outdated Show resolved Hide resolved
src/projections/adams.cpp Outdated Show resolved Hide resolved
docs/source/operations/projections/peirce_q_d.rst Outdated Show resolved Hide resolved
docs/source/operations/projections/peirce_q_d.rst Outdated Show resolved Hide resolved
docs/plot/plotdefs.json Outdated Show resolved Hide resolved
@tcwilkinson
Copy link
Contributor Author

Have set the default type to +type=nhemisphere for backward compatibility with implementation until now, but happy to change now if you think better going forward.

@rouault
Copy link
Member

rouault commented Dec 15, 2021

Have set the default type to +type=nhemisphere for backward compatibility with implementation until now, but happy to change now if you think better going forward.

I don't think restricting to nhemisphere makes sense at all. This option shouldn't be available

…compatibility and real-world usefulness

also added comments, doi reference and attribution about the addition of horizontal and vertical types
@tcwilkinson
Copy link
Contributor Author

Have set the default type to +type=nhemisphere for backward compatibility with implementation until now, but happy to change now if you think better going forward.

I don't think restricting to nhemisphere makes sense at all. This option shouldn't be available

Can think of mapping applications where this and shemisphere would make more sense than convoluted alternatives (e.g. diamond with points above lat>0 will produce a square, but lat<0 will produce the four triangular spokes only).

But agree that these are edge cases and doesn't make sense as default even if backward compatible.

Is +type=diamond is better as default (since it is backward compatible with coords for implementation until now, but shows full range)?

docs/source/operations/projections/peirce_q.rst Outdated Show resolved Hide resolved
docs/source/operations/projections/peirce_q.rst Outdated Show resolved Hide resolved
src/projections/adams.cpp Outdated Show resolved Hide resolved
test/gie/peirce_q.gie Outdated Show resolved Hide resolved
@tcwilkinson tcwilkinson marked this pull request as ready for review December 15, 2021 15:07
@tcwilkinson tcwilkinson changed the title Fix for Peirce Quincuncial projection Fix and additional options for Peirce Quincuncial projections Dec 15, 2021
@rouault rouault modified the milestone: 9.0.0 Dec 20, 2021
@rouault rouault merged commit 5fe037f into OSGeo:master Dec 20, 2021
github-actions bot pushed a commit that referenced this pull request Dec 20, 2021
This fixes the current forward implementation of Peirce Quincuncial proj to correctly flip/reflect out the southern hemisphere to four triangles, and rotate entire result to a square or diamond. (It there resolves the issues identified with pull request #2230 , where southern hemisphere was wrongly projected over northern, and reverses the restriction to northern hemisphere introduced there). It also adds additional lateral projection of the hemispheres.

- This PR adds an optional parameter `+type` which allows selection of projection. The `+type=square` and `+type=diamond` types match in principle ESRI's twin implementations of square and diamond PQ projs. The **default** if not specified is `+type=diamond`.

- The previous behaviour restricted to the northern hemisphere can be reproduced using the `+type=nhemisphere`, though this is an edge case only.

- An additional `+type=horizontal` and `+type=vertical` rectangular lateral versions have been added that place each hemisphere side-by-side. This is primarily to allow creation of projections such as Greiger Triptychial, which also require the additional optional params `scrollx` or `scrolly` in order to shift parts of the projection from one side of the map to the other. 

- Additional documentation has been added to proj description, including quoting the usual meridian used in common usage of projection, and images showing the different types.
@tcwilkinson tcwilkinson deleted the feature_fix_peirce_q branch December 26, 2021 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants