Geometry Boost List: Re: [geometry] Moving projections from extensions

Subject: Re: [geometry] Moving projections from extensions
From: Barend Gehrels (barend_at_[hidden])
Date: 2016-06-22 08:01:46

Hi Adam,

Op 20-6-2016 om 17:59 schreef Adam Wulkiewicz:
> Hi,
>
> I'm planning to move the projections from extensions. I've looked at 
> the code I have some questions and a proposal of some minor changes to 
> the interface, to simplify it.

The projections are not completely finished. So as long as the interface 
is not clear, we should not yet move it.

>
> The first question is:
> Should the user be able to use the projections directly or should 
> he/she always use the transform() function with a strategy?

The user should also be able to use them directly. So the choice is 
either directly, or use the transform function.

>
> Currently there are several ways of using a projection, e.g.:
>
> // compile-time
> projections::parameterspar=projections::init("+ellps=WGS84+units=m");
> projections::robin_spheroid<point_ll,point_xy>prj(par);
> bool ok = prj.forward(pt_ll,pt_xy);

This should be possible.

>
> //run-time
> projections::parameterspar=projections::init("+proj=robin+ellps=WGS84+units=m");
>     // or
> projections::parameterspar=projections::init(epsg);
> projections::factory<point_ll,point_xy>fac;
> boost::shared_ptr<projections::projection<point_ll,point_xy>>prj(fac.create_new(par));
> bool ok prj->forward(pt_ll,pt_xy);

This should be possible too, and is probably the main usage.

>
> // also runtime
>     projections::project_transformer
> <
> point_ll_deg,
> point_xy
> >projection("+proj=robin+ellps=WGS84+units=m"); transform(pt_ll,pt_xy,projection);

This way is not yet tested or worked out. But it should be possible too, 
I think.

>
> There are several problems with this API:
> - it is complex

There are three methods, but apart from that, I don't see the 
complexity. The three samples are relatively straightforward. But maybe 
the runtime option can be simplified.

I think there should be a static and runtime version.

> - the implementation details (factory) are exposed to the user

This factory is not an implementation detail. So it is intended to be 
exposed to the user. But maybe we can improve it.

> - the parameters are always stored as doubles
Should be enhanced indeed

> - the transform strategy and projections require points as template 
> parameters
Yes

> - the transform strategy always use the runtime projection (factory, 
> dynamic polymorphism, etc.)
We might create a static strategy for each projection, which is quite 
verbose but because all these is generated code, it is an option.

> - only proj4 strings and EPSG codes can be used to define a 
> projection, there may be other ways like ESRI-flavored WKT 
> (https://www.nceas.ucsb.edu/scicomp/recipes/projections) or compile 
> time definition like bg::srs::spheroid
Indeed.

> - EPSG codes are simply used to generate proj4 string and then this 
> string is parsed, I suspect that the EPSG could be used to directly 
> fill the projection parameters
There is an option for that too, even compile time.

>
> So in general I propose to hide as much as we can and expose only 
> crucial parts of the API.
> For run-time projections use PIMPL idiom and wrap the pointer inside.
> Require only to define CalculationType for projection and use double 
> as default.
> In transform strategy take projection as template parameter and use 
> run-time projection as default.
>
> // compile-time
> projections::robin_spheroid<>prj("+ellps=WGS84+units=m");
> projections::robin_spheroid<>prj(srs::spheroid<>());
> projections::forward(pt_ll,pt_xy, prj);
>
> // run-time
> projections::projection<> prj("+proj=robin+ellps=WGS84+units=m");
> projections::projection<> prj(epsg);
> projections::forward(pt_ll,pt_xy, prj);
>
> // also runtime
> projections::project_transformer<>projection("+proj=robin+ellps=WGS84+units=m");
> transform(pt_ll,pt_xy,projection);

This looks indeed simpler. And probably it is possible. This should be 
changed in the generation.

Note that also possible should be:

typedef boost::geometry::linestring<any point type> ls;
ls line_ll, line_xy; // fill line_ll
projections::forward(line_ls, line_xy, prj);

So the line is automatically projected, where, if necessary, extra 
points are added because straight lines will usually become curved and 
vv is also possible. Same for most other geometry types (multi_point 
excepted).

This does not need to be implemented in the first release, but we need 
to take this into account.

>
> If the user shouldn't use the projections directly (only transform + 
> strategy) we could leave the parameters type (filled in the transform 
> strategy) as it is now and pass it into the projections instead of 
> proj4 string or epsg code. Then we wouldn't be forced to implement 
> constructors for all inputs in every projection. But parameters should 
> also take the CalculationType and I don't see a reason to have init() 
> function (this also causes problems with multiple representations, see 
> below):
>
> projections::parameters<> par("+ellps=WGS84+units=m");
> projections::parameters<> par(epsg);
>
> projections::robin_spheroid<> prj(par);

