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

Transformation: no longer do vertical trasnformation when doing compound CRS to 2D CRS / add --3d to cs2cs #3119

Merged
merged 1 commit into from
Mar 17, 2022

Conversation

rouault
Copy link
Member

@rouault rouault commented Mar 16, 2022

Previously, when computing transformation between a compound CRS and a
geographic/projected 2D CRS, the behaviour was similar to implicitly
promoting the 2D CRS to 3D CRS in the pipeline computation logic, hence
a geoid model could be applied. But note that when doing a geographic 3D
to geographic/projected 2D CRS transformation, we did not do this implicit
promotion and if a Helmert transformation existed between the datums, it
was done only in 2D. So this is a bit inconsistent and triggered the
comment in #2318 (comment)

With this commit, we no longer do any vertical transformation when doing
compound CRS to the 2D CRS, but just take the transformation of the
horizontal part of the compound CRS to the 2D CRS.
Said otherwise, NAD83+NAVD88 to NAD83 will no longer lead to the
application of the geoid model. Unless you explicitly ask for the
promotion NAD83 to 3D.

Also related, in #1563 that went to 6.3.0,
I changed cs2cs to automatically promote to 3D the CRS as soon as one of
them was compound, for the sake of being consistent with the past
behaviour. But it then becomes difficult to predict PROJ behaviour
depending on which level of it you consider...
This commit undoes that and adds an explicit --3d switch to cs2cs, similarly to
projinfo, to ask for promotion.

Other bug fix found in the process, when using legacy syntax, +init=epsg:4979,
(WGS 84 3D), the resulting CRS was 2D and not 3D.

CC @kbevers @hobu @snowman2 : I'd be interested in hearing if you think this change of behaviour is a good idea. I think it is valuable as it makes PROJ behavior more predictable (and "correct"), but this has a price w.r.t backward compatibility. I believe this is material only candidate for 9.1, not 9.0.x (well, we could possibly backport the addition of the --3d switch to 9.0.x, but not the suppression of the automatic promotion, so that people can anticipate the future change of behaviour by opting in)

…und CRS to 2D CRS / add --3d to cs2cs

Previously, when computing transformation between a compound CRS and a
geographic/projected 2D CRS, the behaviour was similar to implicitly
promoting the 2D CRS to 3D CRS in the pipeline computation logic, hence
a geoid model could be applied. But note that when doing a geographic 3D
to geographic/projected 2D CRS transformation, we *did* not do this implicit
promotion and if a Helmert transformation existed between the datums, it
was done only in 2D. So this is a bit inconsistent and triggered the
comment in OSGeo#2318 (comment)

With this commit, we no longer do any vertical transformation when doing
compound CRS to the 2D CRS, but just take the transformation of the
horizontal part of the compound CRS to the 2D CRS.
Said otherwise, NAD83+NAVD88 to NAD83 will no longer lead to the
application of the geoid model. Unless you explicitly ask for the
promotion NAD83 to 3D.

Also related, in OSGeo#1563 that went to 6.3.0,
I changed cs2cs to automatically promote to 3D the CRS as soon as one of
them was compound, for the sake of being consistent with the past
behaviour. But it then becomes difficult to predict PROJ behaviour
depending on which level of it you consider...
This commit undoes that and adds an explicit --3d switch to cs2cs, similarly to
projinfo, to ask for promotion.

Other bug fix found in the process, when using legacy syntax, +init=epsg:4979,
(WGS 84 3D), the resulting CRS was 2D and not 3D.
@EvertEt
Copy link

EvertEt commented Mar 16, 2022

This looks like a great improvement to the predictability of transformations, thank you!

Is there a way to "demote" the compound CRS to 2D to force this behaviour in older versions? (EPSG or WKT representation would be fine)

@kbevers
Copy link
Member

kbevers commented Mar 16, 2022

I think this change makes sense. I agree that is should only go to 9.1.x.

@hobu
Copy link
Contributor

hobu commented Mar 16, 2022

Said otherwise, NAD83+NAVD88 to NAD83 will no longer lead to the application of the geoid model. Unless you explicitly ask for the promotion NAD83 to 3D.

What are some examples of the syntax and shorthand to do the promotion? We use EPSG:NAD83+NAVD88 quite a bit.

@rouault
Copy link
Member Author

rouault commented Mar 16, 2022

EPSG:NAD83+NAVD88

this is invalid, but "NAD83 + NAVD88 height" or "EPSG:4269+5703" are strings recognized by proj_create().

What are some examples of the syntax and shorthand to do the promotion?

There's no shorthand name to do the promotion (was discussed recently in #3075). Promotion only makes sense for geographic 2D CRS (compound CRS are already 3D by definition). You need to use the proj_crs_promote_to_3D() C API, or the --3d switch of projinfo and (just added in this PR) cs2cs, or the WKT resulting from "projinfo --3d NAD83 -o WKT2:2019 -q"

@hobu
Copy link
Contributor

hobu commented Mar 16, 2022

This change means EPSG:4326+5703 or EPSG:26915+5703 will not magically promote anymore, correct?

@rouault
Copy link
Member Author

rouault commented Mar 16, 2022

This change means EPSG:4326+5703 or EPSG:26915+5703 will not magically promote anymore, correct?

Those CRSs are compoundCRS. They are already 3D. Promoting them is a no-op. Promotion only makes sense for a 2D geographic or projected CRS, to add an ellipsoidal height to it.
The change affects trying to transform such a compoundCRS to a target 2D geographic or projected CRS. Before the change, that target CRS was implicitly promoted to 3D. With the change, it is no longer ==> no vertical transformation is done.

@snowman2
Copy link
Contributor

This makes sense to me 👍

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

5 participants