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

WIP: CMake-based build system #506

Open
wants to merge 20 commits into
base: master
from

Conversation

@rpavlik
Copy link
Contributor

rpavlik commented Nov 27, 2019

In theory this should make build setup and configuration a lot easier, letting e.g. distributions skip plugins they don't want if there is a dependency they can't use or something, etc. (MeshLab is being removed from Debian because getting a newer, qt5-using version to build was too hard for the current maintainer, etc - https://tracker.debian.org/pkg/meshlab ) Was surprisingly easy to get these CMake scripts generated and going quickly - it's not complete yet, but a surprisingly large subset of the package works, and the only bundled dependencies being used (required) so far are jhead and vcglib.

Right now all CMakeLists.txt files besides the main one are being generated with a Python script and some Jinja2 templates: I'll remove those before it's ready to merge.

Additionally, since this is a much cleaner build with fewer things disabled, etc, a lot more warnings (many of which look like they are pointing out real bugs) show up.

@jmespadero

This comment has been minimized.

Copy link
Contributor

jmespadero commented Nov 28, 2019

I won't wait until this is finished to thank you for the effort you are doing to keep meshlab updated.

@simonschmeisser

This comment has been minimized.

Copy link

simonschmeisser commented Nov 28, 2019

I would really appreciate having a newer release based on CMake and have it in Debian. I can try to help you wherever needed!

@jmespadero

This comment has been minimized.

Copy link
Contributor

jmespadero commented Dec 2, 2019

Hi @rpavlik

I have created a fork of VCGlib with lots of warnings and minor bugs solved, as well as CMakeFiles.txt for directories apps/samples and apps/test. Just like the work you are doing here.

If you want to give an opportunity... just use jmespadero/vcglib instead of cnr-isti-vclab/vcglib.

Branches master and devel are equal.

@rpavlik

This comment has been minimized.

Copy link
Contributor Author

rpavlik commented Dec 2, 2019

@jmespadero ah, cool! I didn't have to add any files to vcglib myself to get this building, but I'll take a look when I can.

I'm at a stopping point for now here, I have all the "easy" stuff building and installing nicely, tested on Linux (in install tree and build tree). There are some plugins that are still disabled, mainly those with extra dependencies.

There are some patches in this branch that should be pulled out and merged separately since they aren't necessarily specific to using CMake.

@rpavlik

This comment has been minimized.

Copy link
Contributor Author

rpavlik commented Dec 2, 2019

BTW - my focus is also on building with external libraries rather than bundled versions - the cmake changes to use bundled versions optionally if the external ones can't be found shouldn't be too rough, I just can't put any more time into it at the moment.

@rpavlik rpavlik force-pushed the rpavlik:cmake branch 2 times, most recently from 4ea6eeb to 7ea377b Dec 3, 2019
@jmespadero

This comment has been minimized.

Copy link
Contributor

jmespadero commented Dec 4, 2019

Hi @rpavlik ,

I have compiled this branch and some filters are missing (for example, FF_VERT_SELECTION and FF_FACE_SELECTION). I have search where the problem could be, but the files meshlabplugins/filter_func/filter_func.h and meshlabplugins/filter_func/filter_func.cpp seems to be the same than original meshlab master branch.

Is it due to WIP state? Should I wait or should I investigate the cause? Can I help with this task?

@alemuntoni

This comment has been minimized.

Copy link
Member

alemuntoni commented Dec 4, 2019

Hi @rpavlik,
thank you again also for this pull request!
It would be great to have also a cmake build system in meshlab, although we would prefer to keep qmake as default.
Let me know if you're planning to fix the missing filters mentioned by @jmespadero! I'm not very familiar with cmake, so I can't help on fixing them :(

@rpavlik

This comment has been minimized.

Copy link
Contributor Author

rpavlik commented Dec 4, 2019

@jmespadero So if you look at src/CMakeLists.txt you'll see the same plugin lists as in the qmake .pro files. There are a few that are commented out, mostly because they need a dependency that I hadn't yet added to the build system (whether by finding or detecting). filter_func is one of those plugins: it needs libmuparser. Because of the manual addition of the _UNICODE define and usage of the wide-string APIs (added somewhat recently), we can't directly use system-provided muparser on Linux, and I avoided dealing with that potentially hairy problem since I don't use that plugin myself.

You're welcome to dig into the CMake files and see if you can follow the same pattern I used to make these:

  • The main CMakeLists.txt file is manually edited, as is the external.cmake file that creates targets for using bundled libraries, but the rest are all generated by ./make-cmake.py
  • For a given directory with last component "x", if there's a src/templates/x.cmake file, it will use that - otherwise it will use src/templates/CMakeLists.template.cmake (which serves as the base template) to generate the CMakeLists.txt
    • The templates are all plain old Jinja2 templates, nothing magic. Lots of docs out there on how to modify them.
    • For best results, you do want to have cmake-format installed locally and lightly modified (see the top of make-cmake.py) so that it is applied before saving the file.
  • If you'd rather edit the CMakeLists.txt files directly for now, that's fine: just ping me or send a PR to me and I can port your changes into the templates.
  • I imagine that eventually the CMakeLists.txt-generating scripts and templates will go away, since that's not a very typical way of working with CMake - however, while the builds are all so similar it was easy to do and meant I didn't have to figure out e.g. the license headers right away :)

