LGUG2Z / komorebi

A tiling window manager for Windows ๐Ÿ‰
https://lgug2z.github.io/komorebi/
Other
9.5k stars 191 forks source link

[BUG]: Komorebi doesn't start on a VDI using RDP #883

Closed azinsharaf closed 4 months ago

azinsharaf commented 4 months ago

Describe the bug Upgrading to 0.1.27-dev.0 breaks komorebi on a VM machine connected via RDP.

To Reproduce running komorebic start --config "C:\Users\<username>\.config\komorebi\komorebi.json" --whkd returns this:

Start-Process 'komorebi.exe' -ArgumentList '--config="C:\Users\asharaf\.config\komorebi\komorebi.json"' -WindowStyle hidden
Waiting for komorebi.exe to start...komorebi.exe did not start... Trying again
Start-Process 'komorebi.exe' -ArgumentList '--config="C:\Users\asharaf\.config\komorebi\komorebi.json"' -WindowStyle hidden
Waiting for komorebi.exe to start...komorebi.exe did not start... Trying again
Start-Process 'komorebi.exe' -ArgumentList '--config="C:\Users\asharaf\.config\komorebi\komorebi.json"' -WindowStyle hidden
Waiting for komorebi.exe to start...komorebi.exe did not start... Trying again

Running komorebi.exe directly for detailed error output

error: unexpected argument ''--config="C:\Users\asharaf\.config\komorebi\komorebi.json"'' found

Usage: komorebi.exe [OPTIONS]

For more information, try '--help'.

Operating System

OS Name:                   Microsoft Windows 10 Pro
OS Version:                10.0.19045 N/A Build 19045

komorebic check Output

KOMOREBI_CONFIG_HOME detected: C:\Users\asharaf\.config\komorebi

Looking for configuration files in C:\Users\asharaf\.config\komorebi

Found komorebi.json; this file can be passed to the start command with the --config flag

Found C:\Users\asharaf\.config\whkd\whkdrc; key bindings will be loaded from here when whkd is started, and you can start it automatically using the --whkd flag

Additional context To troubleshoot it further, i built the latest release 0.1.27-dev from the source. then I cloned the https://github.com/LGUG2Z/win32-display-data and in its src folder i created a main.rs file with the following content:

fn main() {
    win32_display_data::connected_displays_all();
}

then i added two debug lines in device.rs file. The snippet is:

