Performance improvement to cropbox #1210
Conversation
| @@ -3,6 +3,7 @@ Software License Agreement (BSD License) | |||
| Point Cloud Library (PCL) - www.pointclouds.org | |||
| Copyright (c) 2009-2012, Willow Garage, Inc. | |||
| Copyright (c) 2012-, Open Perception, Inc. | |||
| Copyright (c) 2015-, Google, Inc. | |||
There was a problem hiding this comment.
-1 If you really want Google in there, make it 2015 only.
There was a problem hiding this comment.
Done. Thanks for the feedback.
|
Otherwise +1, Thanks! |
|
Where this Google copyright comes from at all? Also can you please add spaces between function names and braces as required by the style guide? Thanks! |
|
Thanks for the feedback. I've fixed the style issues and changed LICENSE.txt to only list Google in 2015. @taketwo , as I'm a Google employee, I'd like to add the Google copyright to the LICENSE.txt. I'm excited to get these changes in, as I've seen the calls I'm making to cropbox improve in speed drastically. Those isIdentity() calls are expensive! Thanks! |
|
I can see how your changes improve the performance. As such, they are definitely valuable to PCL. However, in my opinion, this contribution is just too small on the library scale in order to add your employer's copyright to the main LICENSE file. If you insist on keeping the copyright, as a compromise I would propose to add it only to the files you actually change. If you grep through PCL codebase you'll see such additional copyrights here and there, so it's a common practice. @jspricke What do you think? It seems to be more or less in line with @rbrusu's opinion which he expressed in our mail exchange sometime last year. |
|
@taketwo , that's okay by me. |
|
Great! Could you please squash the commits into a single one? I'll merge then. |
|
Done! Thanks. |
Performance improvement to cropbox
Move matrix identity checks out of inner loops in cropbox applyFilter to increase performance. Add Google to LICENSE.txt