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

Add vtk to build visualization #31

Merged
merged 25 commits into from
May 14, 2021
Merged

Add vtk to build visualization #31

merged 25 commits into from
May 14, 2021

Conversation

Tobias-Fischer
Copy link
Contributor

@Tobias-Fischer Tobias-Fischer commented Apr 1, 2021

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@Tobias-Fischer
Copy link
Contributor Author

This is needed to build rtabmap in the RoboStack project
/cc @wolfv @traversaro

@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

@Tobias-Fischer Tobias-Fischer changed the title Add vtk to build visualization Add vtk and qt to build visualization Apr 1, 2021
@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

conda-forge-linter and others added 2 commits April 1, 2021 21:42
@hobu
Copy link
Contributor

hobu commented Apr 2, 2021

I'm worried that VTK and QT dependencies of PCL are going to make it difficult to bump versions and rerender in the future. VTK and QT (QT especially) are not upgraded at their normal release pace in Conda/Forge.

I don't know if it is possible to create a sub package or something that makes this part of PCL optionally installed. It is probably a lot more effort than it is worth.

@Tobias-Fischer
Copy link
Contributor Author

We can create two variants, one with QT/VTK and one without. I'll try getting it to build first, and then introduce the variant.

@hobu
Copy link
Contributor

hobu commented Apr 3, 2021

If it is a big hassle don't worry about it.

@Tobias-Fischer Tobias-Fischer reopened this Apr 8, 2021
@Tobias-Fischer Tobias-Fischer changed the title Add vtk and qt to build visualization [WIP] Add vtk and qt to build visualization Apr 8, 2021
@Tobias-Fischer Tobias-Fischer changed the title [WIP] Add vtk and qt to build visualization [WIP] Add vtk to build visualization Apr 11, 2021
@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

@Tobias-Fischer
Copy link
Contributor Author

Is there some way to enforce pulling in the most recent build? Osx fails because it pulls in the previous build of vtk (build num 6), not the very latest build (build num 7)

@Tobias-Fischer
Copy link
Contributor Author

@conda-forge-admin, please rerender

@Tobias-Fischer Tobias-Fischer changed the title [WIP] Add vtk to build visualization Add vtk to build visualization May 12, 2021
@Tobias-Fischer
Copy link
Contributor Author

Hi @hobu @chambbj,
This is now ready for your review. It adds support for the visualization subsystem by enabling the WITH_OPENGL, WITH_QT and WITH_VTK flags. To enable building with VTK9, a patch is included from PointCloudLibrary/pcl#4262 that is already merged upstream.

It would be great if you could have a look and let me know what you think.

Copy link
Contributor

@chambbj chambbj left a comment

Choose a reason for hiding this comment

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

I'm fine with this. Objections @hobu?

@chambbj
Copy link
Contributor

chambbj commented May 14, 2021

@Tobias-Fischer I guess I will only add that it could be helpful to cross-reference the patch to the PCL fix so that it is a little more obvious when we can back out the patch. E.g., patch 4540 corresponds to PointCloudLibrary/pcl#4540 and has yet to be merged. I'll be honest, we clearly haven't been great about watching these, but it could help down the road.

@hobu hobu merged commit 1873f25 into conda-forge:master May 14, 2021
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.

4 participants