[macOS] Enable QML / image-IO plugins to load on Apple Silicon#98
[macOS] Enable QML / image-IO plugins to load on Apple Silicon#98NeverGET wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces macOS-specific fixes for CMake configurations and Qt3D rendering. It explicitly requests Qt6GuiPrivate and configures RPATH usage on macOS to prevent plugin loading failures. Additionally, it adds placeholder normals buffers to CameraLocatorEntity and PointCloudEntity to prevent renderer crashes on the macOS Metal RHI backend. The review feedback highlights memory leaks in both entities due to QBuffer and QAttribute allocations lacking a parent QObject, and suggests parenting them to customGeometry to ensure proper cleanup.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| auto normalDataBuffer = new QBuffer; | ||
| normalDataBuffer->setData(normalData); | ||
| auto normalAttribute = new QAttribute; |
There was a problem hiding this comment.
The QBuffer and QAttribute instances are allocated on the heap using new but are not given a parent QObject (or QNode). In Qt3D, adding an attribute to a geometry does not transfer ownership, which leads to a memory leak when the entity or geometry is destroyed. Parenting them to customGeometry ensures they are properly cleaned up.
| auto normalDataBuffer = new QBuffer; | |
| normalDataBuffer->setData(normalData); | |
| auto normalAttribute = new QAttribute; | |
| auto normalDataBuffer = new QBuffer(customGeometry); | |
| normalDataBuffer->setData(normalData); | |
| auto normalAttribute = new QAttribute(customGeometry); |
| auto normalDataBuffer = new QBuffer; | ||
| QByteArray normalData(reinterpret_cast<const char*>(normals.data()), npoints * 3 * static_cast<int>(sizeof(float))); | ||
| normalDataBuffer->setData(normalData); | ||
| auto normalAttribute = new QAttribute; |
There was a problem hiding this comment.
The QBuffer and QAttribute instances are allocated on the heap without a parent QObject. Since PointCloudEntity::setData can be called multiple times to update the point cloud, this will leak memory on every update. Parenting them to customGeometry ensures they are automatically deleted when the geometry is destroyed or replaced.
auto normalDataBuffer = new QBuffer(customGeometry);
QByteArray normalData(reinterpret_cast<const char*>(normals.data()), npoints * 3 * static_cast<int>(sizeof(float)));
normalDataBuffer->setData(normalData);
auto normalAttribute = new QAttribute(customGeometry);|
Hi, thanks for the PR. Because I am generally more into the build system stuff, I'll only comment on that for now ^^. While setting Take a look at this file in the Meshroom repo (at least I usually use So my approach/recommendation would be to do the following:
Sorry for doing this nit-picking here, but I think we should make the whole thing somewhat more robust if we are doing this overhaul... |
|
Otherwise I agree with the AI that we are probably leaking memory big time, I just found the Qt documentation on ownership as a reference, so maybe look into that :D. |
- find_package(Qt6GuiPrivate): some Qt distributions (notably aqtinstall framework builds on macOS) do not auto-create the private-module targets via find_package(Qt6Gui), so Qt6::GuiPrivate is missing and configuration fails. Harmless where it is already provided. - Relocatable rpaths: the QML / imageformats plugins link AliceVision frameworks and their dependencies (e.g. libceres) via @rpath but installed no rpath of their own, so dyld fell back to system/Homebrew copies — notably a libceres lacking the glog symbols statically bundled into AliceVision's — and the plugins failed to load ("Symbol not found"). Set relocatable @loader_path rpaths matching a Meshroom standalone layout (plugins under qtPlugins/qml/<Module>/ and qtPlugins/imageformats/ sit beside aliceVision/lib), which keeps the binaries redistributable — no absolute or system-specific paths are baked in. No-op where rpath is unused.
The per-vertex-color material used for the SfM point cloud and camera locators declares a vertexNormal input. On the macOS Metal RHI backend, a geometry that omits an attribute its shader declares fails pipeline-state creation and crashes the renderer (e.g. on 3D viewer resize). These entities have no meaningful surface normal, so attach a constant placeholder to satisfy the vertex descriptor; the value is inert because the material is unlit (driven by vertex color), so rendering is unchanged on every platform. The buffer and attribute are parented to the geometry so Qt owns and frees them (no leak when the point cloud is rebuilt).
c5226fb to
d87e749
Compare
|
Hi @philippremy — thanks a lot for the detailed guidance; the references were exactly what I needed. Rpaths (redistributability): you were right that if(APPLE)
set(CMAKE_MACOSX_RPATH 1)
set(CMAKE_INSTALL_RPATH
"@loader_path/../../../aliceVision/lib" # qml/<Module>/ -> <root>/aliceVision/lib
"@loader_path/../../aliceVision/lib" # imageformats/ -> <root>/aliceVision/lib
"@loader_path"
"@loader_path/..")
endif()Your guess was spot-on — Memory leak: fixed — the placeholder normal One thing I noticed while in there: the existing Thanks again for the careful review! |
Description
Enables the QtAliceVision QML and image-IO plugins to load on macOS, so Meshroom's 3D viewer (SfM points, camera frustums, depth maps) and EXR image display work on Apple Silicon with the Metal RHI backend. Companion to the Meshroom macOS viewer PR (alicevision/Meshroom#3030) and built on AliceVision's macOS support (alicevision/AliceVision#2019).
Changes
CMAKE_INSTALL_RPATH_USE_LINK_PATHon Apple). The plugins link AliceVision frameworks and their dependencies (e.g.libceres) via@rpathbut previously installed no rpath, so dyld fell back to system/Homebrew copies — notably alibcereslacking the glog symbols statically bundled into AliceVision's — and the plugins failed to load with "Symbol not found". Baking the link-time library directories into each plugin's rpath resolves the bundled copies. No-op where rpath isn't used.vertexNormalinput; on Metal RHI a geometry that omits it fails pipeline-state creation and crashes the renderer (e.g. on viewer resize). A constant placeholder satisfies the descriptor; it is inert because the material is unlit (vertex-color driven), so rendering is unchanged everywhere.Qt6GuiPrivateexplicitly. Some Qt distributions (notably aqtinstall framework builds on macOS) don't auto-create the private-module targets viafind_package(Qt6Gui), soQt6::GuiPrivateis missing and configuration fails. Harmless where already provided.Notes
@philippremy — on alicevision/Meshroom#3030 you offered to submit the rpath fix ("a very simple PR"); this implements it via
CMAKE_INSTALL_RPATH_USE_LINK_PATH. Full credit to you for identifying that the right fix is build-time rpaths, not runtime env vars — happy to defer to your version or adjust.Validated on Apple Silicon: rebuilt and confirmed the
AliceVision,SfmDataEntity, andDepthMapEntitymodules load with noinstall_name_toolpatching and noDYLD_*overrides. One observation: with the current AliceVision install the baked rpaths also include/opt/homebrew/lib(AliceVision itself links some Homebrew transitive deps such asmetis); it is ordered after the AliceVision lib dir so it is harmless, and would not appear with a fully self-contained AliceVision build.