I agree that the init function can be skipped. The current API is a bit 
close to the proj4 API , which also calls pj_init (or init_plus). But we 
can hide it indeed.

>
> Now, regarding the other kinds of projections definitions, both proj4 
> an WKTs are strings so currently it wouldn't be possible to implement 
> both unless there was some other function like init_wkt(), but i think 
> it's not elegant. Instead I propose wrapping the parameters, like this:
>
> usingnamespaceprojections;
> parameters<> par= proj4("+ellps=WGS84+units=m");
> parameters<> par = epsg(epsg_code);
> parameters<> par = static_epsg<EPSG>();
> parameters<> par = spheroid<>();
> parameters<> par = wkt("..."); // maybe in the future

OK for me, it looks good.

>
>     // still the user should use:
> project_transformer<>projection(proj4("+proj=robin+ellps=WGS84+units=m"));
> project_transformer<>projection(epsg(epsg_code));
> project_transformer<>projection(spheroid<>()); // compile-time error
> project_transformer<aea_ellipsoid<> >projection(spheroid<>()); // ok
> transform(pt_ll,pt_xy,projection);
>
> This would make the code more clear too, and semantically separate 
> Boost.Geometry from the internal implementation being Proj4.

It looks good indeed.

>
> There are also other things I'd like to ask about, i.e.:
>
> What do you think about getting rid of forward and inverse methods and 
> instead figuring it out from the input types? 

Yes, the transform function should do that indeed. I agree. And it is 
easy to implement.

> The projection would always perform a forward projection if the 
> cartesian geometry was the second parameter and inverse projection if 
> it was the first parameter. I'd be similar to boost::lexical_cast 
> which gets the direction from template parameters. In such case the 
> user would be forced to pass the correct point/geometries types so 
> this is limiting.
>
> This could be implemented only in the transform strategy (so the 
> projections would still have forward() and inverse() functions), then 
> project_inverse_transformer wouldn't be needed.

Maybe it is more convenient that the called method should figure it out. 
The transform strategy should be neutral to inverse/forward. But it can 
be implemented on different levels, we should carefully compare the 
options here.

> Another way could be project_transformer automatically figuring out 
> the projection direction and explicit project_forward_transformer, 
> project_inverse_transformer strategies.
>
> I'm also confused about the names of projections. AFAIU the ones 
> marked as xxx_ellipsoid map from ellipsoid of revolution or spheroid 
> and the ones marked as xxx_spheroid map from a sphere. There is also 
> e.g. cass_spheroid which AFAIU can work with ellipsoid of revolution. 
> Are these simple inaccuracies or am I missing something?
It is by the generation. I have to look this up, but if the proj4 source 
supports an ellipsoid for it (even if it does not make sense), it is 
generated too. We can always adapt the generation.

>
> Currently the names are directly derived from Proj4 parameter names 
> but in WKT they're different (PROJECTION["Transverse_Mercator"]) so 
> users which doesn't know Proj4 could be confused.
> What do you think about naming projections using full names, e.g.:
>
> projections::aea_ellipsoid -> projections::albers_equal_area
> projections::cass_spheroid -> projections::cassini
> projections::tmerc_ellipsoid -> projections::transverse_mercator

I kept all the proj4 names and I think that is more convenient than 
adding another translation table. For users, who usually know proj4 (it 
is quite well-known), it is also more convenient.
Having said that, I agree that cassini looks better than just cass, and 
if WKT has a complete map of all translations, we could consider that too.

>
> This would again be more clear and general. And wouldn't be confusing 
> if we had tri-axial ellipsoid projections in the future.

Thanks for your input.
Regards, Barend