nobleo / rviz_satellite

Display internet satellite imagery in RViz
Apache License 2.0
569 stars 238 forks source link

Fix time synchronization of NavSat transform lookup #88

Closed beetleskin closed 4 years ago

beetleskin commented 4 years ago

Previously, the FrameManager's tf cache was used to look this up. However, this cache is not suitable since its designed for render-frame timings and only for the RViz' fixed-frame. Using the normal TfBuffer resolves some tile offsets when tf is not perfectly up to date.

Test-Plan: @schra Run with different flavors of our stack, confirm watertight transforms (no glitches left, always correct drawing).

beetleskin commented 4 years ago

Can you please elaborate what problem you exactly observe in the first place?

When tf is not perfectly in sync/up2date (which it almost never is), we observe tile jumps. The later the tf and the faster the robot movement, the bigger the jumps. Those jumps happen, when we switch from one center-tile to another. I'm not yet sure why we have those huge jumps, since tf timing is only a tiny little bit off, nothing spectectular. I suspect that even small rotation errors have a big influence.

I have no clue how we never saw this before??

Same here .. I could swear this was pretty stable after my last fix. But maybe something small changed upstream (RViz or tf for instance)? Its very hard to track those changes (fyi).

Why is it ok to leave the getTransform() call in line 675 of src/aerialmap_display.cpp (https://github.com/gareth-cross/rviz_satellite/pull/88/files#diff-98bebf14889792e0de19258987190f0dR675)? Shouldn't we use tfbuffer there too?

When its about transforming stuff into the _fixedframe, one should always use the FrameManager, since it speeds up things and helps with the timing.