From a7f1eb4c25fb80fc239d3dba1cab8e9afda42400 Mon Sep 17 00:00:00 2001 From: fufesou Date: Sat, 12 Sep 2026 00:19:10 +0800 Subject: [PATCH] fix(cursor): address sizing and capture review findings --- flutter/lib/desktop/pages/remote_page.dart | 10 +- flutter/lib/models/model.dart | 23 ++- flutter/linux/cursor_size.h | 5 +- flutter/test/cursor_local_size_test.dart | 43 +++-- .../test/cursor_measurement_failure_test.dart | 181 ++++++++++++++++++ flutter/test/cursor_paint_scale_test.dart | 4 + flutter/test/cursor_resize_limit_test.dart | 43 +++++ flutter/windows/runner/cursor_size.h | 34 +++- flutter/windows/runner/flutter_window.cpp | 6 +- src/server/drm_capturer.rs | 27 ++- src/server/drm_capturer/cursor.rs | 54 ++++-- src/server/drm_capturer/cursor/tests.rs | 54 ++++++ 12 files changed, 434 insertions(+), 50 deletions(-) create mode 100644 flutter/test/cursor_measurement_failure_test.dart create mode 100644 flutter/test/cursor_resize_limit_test.dart create mode 100644 src/server/drm_capturer/cursor/tests.rs diff --git a/flutter/lib/desktop/pages/remote_page.dart b/flutter/lib/desktop/pages/remote_page.dart index 86851fa26..b6c334982 100644 --- a/flutter/lib/desktop/pages/remote_page.dart +++ b/flutter/lib/desktop/pages/remote_page.dart @@ -1121,21 +1121,23 @@ class _ImagePaintState extends State { // changes, so read it live to follow the window across monitors. final dpr = MediaQuery.devicePixelRatioOf(context); - bool isViewAdaptive() => c.viewStyle.style == kRemoteViewStyleAdaptive; + bool isViewScaled() => + c.viewStyle.style == kRemoteViewStyleAdaptive || + c.viewStyle.style == kRemoteViewStyleCustom; bool isViewOriginal() => c.viewStyle.style == kRemoteViewStyleOriginal; mouseRegion({child}) => Obx(() { final useLocalSize = !isWeb && (isLinux || isMacOS || isWindows) && !zoomCursor.value && - isViewAdaptive(); + isViewScaled(); if (useLocalSize) _localCursorSize.ensureLoaded(dpr); double getCursorScale() { var c = Provider.of(context); var cursorScale = 1.0; if (isWindows) { // debug win10 - if (zoomCursor.value && isViewAdaptive()) { + if (zoomCursor.value && isViewScaled()) { cursorScale = s * c.devicePixelRatio; } } else { @@ -1296,7 +1298,6 @@ class _ImagePaintState extends State { {bool useLocalSize = false}) { final cursor = Provider.of(context); final cache = cursor.cache ?? preDefaultCursor.cache; - if (useLocalSize && _localCursorSize.value == null) return MouseCursor.defer; cache?.localSize = useLocalSize ? _localCursorSize.value : null; return buildCursorOfCache(cursor, scale, cache); } @@ -1305,7 +1306,6 @@ class _ImagePaintState extends State { {bool useLocalSize = false}) { final cursor = Provider.of(context); final cache = preForbiddenCursor.cache; - if (useLocalSize && _localCursorSize.value == null) return MouseCursor.defer; cache?.localSize = useLocalSize ? _localCursorSize.value : null; return buildCursorOfCache(cursor, scale, cache); } diff --git a/flutter/lib/models/model.dart b/flutter/lib/models/model.dart index ece33f5a4..eaa82ed6e 100644 --- a/flutter/lib/models/model.dart +++ b/flutter/lib/models/model.dart @@ -2900,6 +2900,15 @@ class CursorData { scale = _visibleSize == 0 ? 1.0 : max(localSize!, kMinCursorSize) / _visibleSize; + // Sparse peer artwork must not amplify the native bitmap allocation. + const maxNativeCursorBitmapSize = 512; + final maxScale = maxNativeCursorBitmapSize / max(width, height); + if (scale > maxScale) { + if (oldScale != maxScale) { + debugPrint('Cursor $id exceeds the native bitmap limit; reducing scale'); + } + scale = maxScale; + } } else if (scale != 1.0) { // Update data if scale changed. final tgtWidth = (width * scale).toInt(); @@ -3461,6 +3470,8 @@ class CursorModel with ChangeNotifier { if (await _updateCache(rgba, image, id, hotx, hoty, width, height)) { _images[id]?.item1.dispose(); _images[id] = Tuple3(image, hotx, hoty); + } else { + image.dispose(); } // Update last cursor data. @@ -3484,7 +3495,8 @@ class CursorModel with ChangeNotifier { final pixels = await image.toByteData(format: ui.ImageByteFormat.rawStraightRgba); if (pixels == null) { - throw StateError('Could not read straight-alpha cursor pixels'); + debugPrint('Could not read straight-alpha cursor pixels: $id'); + return false; } imgOrigin = img2.Image.fromBytes( width: w, height: h, bytes: pixels.buffer, order: img2.ChannelOrder.rgba); @@ -3493,13 +3505,18 @@ class CursorModel with ChangeNotifier { ByteData? imgBytes = await image.toByteData(format: ui.ImageByteFormat.png); if (imgBytes == null) { + debugPrint('Could not encode cursor PNG: $id'); return false; } data = imgBytes.buffer.asUint8List(); if (isLinux || isMacOS) { // Preserve the PNG's straight-alpha colors when resizing native cursors. - imgOrigin = img2.decodePng(data) ?? - (throw const FormatException('Invalid native cursor PNG')); + final decoded = img2.decodePng(data); + if (decoded == null) { + debugPrint('Invalid native cursor PNG: $id'); + return false; + } + imgOrigin = decoded; } } final cache = CursorData( diff --git a/flutter/linux/cursor_size.h b/flutter/linux/cursor_size.h index b3c676628..9100b4257 100644 --- a/flutter/linux/cursor_size.h +++ b/flutter/linux/cursor_size.h @@ -98,7 +98,10 @@ inline void Register(FlView* view) { } if (error) g_warning("Cursor size response failed: %s", error->message); }, view, nullptr); - g_object_set_data_full(G_OBJECT(view), "cursor-size-channel", channel, g_object_unref); + g_object_set_data_full(G_OBJECT(view), "cursor-size-channel", channel, [](gpointer data) { + fl_method_channel_set_method_call_handler(FL_METHOD_CHANNEL(data), nullptr, nullptr, nullptr); + g_object_unref(data); + }); } } // namespace cursor_size diff --git a/flutter/test/cursor_local_size_test.dart b/flutter/test/cursor_local_size_test.dart index f98876c16..354f1e5cd 100644 --- a/flutter/test/cursor_local_size_test.dart +++ b/flutter/test/cursor_local_size_test.dart @@ -25,15 +25,17 @@ class _Image extends ChangeNotifier implements ImageModel { } class _Canvas extends ChangeNotifier implements CanvasModel { - _Canvas(this.devicePixelRatio); + _Canvas(this.devicePixelRatio, {required this.style, required this.scale}); + + final String style; @override final double devicePixelRatio; @override final imageOverflow = false.obs; @override - final viewStyle = ViewStyle( - style: kRemoteViewStyleAdaptive, + late final viewStyle = ViewStyle( + style: style, width: _viewport.width, height: _viewport.height, displayWidth: 400, @@ -44,7 +46,7 @@ class _Canvas extends ChangeNotifier implements CanvasModel { @override Size get size => _viewport; @override - double get scale => 0.5; + final double scale; @override double get x => 0; @override @@ -200,8 +202,15 @@ void main() { if (Platform.isWindows) expect(cursor.data!.length, 25 * 34 * 4); }); - for (final dpr in [1.0, 2.0, 3.0]) { - testWidgets('Zoom off at local DPR $dpr ignores remote raster density', + for (final (dpr, style, scale) in [ + for (final dpr in [1.0, 2.0, 3.0]) + for (final (style, scale) in [ + (kRemoteViewStyleAdaptive, 0.5), + (kRemoteViewStyleCustom, 0.25), + (kRemoteViewStyleCustom, 2.0), + ]) (dpr, style, scale), + ]) { + testWidgets('$style scale=$scale DPR=$dpr preserves local size and hotspot', (tester) async { final channel = Platform.isWindows ? SystemChannels.mouseCursor @@ -227,18 +236,20 @@ void main() { addTearDown(() => tester.binding.defaultBinaryMessenger .setMockMethodCallHandler(sizeChannel, null)); final cursor = _Cursor(_arrow(1)); + final zoom = false.obs; await tester.pumpWidget(MediaQuery( data: MediaQueryData(devicePixelRatio: dpr), child: MultiProvider( providers: [ ChangeNotifierProvider(create: (_) => _Image()), - ChangeNotifierProvider(create: (_) => _Canvas(dpr)), + ChangeNotifierProvider( + create: (_) => _Canvas(dpr, style: style, scale: scale)), ChangeNotifierProvider.value(value: cursor), ], child: ImagePaint( ffi: _FFI(), id: 'local-size-test', - zoomCursor: false.obs, + zoomCursor: zoom, cursorOverImage: true.obs, keyboardEnabled: true.obs, remoteCursorMoved: false.obs, @@ -246,12 +257,14 @@ void main() { ), )); await tester.pump(); - expect(registered, hasLength(1)); - final original = registered.single; + expect(registered, isNotEmpty); + expect(cursor.cache.localSize, Platform.isWindows ? 23.0 * dpr : 23.0); + final original = registered.last; + final beforeDensityChange = registered.length; cursor.cache = _arrow(2); cursor.notifyListeners(); await tester.pump(); - expect(registered, hasLength(2)); + expect(registered, hasLength(beforeDensityChange + 1)); final retina = registered.last; expect( [retina['width'], retina['height'], retina['hotX'], retina['hotY']], @@ -264,6 +277,14 @@ void main() { reason: 'Only the remote raster density changed; the local cursor must ' 'retain its size and hotspot.', ); + zoom.value = true; + await tester.pump(); + expect(cursor.cache.localSize, isNull); + final zoomScale = scale * (Platform.isWindows ? dpr : 1); + expect(cursor.cache.scale, + zoomScale < kMinCursorSize / 34 ? kMinCursorSize / 34 : zoomScale); + expect(cursor.cache.hotx / cursor.cache.scaledWidth, closeTo(4 / 17, 1e-9)); + expect(cursor.cache.hoty / cursor.cache.scaledHeight, closeTo(4 / 23, 1e-9)); await tester.pumpWidget(const SizedBox.shrink()); cursor.dispose(); }); diff --git a/flutter/test/cursor_measurement_failure_test.dart b/flutter/test/cursor_measurement_failure_test.dart new file mode 100644 index 000000000..242b2d370 --- /dev/null +++ b/flutter/test/cursor_measurement_failure_test.dart @@ -0,0 +1,181 @@ +import 'dart:async'; +import 'dart:io'; +import 'dart:ui' as ui; + +import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_hbb/consts.dart'; +import 'package:flutter_hbb/desktop/pages/remote_page.dart'; +import 'package:flutter_hbb/models/input_model.dart'; +import 'package:flutter_hbb/models/model.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:get/get.dart'; +import 'package:image/image.dart' as img; +import 'package:provider/provider.dart'; + +const _viewport = Size(200, 160); + +class _Image extends ChangeNotifier implements ImageModel { + @override + bool get useTextureRender => false; + @override + ui.Image? get image => null; + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +class _Canvas extends ChangeNotifier implements CanvasModel { + _Canvas(this.devicePixelRatio, {required this.style, required this.scale}); + + final String style; + + @override + final double devicePixelRatio; + @override + final imageOverflow = false.obs; + @override + late final viewStyle = ViewStyle( + style: style, + width: _viewport.width, + height: _viewport.height, + displayWidth: 400, + displayHeight: 320, + ); + @override + bool get cursorEmbedded => false; + @override + Size get size => _viewport; + @override + final double scale; + @override + double get x => 0; + @override + double get y => 0; + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +class _Cursor extends ChangeNotifier implements CursorModel { + _Cursor(this.cache); + + @override + CursorData cache; + @override + ui.Image? get image => null; + @override + double get hotx => cache.hotxOrigin; + @override + double get hoty => cache.hotyOrigin; + @override + final Set cachedKeys = {}; + @override + void addKey(String key) => cachedKeys.add(key); + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +class _Input extends Fake implements InputModel { + @override + final relativeMouseMode = false.obs; +} + +class _Peer extends Fake implements FfiModel { + @override + final pi = PeerInfo(); + @override + bool get isPeerLinux => false; +} + +class _FFI extends Fake implements FFI { + @override + final inputModel = _Input(); + @override + final ffiModel = _Peer(); +} + +CursorData _cursor(img.Image image, + {String id = 'sprite', double hotspot = 0}) { + return CursorData( + peerId: 'local-size-test', + id: id, + image: image, + scale: 1, + data: Platform.isWindows + ? image.getBytes(order: img.ChannelOrder.bgra) + : Uint8List.fromList(img.encodePng(image)), + hotxOrigin: hotspot, + hotyOrigin: hotspot, + width: image.width, + height: image.height, + ); +} + +CursorData _arrow(int density) { + final image = + img.Image(width: 17 * density, height: 23 * density, numChannels: 4); + img.fill(image, color: img.ColorRgba8(255, 255, 255, 255)); + return _cursor(image, id: 'arrow-$density', hotspot: 4.0 * density); +} + +void main() { + testWidgets('pending or failed size measurement preserves the remote cursor', + (tester) async { + final channel = Platform.isWindows + ? SystemChannels.mouseCursor + : const MethodChannel('flutter_custom_cursor'); + final registered = >[]; + tester.binding.defaultBinaryMessenger.setMockMethodCallHandler(channel, + (call) async { + if (call.method.startsWith('createCustomCursor')) { + final arguments = call.arguments as Map; + registered.add(arguments); + return arguments['name']; + } + return null; + }); + addTearDown(() => tester.binding.defaultBinaryMessenger + .setMockMethodCallHandler(channel, null)); + const sizeChannel = MethodChannel('org.rustdesk.rustdesk/cursor'); + final measurement = Completer(); + tester.binding.defaultBinaryMessenger + .setMockMethodCallHandler(sizeChannel, (_) => measurement.future); + addTearDown(() => tester.binding.defaultBinaryMessenger + .setMockMethodCallHandler(sizeChannel, null)); + final cursor = _Cursor(_arrow(2)); + await tester.pumpWidget(_view(cursor)); + expect(registered, isNotEmpty); + measurement.completeError(PlatformException(code: 'cursor_size')); + await tester.pump(); + expect(tester.takeException(), isA()); + expect(cursor.cache.localSize, isNull); + final scale = Platform.isWindows ? 1.0 : 0.5; + expect(cursor.cache.scaledWidth, 34 * scale); + expect(cursor.cache.scaledHeight, 46 * scale); + expect(cursor.cache.hotx, 8 * scale); + expect(cursor.cache.hoty, 8 * scale); + expect(registered.last['hotX'], 8 * scale); + expect(registered.last['hotY'], 8 * scale); + await tester.pumpWidget(const SizedBox.shrink()); + cursor.dispose(); + }); +} + +Widget _view(_Cursor cursor) => MediaQuery( + data: const MediaQueryData(devicePixelRatio: 2), + child: MultiProvider( + providers: [ + ChangeNotifierProvider(create: (_) => _Image()), + ChangeNotifierProvider(create: (_) => _Canvas(2, + style: kRemoteViewStyleAdaptive, scale: 0.25)), + ChangeNotifierProvider.value(value: cursor), + ], + child: ImagePaint( + ffi: _FFI(), + id: 'measurement-failure-test', + zoomCursor: false.obs, + cursorOverImage: true.obs, + keyboardEnabled: true.obs, + remoteCursorMoved: false.obs, + ), + ), + ); diff --git a/flutter/test/cursor_paint_scale_test.dart b/flutter/test/cursor_paint_scale_test.dart index 7b393aaed..61a00af4b 100644 --- a/flutter/test/cursor_paint_scale_test.dart +++ b/flutter/test/cursor_paint_scale_test.dart @@ -107,6 +107,10 @@ void main() { (kRemoteViewStyleAdaptive, true, 3.0, (48, 64), 0.375, 0.375, true), (kRemoteViewStyleOriginal, false, 2.0, (48, 64), 0.5, 0.5, true), (kRemoteViewStyleOriginal, true, 2.0, (48, 64), 0.5, 0.5, true), + for (final zoom in [false, true]) + for (final scale in [0.25, 2.0]) + for (final texture in [false, true]) + (kRemoteViewStyleCustom, zoom, 2.0, (48, 64), scale, scale, texture), ( kRemoteViewStyleAdaptive, false, diff --git a/flutter/test/cursor_resize_limit_test.dart b/flutter/test/cursor_resize_limit_test.dart new file mode 100644 index 000000000..50e7c14f5 --- /dev/null +++ b/flutter/test/cursor_resize_limit_test.dart @@ -0,0 +1,43 @@ +import 'dart:math'; +import 'dart:typed_data'; + +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:flutter_hbb/models/model.dart'; +import 'package:image/image.dart' as img; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + test('sparse remote artwork cannot amplify the native bitmap without limit', + () { + const sourceSize = 64; + const bitmapLimit = 512; + final image = img.Image(width: sourceSize, height: sourceSize, numChannels: 4) + ..setPixelRgba(0, 0, 255, 255, 255, 255); + final cache = CursorData( + peerId: 'sparse-cursor', + id: '1', + image: image, + scale: 1, + data: Uint8List.fromList(img.encodePng(image)), + hotxOrigin: 32, + hotyOrigin: 48, + width: sourceSize, + height: sourceSize, + )..localSize = sourceSize.toDouble(); + final messages = []; + final previous = debugPrint; + debugPrint = (String? message, {int? wrapWidth}) { + messages.add(message ?? ''); + }; + addTearDown(() => debugPrint = previous); + + cache.updateGetKey(1); + + expect(max(cache.scaledWidth, cache.scaledHeight), bitmapLimit); + expect(cache.hotx, 32 * cache.scaledWidth / sourceSize); + expect(cache.hoty, 48 * cache.scaledHeight / sourceSize); + expect(messages.single, contains('bitmap limit')); + }); +} diff --git a/flutter/windows/runner/cursor_size.h b/flutter/windows/runner/cursor_size.h index 9edaf085a..be0a642cd 100644 --- a/flutter/windows/runner/cursor_size.h +++ b/flutter/windows/runner/cursor_size.h @@ -71,7 +71,30 @@ inline void IncludeVisiblePixels(const Surface& surface, uint32_t background, } } -inline double SystemSize() { +inline double WindowScale(HWND window) { + const auto user32 = GetModuleHandleW(L"user32.dll"); + const auto window_dpi = reinterpret_cast( + GetProcAddress(user32, "GetDpiForWindow")); + const auto system_dpi = reinterpret_cast( + GetProcAddress(user32, "GetDpiForSystem")); + const auto metrics = reinterpret_cast( + GetProcAddress(user32, "GetSystemMetricsForDpi")); + // Preserve the system-DPI path on Windows versions without per-window metrics. + if (!window_dpi || !system_dpi || !metrics) return 1.0; + const UINT target_dpi = window_dpi(window); + const UINT source_dpi = system_dpi(); + if (!target_dpi || !source_dpi) { + throw std::runtime_error("Could not read the cursor window DPI"); + } + const int target_size = metrics(SM_CXCURSOR, target_dpi); + const int source_size = metrics(SM_CXCURSOR, source_dpi); + if (target_size <= 0 || source_size <= 0) { + throw std::runtime_error("Could not read the DPI-specific cursor metrics"); + } + return static_cast(target_size) / source_size; +} + +inline double SystemSize(HWND window) { HCURSOR cursor = LoadCursorW(nullptr, IDC_ARROW); IconBitmaps bitmaps; if (!cursor || !GetIconInfo(cursor, &bitmaps.info)) { @@ -97,20 +120,21 @@ inline double SystemSize() { if (bounds.right <= bounds.left) { throw std::runtime_error("System cursor has no visible pixels"); } - return (std::max)(bounds.right - bounds.left, bounds.bottom - bounds.top); + return (std::max)(bounds.right - bounds.left, bounds.bottom - bounds.top) * + WindowScale(window); } -inline void Register(flutter::BinaryMessenger* messenger) { +inline void Register(flutter::BinaryMessenger* messenger, HWND window) { flutter::MethodChannel<> channel(messenger, "org.rustdesk.rustdesk/cursor", &flutter::StandardMethodCodec::GetInstance()); - channel.SetMethodCallHandler([](const flutter::MethodCall<>& call, + channel.SetMethodCallHandler([window](const flutter::MethodCall<>& call, std::unique_ptr> result) { if (call.method_name() != "getSystemCursorSize") { result->NotImplemented(); return; } try { - result->Success(flutter::EncodableValue(SystemSize())); + result->Success(flutter::EncodableValue(SystemSize(window))); } catch (const std::exception& error) { result->Error("cursor_size", error.what()); } diff --git a/flutter/windows/runner/flutter_window.cpp b/flutter/windows/runner/flutter_window.cpp index 705cb24e0..a21149cef 100644 --- a/flutter/windows/runner/flutter_window.cpp +++ b/flutter/windows/runner/flutter_window.cpp @@ -101,7 +101,8 @@ bool FlutterWindow::OnCreate() { return false; } RegisterPlugins(flutter_controller_->engine()); - cursor_size::Register(flutter_controller_->engine()->messenger()); + cursor_size::Register(flutter_controller_->engine()->messenger(), + flutter_controller_->view()->GetNativeWindow()); flutter::MethodChannel<> channel( flutter_controller_->engine()->messenger(), @@ -148,7 +149,8 @@ bool FlutterWindow::OnCreate() { auto *flutter_view_controller = reinterpret_cast(controller); auto *registry = flutter_view_controller->engine(); - cursor_size::Register(registry->messenger()); + cursor_size::Register(registry->messenger(), + flutter_view_controller->view()->GetNativeWindow()); TextureRgbaRendererPluginCApiRegisterWithRegistrar( registry->GetRegistrarForPlugin("TextureRgbaRendererPlugin")); FlutterGpuTextureRendererPluginCApiRegisterWithRegistrar( diff --git a/src/server/drm_capturer.rs b/src/server/drm_capturer.rs index 73428b930..8f7b54ff3 100644 --- a/src/server/drm_capturer.rs +++ b/src/server/drm_capturer.rs @@ -632,7 +632,7 @@ async fn recv_thread( let _ = tx.send(Err(err)); return; } - let mutter_cursor = match cursor::Capture::start( + let mut mutter_cursor = match cursor::Capture::start( display, cursor_epoch, displays[wire_idx].name.clone(), @@ -641,8 +641,8 @@ async fn recv_thread( { Ok(cursor) => cursor, Err(error) => { - let _ = tx.send(Err(error)); - return; + log::error!("drm: could not start Mutter cursor capture; using wire cursor: {error:#}"); + None } }; let _ = tx.send(Ok((displays, wire_idx))); @@ -655,11 +655,20 @@ async fn recv_thread( break "stopped".to_owned(); } if let Some(error) = mutter_cursor.as_ref().and_then(cursor::Capture::error) { - break error; + log::error!("drm: {error}; resuming wire cursor capture"); + if let Some(mut cursor) = mutter_cursor.take() { + if let Some(wire) = cursor.wire_cursor.take() { + pending_cursor = Some(wire); + } + if let Err(error) = cursor.stop().await { + log::error!("drm: could not join the Mutter cursor worker: {error:#}"); + } + } } if pending_cursor.is_some() { + let ready = mutter_cursor.as_ref().map(cursor::Capture::ready); let t = shared.transform.load(std::sync::atomic::Ordering::Acquire); - if t != TRANSFORM_PENDING { + if t != TRANSFORM_PENDING && ready.as_deref() != Some(&true) { if let Some((id, width, height, hotx, hoty, raw)) = pending_cursor.take() { deliver_drm_cursor(display, cursor_epoch, id, width, height, hotx, hoty, raw, t); } @@ -781,7 +790,13 @@ async fn recv_thread( raw.len() ); } - if mutter_cursor.is_some() { + // Retain the latest wire shape for a failed Mutter worker, even when + // that shape arrived before the first Mutter sprite. + if let Some(cursor) = mutter_cursor.as_mut() { + cursor.wire_cursor = Some((id, width, height, hotx, hoty, raw.clone())); + } + let ready = mutter_cursor.as_ref().map(cursor::Capture::ready); + if ready.as_deref() == Some(&true) { continue; } let t = shared.transform.load(std::sync::atomic::Ordering::Acquire); diff --git a/src/server/drm_capturer/cursor.rs b/src/server/drm_capturer/cursor.rs index 9dede6c77..3e339b89a 100644 --- a/src/server/drm_capturer/cursor.rs +++ b/src/server/drm_capturer/cursor.rs @@ -9,7 +9,7 @@ use hbb_common::{anyhow::anyhow, bail, log, tokio, ResultType}; use std::{ sync::{ mpsc::{self, Receiver, Sender, TryRecvError}, - Arc, Mutex, + Arc, Mutex, MutexGuard, }, thread::{self, JoinHandle}, time::{Duration, Instant}, @@ -18,6 +18,8 @@ use std::{ mod ffi; mod metadata; mod pipewire; +#[cfg(test)] +mod tests; const BUS: &str = "org.gnome.Mutter.ScreenCast"; const SESSION_INTERFACE: &str = "org.gnome.Mutter.ScreenCast.Session"; @@ -27,10 +29,14 @@ const DBUS_TIMEOUT: Duration = Duration::from_secs(2); const START_TIMEOUT: Duration = Duration::from_secs(5); const POLL_INTERVAL: Duration = Duration::from_millis(20); +pub(super) type WireCursor = (u64, u32, u32, i32, i32, Vec); + pub struct Capture { stop: Option>, thread: Option>, error: Arc>>, + ready: Arc>, + pub(super) wire_cursor: Option, } impl Capture { @@ -41,10 +47,15 @@ impl Capture { let (stop, receiver) = mpsc::channel(); let error = Arc::new(Mutex::new(None)); let worker_error = error.clone(); + let ready = Arc::new(Mutex::new(false)); + let worker_ready = ready.clone(); let thread = thread::Builder::new() .name("drm-cursor".into()) .spawn(move || { - if let Err(error) = run((display, epoch), connector, receiver) { + let result = run(connector, receiver, move |cursor| { + publish((display, epoch), &worker_ready, cursor); + }); + if let Err(error) = result { log::error!("drm: Mutter cursor capture failed: {error:#}"); *worker_error.lock().unwrap() = Some(format!("Mutter cursor capture: {error:#}")); @@ -54,6 +65,8 @@ impl Capture { stop: Some(stop), thread: Some(thread), error, + ready, + wire_cursor: None, })) } @@ -61,6 +74,10 @@ impl Capture { self.error.lock().unwrap().clone() } + pub fn ready(&self) -> MutexGuard<'_, bool> { + self.ready.lock().unwrap() + } + pub async fn stop(mut self) -> ResultType<()> { drop(self.stop.take()); if let Some(thread) = self.thread.take() { @@ -92,27 +109,30 @@ fn stopped(receiver: &Receiver<()>) -> bool { !matches!(receiver.try_recv(), Err(TryRecvError::Empty)) } -fn run(target: (i32, u64), connector: String, stop: Receiver<()>) -> ResultType<()> { +fn publish(target: (i32, u64), ready: &Mutex, cursor: DrmCursorData) { + // Serialize source switching with wire publication, including pending replay. + let mut ready = ready.lock().unwrap(); + if !*ready { + log::info!("drm: using Mutter cursor metadata for display {}", target.0); + } + *ready = true; + // These sprites already have the monitor's upright orientation and physical scale. + super::set_drm_cursor(target.0, target.1, cursor); +} + +fn run( + connector: String, + stop: Receiver<()>, + publish: impl FnMut(DrmCursorData) + Send + 'static, +) -> ResultType<()> { let session = Session::new()?; let Some(node) = session.start(&super::normalize_connector(&connector), &stop)? else { return Ok(()); }; - let stream = pipewire::Stream::new(node, move |cursor| { - // These sprites already have the monitor's upright orientation and physical scale. - super::set_drm_cursor(target.0, target.1, cursor); - })?; + let stream = pipewire::Stream::new(node, publish)?; let started = Instant::now(); - let mut ready = false; while !stopped(&stop) { - if stream.received()? { - if !ready { - log::info!( - "drm: using Mutter cursor metadata for display {} ({connector})", - target.0 - ); - ready = true; - } - } else if started.elapsed() >= START_TIMEOUT { + if !stream.received()? && started.elapsed() >= START_TIMEOUT { bail!("Timed out waiting for PipeWire cursor metadata"); } session.conn.process(POLL_INTERVAL)?; diff --git a/src/server/drm_capturer/cursor/tests.rs b/src/server/drm_capturer/cursor/tests.rs new file mode 100644 index 000000000..0118b7681 --- /dev/null +++ b/src/server/drm_capturer/cursor/tests.rs @@ -0,0 +1,54 @@ +use super::*; + +fn capture() -> Capture { + Capture { + stop: None, + thread: None, + error: Arc::new(Mutex::new(None)), + ready: Arc::new(Mutex::new(false)), + wire_cursor: None, + } +} + +fn sprite(id: u64) -> DrmCursorData { + DrmCursorData { + id, + width: 1, + height: 1, + hotx: 0, + hoty: 0, + colors: vec![255; 4], + } +} + +#[test] +fn wire_cursor_remains_active_until_a_sprite_is_published() { + let capture = capture(); + assert!(!*capture.ready()); + let target = (-1614401, 1); + publish(target, &capture.ready, sprite(7)); + assert!(*capture.ready()); + let mut cursors = super::super::DRM_CURSOR.lock().unwrap(); + assert_eq!(cursors.remove(&target.0).unwrap().1.id, 7); +} + +#[test] +fn wire_publication_cannot_overwrite_the_first_mutter_sprite() { + let capture = capture(); + let target = (-1614402, 1); + let ready = capture.ready(); + assert!(!*ready); + let worker_ready = capture.ready.clone(); + let (started, receiver) = mpsc::channel(); + let worker = thread::spawn(move || { + started.send(()).unwrap(); + publish(target, &worker_ready, sprite(9)); + }); + receiver.recv_timeout(Duration::from_secs(1)).unwrap(); + super::super::set_drm_cursor(target.0, target.1, sprite(8)); + drop(ready); + worker.join().unwrap(); + assert!(*capture.ready()); + let mut cursors = super::super::DRM_CURSOR.lock().unwrap(); + assert_eq!(cursors.remove(&target.0).unwrap().1.id, 9); +}