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

Add run-time and compile-time projections/transformations parameters. #496

Merged
merged 11 commits into from Sep 14, 2018

Conversation

Projects
None yet
4 participants
@awulkiew
Copy link
Member

awulkiew commented Jun 28, 2018

With this PR transformations' parameters can be expressed in following ways:

  • run-time Proj4 string

    srs::proj4("+proj=tmerc +ellps=WGS84 +units=m");
    
  • run-time

    using namespace srs::dpar;
    parameters<>(proj_tmerc)(ellps_wgs84)(units_m)
    
  • compile-time

    using namespace srs::spar;
    parameters<proj_tmerc, ellps_wgs84, units_m>
    

This PR also changes the way of processing parameters in general. In the original code some parameters like datum or ellps were expanded which resulted in addition of new parameters into parameter list. Default parameters could also be added. I removed this mechanism because otherwise I'd be forced to implement rewriting of static parameters in compile-time which would increase the compilation time. the old behavior is emulated though. Now instead of expanding parameters and adding defaults at the beginning, the parameter is searched on the parameters list and if it's not found only then it may be extracted from other parameter (e.g. ellipsoid from datum) and after that the default may be used. A side effect of this change is that the initialization with Proj4 string is 2x faster in my test which is creation of transformation:

from = "+proj=longlat +ellps=WGS84 +datum=WGS84 +no_defs"
to = "+proj=aea +lat_1=34 +lat_2=40.5 +lat_0=0 +lon_0=-120 +x_0=0 +y_0=-4000000 +ellps=clrk66 +units=m +no_defs"

Replacing string-based parameters with run-time parameters speeds up the creation another 5x. So WRT the previous implementation (string-based always modifying parameters by expanding them and adding defaults) the speedup is 10x.

Replacing run-time parameters with compile-time parameters speeds up the creation another 2x, so WRT the original 20x.

Furthermore it's still possible to initialize transformations passing SRID codes (EPSG, ESRI, IAU2000), but now newly introduced, non-string-based parameters are used for this which means the initialization is faster. Before Proj4 strings were used for this.

I also improved const-corectness of the code, e.g. the input coordinates of projections are not modified so projections does not rely on the internal caller to work properly. This was affecting combined projections (projection calling another projection).

awulkiew added some commits Jun 28, 2018

[srs] Implement new static and dynamic transformation parameters.
There are 3 ways of defining parameters:

// run-time Proj4 string
srs::proj4("+proj=tmerc +ellps=WGS84 +units=m")

// run-time
using namespace srs::dpar;
parameters<>(proj_tmerc)(ellps_wgs84)(units_m)

// compile-time
using namespace srs::spar;
parameters<proj_tmerc, ellps_wgs84, units_m>
[srs] Use newly implemented parameters to define predefined SRID para…
…meters.

Run-time and compile-time EPSG, ESRI and IAU2000 codes.
[srs] Fix compilation errors in projections/grids.hpp
Wrong ifstream.open() argument and shadowing of template argument.
@mloskot

This comment has been minimized.

Copy link
Member

mloskot commented Jun 28, 2018

This is awesome, thanks Adam for this overhaul!

@mloskot

This comment has been minimized.

Copy link
Member

mloskot commented Jun 28, 2018

@awulkiew BTW, how is the PROJ.4 to Boost.Geometry conversion managed now? I can't find anything about the procedure. It used to be semi-automatic, AFAIR.

@awulkiew

This comment has been minimized.

Copy link
Member

awulkiew commented Jun 28, 2018

@mloskot Now it's fully manual. I guess it could be partially automated but it'd rely on consistency of Proj4 and the automation code would have to be re-written because both Proj4 and Boost.Geometry was changed since it was last used. I don't consider it worth it because it was used only for projections (code in srs/projections/proj directory) and they are easy to update manually. It also didn't analyse the code (it'd be a lot more complex if it did that), it generated artifacts which had to be removed by hand, the names were not changed (we still have upper-case names in the code), the indentation was wrong and still is in some cases, etc. The style of the original code was there and to be honest I don't like it: confusing way of writing the expressions, e.g. variables assigned internally in expressions, variables assigned in if conditions, reusing variables to temporarily hold some value not-related with the name of variable, using input parameters of functions to hold temporary values, declarations of variables at the beginning of the scope, use of pointers, etc.

@awulkiew awulkiew force-pushed the awulkiew:feature/projections_interface branch from 20ff4f3 to 5729e6f Jun 28, 2018

@awulkiew awulkiew force-pushed the awulkiew:feature/projections_interface branch from 6bc4259 to de95e2c Jul 3, 2018

@mloskot

This comment has been minimized.

Copy link
Member

mloskot commented Jul 10, 2018

@awulkiew Your explanation makes sense. Thanks. It just made me wonder about the maintenance; how the updates in PROJ.4 are going to be captured, if at all; how to document the procedure, etc.

@awulkiew awulkiew requested review from barendgehrels and vissarion Jul 30, 2018

@awulkiew awulkiew added this to the 1.69 milestone Jul 31, 2018

@barendgehrels

This comment has been minimized.

Copy link
Collaborator

barendgehrels commented Jul 31, 2018

Cool! Will look at it tomorrow

@barendgehrels

This comment has been minimized.

Copy link
Collaborator

barendgehrels commented Aug 5, 2018

I'm OK with merging this! Thanks a lot @awulkiew !

@vissarion

This comment has been minimized.

Copy link
Contributor

vissarion commented Aug 21, 2018

Thanks, I am ok with merging.

@awulkiew

This comment has been minimized.

Copy link
Member

awulkiew commented Sep 14, 2018

Thanks for the reviews!

@awulkiew awulkiew merged commit bf62e05 into boostorg:develop Sep 14, 2018

2 checks passed

ci/circleci: bootstrap Your tests passed on CircleCI!
Details
ci/circleci: minimal Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment