build: macOS (arm64) support, CI lane, and PCL loader validation - #34
Merged
Merged
Conversation
rxdu
force-pushed
the
build/macos-support
branch
from
September 6, 2026 14:49
bf29618 to
95f15b9
Compare
Three Linux assumptions kept quickviz from compiling on macOS. None are fundamental -- the viewer's GL usage is already portable (it requests a 3.2 core + forward-compatible context on Apple, and the shaders are #version 330, under the 4.1 cap macOS enforces). - object_feedback_handler.hpp included <GL/gl.h> unguarded, unlike mesh.cpp and opengl_capability_checker.cpp which already select glad. macOS ships OpenGL as a framework and has no GL/ directory, so this was a hard compile error. Guarded the same way, and taught the existing non-glad branches to use <OpenGL/...> on Apple so VIEWER_WITH_GLAD=OFF works there too. - glad.h includes <KHR/khrplatform.h>, but the vendored copy sat in include/glad/. On Linux this resolved only because Mesa happens to install /usr/include/KHR/khrplatform.h -- an undeclared dependency on a system package. Moved it to include/KHR/, glad's own standard layout, so the vendored copy is used on every platform. - glm was never referenced by CMake at all; it resolved implicitly because Ubuntu's libglm-dev installs into /usr/include. Added find_package(glm) so other prefixes work, and defined GLM_ENABLE_EXPERIMENTAL, which glm >= 1.0 requires for the GTX extensions the renderables use. Linux behaviour is unchanged; the KHR move makes it depend on less. Verified: builds clean on macOS 15 (arm64) with no external shims and no manual compile flags. The GUI itself is still unexercised -- quickviz has no automated suite and the samples need a display.
The lane itself is AppleClang + libc++ on macos-14 with BUILD_TESTING=ON. Two build-plumbing problems had to be fixed before any test could run; neither is macOS-specific, both were latent. - gtest_discover_tests enumerates cases by RUNNING each test binary at BUILD time, with a 5 s timeout. These binaries link GL, cairo and optionally OpenCV/PCL, and where those ship as many small shared libraries the dynamic loader alone exceeds that. Discovery then times out and CMake DELETES the executable, which surfaces much later as a baffling <target>_NOT_BUILT ctest failure. Switched to PRE_TEST discovery, which removes the build-time dependency on process startup. - OpenCV and PCL export their whole dependency prefix onto the include path. Where a system googletest exists there, its headers were found ahead of the vendored copy while the vendored libgtest.a still linked -- an ABI mismatch appearing only as undefined testing::internal symbols. The vendored include dirs are now prepended. KNOWN FAILURE on this lane: PCLLoaderTest.InvalidFileError. PCL 1.15 (what Homebrew ships) parses a garbage .pcd into a zero-point cloud and returns success, so LoadPCDInternal's `== -1` check never fires and no CorruptedFileException is thrown. That is a loader robustness gap, not a platform issue, and it will reach the Ubuntu lanes when they move past PCL 1.14. Left for a separate decision rather than changing point-cloud loading semantics here. Otherwise verified on macOS 15 (arm64): 119/121 pass, the second failure being the aggregate target that reruns the first.
LoadPCDInternal/LoadPLYInternal treated a load as successful whenever PCL did not return -1. PCL's readers do not reliably signal a malformed file that way: given arbitrary non-point-cloud data, PCL 1.15 logs "number of points is zero", defaults HEIGHT, reports that x/y/z were not matched, and returns 0. Garbage therefore loaded as an empty cloud and CorruptedFileException was never thrown. The header is the reliable signal. Every PCD/PLY readable into a PCL point type declares x, y and z, so their absence means the file is not a point cloud. Crucially this stays true for a legitimately EMPTY cloud -- POINTS 0 with a full FIELDS line -- which must keep loading, so the check keys on the header rather than on the point count. Applied to all four load paths: the LoadToPCL templates and the renderer-facing LoadPCD/LoadPLY, which already had the fields in hand. AnalyzeFields is deliberately untouched: it answers "what fields does this file have", and all-false is a legitimate answer to that question. Tests: InvalidPLYFileError closes the gap where the fixture built a garbage .ply that nothing asserted on, and EmptyButValidPCDLoads pins the distinction the fix depends on -- a malformed file and a valid empty one both yield zero points, so only the header separates them. Not a macOS-specific fix. Ubuntu 22.04/24.04 ship PCL 1.12/1.14 and have not hit it yet; they will on the next PCL bump. Verified on macOS 15 (arm64): 123/123 tests pass, up from 121 with 2 failing.
- ThreadSafeQueueTest.PopTimeout asserted that a 50 ms wait returns within
100 ms. That is a claim about how promptly the OS reschedules a blocked
thread, not about the queue, and a loaded runner overshoots it -- CI
measured 143 ms. The lower bound is the real contract and is unchanged;
the upper bound now has a wide margin and guards only against waiting
forever. This was latent on Linux too.
- Benchmarks are excluded from the macOS lane. They open a real GL window
and the hosted macOS runner is headless. Unlike Linux, GLFW initialises
fine there (Cocoa is always present) and fails only later at window
creation ("NSGL: Failed to find a suitable pixel format"), so the process
aborts rather than degrading. The correctness suites already skip GUI
tests on their own. Linux keeps running benchmarks.
Verified on macOS 15 (arm64): 123/123 tests pass with -LE benchmark.
rxdu
force-pushed
the
build/macos-support
branch
from
September 6, 2026 15:00
95f15b9 to
680c0b2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes quickviz build and test on macOS, and fixes several latent issues found along the way. Most are not macOS-specific — Linux masked them.
Portability fixes
object_feedback_handler.hppincluded<GL/gl.h>unguarded, unlikemesh.cppandopengl_capability_checker.cppwhich already select glad. macOS ships OpenGL as a framework and has noGL/directory, so this was a hard compile error. Guarded the same way, and taught the existing non-glad branches to use<OpenGL/...>on Apple soVIEWER_WITH_GLAD=OFFworks there too.khrplatform.hsat ininclude/glad/, so<KHR/khrplatform.h>resolved on Linux only because Mesa happens to install/usr/include/KHR/khrplatform.h— an undeclared dependency on a system package. Moved toinclude/KHR/, glad's own standard layout, so the vendored copy is used everywhere. This makes the Linux build depend on less.libglm-devinstalls into/usr/include. Addedfind_package(glm)andGLM_ENABLE_EXPERIMENTAL, which glm >= 1.0 requires for the GTX extensions the renderables use.Build plumbing (both latent, neither macOS-specific)
gtest_discover_testsenumerates cases by running each test binary at build time, with a 5 s timeout. These binaries link GL, cairo and optionally OpenCV/PCL, and where those ship as many small shared libraries the dynamic loader alone exceeds that. Discovery then times out and CMake deletes the executable, surfacing much later as a baffling<target>_NOT_BUILTctest failure. Switched toPRE_TESTdiscovery.libgtest.astill linked — an ABI mismatch appearing only as undefinedtesting::internalsymbols. The vendored include dirs are now prepended.PCL loader: validate the header, not just the return code
LoadPCDInternal/LoadPLYInternaltreated a load as successful whenever PCL did not return-1. PCL does not honour that: given garbage, 1.15 logs "number of points is zero", defaults HEIGHT, reports x/y/z unmatched, and returns 0. Non-point-cloud data loaded silently as an empty cloud.The header is the reliable signal, and the check keys on it rather than the point count for a specific reason: a malformed file and a legitimately empty one both yield zero points, so rejecting on emptiness would break valid
POINTS 0clouds. Every PCD/PLY readable into a PCL point type declares x/y/z, and that stays true when the cloud is empty.Applied to all four load paths.
AnalyzeFieldsis deliberately untouched — it answers "what fields does this file have", and all-false is a legitimate answer.Two tests:
InvalidPLYFileError(the fixture built a garbage.plythat nothing asserted on) andEmptyButValidPCDLoads, which pins the distinction the fix rests on so it cannot later be "simplified" into a point-count check.Ubuntu 22.04/24.04 ship PCL 1.12/1.14 and have not hit this yet. They will on the next PCL bump.
CI lane
macos-14(arm64), AppleClang + libc++,BUILD_TESTING=ON. The GUI itself is not exercised — there is no automated suite for it and the samples need a display, so only compilation and the non-GUI tests are covered.Verification
macOS 15 (arm64): 123/123 tests pass.
Known, pre-existing and untouched: the
PCLLoaderTestfixture shares a temp directory, so it races underctest -j(9 failures parallel vs 0 serial). Both this lane and the existing Ubuntu lanes run ctest serially, so it is latent on Linux too.