From 94dab87ea94091ededa0017460da87ce6fe4df87 Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Fri, 7 Aug 2026 12:02:15 -0400 Subject: [PATCH 1/2] fix(linux): fall back to live CRTC geometry for KMS capture Avoid aborting when the selected DRM card or CRTC is absent from the cached monitor list. Preserve cached geometry when available and add regression coverage for both fallback paths. --- cmake/compile_definitions/linux.cmake | 1 + src/platform/linux/kmsgrab.cpp | 104 ++++++++------------- src/platform/linux/kmsgrab.h | 88 +++++++++++++++++ tests/unit/platform/linux/test_kmsgrab.cpp | 93 ++++++++++++++++++ 4 files changed, 219 insertions(+), 67 deletions(-) create mode 100644 src/platform/linux/kmsgrab.h create mode 100644 tests/unit/platform/linux/test_kmsgrab.cpp diff --git a/cmake/compile_definitions/linux.cmake b/cmake/compile_definitions/linux.cmake index ed5133377bc..0bac95aa0d8 100644 --- a/cmake/compile_definitions/linux.cmake +++ b/cmake/compile_definitions/linux.cmake @@ -94,6 +94,7 @@ if(LIBDRM_FOUND) list(APPEND PLATFORM_LIBRARIES ${LIBDRM_LIBRARIES}) add_compile_definitions(SUNSHINE_BUILD_DRM) list(APPEND PLATFORM_TARGET_FILES + "${CMAKE_SOURCE_DIR}/src/platform/linux/kmsgrab.h" "${CMAKE_SOURCE_DIR}/src/platform/linux/kmsgrab.cpp") list(APPEND SUNSHINE_DEFINITIONS EGL_NO_X11=1) endif() diff --git a/src/platform/linux/kmsgrab.cpp b/src/platform/linux/kmsgrab.cpp index fdf10a92fc5..96ad1267edc 100644 --- a/src/platform/linux/kmsgrab.cpp +++ b/src/platform/linux/kmsgrab.cpp @@ -21,6 +21,7 @@ // local includes #include "cuda.h" #include "graphics.h" +#include "kmsgrab.h" #include "src/config.h" #include "src/logging.h" #include "src/platform/common.h" @@ -229,29 +230,6 @@ namespace platf { bool connected; ///< Whether the DRM connector is connected. }; - /** - * @brief KMS monitor capture state and DRM resources. - */ - struct monitor_t { - // Connector attributes - std::uint32_t type; ///< Type. - std::uint32_t index; ///< Index. - - // Monitor index in the global list - std::uint32_t monitor_index; ///< Monitor index. - - platf::touch_port_t viewport; ///< Viewport. - }; - - /** - * @brief DRM card, device path, and render-node metadata. - */ - struct card_descriptor_t { - std::string path; ///< Path. - - std::map crtc_to_monitor; ///< Crtc to monitor. - }; - static std::vector card_descriptors; static std::uint32_t from_view(const std::string_view &string) { @@ -979,18 +957,6 @@ namespace platf { BOOST_LOG(info) << "Found monitor for DRM screencasting"sv; - // We need to find the correct /dev/dri/card{nr} to correlate the crtc_id with the monitor descriptor - auto pos = std::find_if(std::begin(card_descriptors), std::end(card_descriptors), [&](card_descriptor_t &cd) { - return cd.path == filestring; - }); - - if (pos == std::end(card_descriptors)) { - // This code path shouldn't happen, but it's there just in case. - // card_descriptors is part of the guesswork after all. - BOOST_LOG(error) << "Couldn't find ["sv << entry.path() << "]: This shouldn't have happened :/"sv; - return -1; - } - // TODO: surf_sd = fb->to_sd(); kms::print(plane.get(), fb.get(), crtc.get()); @@ -1006,40 +972,44 @@ namespace platf { this->env_logical_width = ::platf::kms::env_logical_width; this->env_logical_height = ::platf::kms::env_logical_height; - auto monitor = pos->crtc_to_monitor.find(plane->crtc_id); - if (monitor != std::end(pos->crtc_to_monitor)) { - auto &viewport = monitor->second.viewport; - - width = viewport.width; - height = viewport.height; - - logical_width = viewport.logical_width; - logical_height = viewport.logical_height; - - switch (card.get_panel_orientation(plane->plane_id)) { - case DRM_MODE_ROTATE_270: - BOOST_LOG(debug) << "Detected panel orientation at 90, swapping width and height."; - width = viewport.height; - height = viewport.width; - break; - case DRM_MODE_ROTATE_90: - case DRM_MODE_ROTATE_180: - BOOST_LOG(warning) << "Panel orientation is unsupported, screen capture may not work correctly."; - break; - } - - offset_x = viewport.offset_x; - offset_y = viewport.offset_y; + const platf::touch_port_t live_crtc_viewport { + (int) crtc->x, + (int) crtc->y, + (int) crtc->width, + (int) crtc->height, + (int) crtc->width, + (int) crtc->height, + }; + const auto resolved_viewport = resolve_monitor_viewport(card_descriptors, filestring, plane->crtc_id, live_crtc_viewport); + switch (resolved_viewport.source) { + case monitor_viewport_source_e::live_crtc_missing_card: + BOOST_LOG(warning) << "DRM card ["sv << entry.path() << "] was absent from the cached monitor list; using live CRTC geometry."sv; + break; + case monitor_viewport_source_e::live_crtc_missing_monitor: + BOOST_LOG(warning) << "CRTC ["sv << plane->crtc_id << "] was absent from the cached monitor list; using live CRTC geometry."sv; + break; + case monitor_viewport_source_e::cached: + break; } - // This code path shouldn't happen, but it's there just in case. - // crtc_to_monitor is part of the guesswork after all. - else { - BOOST_LOG(warning) << "Couldn't find crtc_id, this shouldn't have happened :\\"sv; - width = crtc->width; - height = crtc->height; - offset_x = crtc->x; - offset_y = crtc->y; + const auto &viewport = resolved_viewport.viewport; + width = viewport.width; + height = viewport.height; + logical_width = viewport.logical_width; + logical_height = viewport.logical_height; + offset_x = viewport.offset_x; + offset_y = viewport.offset_y; + + switch (card.get_panel_orientation(plane->plane_id)) { + case DRM_MODE_ROTATE_270: + BOOST_LOG(debug) << "Detected panel orientation at 90, swapping width and height."; + width = viewport.height; + height = viewport.width; + break; + case DRM_MODE_ROTATE_90: + case DRM_MODE_ROTATE_180: + BOOST_LOG(warning) << "Panel orientation is unsupported, screen capture may not work correctly."; + break; } plane_id = plane->plane_id; diff --git a/src/platform/linux/kmsgrab.h b/src/platform/linux/kmsgrab.h new file mode 100644 index 00000000000..3cb4e35e56b --- /dev/null +++ b/src/platform/linux/kmsgrab.h @@ -0,0 +1,88 @@ +/** + * @file src/platform/linux/kmsgrab.h + * @brief Portable KMS monitor descriptor and viewport helpers. + */ +#pragma once + +// standard includes +#include +#include +#include +#include +#include +#include +#include + +// local includes +#include "src/platform/common.h" + +namespace platf::kms { + + /** + * @brief KMS monitor capture state and DRM resources. + */ + struct monitor_t { + // Connector attributes + std::uint32_t type; ///< Type. + std::uint32_t index; ///< Index. + + // Monitor index in the global list + std::uint32_t monitor_index; ///< Monitor index. + + platf::touch_port_t viewport; ///< Viewport. + }; + + /** + * @brief DRM card, device path, and monitor metadata. + */ + struct card_descriptor_t { + std::string path; ///< DRM card filename. + std::map crtc_to_monitor; ///< CRTC-to-monitor lookup. + }; + + /** + * @brief Source used to resolve a KMS monitor viewport. + */ + enum class monitor_viewport_source_e { + cached, ///< Viewport came from the cached card and monitor descriptor. + live_crtc_missing_card, ///< Cached card was absent, so live CRTC geometry was used. + live_crtc_missing_monitor, ///< Cached CRTC was absent, so live CRTC geometry was used. + }; + + /** + * @brief Resolved KMS viewport and its source. + */ + struct monitor_viewport_result_t { + platf::touch_port_t viewport; ///< Resolved viewport. + monitor_viewport_source_e source; ///< Source of the resolved viewport. + }; + + /** + * @brief Resolve cached monitor geometry, falling back to the live CRTC. + * + * @param card_descriptors Cached KMS card descriptors from display enumeration. + * @param card_path Filename of the live DRM card being captured. + * @param crtc_id Live DRM CRTC identifier being captured. + * @param live_crtc_viewport Viewport derived from the live CRTC. + * @return Cached monitor viewport when available; otherwise the live CRTC viewport. + */ + inline monitor_viewport_result_t resolve_monitor_viewport( + const std::vector &card_descriptors, + const std::string_view card_path, + const std::uint32_t crtc_id, + const platf::touch_port_t &live_crtc_viewport + ) { + const auto card = std::ranges::find(card_descriptors, card_path, &card_descriptor_t::path); + if (card == std::end(card_descriptors)) { + return {live_crtc_viewport, monitor_viewport_source_e::live_crtc_missing_card}; + } + + const auto monitor = card->crtc_to_monitor.find(crtc_id); + if (monitor == std::end(card->crtc_to_monitor)) { + return {live_crtc_viewport, monitor_viewport_source_e::live_crtc_missing_monitor}; + } + + return {monitor->second.viewport, monitor_viewport_source_e::cached}; + } + +} // namespace platf::kms diff --git a/tests/unit/platform/linux/test_kmsgrab.cpp b/tests/unit/platform/linux/test_kmsgrab.cpp new file mode 100644 index 00000000000..d4fbd661f36 --- /dev/null +++ b/tests/unit/platform/linux/test_kmsgrab.cpp @@ -0,0 +1,93 @@ +/** + * @file tests/unit/platform/linux/test_kmsgrab.cpp + * @brief Tests for portable KMS monitor descriptor helpers. + */ +#include "../../../tests_common.h" + +// standard includes +#include +#include +#include +#include + +// local includes +#include "src/platform/linux/kmsgrab.h" + +namespace { + + /** + * @brief Input and expected output for KMS viewport resolution tests. + */ + struct monitor_viewport_test_case_t { + std::string name; ///< Descriptive test-case name. + std::vector card_descriptors; ///< Cached card descriptors. + std::string card_path; ///< Live DRM card filename. + std::uint32_t crtc_id; ///< Live DRM CRTC identifier. + platf::touch_port_t live_crtc_viewport; ///< Fallback live CRTC geometry. + platf::touch_port_t expected_viewport; ///< Expected resolved geometry. + platf::kms::monitor_viewport_source_e expected_source; ///< Expected geometry source. + }; + + /** + * @brief Parameterized fixture for KMS viewport resolution. + */ + class KmsMonitorViewportTest: public testing::TestWithParam {}; + + TEST_P(KmsMonitorViewportTest, ResolvesCachedOrLiveCrtcGeometry) { + const auto &test_case = GetParam(); + const auto result = platf::kms::resolve_monitor_viewport( + test_case.card_descriptors, + test_case.card_path, + test_case.crtc_id, + test_case.live_crtc_viewport + ); + + EXPECT_EQ(result.source, test_case.expected_source); + EXPECT_EQ(result.viewport.offset_x, test_case.expected_viewport.offset_x); + EXPECT_EQ(result.viewport.offset_y, test_case.expected_viewport.offset_y); + EXPECT_EQ(result.viewport.width, test_case.expected_viewport.width); + EXPECT_EQ(result.viewport.height, test_case.expected_viewport.height); + EXPECT_EQ(result.viewport.logical_width, test_case.expected_viewport.logical_width); + EXPECT_EQ(result.viewport.logical_height, test_case.expected_viewport.logical_height); + } + + INSTANTIATE_TEST_SUITE_P( + KmsMonitorViewportCases, + KmsMonitorViewportTest, + testing::Values( + monitor_viewport_test_case_t { + "cached monitor", + {{"card2", {{42, {10, 1, 0, {100, 200, 2560, 1600, 1280, 800}}}}}}, + "card2", + 42, + {0, 0, 1920, 1080, 1920, 1080}, + {100, 200, 2560, 1600, 1280, 800}, + platf::kms::monitor_viewport_source_e::cached, + }, + monitor_viewport_test_case_t { + "missing secondary card", + {{"card1", {{17, {11, 1, 0, {0, 0, 1920, 1080, 1920, 1080}}}}}}, + "card2", + 42, + {0, 0, 2560, 1600, 2560, 1600}, + {0, 0, 2560, 1600, 2560, 1600}, + platf::kms::monitor_viewport_source_e::live_crtc_missing_card, + }, + monitor_viewport_test_case_t { + "missing monitor on cached card", + {{"card2", {{17, {10, 1, 0, {0, 0, 1920, 1080, 1920, 1080}}}}}}, + "card2", + 42, + {300, 0, 2560, 1600, 2560, 1600}, + {300, 0, 2560, 1600, 2560, 1600}, + platf::kms::monitor_viewport_source_e::live_crtc_missing_monitor, + } + ), + [](const testing::TestParamInfo &info) { + std::string name = info.param.name; + std::ranges::replace(name, ' ', '_'); + return name; + } + ); + +} // namespace From 0cbb28c921e849d70d9c1b7487842a2f3d00d8df Mon Sep 17 00:00:00 2001 From: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com> Date: Fri, 7 Aug 2026 12:46:41 -0400 Subject: [PATCH 2/2] chore: sonar fixes --- src/platform/linux/kmsgrab.cpp | 8 ++++-- src/platform/linux/kmsgrab.h | 50 ++++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/platform/linux/kmsgrab.cpp b/src/platform/linux/kmsgrab.cpp index 96ad1267edc..dc9e725ccde 100644 --- a/src/platform/linux/kmsgrab.cpp +++ b/src/platform/linux/kmsgrab.cpp @@ -982,13 +982,15 @@ namespace platf { }; const auto resolved_viewport = resolve_monitor_viewport(card_descriptors, filestring, plane->crtc_id, live_crtc_viewport); switch (resolved_viewport.source) { - case monitor_viewport_source_e::live_crtc_missing_card: + using enum monitor_viewport_source_e; + + case live_crtc_missing_card: BOOST_LOG(warning) << "DRM card ["sv << entry.path() << "] was absent from the cached monitor list; using live CRTC geometry."sv; break; - case monitor_viewport_source_e::live_crtc_missing_monitor: + case live_crtc_missing_monitor: BOOST_LOG(warning) << "CRTC ["sv << plane->crtc_id << "] was absent from the cached monitor list; using live CRTC geometry."sv; break; - case monitor_viewport_source_e::cached: + case cached: break; } diff --git a/src/platform/linux/kmsgrab.h b/src/platform/linux/kmsgrab.h index 3cb4e35e56b..0c0579054f1 100644 --- a/src/platform/linux/kmsgrab.h +++ b/src/platform/linux/kmsgrab.h @@ -11,6 +11,7 @@ #include #include #include +#include #include // local includes @@ -36,6 +37,47 @@ namespace platf::kms { * @brief DRM card, device path, and monitor metadata. */ struct card_descriptor_t { + /** + * @brief Construct a DRM card descriptor. + * + * @param card_path DRM card filename. + * @param monitors CRTC-to-monitor lookup. + */ + card_descriptor_t(std::string card_path, std::map monitors): + path {std::move(card_path)}, + crtc_to_monitor {std::move(monitors)} { + } + + /** + * @brief Copy a DRM card descriptor. + * + * @param other Descriptor to copy. + */ + card_descriptor_t(const card_descriptor_t &other) = default; + + /** + * @brief Move a DRM card descriptor without throwing. + * + * @param other Descriptor to move. + */ + card_descriptor_t(card_descriptor_t &&other) noexcept = default; + + /** + * @brief Copy-assign a DRM card descriptor. + * + * @param other Descriptor to copy. + * @return Reference to this descriptor. + */ + card_descriptor_t &operator=(const card_descriptor_t &other) = default; + + /** + * @brief Move-assign a DRM card descriptor without throwing. + * + * @param other Descriptor to move. + * @return Reference to this descriptor. + */ + card_descriptor_t &operator=(card_descriptor_t &&other) noexcept = default; + std::string path; ///< DRM card filename. std::map crtc_to_monitor; ///< CRTC-to-monitor lookup. }; @@ -72,17 +114,19 @@ namespace platf::kms { const std::uint32_t crtc_id, const platf::touch_port_t &live_crtc_viewport ) { + using enum monitor_viewport_source_e; + const auto card = std::ranges::find(card_descriptors, card_path, &card_descriptor_t::path); if (card == std::end(card_descriptors)) { - return {live_crtc_viewport, monitor_viewport_source_e::live_crtc_missing_card}; + return {live_crtc_viewport, live_crtc_missing_card}; } const auto monitor = card->crtc_to_monitor.find(crtc_id); if (monitor == std::end(card->crtc_to_monitor)) { - return {live_crtc_viewport, monitor_viewport_source_e::live_crtc_missing_monitor}; + return {live_crtc_viewport, live_crtc_missing_monitor}; } - return {monitor->second.viewport, monitor_viewport_source_e::cached}; + return {monitor->second.viewport, cached}; } } // namespace platf::kms