-
Notifications
You must be signed in to change notification settings - Fork 956
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stepping back. Eigen::Affine3f is better than the recently introduced changes in #20 #29
Comments
Hi again ;) I also thought about this issue but haven't looked at it into detail. My thought is that the input cloud is already expected to arrive in order. Using this fact, we could transform all 16 / 32 / 64 scan rings simultaneously, using the same transform for all rings instead of creating a transformation per point. This would reduce transformation creations by the factor of scan rings. |
To be fair, as you can see in the picture, the operation pointAssociateToMap isn't very important in terms of CPU usage, in laserMapping. So, it is mostly a matter of code readability, not performance. On the other hand, the contribution to total calculation in TransformToEnd is more significant Can anyone explain to me what this line of code is supposed to mean?
|
Very interesting figures! Sometimes there is an option to optimize, but the effect is negligible. In this case I would prefer readability, too. Regarding the code line you asked about: The scanRegistration component sets the intensity of a point to its ring number + a weighted time factor based on its horizontal angle. The idea is to encode the scan index and relative point time within the point intensity. This way you can access the scan index by:
and its relative scan time via:
However, there are currently some issues in the calculation of the involved angles, resulting in wrong interpolations of the relative times as I mentioned in my other issue. Even though it still seems to work somehow... Btw.: The magic constant here (10) is the result of (1 / SCAN_PERIOD) in my opinion. |
@facontidavide |
@facontidavide what software you use in above pitcure? |
Hotspot from kdab. You can find it on github. It is a graphical interface to visualize the results of perf |
@alittleharry I totally agree with you. I never argued that the correction of the cloud distortion is negligible. I was referring to some code optimizations, which - according to the performance figures - do not really contribute to the overall runtime. |
It's absolutely useful! |
Hi,
with the recent PR I wanted to test that there could be a considerable improve in execution time avoiding useless calculations of cos and sin.
Introducing rotateX, rotateY and rotateZ, I believe that the code become more readable.
But thinking twice, I think that the way I would do it myself would be to create an affine transform once and use it for each of the points, instead of doing multiple, simple rotations and translations.
Before doing this change, I wanted to get some feedback from the other users and the author...
The text was updated successfully, but these errors were encountered: