Fix animation start pose being applied twice in Animate.trplot

[RoboX2020]

Fixes #171.

Animate.trplot built the frame geometry already placed at the requested start pose. The animation then applied the absolute pose again when drawing, so the frame showed up at the wrong place: asking for a start of (-1, 0, 2) displayed it at (-2, 0, 4).

The geometry is now built at the identity and the start pose is applied once through _draw(self.start), which matches how the end pose and the intermediate frames are already drawn.

Test: added a case to tests/base/test_transforms3d_plot.py that checks the drawn frame at the start and end poses for the line, arrow and rviz styles. It fails on master and passes with this change. Full suite: 348 passed, 3 skipped. The interactive animation loop itself is not exercised in the headless tests; the endpoint poses are.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[petercorke]

Thank you for your interest in SMTB, and for tracking this one down. The double application of the start pose in Animate.trplot was exactly what #171 described, and your fix, building the geometry at the identity and drawing the start pose once through _draw, is clean and consistent with how the end pose and intermediate frames are handled. I've checked it against a static trplot at both the start and end poses for the line and rviz styles, and the frame labels, and it matches. For the arrow style the shafts match too; only the arrowhead wings differ slightly, which was already the case for every animated frame.

One suggestion on the test, which isn't a blocker: it currently passes even if the self._draw(self.start) line is removed, because it calls _draw itself and computes expected from the stored points. A stronger version would plot trplot(start) on a plain axes, then check that the Animate artists have the same coordinates straight after trplot(end, start=start), and again after _draw(end). That would fail if either half of the fix were lost, and it would exercise the rviz style too.

I've approved this. Merging is up to the RAI maintainers, so it may take a little while. Thanks again for the contribution!

[petercorke]

A quick note on the failing sphinx / sphinx check: this isn't caused by your change, so please don't worry about it. All of the unit-test jobs and codecov pass.

The docs build itself completes. It fails on its final step, which tries to push the built docs to gh-pages. For pull requests from a fork, GitHub gives the workflow a read-only token, so that push is always rejected with a 403. It would happen to any contributor's PR right now.

The fix is already open as #222, which stops the docs job running on pull requests at all. It's waiting on review by the RAI maintainers. Once that merges, the check will go away, and refreshing this PR (for example with "Update branch") will pick up the fix. There's nothing you need to change on your side.

[RoboX2020]

Thanks Peter, and thanks for checking it against a static trplot and for the note on the sphinx check.

I've pushed a stronger test along the lines you suggested. It compares the Animate frame's axis artists with a static trplot, once straight after trplot(end, start=start) and again after _draw(end), for the line, arrow (shafts only) and rviz styles. It fails if either half of the fix is removed.