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-23 12:38:24

Hi Adam,

Op 22-6-2016 om 18:39 schreef Adam Wulkiewicz:
> Barend Gehrels wrote:
>> Op 20-6-2016 om 17:59 schreef Adam Wulkiewicz:
> <snip>
>>
>>>
>>> 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.
>>
>
> I was also thinking about an alternative API, based on projection tags:
>
>     // run-time
> projections::projection<> prj("+proj=robin+ellps=WGS84+units=m");
>
> // compile-time
> projections::projection<robin>prj("+ellps=WGS84+units=m");
>
> So robin (or robin_tag) would be a tag passed into projection<> as 
> template parameter. For a default tag the projection would be runtime 
> and for non-default tag the projection would be compile-time.

This looks also good. The _tag suffix is mainly used internally, but it 
is actually exposed to the user when registering new types, so I think 
it could be used indeed. But without is also fine and more conform with 
the initialization string.

> And then it would be possible to pass only tag to transform strategy, 
> so no need to pass the whole projection type.
>
> // run-time
> projections::project_transformer<>projection("+proj=robin+ellps=WGS84+units=m");
> transform(pt_ll,pt_xy,projection); // compile-time
> projections::project_transformer<robin>projection("+ellps=WGS84+units=m");transform(pt_ll,pt_xy,projection);
>
> This could simplify the generic implementation. E.g. the constructors 
> taking all of those possible input parameters directly in projection 
> (mentioned below) could be implemented only once. Otherwise we'd be 
> forced to implement all constructors in every static projection.
>
> On the other hand it could be harder to understand when projection is 
> dynamic and when static.
>
> What do you think?

It looks very good. I think the difference between static (specify in 
source code) and dynamic (in initialization string) is clear enough.

>
>> 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.
>>
>
> Wouldn't it be the transform() function's purpose?

Yes, that is true.

> Shouldn't projections (and transform strategies) only project between 
> points?

OK

> This allows to logically separate the code nicely. Otherwise parts of 
> projections code like projections::forward() would call transform() 
> which would use Pt-Pt projection in a strategy, so call 
> projections::forward().

Indeed, I agree.

>
>>
>>>
>>> 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.
>
> There could be the third function (besides forward() and inverse()) 
> picking projection's direction automatically.
> And three corresponding strategies simply calling these three functions.
> So:
>
> projections::project(pt1, pt2, prj); // forward or inverse based on 
> input point types

OK for me.

>
>> 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.
>
> And by "proj4 names" do you mean the names of functions used in the 
> Proj4 sources?
> Because the projection parameters has different names in Proj4 string 
> format. I'd expect the users are more familiar with these names, not 
> function names.
>
> E.g. for aea_ellipsoid (e.g. 
> http://spatialreference.org/ref/epsg/2964/) I see that there are 
> basically 3 formats/names:
>
> OGC WKT: PROJECTION["Albers_Conic_Equal_Area"]
> Proj4: +proj=aea
> ESRI WKT: PROJECTION["Albers"]

OK

>
>>
>>>
>>> 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.
>
> I'd like to do the necessary work now if you're still ok with that.

Sure I am, it would be great to make new progress here. Do you also want 
to update the conversion program?

Regards, Barend