GeoProjection: Fix the handling of left handed Unreal location values#9590
GeoProjection: Fix the handling of left handed Unreal location values#9590berndgassmann wants to merge 1 commit intocarla-simulator:ue4-devfrom
Conversation
Use a helper class LocationRightHanded to allow the implemenation of the GeoProjections to perform their projections in a right handed coordinate frame. The explicit handling using LocationRightHanded revealed error in OffsetTransform: result Location is now transformed back to Unreal coordinates and Rotation is fixed. Allow OpenDRIVE offsets for any type of projection Fix UniversalTransverseMercatorParams::operator!=()
|
Hey @berndgassmann, thank you for the PR. The implementation is correct, and it fixes the issue with the offset transform. I've left some comments regarding the exposure to the PythonAPI. The code itself works fine but It'd be great if a couple of things were changed. |
|
Sure I can change...
did you click on publishing of the comments yet?
:-)
|
| @@ -359,21 +381,64 @@ void export_geom() { | |||
|
|
|||
| class_<cg::WebMercatorParams>("GeoProjectionWebMerc") | |||
| .def(init<cg::Ellipsoid>((arg("ellps")=cg::Ellipsoid()))) | |||
There was a problem hiding this comment.
[Applied to all projections]
There are two inits, one with the offset transfom and one without it. I'd merge them into one.
| .def(init<cg::Ellipsoid>((arg("ellps")=cg::Ellipsoid()))) | ||
| .def(init<cg::Ellipsoid, boost::optional<cg::OffsetTransform>>((arg("ellps")=cg::Ellipsoid(), arg("offset")=boost::python::object()))) | ||
| .def_readwrite("ellps", &cg::WebMercatorParams::ellps) | ||
| .add_property("offset", |
There was a problem hiding this comment.
[Applied to all projections]
Is this really the best way to define the offset? It feels convoluted. Why not just pass the default offset? Or None?
And if that isn't possible, at least try to merge the four offsets into one template to avoid repeated code
|
There, I though I had published them, sorry |
2 participants
Use a helper class LocationRightHanded to allow the implemenation of the GeoProjections to perform their projections in a right handed coordinate frame.
The explicit handling using LocationRightHanded revealed error in OffsetTransform: result Location is now transformed back to Unreal coordinates and Rotation is fixed.
Allow OpenDRIVE offsets for any type of projection
Fix UniversalTransverseMercatorParams::operator!=()
This fix should contain the fixes which #9552 tried to tackle by using a more general solution. Especially this solution only applies the left-handed/right-handed conversion to the carla::geom::Location values, because all offset values provided by the OpenDRIVE map itself have to be considered to be in right-handed notation.
Description
Fixes #9530
Fixes #9389
And implements #9552 in a more complete manner.
Where has this been tested?
Possible Drawbacks
This change is