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 :). |
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. |
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
CUDA has long been removed and even longer been deprecated on Apple platforms. Guard any CUDA functionality behind cmake_dependent_option so it never gets used on Apple platforms. Signed-off-by: Philipp Remy <[email protected]>
Introduces a CMake function called av_conditional_option, which allows (compared to cmake_dependent_option) for the user to overwrite it easily and does not hide it from the TUI/GUI. Signed-off-by: Philipp Remy <[email protected]>
This is mainly useful for Apple targets (and therefore only enabled for Apple targets by default) where the system compiler (aka AppleClang) supports OpenMP but does not ship the required library and headers. Explicitly pass the required CMake flags to CMake for projects that depend on OpenMP. Signed-off-by: Philipp Remy <[email protected]>
PopSIFT has a hard dependency on CUDA being available and therefore cannot be used on any Apple platforms. Signed-off-by: Philipp Remy <[email protected]>
…e targets On Apple targets, optimized (=faster) implementations of BLAS/LAPACK and Sparse Solvers (since 2017) are available in Accelerate.framework (a system-provided Framework existent on every machine by default). BLAS and LAPACK are used by multiple dependencies, each of them being capable of handling Apple's Accelerate.framework. SuiteSparse is *one* possible backend for Sparse Solvers in Ceres, but it can use 'AccelerateSparse' (= Accelerate.framework) as an alternative. During testing, no performance difference was noticable. This relieves Apple targets from needing to have a Fortran compiler installed and reduces final bundle size by ~160MB (GraphBLAS in SuiteSparse is *huge*). Signed-off-by: Philipp Remy <[email protected]>
…res on Apple targets Ceres can utilize the system Accelerate.framework instead of using SuiteSparse to enable fast Sparse Solvers. Therefore, don't require SuiteSparse on Apple platforms. Signed-off-by: Philipp Remy <[email protected]>
…edded Geogram As we can now safely rely on CMAKE_OSX_ARCHITECTURES when determining which architecture to build on Apple targets, use CMAKE_OSX_ARCHITECTURES instead of CMAKE_SYSTEM_PROCESSOR when setting the Vorpaline Platforms. This enables cross-compiling Geogram on Apple targets from x86_64 to arm64 and vice-versa. Signed-off-by: Philipp Remy <[email protected]>
This commit introduces two changes: (1) Bumps the version of libPNG from 1.6.39 to 1.6.50 (appears to be ABI and API compatible), as 1.6.39 requires a non-existing fp16.h header on macOS. (2) Sets DPNG_ARM_NEON to ON, if an ARM processor is used. NEON is broadly supported on reasonably modern ARM CPUs. Signed-off-by: Philipp Remy <[email protected]>
Bump version of libVPX from v1.13.0 to v1.15.2 (appears to be ABI and API compatible), because v1.13.0 uses Linux specific linker flags on Apple targets. Signed-off-by: Philipp Remy <[email protected]>
…SX Architecture This patch automatically selects the correct ONNXRuntime binary, based on CMAKE_OSX_ARCHITECTURES (which is used throughout the whole CMake infrastructure) and allows for cross-compiling macOS for x86_64 and vice-versa. Signed-off-by: Philipp Remy <[email protected]>
XercesC is a required hard dependency of libE57Format it cannot build as an embedded project on its own. Allow the user to rely on building a XercesC library as a dependency. Because it is *only* required by libE57Format, make it an av_conditional_option() based on whether libE57Format should be build. Signed-off-by: Philipp Remy <[email protected]>
Bump version of pcl from v1.13.0 to v1.15.1 (appears to be API and ABI compatible). This fixes compilation errors on Apple targets. Signed-off-by: Philipp Remy <[email protected]>
AliceVision does not use SWIG functionality with regular expression matching. Signed-off-by: Philipp Remy <[email protected]>
If ALICEVISION_USE_RPATH is set, tell libVPX to set its install name directory to @rpath using environment LDFLAGS, as the autoconf script does not have a native switch for that. This commit fixes a build logic issue in libVPX, where the configure script is only able to detect Darwin up to version 24.X.X (macOS Sonoma). macOS 26 Tahoe is Darwin 25.X.X, causing the configure script to fall back to generic-gnu, which assumes a Linux host OS. As a workaround in CMake, we detect (using CMAKE_SYSTEM_VERSION) if the host Darwin is > 2. In that case, we explicitly set the newest available toolchain for libVPX, which logically is [x86_64/arm64]-darwin24-gcc. FIXME: To be removed when libVPX either changes its detection logic or a newer ABI- and API-compatible version with proper Darwin > 24 support is released. Signed-off-by: Philipp Remy <[email protected]>
Bump Boost to 1.90.0 (the latest at the time of writing) and switch from b2 to the CMake build system to integrate better with AliceVision's dependency builder orchestrator. Some libraries, which b2 includes by default, are no longer installed and cause CCTag and AliceVision to use non-existent header files. Add these manually to the list. Signed-off-by: Philipp Remy <[email protected]>
As for other dependencies, do not build the tests for Ceres (as they are not needed and not tested). Besides from saving compilation time, this allows for cross-compiling Ceres on macOS. Otherwise, Ceres will build an embedded glog but does not pass through CMAKE_OSX_ARCHITECTURES, causing linking errors because of mismatched architectures. Signed-off-by: Philipp Remy <[email protected]>
If the user explicitly disables rpath handling when building AliceVision by setting ALICEVISION_USE_RPATH=OFF, respect this in the embedded dependencies. This mainly fixes some older dependencies, where this is not set to ON by default and therefore results in a library with absolute install names (which drastically reduces portability of the resulting binary). Signed-off-by: Philipp Remy <[email protected]>
Explicitly pass through top-level CMake variables when building AliceVision as an external project in Dependencies.cmake. This change allows for the following configure step to respect any choices the user made at the top level. FIXME: We should ideally only have one place to set the options - the very top-level CMakeLists.txt. But that would be a breaking change, as we would need to remove all "AV_" prefixed variables and replace them with the src/CMakeLists.txt "ALICEVISION_" prefix style. Signed-off-by: Philipp Remy <[email protected]>
aliceVision_selectConnectedViews linked to aliceVision_depthMap although it does not use the library. This caused a linker issue on macOS, as the target was not guarded behind a ALICEVISION_HAVE_CUDA check. Remove the unnecessary link completely. Signed-off-by: Philipp Remy <[email protected]>
Adapt the install rpaths on Apple targets to accommodate for the typical library paths. This includes the standard Unix paths (lib, next-to-loading-binary) and the Apple bundle structures (Libraries, Frameworks). Add every possibility for @loader_path (for dynamic libraries) and @executable_path (for executables) to ensure proper lookup based on the context. Signed-off-by: Philipp Remy <[email protected]>
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 <[email protected]>
…rpath If ALICEVISION_USE_RPATH is set, explicitly set the install name directory for the ffmpeg libraries to @rpath using the available autoconf CLI argument. Signed-off-by: Philipp Remy <[email protected]>
…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 <[email protected]>
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 <[email protected]>
…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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Respect the user's choice to use static libraries for the dependencies Signed-off-by: Philipp Remy <[email protected]>
Fixes some configuration errors introduced by rebasing onto the new CMake infrastructure. Signed-off-by: Philipp Remy <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
c35036d to
553d389
Compare
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 <[email protected]>
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 <[email protected]>
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 <[email protected]>
Simplify some of the functions in darwin_bundle.py. DISCLAIMER: This was done using AI assistance. Signed-off-by: Philipp Remy <[email protected]>
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.