Camera of SDL Viewer works with ECEF

[catevita]

by adding the input parameter --ecef it is possible to start the viewing hovering over palo alto and by pressing G it realigns the point of view with the earth center ("gravity")

the camera structure had redundant member variables for expressing the angles and this has been cleaned up with an eye on backward compatibility.

[catevita]

PTAL @nnmm fyi @SirVer

[nnmm]

Summarizing what we talked about offline: It would be great if the point cloud couldn't tilt in weird ways, like the local point clouds. That is, have only two degrees of freedoms (pitch and yaw) wrt. the ground plane.

At a high level, I see two possible approaches:

• The camera is essentially a transform with an internal representation that enforces the no-roll constraint. We could allow the camera to store an additional world_to_local transform. That transform is composed with the camera transform itself, which would aqcuire the meaning of local_to_camera (instead of world_to_camera), in the get_world_to_gl() function. The camera functionality doesn't change at all otherwise, so it's a pretty minimal change, and you get the constraint for free.
• Re-parametrize the transform so that rotations are interpreted not as local to the camera coordinates, but relative to the earth_vector and north. That would require special casing (i.e. the CameraPose struct you introduced) which would not be needed with the first option.

It seems to me that the first is simpler and more flexible. What do you think?

[catevita]

I'd stick with quaternions, I just needed to pick the correct axis to apply the rotations. The solution is more general and does neither require casing nor additional redundancy information to be stored.

[nnmm]

Is this ready to be reviewed, i.e. is the constrained camera movement already included?

[catevita]

yes, the camera is now constrained

[nnmm]

I'm sorry for the large delay. I finally tested it and it seems that on this branch, the camera now tilts in weird ways even when not using ECEF at all. I also can't see anything when using my test ECEF octree.

This brings me back to my worries that this approach is too complex and hard to get right – just lots of fiddly math. I think we should look at just adding an extra transform to the camera again. I'd like to hear what you think about this, and to give you a better idea of what I mean, I set up a proof-of-concept here: https://github.com/googlecartographer/point_cloud_viewer/compare/nnmm/ecef_camera

I think the benefits of that would be:

• Less code (no set_displacement()) and less math
• Nothing PAO- or ECEF-specific goes in this repository
• The viewing convention (North is "up") is not hardcoded anymore The drawbacks are
• You'd have to give it an extra protobuf each time you view an ECEF octree
• The transform is not determined automatically

You were worried about introducing redundancy, but I don't think we'd incur meaningful redundancy here (where different representations can get out of sync) – the local transform is set once and never modified.

[feuerste]

I like @nnmm's approach. The only thing I would change is not to rely on the proto, but automatically determine the transformation, similar to how it is done in https://github.com/lyft/avmapping-geometry/blob/master/xray_extensions/src/bin/build_xray_quadtree.rs, which would just require minor refactorings for extensions.

[catevita]

I had a quick look and it makes a lot of sense to change in these suggested directions!

[feuerste]

See https://github.com/googlecartographer/point_cloud_viewer/pull/274 and https://github.com/lyft/avmapping-geometry/pull/398 for my proposed changes