macOS: enable realsense-viewer (ImGui GLSL 120) + relocatable install RPATH

[keskinonur]

Description

Enable realsense-viewer (and related GUI tools) on Apple Silicon / macOS, where the build was previously gated with if(NOT APPLE).

Root cause of the historical breakage was not the viewer itself, but ImGui's OpenGL3 backend defaulting to #version 150 (GL 3.2 core) while GLFW windows are created without version hints, so macOS provides a legacy 2.1 compatibility context. Pass #version 120 on Apple so shaders match the context.

Also:

• Ignore degenerate saved window sizes (e.g. 0×0 after a headless/session-less run) so later launches are not invisible.
• Apple install RPATH: use @loader_path / @loader_path/../lib and never bake absolute build-tree paths into installed tools.
• Force GLSL rendering/processing off on Apple (core 3.2 cannot be mixed with the fixed-function 2D path); use the existing fixed-function pointcloud path; fix texcoord-before-vertex order there.

Motivation

Verified on macOS arm64 with an Ethernet RealSense D555 (DDS): viewer launches from an install prefix without DYLD_LIBRARY_PATH, enumerates the device, streams depth/color/IR, and exposes Motion Module. rs-depth-quality and rs-rosbag-inspector build and install alongside.

Changes

Area	Change
tools/CMakeLists.txt	Drop if(NOT APPLE) around realsense-viewer
common/ux-window.cpp	Apple ImGui #version 120; force GLSL flags off; ignore <320×240 saved size
tools/rosbag-inspector/…	Same ImGui GLSL pin
CMake/unix_config.cmake + tools/tools_common.cmake	Relocatable install RPATH on Apple
src/gl/pc-shader.cpp	Fixed-function pointcloud: texcoord order + enable texture
common/viewer.cpp	Clear sticky GL errors before 3D; safe aspect for gluPerspective
Python / frame API (same commits)	Bind combined_motion.orientation; throw if get_motion_data() used on combined frames

Testing

• macOS arm64 build with BUILD_GRAPHICAL_EXAMPLES=ON
• realsense-viewer runs from install prefix; otool RPATH is only @loader_path + @loader_path/../lib (no build-tree absolute path)
• Live D555 over DDS: depth stream visible; device info correct
• Python smoke: stream + combined IMU

Notes

• Full modern GLSL 3D (shader version 130 / float FBOs) still requires a core 3.2 context and is intentionally not enabled on Apple in this PR.
• Separate from the Fast-DDS macOS multicast SO_REUSEPORT work (Set SO_REUSEPORT on multicast input sockets for Apple platforms eProsima/Fast-DDS#6471).

[sysrsbuild-gh-agentic]

Automated Code Review by rs-agentic bot

This PR enables realsense-viewer and GUI tools on Apple Silicon / macOS. The root cause diagnosis (ImGui OpenGL3 backend defaulting to #version 150 on a GL 2.1 legacy context) is correct, and the approach is sound. Several issues noted below require attention before merging.

Summary of findings:

• BUG: glEnable(GL_TEXTURE_2D) is added in pc-shader.cpp but never matched with glDisable(GL_TEXTURE_2D) after draw calls -- can corrupt subsequent render state.
• BUG: The exception in rs_frame.hpp references get_combined_motion_data() but it is not clear this method is publicly exposed; verify it exists and is documented.
• Enhancement: GLSL flags are force-disabled on Apple via set() (not just set_default()), permanently preventing future macOS users from enabling GLSL even on compatible hardware.
• Enhancement: Magic numbers 320/240 in window size guard should be named constants.
• Enhancement: double-to-float cast in the Python orientation getter loses precision silently.
• Note: CMAKE_BUILD_RPATH set globally to only @loader_path in unix_config.cmake may be too restrictive for non-tool targets during build.
• Note: BUILD_RPATH in tools_common.cmake includes absolute build-tree paths, which contradicts the comment above it (though technically harmless since it only affects build-tree binaries, not installed ones).
• Missing tests: No unit/integration tests for the new Python combined_motion.orientation binding.

[sysrsbuild-gh-agentic]

[rs-agentic bot] BUG: Missing glDisable(GL_TEXTURE_2D) after draw calls

glEnable(GL_TEXTURE_2D) is added here to enable texturing for the fixed-function path, but there is no corresponding glDisable(GL_TEXTURE_2D) call after the glEnd() blocks (for both the QUADS and POINTS paths). This leaves texture state globally enabled.

On macOS (and any path where GLSL is off), subsequent rendering passes that do not bind a valid texture will sample garbage, causing visual artifacts or black geometry. Add glDisable(GL_TEXTURE_2D) after each glEnd() block that follows this glEnable, or pair the enable/disable around the entire fill/points conditional block.

glEnable(GL_TEXTURE_2D);
// ... QUADS and POINTS draw calls ...
glDisable(GL_TEXTURE_2D);  // <-- add this

[sysrsbuild-gh-agentic]

[rs-agentic bot] BUG + Breaking API Change: Verify get_combined_motion_data() exists and document the breaking change

Two concerns here:

1. Missing alternative method: The exception message tells users to call get_combined_motion_data(), but this method does not appear in the public C++ API of motion_frame visible in this diff. If it does not exist, this exception leaves users with no valid code path at all. Please verify the method is present and accessible (or add it in this PR).

2. Breaking API change: Previously, get_motion_data() on a COMBINED_MOTION frame silently returned garbage floats. Now it throws std::logic_error. Any existing user code that called get_motion_data() on combined frames will now crash at runtime. This is a breaking change and should be explicitly called out in the PR description and ideally in CHANGELOG.md or release notes.

[sysrsbuild-gh-agentic]

[rs-agentic bot] Enhancement: Force-disabling GLSL prevents future user override

Using config_file::instance().set(...) (not just set_default(...)) permanently overwrites any user-configured GLSL preference on Apple. If a future macOS version or a user's system does support a proper OpenGL 3.2 core context (e.g., via explicit GLFW hints), they will have no way to enable GLSL without modifying source code.

Consider instead detecting the actual OpenGL context version at runtime, e.g.:

#ifdef __APPLE__
    // Force GLSL off: macOS legacy 2.1 context cannot support GLSL 1.30 shaders.
    // Only override saved config, not a runtime capability check.
    const int gl_major = ...; // glGetIntegerv(GL_MAJOR_VERSION, ...)
    if (gl_major < 3) {
        config_file::instance().set(configurations::performance::glsl_for_processing, false);
        config_file::instance().set(configurations::performance::glsl_for_rendering, false);
    }
#endif

Or at minimum add a comment explaining that this is intentionally permanent until the codebase supports core 3.2 contexts on Apple.

[sysrsbuild-gh-agentic]

[rs-agentic bot] Enhancement: Magic numbers 320/240 should be named constants

The minimum window size thresholds 320 and 240 are magic numbers. Per the project's code conventions, these should be defined as named constants to improve readability and maintainability:

constexpr int MIN_PLAUSIBLE_WINDOW_WIDTH  = 320;
constexpr int MIN_PLAUSIBLE_WINDOW_HEIGHT = 240;

if (w >= MIN_PLAUSIBLE_WINDOW_WIDTH && h >= MIN_PLAUSIBLE_WINDOW_HEIGHT)

This also makes it easier to find and adjust the threshold if the minimum supported display resolution changes.

[sysrsbuild-gh-agentic]

[rs-agentic bot] Note: Global CMAKE_BUILD_RPATH "@loader_path" may be too narrow

Setting CMAKE_BUILD_RPATH globally to only "@loader_path" means all Apple targets (not just tools) will have only the loader's directory in the build-tree RPATH. Libraries built into subdirectories of ${CMAKE_BINARY_DIR} (e.g. ${CMAKE_BINARY_DIR}/Release/, ${CMAKE_BINARY_DIR}/lib/) may not be found at link time during the build.

tools_common.cmake overrides this per-target with absolute build paths for tools, but other target types (examples, unit tests, Python bindings) that do not go through tools_target_config will only have @loader_path. Please verify that examples (BUILD_EXAMPLES) and Python bindings (BUILD_PYTHON_BINDINGS) still build and run correctly from the build tree on Apple with just @loader_path.

[sysrsbuild-gh-agentic]

[rs-agentic bot] Note: BUILD_RPATH contains absolute build-tree paths (comment is misleading)

The comment above says "never bake absolute build-tree paths", but BUILD_RPATH here explicitly includes ${CMAKE_BINARY_DIR}/Release and ${CMAKE_BINARY_DIR} -- both absolute paths. While BUILD_RPATH only affects binaries in the build tree (not installed binaries), the comment is misleading.

Please either:

1. Clarify the comment to distinguish BUILD_RPATH (build-tree only, absolute paths acceptable) from INSTALL_RPATH (installed binaries, must be relocatable), or
2. Remove the absolute paths from BUILD_RPATH if they are not needed (since @loader_path plus CMake's own link path resolution should suffice during build).

As written, otool -L on a build-tree binary will show both @loader_path AND the absolute build paths -- not a bug, but it is inconsistent with the stated goal.

[sysrsbuild-gh-agentic]

[rs-agentic bot] Enhancement: Double-to-float precision loss in orientation getter

rs2_combined_motion::orientation fields are stored as double (full quaternion double precision for IMU fusion accuracy), but this getter casts them to float via rs2_quaternion. This silently loses ~7 decimal digits of precision.

For orientation data used in sensor fusion or dead reckoning, this loss can accumulate over time. Consider one of:

1. Expose the orientation as a Python tuple of doubles directly: return py::make_tuple(self.orientation.x, self.orientation.y, self.orientation.z, self.orientation.w);
2. Or at minimum document the precision loss in a Python docstring so users are aware.

The setter path (float -> double) has no precision loss and is fine.

[sysrsbuild-gh-agentic]

[rs-agentic bot] Missing Tests

No automated tests are added in this PR for any of the changed areas. In particular:

• Python binding (wrappers/python/c_files.cpp): The new combined_motion.orientation property has no Python test. Per project testing guidelines (.github/skills/testing.md), new API additions should have test coverage in unit-tests/. Please add a test that:

  1. Creates or retrieves a combined_motion frame with known orientation data
  2. Verifies that .orientation returns the expected x, y, z, w values
  3. Verifies that calling get_motion_data() on a COMBINED_MOTION frame raises RuntimeError (the Python exception wrapping std::logic_error)

• macOS viewer smoke test: Although manual testing was done on arm64, a CI-runnable check (e.g., a headless launch with --no-device flag or similar) would prevent regressions on future macOS CI runs.

• Window restore guard (common/ux-window.cpp): No test for the 0x0 saved-size protection. Consider adding a unit test that persists a degenerate window size (e.g., 0x0, 100x50) to config and verifies the viewer does not attempt to restore it.

[ashrafk93]

@keskinonur
Thanks for the contribution! I tried this on macOS (Apple Silicon) with a USB D455. The viewer builds and the window opens, but I can't get a USB camera to show up:

With sudo (needed for libusb USB access), it crashes at startup — root can't reach the WindowServer: Cocoa: Failed to find service port for display → map::at: key not found.
Without sudo, the GUI opens but it's "Add Source (0 available)" — libusb can't open the device without root.
Did you test only with a DDS device (the D555)? DDS goes over the network and doesn't need root, so a non-sudo viewer would work fine there which would explain why it works for you but not for a USB camera.