pub fn connected_displays_all() -> impl Iterator<Item = Result<Device, SysError>> {
    unsafe {
        let device_info_map = match get_device_info_map() {
            Ok(info) => info,
            Err(e) => return Either::Right(once(Err(e))),
        };

        let hmonitors = match enum_display_monitors() {
            Ok(monitors) => monitors,
            Err(e) => return Either::Right(once(Err(e))),
        };
    dbg!(&hmonitors);

        Either::Left(hmonitors.into_iter().flat_map(move |hmonitor| {
            let display_devices = match get_display_devices_from_hmonitor(hmonitor) {
                Ok(p) => p,
                Err(e) => return vec![Err(e)],
            };

       dbg!(&display_devices);

then ran cargo run in its src folder. this is the result:


 โ•ญโ”€ ~\Downloads\win32-display-data-master\win32-display-data-master\src 
 โ•ฐโ”€ฮป cargo run
   Compiling win32-display-data v0.1.0 (C:\Users\asharaf\Downloads\win32-display-data-master\win32-display-data-master)
warning: unused implementer of `Iterator` that must be used
 --> src\main.rs:2:5
  |
2 |     win32_display_data::connected_displays_all();
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: iterators are lazy and do nothing unless consumed
  = note: `#[warn(unused_must_use)]` on by default

warning: `win32-display-data` (bin "win32-display-data") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 1.61s
     Running `C:\Users\asharaf\Downloads\win32-display-data-master\win32-display-data-master\target\debug\win32-display-data.exe`
azinsharaf commented 4 months ago

on version 0.1.27:

โ•ฐโ”€ฮป komorebi.exe
2024-06-21T17:14:35.331377Z  INFO foreground_lock_timeout: komorebi::windows_api: current value of ForegroundLockTimeout is 0
2024-06-21T17:14:35.335438Z  INFO komorebi: creating window manager from static configuration file: C:\Users\asharaf\.config\komorebi\komorebi.json
2024-06-21T17:14:35.345024Z  INFO init: komorebi::window_manager: initialising
Error:
   0: there is no monitor with that index

Location:
   komorebi\src\window_manager.rs:428

   1: BaseThreadInitThunk<unknown>
      at <unknown source file>:<unknown line>
   2: RtlUserThreadStart<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Run with RUST_BACKTRACE=full to include source snippets.

komorebic monitor-information

thread 'main' panicked at komorebic\src/main.rs:1255:23:
No connection could be made because the target machine actively refused it. (os error 10061)
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
LGUG2Z commented 4 months ago

I think this one is gonna require me spinning up a VDI somewhere ๐Ÿ˜“

Is there a way I could reproduce a similar environment on a Azure?

azinsharaf commented 4 months ago

this is a work machine so i am not sure how you can spin it up free (or cheap). We are using Workspot Inc. (using win 10) as an enterprise solution. Maybe using Azure Virtual Desktop could be a similar environment?

azinsharaf commented 4 months ago

@LGUG2Z I am wondering if you have had a chance to work on this. I am not able to use komorebi anymore. I understand that it isn't a common case, but hopefully there would be a solution.

LGUG2Z commented 4 months ago
image

I tried reproducing this on an Amazon Lightsail Windows Server 2022 instance but I was not able to ^

LGUG2Z commented 4 months ago

Can you try running your experiment from before again, but this time explicitly call the iterator with a for loop? I think the underlying functions weren't actually called based on the output:

  = note: iterators are lazy and do nothing unless consumed
  = note: `#[warn(unused_must_use)]` on by default
fn main() {
    let x = win32_display_data::connected_displays_all();
    for y in x {
        dbg!(y);
    }
}
azinsharaf commented 4 months ago
 โ•ญโ”€ win32-display-data\src on master 
 โ•ฐโ”€ฮป cargo run
warning: unused `Result` that must be used
 --> src\main.rs:4:9
  |
4 |         dbg!(y);
  |         ^^^^^^^
  |
  = note: this `Result` may be an `Err` variant, which should be handled
  = note: `#[warn(unused_must_use)]` on by default
  = note: this warning originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `win32-display-data` (bin "win32-display-data") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
     Running `C:\Users\asharaf\win32-display-data\target\debug\win32-display-data.exe`
[src\main.rs:4:9] y = Err(
    ListingDevicesFailed(
        QueryDisplayConfigFailed(
            Error {
                code: HRESULT(0x80070057),
                message: "The parameter is incorrect.",
            },
        ),
    ),
)
LGUG2Z commented 4 months ago

Can you try doing a search and replace in device.rs for QDC_ONLY_ACTIVE_PATHS to QDC_ALL_PATHS and then try running the same main.rs snippet again?

azinsharaf commented 4 months ago

getting the exact same error.

LGUG2Z commented 4 months ago

https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-querydisplayconfig#parameters

The only other valid parameter in the docs for this function is QDC_DATABASE_CURRENT, worth giving it a try too ๐Ÿค”

    let mut path_count = 0;
    let mut mode_count = 0;
    GetDisplayConfigBufferSizes(QDC_DATABASE_CURRENT, &mut path_count, &mut mode_count)
        .ok()
        .map_err(SysError::GetDisplayConfigBufferSizesFailed)?;
    let mut display_paths = vec![DISPLAYCONFIG_PATH_INFO::default(); path_count as usize];
    let mut display_modes = vec![DISPLAYCONFIG_MODE_INFO::default(); mode_count as usize];
    QueryDisplayConfig(
        QDC_DATABASE_CURRENT,
        &mut path_count,
        display_paths.as_mut_ptr(),
        &mut mode_count,
        display_modes.as_mut_ptr(),
        Some(&mut DISPLAYCONFIG_TOPOLOGY_INTERNAL),
    )
    .ok()
    .map_err(SysError::QueryDisplayConfigFailed)?;

You should probably try different DISPLAYCONFIG_TOPOLOGY values too: https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ne-wingdi-displayconfig_topology_id

LGUG2Z commented 4 months ago

Actually I wonder if your specific VDI configuration falls under this: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-querydisplayconfig#head-mounted-and-specialized-monitors

azinsharaf commented 4 months ago

https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-querydisplayconfig#parameters

The only other valid parameter in the docs for this function is QDC_DATABASE_CURRENT, worth giving it a try too ๐Ÿค”

    let mut path_count = 0;
    let mut mode_count = 0;
    GetDisplayConfigBufferSizes(QDC_DATABASE_CURRENT, &mut path_count, &mut mode_count)
        .ok()
        .map_err(SysError::GetDisplayConfigBufferSizesFailed)?;
    let mut display_paths = vec![DISPLAYCONFIG_PATH_INFO::default(); path_count as usize];
    let mut display_modes = vec![DISPLAYCONFIG_MODE_INFO::default(); mode_count as usize];
    QueryDisplayConfig(
        QDC_DATABASE_CURRENT,
        &mut path_count,
        display_paths.as_mut_ptr(),
        &mut mode_count,
        display_modes.as_mut_ptr(),
        Some(&mut DISPLAYCONFIG_TOPOLOGY_INTERNAL),
    )
    .ok()
    .map_err(SysError::QueryDisplayConfigFailed)?;

You should probably try different DISPLAYCONFIG_TOPOLOGY values too: https://learn.microsoft.com/en-us/windows/win32/api/wingdi/ne-wingdi-displayconfig_topology_id

i tried some of them , same result.

azinsharaf commented 4 months ago

Actually I wonder if your specific VDI configuration falls under this: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-querydisplayconfig#head-mounted-and-specialized-monitors

i doubt if my machine as an specialized one (like medical device display, etc.).

LGUG2Z commented 4 months ago

Not the most elegant solution but I think this should work: https://github.com/LGUG2Z/komorebi/commit/d2782f3b919182f8091ec388792abc211d668554

https://github.com/LGUG2Z/win32-display-data/commit/fc64bd1b9973acb81b570a66e6458f36ec8f18a2

azinsharaf commented 4 months ago

thanks for working on this. I built it from the source again but getting the same there is no monitor with that index error again.

> komorebi
2024-07-13T19:07:22.223199Z  INFO foreground_lock_timeout: komorebi::windows_api: current value of ForegroundLockTimeout is 0
2024-07-13T19:07:22.226704Z  INFO komorebi: creating window manager from static configuration file: C:\Users\asharaf\.config\komorebi\komorebi.json
2024-07-13T19:07:22.227157Z  INFO komorebi::border_manager: purging known borders: []
2024-07-13T19:07:22.236209Z  INFO init: komorebi::window_manager: initialising
Error:
   0: there is no monitor with that index

Location:
   komorebi\src\window_manager.rs:447

  โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ” BACKTRACE โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”โ”
                                โ‹ฎ 4 frames hidden โ‹ฎ
   5: komorebi::window_manager::WindowManager::enforce_workspace_rules::h2edad96ce26670d1
      at <unknown source file>:<unknown line>
   6: komorebi::process_command::<impl komorebi::window_manager::WindowManager>::handle_workspace_rules::h8601b85a23f29567
      at <unknown source file>:<unknown line>
   7: komorebi::static_config::StaticConfig::postload::h73532d8ba5e44e8b
      at <unknown source file>:<unknown line>
   8: <komorebi::Opts as clap_builder::derive::CommandFactory>::command::hfc37937ef0602cea
      at <unknown source file>:<unknown line>
   9: std::sys_common::backtrace::__rust_begin_short_backtrace::h32d5958cee44481c
      at <unknown source file>:<unknown line>
  10: std::rt::lang_start::{{closure}}::hdec92c7375ba689a
      at <unknown source file>:<unknown line>
  11: std::rt::lang_start_internal::closure$2<unknown>
      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library\std\src\rt.rs:148
  12: std::panicking::try::do_call<unknown>
      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library\std\src\panicking.rs:552
  13: std::panicking::try<unknown>
      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library\std\src\panicking.rs:516
  14: std::panic::catch_unwind<unknown>
      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library\std\src\panic.rs:142
  15: std::rt::lang_start_internal<unknown>
      at /rustc/07dca489ac2d933c78d3c5158e3f43beefeb02ce/library\std\src\rt.rs:148
  16: main<unknown>
      at <unknown source file>:<unknown line>
  17: invoke_main<unknown>
      at D:\a01\_work\9\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
  18: __scrt_common_main_seh<unknown>
      at D:\a01\_work\9\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
  19: BaseThreadInitThunk<unknown>
      at <unknown source file>:<unknown line>
  20: RtlUserThreadStart<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Run with RUST_BACKTRACE=full to include source snippets.
azinsharaf commented 4 months ago

but running the src\main.rs in win32-display (after adding debugging lines) returns this:

 โ•ฐโ”€ฮป cargo run
   Compiling win32-display-data v0.1.0 (C:\Users\asharaf\win32-display-data)
warning: unused `Result` that must be used
 --> src\main.rs:4:9
  |
4 |         dbg!(y);
  |         ^^^^^^^
  |
  = note: this `Result` may be an `Err` variant, which should be handled
  = note: `#[warn(unused_must_use)]` on by default
  = note: this warning originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `win32-display-data` (bin "win32-display-data") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 1.59s
     Running `C:\Users\asharaf\win32-display-data\target\debug\win32-display-data.exe`
[src\device.rs:144:6] &hmonitors = [
    HMONITOR(
        65537,
    ),
]
[src\device.rs:152:10] &display_devices = []
LGUG2Z commented 4 months ago

I think we are getting closer, can you try commenting out this line? https://github.com/LGUG2Z/win32-display-data/blob/fc64bd1b9973acb81b570a66e6458f36ec8f18a2/src/device.rs#L359

azinsharaf commented 4 months ago

output:

 โ•ฐโ”€ฮป cargo run
warning: unused import: `windows::Win32::Graphics::Gdi::DISPLAY_DEVICE_ACTIVE`
  --> src\device.rs:37:5
   |
37 | use windows::Win32::Graphics::Gdi::DISPLAY_DEVICE_ACTIVE;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

warning: function `flag_set` is never used
   --> src\device.rs:131:4
    |
131 | fn flag_set<T: std::ops::BitAnd<Output = T> + PartialEq + Copy>(t: T, flag: T) -> bool {
    |    ^^^^^^^^
    |
    = note: `#[warn(dead_code)]` on by default

warning: `win32-display-data` (lib) generated 2 warnings (run `cargo fix --lib -p win32-display-data` to apply 1 suggestion)
warning: unused `Result` that must be used
 --> src\main.rs:4:9
  |
4 |         dbg!(y);
  |         ^^^^^^^
  |
  = note: this `Result` may be an `Err` variant, which should be handled
  = note: `#[warn(unused_must_use)]` on by default
  = note: this warning originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `win32-display-data` (bin "win32-display-data") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
     Running `C:\Users\asharaf\win32-display-data\target\debug\win32-display-data.exe`
[src\device.rs:144:6] &hmonitors = [
    HMONITOR(
        65537,
    ),
]
[src\device.rs:152:10] &display_devices = []
LGUG2Z commented 4 months ago

I have tried replicating what the old code for monitor information resolution does here if you can try running the debug loop against it: https://github.com/LGUG2Z/win32-display-data/commit/087c128e69b29e3def32603e4667b4c57ca7954c

azinsharaf commented 4 months ago

it returns more information:

 โ•ฐโ”€ฮป cargo run
warning: unused `Result` that must be used
 --> src\main.rs:4:9
  |
4 |         dbg!(y);
  |         ^^^^^^^
  |
  = note: this `Result` may be an `Err` variant, which should be handled
  = note: `#[warn(unused_must_use)]` on by default
  = note: this warning originates in the macro `dbg` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: `win32-display-data` (bin "win32-display-data") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.06s
     Running `C:\Users\asharaf\win32-display-data\target\debug\win32-display-data.exe`
[src\main.rs:4:9] y = Ok(
    Device {
        hmonitor: 65537,
        size: RECT {
            left: 0,
            top: 0,
            right: 3440,
            bottom: 1440,
        },
        work_area_size: RECT {
            left: 0,
            top: 0,
            right: 3440,
            bottom: 1400,
        },
        device_name: "\\\\.\\DISPLAY1",
        device_description: "RDPUDD Chained DD",
        device_key: "\\REGISTRY\\Machine\\System\\CurrentControlSet\\Services\\RDPUDD\\Device0",
        device_path: "",
        output_technology: None,
    },
)
LGUG2Z commented 4 months ago

I think this is the fix! ๐ŸŽ‰

azinsharaf commented 4 months ago

excited! Do you merge the branch or i can use the new rev # to test it?

LGUG2Z commented 4 months ago

https://github.com/LGUG2Z/komorebi/commit/3c8a6cb7bd7cd8a29602155012eeef6f8193d8c1 Updated here ๐Ÿคž

azinsharaf commented 4 months ago

:(

 โ•ฐโ”€ฮป komorebi
2024-07-13T23:24:24.314885Z  INFO foreground_lock_timeout: komorebi::windows_api: current value of ForegroundLockTimeout is 0
2024-07-13T23:24:24.318570Z  INFO komorebi: creating window manager from static configuration file: C:\Users\asharaf\.config\komorebi\komorebi.json
2024-07-13T23:24:24.319006Z  INFO komorebi::border_manager: purging known borders: []
2024-07-13T23:24:24.327386Z  INFO init: komorebi::window_manager: initialising
2024-07-13T23:24:24.327641Z ERROR init: komorebi: panicked at komorebi\src\windows_api.rs:244:19:
removal index (is 18446744073709551615) should be < len (is 0) panic.file="komorebi\\src\\windows_api.rs" panic.line=244 panic.column=19
LGUG2Z commented 4 months ago

Almost there ๐Ÿคž https://github.com/LGUG2Z/komorebi/commit/7b854616f78bac7cfea3fb8d2d83a76ebf16bcd1

azinsharaf commented 4 months ago
2024-07-13T23:55:08.063502Z ERROR init: komorebi: panicked at komorebi\src\windows_api.rs:819:23:
removal index (is 18446744073709551615) should be < len (is 0) panic.file="komorebi\\src\\windows_api.rs" panic.line=819 panic.column=23
LGUG2Z commented 4 months ago

Think I got all of the calls where we try to compute the device_id here: https://github.com/LGUG2Z/komorebi/commit/74350890106521463fc4e97c4c60591261092ef6

tl;dr of the root cause: RDPUDD Chained DD virtual monitor devices don't seem to ever have a device_id (unlike Remote Display Adapter devices which do)

azinsharaf commented 4 months ago

it works now! Thank you so much!! <3

LGUG2Z commented 4 months ago

I'll clean this up and add it for the v0.1.28 release ๐ŸŽ‰