Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
azinsharaf opened this issue Jun 17, 2024 · 29 comments
Closed

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

azinsharaf opened this issue Jun 17, 2024 · 29 comments
Labels
bug Something isn't working

Comments

@azinsharaf
Copy link

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 azinsharaf added the bug Something isn't working label Jun 17, 2024
@azinsharaf azinsharaf changed the title [BUG]: Short descriptive title [BUG]: Komorebi doesn't start on a VDI using RDP Jun 17, 2024
@azinsharaf
Copy link
Author

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
Copy link
Owner

LGUG2Z commented Jun 21, 2024

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
Copy link
Author

azinsharaf commented Jun 21, 2024

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
Copy link
Author

@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
Copy link
Owner

LGUG2Z commented Jul 11, 2024

image

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

@LGUG2Z
Copy link
Owner

LGUG2Z commented Jul 11, 2024

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
Copy link
Author

 ╭─ 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
Copy link
Owner

LGUG2Z commented Jul 12, 2024

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
Copy link
Author

getting the exact same error.

@LGUG2Z
Copy link
Owner

LGUG2Z commented Jul 12, 2024

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
Copy link
Owner

LGUG2Z commented Jul 12, 2024

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
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

LGUG2Z commented Jul 13, 2024

Not the most elegant solution but I think this should work: d2782f3

LGUG2Z/win32-display-data@fc64bd1

@azinsharaf
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

LGUG2Z commented Jul 13, 2024

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
Copy link
Author

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
Copy link
Owner

LGUG2Z commented Jul 13, 2024

I have tried replicating what the old code for monitor information resolution does here if you can try running the debug loop against it: LGUG2Z/win32-display-data@087c128

@azinsharaf
Copy link
Author

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
Copy link
Owner

LGUG2Z commented Jul 13, 2024

I think this is the fix! 🎉

@azinsharaf
Copy link
Author

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

@LGUG2Z
Copy link
Owner

LGUG2Z commented Jul 13, 2024

3c8a6cb Updated here 🤞

@azinsharaf
Copy link
Author

:(

 ╰─λ 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
Copy link
Owner

LGUG2Z commented Jul 13, 2024

Almost there 🤞 7b85461

@azinsharaf
Copy link
Author

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
Copy link
Owner

LGUG2Z commented Jul 14, 2024

Think I got all of the calls where we try to compute the device_id here: 7435089

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
Copy link
Author

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

@LGUG2Z
Copy link
Owner

LGUG2Z commented Jul 14, 2024

I'll clean this up and add it for the v0.1.28 release 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants