[macOS] Fix Metal RHI 3D viewer crashes and enable the viewer on Apple Silicon#3030
Conversation
|
This is AWESOME! I ran into all these issues and did some very dirty hacking to get a bare minimum of it working 😅. |
|
You're welcome, BTW I bundled the app and using daily 😄. |
| ambient: root.ambient | ||
| shininess: root.shininess | ||
| specular: root.specular | ||
| ambient: "#111" | ||
| shininess: 1.0 | ||
| specular: "#000" |
There was a problem hiding this comment.
What's the reason to change that?
There was a problem hiding this comment.
Actually the mesh preview is looking a bit phased off, like whiteish. I tried to make it look like full saturated colour but it didn't worked either and I was feeling very happy about the app is working, I directly go into PR.
Tldr; it just a mess I forgot to revert
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3030 +/- ##
===========================================
- Coverage 85.38% 85.33% -0.06%
===========================================
Files 73 73
Lines 11406 11414 +8
===========================================
+ Hits 9739 9740 +1
- Misses 1667 1674 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| @@ -196,12 +196,15 @@ def matchDescription(self, value, strict=True): | |||
| class GroupAttribute(Attribute): | |||
| """ A macro Attribute composed of several Attributes """ | |||
| @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") | |||
There was a problem hiding this comment.
| @deprecated.depreciateParam("group", "Param 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") | |
| @deprecated.depreciateParam("group", "Param argument 'group' on {name} should not be used anymore. Please use 'commandLineGroup' instead") | |
| @deprecated.depreciateParam("groupDesc", "GroupAttribute argument 'groupDesc' on {name} should not be used anymore. Please use 'items' instead") |
|
@NeverGET Could you rebase this PR onto the develop branch (instead of a merge)? |
Will do. I think I have a valid AdaptiveCpp build now and will report any issues asap. I'll be on a train ride tonight, so maybe I can take it for a spin then. |
|
I just went ahead and did the testing: (+): It does not crash anymore, models and images are loaded correctly and (afaict) the 3D Viewer now works correctly on macOS (🚀)1 (-): I don't know what to think about the
Everything else LGTM. But I would really prefer to not use/introduce more environment variables but instead try to reduce them in general. |
4097642 to
a6d4304
Compare
|
Hi @fabiencastan @philippremy — apologies for the long silence on this one. The past couple of months turned out to be unexpectedly busy and I only now got the time to come back and properly address the review. Thanks a lot for the detailed feedback and for taking the time to test it on macOS. I've pushed an updated branch that addresses every point:
What remains is the macOS RHI/shader work — Metal normal fixes, OpenGL fallback techniques, the cgroup fix, and the Qt plugin-path existence guards. I re-tested locally on macOS (Apple Silicon, Metal RHI): the full pipeline recomputes end to end and the 3D viewer works. Let me know if anything else needs adjusting. |
|
Hello, Thanks for your contribution.
Thanks ! |
a6d4304 to
d62e860
Compare
|
Hi @servantftransperfect — thanks, both points are fair. cgroup: you're right, I had the semantics backwards. Looking at the For context on why I touched that file in the first place: while debugging the macOS crashes I also noticed Meshroom wasn't using all CPU threads on my machine, and I tried a few things to nudge it — that change was one of those experiments and ended up being a leftover. With it dropped, AliceVision's own detection takes over, which is what should happen. OpenGL fallback shaders: no real-world use case. The actual macOS Metal fix is in the modified Branch is force-pushed; the PR now has 6 focused commits. |
|
I am not sure to know why, but without your PR, the bounding box and the widgets were correctly lighted using the directional light (in the 3D viewer). Now using your PR, as you explicitely added wrong normals, the lighting is weird. I am not sure we wanted (in the first place) those widgets to be affected by the lighting. Would you have time finding a solution to make them unlit ? I can see in the doc there is a PerVertexColorMaterial which requires to replace normals with color. |
d62e860 to
fb8c845
Compare
|
Hi @servantftransperfect — thanks for the pointer, agreed the uniform
It looks like Qt 6.8's RHI shader generation for What I landed instead — same intent, different lever: Keep PhongMaterial {
ambient: "#FFF" // grid color, unchanged
diffuse: "#000"
specular: "#000"
shininess: 0
}With Net effect: the lighting weirdness you flagged is gone, and the macOS Metal path stays on shader/material combinations Qt is happy with. Force-pushed; the PR's second commit is now "Add normals and neutralize lighting on widget geometries" and reflects this. Separate observation (not part of this PR): there's a pre-existing Qt3D-on-Metal crash when the user resizes the 3D viewer. The remaining |
|
Please remove your commit about groupDesc.
|
Metal RHI strictly validates that vertex shader inputs match the vertex descriptor provided by the geometry. The SphericalHarmonics shaders declared a vertexNormal input, but custom Qt3D geometries (Grid3D, BoundingBox, Locator3D) and some loaded meshes don't provide normal attributes, causing Metal pipeline creation to fail with a crash. Instead of requiring normals as a vertex attribute, compute flat face normals in the fragment shader using dFdx/dFdy screen-space derivatives of the world position. This produces correct lighting for flat-shaded geometry and works regardless of whether the mesh provides normals. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Built-in Qt3D materials (PhongMaterial, DiffuseSpecularMaterial) expect a vertexNormal attribute in the vertex descriptor. On Metal RHI, if a geometry lacks this attribute, the render pipeline state creation fails because Metal strictly validates that all shader inputs are satisfied by the vertex descriptor. Add default upward-facing (0, 1, 0) normal attributes to: - Grid3D.qml (dynamic count matching grid vertices) - BoundingBox.qml edges (24 vertices) - Locator3D.qml (6 vertices) A uniform normal would otherwise drive a noticeable lighting artefact on these line geometries when consumed by Phong's diffuse/specular terms. Neutralize the lighting equation by zeroing diffuse and specular on the PhongMaterial used by Grid3D and BoundingBox edges; the Phong sum then collapses to its ambient term, giving a flat unlit appearance with the normals serving purely as Metal descriptor plumbing. Locator3D already uses PerVertexColorMaterial, so its material is unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Meshes loaded via SceneLoader (OBJ, PLY, etc.) may lack normal attributes. When these meshes are rendered with built-in Qt3D materials that require vertexNormal, Metal RHI crashes due to missing vertex descriptor entries. Add Scene3DHelper.ensureNormals(entity) that traverses all QGeometry children of a loaded entity and adds default (0, 1, 0) normal attributes to any geometry missing them. Call this method in MediaLoader's sceneLoaderPostProcess before material setup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add PySide6's bundled QML modules directory (PySide6/Qt/qml) to the QML engine import path, guarded by an existence check. This lets the QML engine locate the Qt3D and QtQuick.Scene3D modules the 3D viewer depends on when running against a PySide6 wheel (notably on macOS); it is a no-op where the directory is absent, so other platforms are unaffected. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Guard QT_PLUGIN_PATH / QML2_IMPORT_PATH setup behind directory-existence checks: on macOS with PySide6 these directories may not exist, and adding invalid paths causes plugin loading failures. - On macOS, do not set a dynamic-linker path variable: AliceVision libraries are resolved through the rpaths embedded in the bundle (LD_LIBRARY_PATH is Linux-only and nothing analogous is needed on macOS). - Default the Qt RHI backend to Metal on macOS (QSG_RHI_BACKEND=metal) via setdefault so it stays overridable, keeping OpenGL as the default elsewhere. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fb8c845 to
0c2672e
Compare
|
Hi @servantftransperfect, @philippremy, @fabiencastan — thanks for the thorough review and the patience on this one. I've pushed an update that addresses everything raised and tightens the scope: Review points
Housekeeping
Companion PR This PR now focuses purely on the Meshroom-side Metal RHI viewer fixes — everything additive and platform-guarded — and I re-tested the full SfM → DepthMap → Meshing → Texturing pipeline plus the 3D viewer on Apple Silicon. Happy to adjust anything further. Thanks again! |
servantftransperfect
left a comment
There was a problem hiding this comment.
Thank you for this PR !
37993f4
into
alicevision:develop
Description
Adds macOS support for Meshroom's Qt 3D viewer on the Metal RHI backend. On macOS the Qt Quick scene graph runs on Metal, whose pipeline-state creation strictly validates that every vertex-shader input is satisfied by the geometry's vertex descriptor. Several viewer geometries provide only
vertexPosition, while the built-in Qt3D materials they use declarevertexNormal— so on Metal the pipeline fails to build and the app crashes. OpenGL tolerates this, which is why it only surfaces on macOS.This PR makes the viewer build valid Metal pipelines with no behavioral change on other platforms, plus the macOS-specific environment setup needed to run against a PySide6 wheel. It builds on the now-merged macOS support in AliceVision (alicevision/AliceVision#2019). The 3D SfM/camera/depth overlays additionally need a small companion fix in QtAliceVision (plugin rpaths + descriptor normals) — see the linked QtAliceVision PR.
Features list
vertexNormalinput; reconstruct a flat normal fromdFdx/dFdyof world position in the fragment shader.diffuse/specular= black) so the constant normal can't produce wrong shading; widgets render unlit, as before.Scene3DHelper.ensureNormals()adds default normals to OBJ/PLY geometries lacking them, so built-in lit materials build valid pipelines.Implementation remarks
All changes are additive and platform-guarded — Linux/Windows are unaffected (the shader normal reconstruction is valid on every backend; the env changes are macOS-only).
Addresses the earlier review: the
DYLD_FALLBACK_LIBRARY_PATH/AV_BUNDLEhandling, thecgroupchange, the speculative OpenGL fallback techniques, and the out-of-scopegroupDesccommit have all been removed; the branch is rebased onto currentdevelop.Tested on macOS / Apple Silicon (PySide6 6.10.2, Qt 6.10.2, Metal RHI) against a clean upstream AliceVision
developbuild: the 3D viewer opens and resizes without crashing, and SfM points + cameras, the textured mesh (EXR), and per-view depth maps all render; the full SfM → DepthMap → Meshing → Texturing pipeline runs.Thanks to @philippremy for the AliceVision macOS support (#2019) and to the @alicevision team.