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.
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?
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?
Shouldn't projections (and transform strategies) only project
between points?
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().
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:
using namespace projections;
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
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"]
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.
Regards,
Adam