@alemuntoni: How do people normally use the files in plugins_experimental and plugins_unsupported? I assume some folks build them or they wouldn't be included, but I wasn't sure how to include them in the build system.

@rpavlik rpavlik force-pushed the rpavlik:cmake branch from 7ea377b to 1dda30f Dec 4, 2019
@rpavlik

This comment has been minimized.

Copy link
Contributor Author

rpavlik commented Dec 4, 2019

OK, I've now enabled a few more plugins:

  • io_3ds
  • filter_csg
  • filter_func
  • filter_isoparametrization
  • filter_sdfgpu

This leaves only the following missing from the CMake build in terms of plugin parity vs the qmake build of meshlab_full:

  • io_ctm
  • filter_qhull
  • filter_ssynth
@alemuntoni

This comment has been minimized.

Copy link
Member

alemuntoni commented Dec 5, 2019

@rpavlik plugins_experimental are plugins that have been developed but builded only by the people that developed them. They are not robust enough to be included in meshlab plugins.
In plugins_unsupported there are old plugins not supported anymore because replaced or based on unsupported external libraries. They are there only for historical reasons.
I'll add some readmes to explain this!
In both cases there is no need to create the cmake build :)

@rpavlik rpavlik force-pushed the rpavlik:cmake branch from 1dda30f to 1787a08 Dec 5, 2019
@rpavlik

This comment has been minimized.

Copy link
Contributor Author

rpavlik commented Dec 5, 2019

OK, I have all the plugins building. There's some TODOs left (and I need to put on license headers) but it's worth trying. Should work on Linux, no idea if it works on Windows or Mac.

The first few commits are all build-system-independent fixes that I'll pull into a separate PR.

@rpavlik rpavlik force-pushed the rpavlik:cmake branch from 1787a08 to 7c49db6 Dec 5, 2019
@rpavlik

This comment has been minimized.

Copy link
Contributor Author

rpavlik commented Dec 5, 2019

OK, now all libraries you can use a system version of (if one exists in Debian). There are a few where using CMake requires you to use a system version despite a bundled one existing - from a packaging point of view this doesn't bother me much, but I'll probably poke at that next week a little bit.

@rpavlik

This comment has been minimized.

Copy link
Contributor Author

rpavlik commented Dec 5, 2019

Can also add plugins from experimental, etc.

image

@cignoni

This comment has been minimized.

Copy link
Contributor

cignoni commented Dec 5, 2019

plugins in unsupported and experimental folders should not be compiled. Most of them are there just for historical reasons, some are plain duplicates with lesser features or even could not compile just for incompatibility with newer version of the vcglib

@rpavlik

This comment has been minimized.

Copy link
Contributor Author

rpavlik commented Dec 5, 2019

Would it make sense to remove them since they're in version control anyway? FWIW, many of the dependencies bundled in external are only used by plugins in those two places.

@rpavlik rpavlik force-pushed the rpavlik:cmake branch 3 times, most recently from 3a9387c to 147fbb0 Dec 5, 2019
@rpavlik rpavlik force-pushed the rpavlik:cmake branch from 147fbb0 to e671197 Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.