Merge pull request #6079 from dignow/fix/change_display_resolution

Fix/change display resolution

flutter/lib/models/model.dart

@@ -430,14 +430,12 @@ class FfiModel with ChangeNotifier {
       Map<String, dynamic> evt, SessionID sessionId, String peerId) {
     final curDisplay = int.parse(evt['display']);
 
-    // The message should be handled by the another UI session.
+    if (_pi.currentDisplay != kAllDisplayValue) {
-    if (isChooseDisplayToOpenInNewWindow(_pi, sessionId)) {
+      if (bind.peerGetDefaultSessionsCount(id: peerId) > 1) {
         if (curDisplay != _pi.currentDisplay) {
           return;
         }
       }
-
-    if (_pi.currentDisplay != kAllDisplayValue) {
       _pi.currentDisplay = curDisplay;
     }
 
@@ -825,6 +823,7 @@ class FfiModel with ChangeNotifier {
       }
       _pi.displays = newDisplays;
       _pi.displaysCount.value = _pi.displays.length;
+
       if (_pi.currentDisplay == kAllDisplayValue) {
         updateCurDisplay(sessionId);
         // to-do: What if the displays are changed?
@@ -2282,7 +2281,7 @@ class PeerInfo with ChangeNotifier {
     if (currentDisplay == kAllDisplayValue) {
       return null;
     }
-    if (currentDisplay > 0 && currentDisplay < displays.length) {
+    if (currentDisplay >= 0 && currentDisplay < displays.length) {
       return displays[currentDisplay];
     } else {
       return null;

src/core_main.rs

@@ -1,4 +1,4 @@
-#[cfg(not(any(target_os = "android", target_os = "ios")))]
+#[cfg(windows)]
 use crate::client::translate;
 #[cfg(not(debug_assertions))]
 #[cfg(not(any(target_os = "android", target_os = "ios")))]

src/server/connection.rs

@@ -611,7 +611,7 @@ impl Connection {
                                 _ => {},
                             }
                         }
-                        Some(message::Union::PeerInfo(_)) => {
+                        Some(message::Union::PeerInfo(..)) => {
                             conn.refresh_video_display(None);
                         }
                         _ => {}
@@ -1132,11 +1132,13 @@ impl Connection {
                 self.send(msg_out).await;
             }
 
-            match super::display_service::get_displays().await {
+            match super::display_service::update_get_sync_displays().await {
                 Err(err) => {
                     res.set_error(format!("{}", err));
                 }
                 Ok(displays) => {
+                    // For compatibility with old versions, we need to send the displays to the peer.
+                    // But the displays may be updated later, before creating the video capturer.
                     pi.displays = displays.clone();
                     pi.current_display = self.display_idx as _;
                     res.set_peer_info(pi);
@@ -2139,13 +2141,12 @@ impl Connection {
                         ..Default::default()
                     });
                 }
-
-                // send display changed message
-                if let Some(msg_out) =
-                    video_service::make_display_changed_msg(self.display_idx, None)
-                {
-                    self.send(msg_out).await;
             }
+
+            // Send display changed message.
+            // For compatibility with old versions ( < 1.2.4 ).
+            if let Some(msg_out) = video_service::make_display_changed_msg(self.display_idx, None) {
+                self.send(msg_out).await;
             }
         }
     }

src/server/display_service.rs

@@ -5,6 +5,12 @@ use crate::virtual_display_manager;
 use hbb_common::get_version_number;
 use hbb_common::protobuf::MessageField;
 use scrap::Display;
+#[cfg(target_os = "linux")]
+use std::sync::atomic::{AtomicBool, Ordering};
+
+// https://github.com/rustdesk/rustdesk/discussions/6042, avoiding dbus call
+#[cfg(target_os = "linux")]
+pub(super) static IS_X11: AtomicBool = AtomicBool::new(false);
 
 pub const NAME: &'static str = "display";
 
@@ -19,6 +25,71 @@ lazy_static::lazy_static! {
     // Initial primary display index.
     // It should only be updated when the rustdesk server is started, and should not be updated when displays changed.
     pub static ref PRIMARY_DISPLAY_IDX: usize = get_primary();
+    static ref SYNC_DISPLAYS: Arc<Mutex<SyncDisplaysInfo>> = Default::default();
+}
+
+#[derive(Default)]
+struct SyncDisplaysInfo {
+    displays: Vec<DisplayInfo>,
+    is_synced: bool,
+}
+
+impl SyncDisplaysInfo {
+    fn check_changed(&mut self, displays: Vec<DisplayInfo>) {
+        if self.displays.len() != displays.len() {
+            self.displays = displays;
+            self.is_synced = false;
+            return;
+        }
+        for (i, d) in displays.iter().enumerate() {
+            if d != &self.displays[i] {
+                self.displays = displays;
+                self.is_synced = false;
+                return;
+            }
+        }
+    }
+
+    fn get_update_sync_displays(&mut self) -> Option<Vec<DisplayInfo>> {
+        if self.is_synced {
+            return None;
+        }
+        self.is_synced = true;
+        Some(self.displays.clone())
+    }
+}
+
+// This function is really useful, though a duplicate check if display changed.
+// The video server will then send the following messages to the client:
+//  1. the supported resolutions of the {idx} display
+//  2. the switch resolution message, so that the client can record the custom resolution.
+pub(super) fn check_display_changed(
+    ndisplay: usize,
+    idx: usize,
+    (x, y, w, h): (i32, i32, usize, usize),
+) -> Option<DisplayInfo> {
+    #[cfg(target_os = "linux")]
+    {
+        // wayland do not support changing display for now
+        if !IS_X11.load(Ordering::SeqCst) {
+            return None;
+        }
+    }
+
+    let lock = SYNC_DISPLAYS.lock().unwrap();
+    // If plugging out a monitor && lock.displays.get(idx) is None.
+    //  1. The client version < 1.2.4. The client side has to reconnect.
+    //  2. The client version > 1.2.4, The client side can handle the case becuase sync peer info message will be sent.
+    // But it is acceptable to for the user to reconnect manually, becuase the monitor is unplugged.
+    let d = lock.displays.get(idx)?;
+    if ndisplay != lock.displays.len() {
+        return Some(d.clone());
+    }
+    if !(d.x == x && d.y == y && d.width == w as i32 && d.height == h as i32) {
+        Some(d.clone())
+    } else {
+        None
+    }
 }
 
 #[inline]
@@ -74,45 +145,29 @@ pub fn is_privacy_mode_supported() -> bool {
     return false;
 }
 
-#[derive(Default)]
-struct StateDisplay {
-    synced_displays: Vec<DisplayInfo>,
-}
-
-impl super::service::Reset for StateDisplay {
-    fn reset(&mut self) {
-        self.synced_displays.clear();
-    }
-}
-
 pub fn new() -> GenericService {
-    let svc = EmptyExtraFieldService::new(NAME.to_owned(), false);
+    let svc = EmptyExtraFieldService::new(NAME.to_owned(), true);
-    GenericService::repeat::<StateDisplay, _, _>(&svc.clone(), 300, run);
+    GenericService::run(&svc.clone(), run);
     svc.sp
 }
 
-fn check_get_displays_changed_msg(last_synced_displays: &mut Vec<DisplayInfo>) -> Option<Message> {
+fn displays_to_msg(displays: Vec<DisplayInfo>) -> Message {
-    let displays = try_get_displays().ok()?;
-    if displays.len() == last_synced_displays.len() {
-        return None;
-    }
-
-    // Display to DisplayInfo
-    let displays = to_display_info(&displays);
-    if last_synced_displays.len() == 0 {
-        *last_synced_displays = displays;
-        None
-    } else {
     let mut pi = PeerInfo {
         ..Default::default()
     };
     pi.displays = displays.clone();
+    // current_display should not be used in server.
+    // It is set to 0 for compatibility with old clients.
     pi.current_display = 0;
     let mut msg_out = Message::new();
     msg_out.set_peer_info(pi);
-        *last_synced_displays = displays;
+    msg_out
-        Some(msg_out)
 }
+
+fn check_get_displays_changed_msg() -> Option<Message> {
+    check_update_displays(&try_get_displays().ok()?);
+    let displays = SYNC_DISPLAYS.lock().unwrap().get_update_sync_displays()?;
+    Some(displays_to_msg(displays))
 }
 
 #[cfg(all(windows, feature = "virtual_display_driver"))]
@@ -120,11 +175,28 @@ pub fn try_plug_out_virtual_display() {
     let _res = virtual_display_manager::plug_out_headless();
 }
 
-fn run(sp: EmptyExtraFieldService, state: &mut StateDisplay) -> ResultType<()> {
+fn run(sp: EmptyExtraFieldService) -> ResultType<()> {
-    if let Some(msg_out) = check_get_displays_changed_msg(&mut state.synced_displays) {
+    #[cfg(target_os = "linux")]
+    {
+        IS_X11.store(scrap::is_x11(), Ordering::SeqCst);
+    }
+
+    while sp.ok() {
+        sp.snapshot(|sps| {
+            if sps.has_subscribes() {
+                SYNC_DISPLAYS.lock().unwrap().is_synced = false;
+                bail!("new subscriber");
+            }
+            Ok(())
+        })?;
+
+        if let Some(msg_out) = check_get_displays_changed_msg() {
             sp.send(msg_out);
             log::info!("Displays changed");
         }
+        std::thread::sleep(Duration::from_millis(300));
+    }
+
     Ok(())
 }
 
@@ -167,8 +239,20 @@ pub(super) fn get_original_resolution(
     .into()
 }
 
-pub fn to_display_info(all: &Vec<Display>) -> Vec<DisplayInfo> {
+#[cfg(target_os = "linux")]
-    all.iter()
+pub(super) fn get_sync_displays() -> Vec<DisplayInfo> {
+    SYNC_DISPLAYS.lock().unwrap().displays.clone()
+}
+
+pub(super) fn get_display_info(idx: usize) -> Option<DisplayInfo> {
+    SYNC_DISPLAYS.lock().unwrap().displays.get(idx).cloned()
+}
+
+// Display to DisplayInfo
+// The DisplayInfo is be sent to the peer.
+pub(super) fn check_update_displays(all: &Vec<Display>) {
+    let displays = all
+        .iter()
         .map(|d| {
             let display_name = d.name();
             let original_resolution = get_original_resolution(&display_name, d.width(), d.height());
@@ -184,32 +268,34 @@ pub fn to_display_info(all: &Vec<Display>) -> Vec<DisplayInfo> {
                 ..Default::default()
             }
         })
-        .collect::<Vec<DisplayInfo>>()
+        .collect::<Vec<DisplayInfo>>();
+    SYNC_DISPLAYS.lock().unwrap().check_changed(displays);
 }
 
 pub fn is_inited_msg() -> Option<Message> {
     #[cfg(target_os = "linux")]
-    if !scrap::is_x11() {
+    if !IS_X11.load(Ordering::SeqCst) {
         return super::wayland::is_inited();
     }
     None
 }
 
-pub async fn get_displays() -> ResultType<Vec<DisplayInfo>> {
+pub async fn update_get_sync_displays() -> ResultType<Vec<DisplayInfo>> {
     #[cfg(target_os = "linux")]
     {
-        if !scrap::is_x11() {
+        if !IS_X11.load(Ordering::SeqCst) {
             return super::wayland::get_displays().await;
         }
     }
-    Ok(to_display_info(&try_get_displays()?))
+    check_update_displays(&try_get_displays()?);
+    Ok(SYNC_DISPLAYS.lock().unwrap().displays.clone())
 }
 
 #[inline]
 pub fn get_primary() -> usize {
     #[cfg(target_os = "linux")]
     {
-        if !scrap::is_x11() {
+        if !IS_X11.load(Ordering::SeqCst) {
             return match super::wayland::get_primary() {
                 Ok(n) => n,
                 Err(_) => 0,

src/server/video_service.rs

@@ -18,7 +18,16 @@
 // to-do:
 // https://slhck.info/video/2017/03/01/rate-control.html
 
-use super::{service::ServiceTmpl, video_qos::VideoQoS, *};
+#[cfg(target_os = "linux")]
+use super::display_service::IS_X11;
+use super::{
+    display_service::{check_display_changed, get_display_info},
+    service::ServiceTmpl,
+    video_qos::VideoQoS,
+    *,
+};
+#[cfg(target_os = "linux")]
+use crate::common::SimpleCallOnReturn;
 #[cfg(windows)]
 use crate::{platform::windows::is_process_consent_running, privacy_win_mag};
 use hbb_common::{
@@ -38,7 +47,7 @@ use scrap::{
     CodecName, Display, TraitCapturer,
 };
 #[cfg(target_os = "linux")]
-use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::atomic::Ordering;
 #[cfg(windows)]
 use std::sync::Once;
 use std::{
@@ -49,7 +58,6 @@ use std::{
 };
 
 pub const NAME: &'static str = "video";
-pub const OPTION_DISPLAY_CHANGED: &'static str = "changed";
 pub const OPTION_REFRESH: &'static str = "refresh";
 
 lazy_static::lazy_static! {
@@ -63,10 +71,6 @@ lazy_static::lazy_static! {
     pub static ref IS_FOREGROUND_WINDOW_ELEVATED: Arc<Mutex<bool>> = Default::default();
 }
 
-// https://github.com/rustdesk/rustdesk/discussions/6042, avoiding dbus call
-#[cfg(target_os = "linux")]
-static IS_X11: AtomicBool = AtomicBool::new(false);
-
 #[inline]
 pub fn notify_video_frame_fetched(conn_id: i32, frame_tm: Option<Instant>) {
     FRAME_FETCHED_NOTIFIER.0.send((conn_id, frame_tm)).ok();
@@ -165,35 +169,6 @@ pub fn new(idx: usize) -> GenericService {
     vs.sp
 }
 
-fn check_display_changed(
-    last_n: usize,
-    last_current: usize,
-    last_width: usize,
-    last_height: usize,
-) -> bool {
-    #[cfg(target_os = "linux")]
-    {
-        // wayland do not support changing display for now
-        if !IS_X11.load(Ordering::SeqCst) {
-            return false;
-        }
-    }
-
-    let displays = match try_get_displays() {
-        Ok(d) => d,
-        _ => return false,
-    };
-
-    let n = displays.len();
-    if n != last_n {
-        return true;
-    };
-    match displays.get(last_current) {
-        Some(d) => d.width() != last_width || d.height() != last_height,
-        None => true,
-    }
-}
-
 // Capturer object is expensive, avoiding to create it frequently.
 fn create_capturer(
     privacy_mode_id: i32,
@@ -323,7 +298,6 @@ fn check_uac_switch(privacy_mode_id: i32, capturer_privacy_mode_id: i32) -> Resu
 }
 
 pub(super) struct CapturerInfo {
-    pub name: String,
     pub origin: (i32, i32),
     pub width: usize,
     pub height: usize,
@@ -415,7 +389,6 @@ fn get_capturer(
         portable_service_running,
     )?;
     Ok(CapturerInfo {
-        name,
         origin,
         width,
         height,
@@ -431,16 +404,21 @@ fn run(vs: VideoService) -> ResultType<()> {
     #[cfg(not(any(target_os = "android", target_os = "ios")))]
     let _wake_lock = get_wake_lock();
 
-    #[cfg(target_os = "linux")]
+    // Wayland only support one video capturer for now. It is ok to call ensure_inited() here.
-    {
+    //
-        IS_X11.store(scrap::is_x11(), Ordering::SeqCst);
-    }
-
     // ensure_inited() is needed because clear() may be called.
     // to-do: wayland ensure_inited should pass current display index.
     // But for now, we do not support multi-screen capture on wayland.
     #[cfg(target_os = "linux")]
     super::wayland::ensure_inited()?;
+    #[cfg(target_os = "linux")]
+    let _wayland_call_on_ret = SimpleCallOnReturn {
+        b: true,
+        f: Box::new(|| {
+            super::wayland::clear();
+        }),
+    };
+
     #[cfg(windows)]
     let last_portable_service_running = crate::portable_service::client::running();
     #[cfg(not(windows))]
@@ -471,16 +449,6 @@ fn run(vs: VideoService) -> ResultType<()> {
     c.set_use_yuv(encoder.use_yuv());
     VIDEO_QOS.lock().unwrap().store_bitrate(encoder.bitrate());
 
-    if sp.is_option_true(OPTION_DISPLAY_CHANGED) {
-        log::debug!("Broadcasting display changed");
-        broadcast_display_changed(
-            display_idx,
-            &sp,
-            Some((c.name.clone(), c.origin.clone(), c.width, c.height)),
-        );
-        sp.set_option_bool(OPTION_DISPLAY_CHANGED, false);
-    }
-
     if sp.is_option_true(OPTION_REFRESH) {
         sp.set_option_bool(OPTION_REFRESH, false);
     }
@@ -518,7 +486,8 @@ fn run(vs: VideoService) -> ResultType<()> {
         }
         drop(video_qos);
 
-        if sp.is_option_true(OPTION_DISPLAY_CHANGED) || sp.is_option_true(OPTION_REFRESH) {
+        if sp.is_option_true(OPTION_REFRESH) {
+            let _ = try_broadcast_display_changed(&sp, display_idx, &c);
             bail!("SWITCH");
         }
         if codec_name != Encoder::negotiated_codec() {
@@ -540,14 +509,9 @@ fn run(vs: VideoService) -> ResultType<()> {
         let now = time::Instant::now();
         if last_check_displays.elapsed().as_millis() > 1000 {
             last_check_displays = now;
-
+            // This check may be redundant, but it is better to be safe.
-            // Capturer on macos does not return Err event the solution is changed.
+            // The previous check in `sp.is_option_true(OPTION_REFRESH)` block may be enough.
-            #[cfg(target_os = "macos")]
+            try_broadcast_display_changed(&sp, display_idx, &c)?;
-            if check_display_changed(c.ndisplay, c.current, c.width, c.height) {
-                sp.set_option_bool(OPTION_DISPLAY_CHANGED, true);
-                log::info!("Displays changed");
-                bail!("SWITCH");
-            }
         }
 
         frame_controller.reset();
@@ -624,13 +588,9 @@ fn run(vs: VideoService) -> ResultType<()> {
                 }
             }
             Err(err) => {
-                if check_display_changed(c.ndisplay, c.current, c.width, c.height) {
+                // This check may be redundant, but it is better to be safe.
-                    log::info!("Displays changed");
+                // The previous check in `sp.is_option_true(OPTION_REFRESH)` block may be enough.
-                    #[cfg(target_os = "linux")]
+                try_broadcast_display_changed(&sp, display_idx, &c)?;
-                    super::wayland::clear();
-                    sp.set_option_bool(OPTION_DISPLAY_CHANGED, true);
-                    bail!("SWITCH");
-                }
 
                 #[cfg(windows)]
                 if !c.is_gdi() {
@@ -638,7 +598,6 @@ fn run(vs: VideoService) -> ResultType<()> {
                     log::info!("dxgi error, fall back to gdi: {:?}", err);
                     continue;
                 }
-
                 return Err(err.into());
             }
             _ => {
@@ -671,9 +630,6 @@ fn run(vs: VideoService) -> ResultType<()> {
         }
     }
 
-    #[cfg(target_os = "linux")]
-    super::wayland::clear();
-
     Ok(())
 }
 
@@ -889,58 +845,52 @@ fn get_wake_lock() -> crate::platform::WakeLock {
 }
 
 #[inline]
-fn broadcast_display_changed(
+fn try_broadcast_display_changed(
-    display_idx: usize,
     sp: &GenericService,
-    display_meta: Option<(String, (i32, i32), usize, usize)>,
+    display_idx: usize,
+    cap: &CapturerInfo,
+) -> ResultType<()> {
+    if let Some(display) = check_display_changed(
+        cap.ndisplay,
+        cap.current,
+        (cap.origin.0, cap.origin.1, cap.width, cap.height),
     ) {
-    if let Some(msg_out) = make_display_changed_msg(display_idx, display_meta) {
+        log::info!("Display {} changed", display);
+        if let Some(msg_out) = make_display_changed_msg(display_idx, Some(display)) {
             sp.send(msg_out);
+            bail!("SWITCH");
         }
     }
-
+    Ok(())
-fn get_display_info_simple_meta(display_idx: usize) -> Option<(String, (i32, i32), usize, usize)> {
-    let displays = display_service::try_get_displays().ok()?;
-    if let Some(display) = displays.get(display_idx) {
-        Some((
-            display.name(),
-            display.origin(),
-            display.width(),
-            display.height(),
-        ))
-    } else {
-        None
-    }
 }
 
 pub fn make_display_changed_msg(
     display_idx: usize,
-    display_meta: Option<(String, (i32, i32), usize, usize)>,
+    opt_display: Option<DisplayInfo>,
 ) -> Option<Message> {
-    let mut misc = Misc::new();
+    let display = match opt_display {
-    let (name, origin, width, height) = match display_meta {
         Some(d) => d,
-        None => get_display_info_simple_meta(display_idx)?,
+        None => get_display_info(display_idx)?,
     };
-    let original_resolution = display_service::get_original_resolution(&name, width, height);
+    let mut misc = Misc::new();
     misc.set_switch_display(SwitchDisplay {
         display: display_idx as _,
-        x: origin.0,
+        x: display.x,
-        y: origin.1,
+        y: display.y,
-        width: width as _,
+        width: display.width,
-        height: height as _,
+        height: display.height,
         cursor_embedded: display_service::capture_cursor_embedded(),
         #[cfg(not(any(target_os = "android", target_os = "ios")))]
         resolutions: Some(SupportedResolutions {
-            resolutions: if name.is_empty() {
+            resolutions: if display.name.is_empty() {
                 vec![]
             } else {
-                crate::platform::resolutions(&name)
+                crate::platform::resolutions(&display.name)
             },
             ..SupportedResolutions::default()
         })
         .into(),
-        original_resolution,
+        original_resolution: display.original_resolution,
         ..Default::default()
     });
     let mut msg_out = Message::new();

src/server/wayland.rs

@@ -146,7 +146,8 @@ pub(super) async fn check_init() -> ResultType<()> {
                 let num = all.len();
                 let primary = super::display_service::get_primary_2(&all);
                 let current = primary;
-                let mut displays = super::display_service::to_display_info(&all);
+                super::display_service::check_update_displays(&all);
+                let mut displays = super::display_service::get_sync_displays();
                 for display in displays.iter_mut() {
                     display.cursor_embedded = is_cursor_embedded();
                 }
@@ -173,10 +174,13 @@ pub(super) async fn check_init() -> ResultType<()> {
                     Some(result) if !result.is_empty() => {
                         let resolution: Vec<&str> = result.split(" ").collect();
                         let w: i32 = resolution[0].parse().unwrap_or(origin.0 + width as i32);
-                        let h: i32 = resolution[2].trim_end_matches(",").parse().unwrap_or(origin.1 + height as i32);
+                        let h: i32 = resolution[2]
+                            .trim_end_matches(",")
+                            .parse()
+                            .unwrap_or(origin.1 + height as i32);
                         (w, h)
                     }
-                    _ => (origin.0 + width as i32, origin.1 + height as i32)
+                    _ => (origin.0 + width as i32, origin.1 + height as i32),
                 };
 
                 minx = 0;
@@ -267,9 +271,6 @@ pub(super) fn get_capturer() -> ResultType<super::video_service::CapturerInfo> {
             let cap_display_info = &*cap_display_info;
             let rect = cap_display_info.rects[cap_display_info.current];
             Ok(super::video_service::CapturerInfo {
-                name: cap_display_info.displays[cap_display_info.current]
-                    .name
-                    .clone(),
                 origin: rect.0,
                 width: rect.1,
                 height: rect.2,

src/tray.rs

@@ -1,9 +1,12 @@
-use crate::{client::translate, ipc::Data};
+use crate::client::translate;
-use hbb_common::{allow_err, log, tokio};
+#[cfg(windows)]
-use std::{
+use crate::ipc::Data;
-    sync::{Arc, Mutex},
+#[cfg(windows)]
-    time::Duration,
+use hbb_common::tokio;
-};
+use hbb_common::{allow_err, log};
+use std::sync::{Arc, Mutex};
+#[cfg(windows)]
+use std::time::Duration;
 
 pub fn start_tray() {
     allow_err!(make_tray());