I find this hard(er) to read, I know what a rotation is but I will have to think a long time to remember what the linear part of a matrix is. It's similar yet not as bad for transpose vs inverse

Would it hurt to only change this for melodic? Or maybe split it into internal and external api cases so that plugins etc still stay compatible? If so I would vote for using Eigen::Isometry3d

@simonschmeisser I don't get your point yet. I think there is no question in applying those changes both in Kinetic and Melodic as there is a severe performance penalty connected with calling Affine3d::rotation() or Affine3d::inverse().

Replacing Eigen::Isometry3d for Eigen::Affine3d applies the more efficient operations automatically when calling rotation() or inverse().

So are you asking to apply this type change in Melodic and keep rotation() and inverse() as is?
I'm definitely open to this. However, this might pose a new issue: Probably it's not possible to assign Isometry3d <- Affine3d, which probably is required when using eigen conversions, isn't it?
If there is consensus on taking this alternative approach, I will target these commits for Kinetic only and provide an alternative PR for Melodic.
@v4hn @davetcoleman Any opinions on that?

My concerns are confirmed: #1096 fails when trying to assign Affine3d (as a result of tf2 conversion) to Isometry3d. Tully accepted a while ago a patch to support Isometry in tf1. So we would need to file a similar patch for tf2 as well. Any volunteers?

So we would need to file a similar patch for tf2 as well. Any volunteers?

I will look into this.

Just noticed that we will need to switch to Eigen::Isometry in geometric_shapes as well!

I agree I think we should not change kinetic-devel, and do it the better way for melodic-devel using Isometry3d

I rebased and retargeted this PR to Kinetic.

I'm not super comfortable making this kind of change in the kinetic branch, but also do not feel super strongly about it. Would like to hear other's thoughts.

Can you motivate your objections? This is just performance improvement. No API changes.

Rebased, to resolve merge conflicts.

I have not yet compiled and tested this PR but from reading through it does look safe. I still have slight theoretical objections about limiting the amount of changes in a somewhat stable branch but that is more a general issue I guess and not to be discussed here.

I'm really happy this has been fixed in a type-safe manner in melodic and can be bribed by proposed speed improvements (do we have a way to put that into numbers?) for kinetic

Would be indeed interesting to put this in numbers, e.g. performing some random RobotState updates...