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

[RFC4_dev] Address code review comments #1826

Merged
merged 57 commits into from
Jan 20, 2020

Conversation

rouault
Copy link
Member

@rouault rouault commented Jan 6, 2020

Continuation of PR #1817. Commits specific to this one start at "Grid refactoring: address review comments of #1777 "

…hgridshift/vgridshift/deformation structures
…stward correction' in the CTable2/NTv1/NTv2 backends
…grid. Used for French NTF to RGF93 transformation using gr3df97a.tif grid
…n switching between (sub)grids (fixes OSGeo#1663)

Given in.txt with
53.999759140 5.144478208 252.6995

Before the fix,
cct  -t 0 -d 4 +proj=pipeline +step +proj=axisswap +order=2,1,3,4 +step +proj=hgridshift +inv +grids=rdtrans2018.gsb +step +proj=vgridshift +grids=naptrans2018.gtx +step +proj=sterea +lat_0=52.156160556 +lon_0=5.387638889 +k=0.9999079 +x_0=155000 +y_0=463000 +ellps=bessel in.txt

returned:
  139079.8814    668306.0302      212.1724        0.0000

It now returns:
  139079.8850    668306.0458      212.1724        0.0000

which meets with the 1mm accuracy the expected result of test point
```
30010049	53.999759140	5.144478208	252.6995	139079.8850	668306.0460	212.1723
```
- Move content of legacy apply_gridshift.cpp and apply_vgridshift.cpp in grids.cpp
- Rename nad_ functions to pj_hgrid_
- Rename internal proj_hgrid_/proj_vgrid_ functions to pj_
@rouault rouault force-pushed the rfc4_code_review branch 2 times, most recently from 3dd85f2 to d279de5 Compare January 7, 2020 03:03
Copy link
Member

@mwtoews mwtoews left a comment

Choose a reason for hiding this comment

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

Just a few minor corrections to docs to address.

docs/source/operations/transformations/xyzgridshift.rst Outdated Show resolved Hide resolved
docs/source/operations/transformations/xyzgridshift.rst Outdated Show resolved Hide resolved
docs/source/operations/transformations/xyzgridshift.rst Outdated Show resolved Hide resolved
docs/source/references.bib Outdated Show resolved Hide resolved
docs/source/resource_files.rst Show resolved Hide resolved
docs/source/usage/environmentvars.rst Outdated Show resolved Hide resolved
@rouault
Copy link
Member Author

rouault commented Jan 7, 2020

Just a few minor corrections to docs to address.

Thanks. Done per 8821b35

@rouault
Copy link
Member Author

rouault commented Jan 17, 2020

Any objection to merge this ?

@kbevers
Copy link
Member

kbevers commented Jan 17, 2020

Any objection to merge this ?

No. In this PR you've taken care of all the things we didn't like about the earlier PR's, so I think this should be fine to merge without further reviews :-)

@rouault
Copy link
Member Author

rouault commented Jan 17, 2020

Actually the title of the PR is a bit misleading. It started with code review-only changes, but commits starting with "pj_bilinear_interpolation_three_samples(): fix MSVC warning" are about other fixes & additions

@kbevers
Copy link
Member

kbevers commented Jan 17, 2020

Okay, I see if I can find the time to look at those commits during the weekend. It'd be nice to get RFC4 stuff fully merge soon.

Copy link
Member

@kbevers kbevers left a comment

Choose a reason for hiding this comment

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

Overall this looks good to me. I just need clarification on the OS X user writable directory and I am happy to merge this

@rouault rouault merged commit a6390b5 into OSGeo:rfc4_dev Jan 20, 2020
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.

3 participants