Re-add support for macOS#2019
Conversation
|
Sorry for the long time to answer, i'm still figuring on how to get access to a mac. |
|
No worries, I'll be busy with exams until the end of February anyways, so I won't be able to work on this full time until then. Just wanted to get this out, so at least the RFC period can start :). |
|
A quick follow up on the Boost/LLVM issue: This appears to be a known upstream problem, which (as of the time of writing) can only be fixed by overwriting LLVM headers, see here and here. So there are essentially three options to approach this (imho):
I strongly prefer option 3.1. (as it is the most controllable from the organization's pov), but if you have other ideas: Please let me know ^^. |
|
Hi @music-dsp-collection — thank you so much for the incredible work on MTL-AliceVision! 🎉 I've been testing the Meshroom 3D viewer with your macOS Metal bundle on Apple Silicon (M4 Max, macOS 26.3) and it's working great. We ran into some Meshroom-side issues where the 3D viewer crashed due to Metal RHI's strict vertex descriptor validation — custom Qt3D geometries (Grid3D, BoundingBox, Locator3D) and loaded meshes didn't always provide the We've put together fixes in a Meshroom PR: alicevision/Meshroom#3030 Key fixes on the Meshroom side:
Everything is running stable now — the full Meshroom pipeline (camera init → feature extraction → matching → structure from motion → texturing) works end-to-end, and the 3D viewer renders loaded meshes with textures via EXR support from QtAliceVisionImageIO. Happy to help with any additional macOS testing! Thanks again for making this possible. 🙏 |
|
Happy to hear that :D! I finished my exams and will be able to start working on this again shortly. Hopefully I'll be able to iron out the last blockers and turn this into a proper pull request soon :). |
Update: One step closer
It seems like this is now the most probable option. A PR in the Boost.Math repo is in the process of being merged, so this could actually be resolved very soon. I'll work on switching to the CMake build system for Boost as soon as the interface is stable (i.e., the PR is merged). Once this is done, I think the infrastructure changes are ready to be merged 🚀. AFAIK the required changes for Boost should land in v1.91.0, which is scheduled to be released on April 8th! |
|
CCTag merged the required PR, so this technically has no blockers left - except that it needs a huge rebase/rework because of the PR that changed the dependency handling. |
My bad, sorry for that. I forgot you had made changes to those files. |
|
No worries, that was something I wanted to tackle anyways. It is muuuch cleaner now and transferring the changes should be as simple as cherrypicking or copy-pasting. I don't expect that to take long :). |
philippremy
force-pushed
the
bits/macos-infra
branch
from
5e73b01 to
c35036d
Compare
|
I reworked everything onto the new infrastructure and even found some bugs on my side along the way. I tested compilation on my Mac (clean, only required tools per Thanks for the patience everyone ^^! EDIT: Woops, didn't mean to rewrite that much history. Commits are now back with their owners :D. |
philippremy
force-pushed
the
bits/macos-infra
branch
from
c35036d to
553d389
Compare
| ) | ||
|
|
||
| set(XERCESC_CMAKE_FLAGS | ||
| # TODO |
There was a problem hiding this comment.
My bad, this should contain -DXercesC_DIR:PATH=${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}/cmake/XercesC.
Should I add a new commit for this or rebase? You can add it, if you want to, of course.
| set_target_properties(${software_name}_exe | ||
| PROPERTIES SOVERSION ${ALICEVISION_SOFTWARE_VERSION_MAJOR} | ||
| VERSION "${ALICEVISION_SOFTWARE_VERSION_MAJOR}.${ALICEVISION_SOFTWARE_VERSION_MINOR}" | ||
| VERSION "${ALICEVISION_SOFTWARE_VERSION_MAJOR}_${ALICEVISION_SOFTWARE_VERSION_MINOR}" |
There was a problem hiding this comment.
The standard is to use dot:
https://cmake.org/cmake/help/latest/prop_tgt/VERSION.html#prop_tgt:VERSION
There was a problem hiding this comment.
Yes, I have seen the dot notation in many places actually, but so far only for shared libraries. And for these it is not an issue, because they always have the .dylib file extension. For Mach-O executables (on macOS) the system went to interpret the last version number as a file extension, because they usually don't have a file extension at all (see the image as an example). While not the biggest issue, e.g. Xcode refuses to load the executable for debugging, because it only checked the file extension (bad practice, I know).
And I thought it would be fine because we created symlinks anyways and as far as I can tell, Meshroom for example will invoke the non-versioned symlinks.
But feel free to reverse, if it breaks stuff I did not notice. Invoking them via the CLI works fine, as far as I know...
There was a problem hiding this comment.
can't we just do that for mac os ?
There was a problem hiding this comment.
Sounds good to me!
servantftransperfect
force-pushed
the
bits/macos-infra
branch
from
deefd5f to
6e0ecfd
Compare
|
I can't compile docker images with the updated boost-1.90. Any reason not to stick with 1.86 ? (Except cmake archive) |
servantftransperfect
force-pushed
the
bits/macos-infra
branch
2 times, most recently
from
c5421f2 to
5487257
Compare
The previous OptimizeForArchitecture only included proper support for x86(_64). The new implementation supports x86(_64), arm(64), ppc and adds newer CPU families. Furthermore, it adapts OFA to account for cross-compilation on Apple targets. When targeting arm64, it selects apple-m1 as the baseline architecture (making it compatible with all Mac Apple Silicon Chips). When targeting x86_64, it selects skylake as the baseline architecture (making it compatible with all Intel Macs not older than 2015). All of this can be manually overridden by sepcifying TARGET_ARCHITECTURE on the command line. The code is derived/copied from https://gitlab.inria.fr/gismo/gismo, which is licensed under MPL-2. Signed-off-by: Philipp Remy <philipp.remy@law-school.de>
Instead of setting CFLAGS or CXXFLAGS manually, simply use the CMake targets OpenMP::OpenMP_C and OpenMP::OpenMP_CXX respectively. This applies to dependencies (MeshSD), nonfree (VLSift) and the AliceVision targets itself. Signed-off-by: Philipp Remy <philipp.remy@law-school.de>
This commit adds support for compiling library targets into Apple Frameworks on macOS. Embedding resources into the bundle is supported as well. It is checked early if the user attempts to build a universal binary, which is currently unsuppprted due to missing support in the dependency building code. Signed-off-by: Philipp Remy <philipp.remy@law-school.de>
Executable file names are suffixed with their respective versions in the format "MAJOR.MINOR". This is fine as long as the executable (as on Windows) has a proper file extension. On macOS (and other Unix OSes as well), the appended version suffix could be falsely interpreted as a file extension. Change "MAJOR.MINOR" to "MAJOR_MINOR" formatting. Signed-off-by: Philipp Remy <philipp.remy@law-school.de>
This commit introduces several changes to the general build infrastructure. Given CMAKE_OSX_ARCHITECTURES, it is now possible to cross-compile the whole project and its dependencies from x86_64 to aarch64 and vice versa on macOS. This is easily achievable because Apple can compile for both architectures with the same sysroot. Choosing the correct flags depends on the underlying build system:
(1) CMake: In CMake, one can just set CMAKE_OSX_ARCHITECTURES to either arm64 or x86_86.
(2) libVPX: A target must be specified using the --target flag (includes the major Darwin version number).
(3) ffmpeg: Must pass the additional CFLAGS, CXXFLAGS and LDFLAGS (with "-arch x86_64/arm64") and the --arch (with x86_64 or aarch64(!)), --enable-cross-compile and --sysroot (xcrun --sdk macosx --show-sdk-path) flags.
(4) Boost: For Boost, it is essentially the same as for CMake, but
Boost.Context requires some special handling for its assembly files.
SWIG will not be cross-compiled, because it is a compile-time tool that
must match the host CPU of the machine performing the build.
Signed-off-by: Philipp Remy <philipp@philippremy.net>
…pple targets where necessary As it might be desirable to later change the embedded rpaths of a binary, "install_name_tool" expects enough padding between the header and the first data section of a Mach-O file. This caused issues when changing the rpaths of the following dependencies, which added little to no padding to the Mach-O header: - ffmpeg Pass "-headerpad_max_install_names" explicitly as linker flags to these dependencies when building them on Apple platforms. This adds the maximum amount of padding to the header (per ld man page: "Automatically adds space for future expansion of load commands such that all paths could expand to MAXPATHLEN."). Signed-off-by: Philipp Remy <philipp@philippremy.net>
As we do not build Ceres with SuiteSparse support on Apple platforms by default (Ceres can utilize Accelerate.framework), do not ask for SuiteSparse on Apple platforms. As an equivalent, make sure to find a Ceres with AccelerateSparse (can be disabled in the same manner as Ceres with SuiteSparse). Signed-off-by: Philipp Remy <philipp@philippremy.net>
…ries for AliceVision Currently, the name is set by CMAKE_SYSTEM_PROCESSOR. However, this does not always match the target architecture, as users can cross-compile on Apple targets. On Apple, use CMAKE_OSX_ARCHITECTURES instead. Signed-off-by: Philipp Remy <philipp.remy@law-school.de>
This commit adds a Python script (darwin_bundle.py) which takes several Mach-O files (or Framework folders) as input and attempts to resolve all required dependencies to create a standalone (i.e, self-contained) bundle. A CMake target for use after the AliceVision build is added to automatically invoke the script with all targets of the project (can be invoked with make darwin-bundle). This works somewhat similar to what the regular bundle target does, but the CMake implementation did not appear to work for Apple targets (out of the box). Signed-off-by: Philipp Remy <philipp@philippremy.net>
Due to the additional requirements and the unique build process of this platform (cross-compiling, environment, ...) move the build instructions to a custom file. This commit also introduces a list of available target architectures in the OFA system, available at src/cmake/OFA/SupportedArchitectures.md. Signed-off-by: Philipp Remy <philipp.remy@law-school.de>
Adapt linker flags to the Apple linker (ld), which does not allow undefined symbols in shared objects (.so) by default, see SWIG issue 2469. Signed-off-by: Philipp Remy <philipp@philippremy.net>
Respect the user's choice to use static libraries for the dependencies Signed-off-by: Philipp Remy <philipp@philippremy.net>
Fixes some configuration errors introduced by rebasing onto the new CMake infrastructure. Signed-off-by: Philipp Remy <philipp@philippremy.net>
Bump version of OpenImageIO from v3.0.9.1 to v3.0.17.0 (appears to be ABI and API compatible), because v3.0.9.1 uses a version of fmt which causes consteval errors on newer Clang compilers. Signed-off-by: Philipp Remy <philipp@philippremy.net>
When building an embedded PXR and these flags are not set, the CMake build system of USD does a funny thing and overwrites PXR_BUILD_GPU_SUPPORT, causing several libraries to link to a non-existent GPU target of OpenSubdiv (which we explicitly build *without* GPU backends, see opensubdiv.cmake). Signed-off-by: Philipp Remy <philipp@philippremy.net>
CCTag merged a patch which removed several Boost dependencies (including boost::math_c99, which we currently cannot build with Boost's CMake build system). FIXME: Switch to a proper release tag once CCTag gets a new release which includes these changes. Signed-off-by: Philipp Remy <philipp@philippremy.net>
liblemon always assumes a static library in its CMake configuration template and therefore cannot be used as a dynamic library. It also still uses the C++17 deprecated "register" keyword, so force it to build with C++14 to avoid a compilation error. FIXME: Remove when upstream liblemon is updated accordingly. Signed-off-by: Philipp Remy <philipp@philippremy.net>
As all other dependencies are built using AV_BUILD_DEPENDENCIES_PARALLEL, this should also be applied to AliceVision in this case to avoid eternal compilation times. Signed-off-by: Philipp Remy <philipp@philippremy.net>
This caused cross-compilation to fail on Apple platforms, because it did not receive the baseline CMAKE_CORE_BUILD_FLAGS we pass to each dependency build. Signed-off-by: Philipp Remy <philipp@philippremy.net>
This commit adds some information about how to correctly build AliceVision for use with Meshroom, as this poses some specific requirements regarding Python, NumPy and the SWIG bindings. Signed-off-by: Philipp Remy <philipp@philippremy.net>
If the user disabled the SWIG bindings, Python might not have been found for the bundler. MacOS always has a (somewhat old) Python 3.9 available, so we can safely use this. Add an explicit search step if Python is not yet available. Signed-off-by: Philipp Remy <philipp@philippremy.net>
We also need to make sure that any SWIG generated .so files are entirely self contained in the bundle. As they cannot rely on more libraries than their respective main library, we can simply remove all rpaths and add a single new one "@loader_path/../../", which directly resolves to the lib folder in the bundle containing all bundled dependencies. That can be done with a simple shell script. Signed-off-by: Philipp Remy <philipp@philippremy.net>
Simplify some of the functions in darwin_bundle.py. DISCLAIMER: This was done using AI assistance. Signed-off-by: Philipp Remy <philipp@philippremy.net>
servantftransperfect
force-pushed
the
bits/macos-infra
branch
from
5487257 to
8b28017
Compare
No, not really - I guess I was still thinking about the What exactly is causing the build failure in Docker (just out of personal curiosity)? |
I'm getting a weird compilation error in boost maths numeric_limits.hpp. BOOST_STATIC_something contains constexpr which generate a constexpr constexpr function(...). |
Uh oh, that doesn't sound good. I think I had the same issue at some point and just forgot about it because I patched it manually. I'll see if I it already fixed upstream and if not, I will open a bug report. The maintainer over there is super helpful and maybe that'll make us future-proof :D. EDIT: Apparently has been fixed two weeks ago, but that will probably only land in Boost 1.91, because it didn't make it into master in time: boostorg/math#1383. So if it wasn't a problem in 1.86, we should definitely revert! Using the develop branch for all Boost stuff doesn't seem a good idea imho :). |
servantftransperfect
force-pushed
the
bits/macos-infra
branch
from
8b28017 to
5d9511f
Compare
servantftransperfect
left a comment
servantftransperfect
left a comment
There was a problem hiding this comment.
That was a pretty long review to do !
So first of all, thanks for this large PR. I am sure many people will be happy to see this available.
I reviewed the changes mostly for non regression.
As it is focused on cmake changes, I checked that our docker containers are still building correctly. The CI is still working correctly.
I am NOT able to validate the APPLE part as we have no access to hardware.
I did some updates related to the comment i did, and i added some changes for the dockerFiles.
|
Thank YOU for this awesome project! The containers are something that I cannot reliably test, so thanks for that as well. For any future PRs, I will try to keep them way smaller. Everything seems fine to me - but as I mentioned over in my proposal in the discussion section, the build will probably always be somewhat brittle (at least for the mac users under us). And that isn't even our fault, but CMake just really gets to its limits with so many dependencies. I will try to do regular testing for macOS, if possible. And maybe I'll be able to spin up some CI for macOS as well. |
servantftransperfect
merged commit 33b6e27
into
alicevision:develop
4 participants
Project support for macOS
This is now ready to be merged ^^.
I know that these look like some heavy changes, but I believe that I did not break any Windows/Linux infrastructure. I tried my best to do CI on both Windows and Linux, and at least with the official GitHub CI, they appeared to work. Compiling embedded dependencies on Linux works as expected, if you don't attempt to build an embedded Assimp - but that issue was already present before (on recent GCCs, one header is missing a
#include <cstdint>, which is a one-line fix).Overview of changes
Each commit has its own description on what changed and why that change is required in my opinion. But as a quick overview:
x86_64was properly supported, now there is very solidarm/arm64, expandedx86_64and rudimentaryppcsupport./src). Now the user can overwrite all variables properly by passing them on the CMake CLI (backwards-comparible).Accelerate.framework, which Ceres can use instead of Suitesparse. This lifts the requirement for a Fortran compiler on macOS and drastically reduces compilation times and bundle size. When running the unit tests I did not notice any performance differences.-arch <ARCH>to the compiler, the project fully supports cross-compiling fromarm64tox86_64on macOS - including all required dependencies. The target architecture can be set by specifyingCMAKE_OSX_ARCHITECTURES=<arm64|x86_64>on the CMake CLI. Note that compiling universal binaries is not supported at this point, because some dependencies do not have the required logic to do this in one step.INSTALL_macOS.mdprovides specific instructions on how to compile this project successfully on macOS.AppleClangis now supported - this requires that OpenMP is installed as an external dependency (enabled by default on macOS), as the Apple SDK does not ship it. Furthermore, the project now uses the unified CMake targetsOpenMP::OpenMP_CandOpenMP::OpenMP_CXXrespectively instead of setting the required flags manually.Open questions
Drawbacks
Because AliceVision has so many dependencies - some with very specific configurations - I think we cannot officially support building the project with package-manager provided dependencies. I just stumbled from roadblock to roadblock when attempting to standardize the required dependencies for the different package managers. So currently the only option is to build the project with
ALICEVISION_BUILD_DEPENDENCIES=ON. However, this matches the recommendation for Linux, so I think this should be fine. If someone wants to attempt to build it with Homebrew/MacPorts/Nix anyway, that might or might not work. But at least we don't have the obligation to ensure compatibility for these cases.There is no CI yet. I decided against CI, because it currently requires to rebuild all dependencies over and over again - the GitHub runners usually only have 3 threads available, so this is quite slow. I tested pre-built dependencies, but that currently does not work because there are some dependencies hard-code absolute paths into their CMake config modules - making them not portable.
There are no unit tests yet. I ran the unit tests on my Mac, and two are currently failing:
acRansac_test.cpp: I think there is some undefined behavior here, which caused a segfault on macOS. See this gist for a detailed description and proposed solution.fundamental10PSolver_test.cpp: The check on line 184 fails. I have no idea why and I don't know anywhere near enough to solve this. Help is much appreciated. For the results see this gist.Future plans
This is the first step to make macOS a fully supported platform for this project again. The next steps would be to add the Apple Metal DepthMap backend and maybe generally abstract the DepthMap library to allow for multiple backends with a unified interface.
Also: Add a proper CI solution and ensure that all unit tests pass.
Feedback is appreciated!
These patches are the result of weeks of debugging, investigating and rebasing - but I am sure that there are still issues and edge cases that I completely missed. So fresh viewpoints and remarks are much appreciated. If anyone has the chance to test this out, please do so and let me know if there are any issues.