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.

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

Raster utils #262

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Raster utils #262

wants to merge 6 commits into from

Conversation

marrts
Copy link
Contributor

@marrts marrts commented Sep 24, 2024

I can separate this into 2 PRs if needed, but this does two things.

  1. It addresses an yaml library issue. This issue is referenced here and only appears for me when I am building in Release. This gets rid of YAML forward declaration, which I'm not convinced was having a major benefit in compile time, but was causing problems. (For reference I am building on Ubuntu 22.04 using colcon with ROS2 Humble)
  2. It moves several of the functions in the plane slicer raster planner into a separate library header so that they can be reused for other planners.

@marip8
Copy link
Member

marip8 commented Sep 24, 2024

Do you think all of these are worth pulling out as utility functions? My impression was that most of these were really just supporting the plane slice raster planner and might not be useful on their own. I don't mind moving them to the plane slice raster header to make them includable, but I wonder if they're worth exposing as general purpose utilities

@marrts
Copy link
Contributor Author

marrts commented Sep 25, 2024

Do you think all of these are worth pulling out as utility functions?

That was my starting place, but looking back now I don't think that it's really necessary to have all of them pulled out. Maybe a mix of putting some in the plane slice header and a new raster_utils file?

For creating the new drawn toolpath plugin for snp I needed to use:

  • convertMeshToVTKPolyData a new method I made that I figured would be reused a lot
  • insertNormals

These are both moreso mesh utilities that a toolpath planner might want.

Most of the other things feel more like plane slicer, or a raster pattern specific task that could be moved back to the original file, but with a header declaration to be includable

@marip8
Copy link
Member

marip8 commented Sep 25, 2024

Okay, I think that sounds like a good plan. Can you update the description/variable names of insertNormals? It took me a while to understand that that function is trying to assign normal vectors to points on a raster segment using the mesh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants