Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Utilize setScaleDirty() and setPaddingDirty() to improve performance #2038

Merged
merged 2 commits into from
May 22, 2020
Merged

Utilize setScaleDirty() and setPaddingDirty() to improve performance #2038

merged 2 commits into from
May 22, 2020

Conversation

peci1
Copy link
Contributor

@peci1 peci1 commented Apr 27, 2020

Description

As requested in moveit/geometric_shapes#109 (comment).

I only found one candidate usage...

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@peci1 peci1 mentioned this pull request Apr 28, 2020
Merged
@peci1
Copy link
Contributor Author

peci1 commented Apr 28, 2020

Could be optimized even more by backport of moveit/geometric_shapes#137 to melodic.

@v4hn
Copy link
Contributor

v4hn commented May 4, 2020

backport in progress: moveit/geometric_shapes#142
Feel free to update the patch to make use of it.

@v4hn
Copy link
Contributor

v4hn commented May 21, 2020

@peci1 we actually moved to depend on noetic-devel for geometric_shapes in master now, but the patch was also backported. Could you update this patch too, please?

@peci1 peci1 requested review from rhaschke and v4hn as code owners May 21, 2020 16:03
@peci1
Copy link
Contributor Author

peci1 commented May 21, 2020

The speedup has been implemented. I found two more places where it can be done.

@codecov-commenter
Copy link

Codecov Report

Merging #2038 into master will increase coverage by 0.00%.
The diff coverage is 77.77%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2038   +/-   ##
=======================================
  Coverage   57.79%   57.79%           
=======================================
  Files         328      328           
  Lines       25664    25665    +1     
=======================================
+ Hits        14833    14834    +1     
  Misses      10831    10831
Impacted Files Coverage Δ
...kinematic_constraints/src/kinematic_constraint.cpp 72.69% <50.00%> (-0.05%) ⬇️
moveit_core/distance_field/src/distance_field.cpp 26.02% <100.00%> (ø)
...eption/point_containment_filter/src/shape_mask.cpp 53.94% <100.00%> (+1.24%) ⬆️
...ma_kinematics_plugin/src/lma_kinematics_plugin.cpp 78.94% <0.00%> (-2.26%) ⬇️
...anning_scene_monitor/src/current_state_monitor.cpp 52.82% <0.00%> (+1.53%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 736219d...804192c. Read the comment docs.

v4hn
v4hn approved these changes May 21, 2020
View reviewed changes
Copy link
Contributor

@v4hn v4hn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks as many times before @peci1 😃

@rhaschke rhaschke merged commit ea42a68 into moveit:master May 22, 2020
pradeepr-roboticist pushed a commit to pradeepr-roboticist/moveit that referenced this pull request Jun 3, 2020
@peci1
Utilize new geometric_shapes functions to improve performance (moveit…
9571880 
…#2038)

Utilize createEmptyBodyFromShapeType(), setScaleDirty() and setPaddingDirty()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhaschke rhaschke rhaschke approved these changes

@v4hn v4hn v4hn approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@peci1 @v4hn @codecov-commenter @rhaschke