embrace trigonometry (fix also issue #164)

[facontidavide]

In theory, this modification is just about readability. It should give EXACTLY the same result, but...

In practice, we fixed also a problem with min/max distance, as reported in #164

Please test it before merging it, to be sure I didn't miss anything.

[facontidavide]

THis is emabrassing.... there is a compilation error. Let me fix it

[SteveMacenski]

@tasilb can you test this? If you set VISUALIZE_FRUSTUM 1 in the depth camera frustum file, it'll publish the points of the frustum (kill performance, but you can see the frustum corners and normal vectors). Just making sure I don't break you guys -- stuff inside updating, stuff outside not, the usual.

[tasilb]

Sure thing, though I might have to take my time with it due to hardware availability

…

________________________________ From: Steve Macenski <[email protected]> Sent: Tuesday, April 7, 2020 11:24:36 AM To: SteveMacenski/spatio_temporal_voxel_layer <[email protected]> Cc: David Tsai <[email protected]>; Mention <[email protected]> Subject: Re: [SteveMacenski/spatio_temporal_voxel_layer] embrace trigonometry (fix also issue #164) (#165) @tasilb<https://github.com/tasilb> can you test this? If you set VISUALIZE_FRUSTUM 1 in the depth camera frustum file, it'll publish the points of the frustum (kill performance, but you can see the frustum corners and normal vectors. Just making sure I don't break you guys -- stuff inside updating, stuff outside not, the usual. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#165 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABJYI5EW7UIR3UCQ2VLL4G3RLNVWJANCNFSM4MDBB5KA>.

[SteveMacenski]

@tasilb have you taken a look at this?

[mlsdpk]

Are there any updates on this? I've noticed a significant improvement in CPU juice after removing the normalize operation

[facontidavide]

I guess the changes are still valid 😆

Using std array instead of vector surely help too

[SteveMacenski]

@mlsdpk if you can validate that this represents the right frustum now, I can merge this in.

[mlsdpk]

Understood! I'll do my best to share results from real hardware, though I cannot guarantee it due to confidentiality concerns.

Is it acceptable if I submit a PR for unit tests to compare the points from pcl::FrustumCulling filter with STVL's depth camera frustum? This would allow us to thoroughly test the changes introduced in this PR also. Please let me know if this approach aligns with your expectations.

[SteveMacenski]

Visualizing the frustum and/or measuring the plane angles and/or measuring the corner points and comparing them to manually computed values from another method would be all ways to validate.

Certainly pcl would be another way :-) Unit tests are always appreciated, but not required for this repository. This is largely in legacy maintenance mode with few/no outstanding issues (beyond this) that hopefully will be superseded by something building on this idea with probabilistic modeling in the medium term future.

I usually require much more formality, but this is an old project that predated that policy and I expect that if Davide tested it, it probably works fine, I just want a 3rd opinion since we're all human