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

createOperations(): emulate PROJ < 6 behavior when doing geocentric <--> geographic transformation between datum with unknown transformation (fixes #3360) #3361

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

rouault
Copy link
Member

@rouault rouault commented Oct 5, 2022

We switch from one arbitrary to the other one. The new behavior is perhaps slightly better because, besides matching PROJ <6 results, it matches also the behavior when operating in the geographic coordinate space between datums without known transformations.
@kbevers opinions ? I considered at that point to add some API switch to select which behavior is desired, but that seems overkill and nobody will figure that they need to use such potential switch, right ?

Likely not appropriate for 9.1.x backport

…--> geographic transformation between datum with unknown transformation (fixes OSGeo#3360)
@kbevers
Copy link
Member

kbevers commented Oct 6, 2022

I don't see why this solution is arbitrary. To me it is obviously correct to do the +proj=cart +inv step using the WGS84 ellipsoid and not the radius of a sphere. This might be easier for human eyes to see than for a computer program that follows strict rules.

In fact, the +proj=longlat +R=6378137 is not really doing anything in the pipeline, as seen below where I've removed it in the last example.

$ echo "4057449.87922115 1236185.10760693 4747287.50008112" | cct -t0 +proj=pipeline +step +inv +proj=cart +ellps=WGS84 +step +proj=longlat +R=6378137 +step +proj=unitconvert +xy_in=rad +xy_out=deg +step +proj=axisswap +order=2,1
 48.4111246681   16.9444298151      -74.6128        0.0000
$ echo "4057449.87922115 1236185.10760693 4747287.50008112" | cct -t0 +proj=pipeline +step +inv +proj=cart +ellps=WGS84 +step +proj=unitconvert +xy_in=rad +xy_out=deg +step +proj=axisswap +order=2,1     
 48.4111246681   16.9444298151      -74.6128        0.0000

Had there been a a projection in play on the output CRS the +R would of course play a role but the +proj=latlong +R=6378137 step would still be unnecessary.

So in conclusion, looks more like a bug fix than a breaking change in behaviour to me.

@rouault
Copy link
Member Author

rouault commented Oct 6, 2022

To me it is obviously correct

well, I'm sill a bit perplexed by that. anyway... Adding the 9.1 backport label then

@rouault rouault removed this from the 9.2.0 milestone Oct 6, 2022
@rouault rouault merged commit 632eb41 into OSGeo:master Oct 6, 2022
github-actions bot pushed a commit that referenced this pull request Oct 6, 2022
createOperations(): emulate PROJ < 6 behavior when doing geocentric <--> geographic transformation between datum with unknown transformation (fixes #3360)
@kbevers
Copy link
Member

kbevers commented Oct 9, 2022

Geocentric (ECEF) does not have an ellipsoid... Right...

But WGS84 as a datum does, and that needs to be taken into consideration when converting from cartesian coordinates to geodetic coordinates.

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.

2 participants