microsoft / windows-rs

Rust for Windows
https://kennykerr.ca/rust-getting-started/
Apache License 2.0
10.33k stars 486 forks source link

Missing `SetFileTypeChoices` method for `FileSavePicker` #3225

Closed JunkuiZhang closed 3 weeks ago

JunkuiZhang commented 3 weeks ago

Summary

As mentioned in the remarks section of Microsoft's documentation at here, you must specify one or more file types using the FileTypeChoices property before calling the PickSaveFileAsync method. Typically, I would use the SetFileTypeChoices method to set this property, but I haven't been able to find it. Could you please advise me on how to set this property?

Crate manifest

No response

Crate code

No response

kennykerr commented 3 weeks ago

Looks like the method is called FileTypeChoices and requires the "Foundation_Collections" feature.

https://github.com/microsoft/windows-rs/blob/88a80f2022ce6796e230922a5c2158a05bc7b0ba/crates/libs/windows/src/Windows/Storage/Pickers/mod.rs#L745-L746

JunkuiZhang commented 3 weeks ago

Well, that method retrieves the file types, but I want to set that property. Wait, do I need to add them manually? Something like this?

let map = dialog.FileTypeChoices()?;
map.add(...);
kennykerr commented 3 weeks ago

That is what the sample does in the docs.

JunkuiZhang commented 3 weeks ago

Sorry to bother you, there are Set*** methods for most properties in windows-rs. So, at the first place I thought it should be something like this:

let map = dialog.FileTypeChoices()?;
map.add(...);
dialog.SetFileTypeChoices(map)?;

So I thought the method was missing, my apologies!

kennykerr commented 3 weeks ago

No prob, some of these APIs can be a little confusing. This "property" cannot be set but instead always returns a reference that must be updated to "set" the property. 🤷

JunkuiZhang commented 3 weeks ago

Really sorry to bother you again, but could you please guide me how to create an IVector?

let picker = FileSavePicker::new()?;
let map: IMap<HSTRING, IVector<HSTRING>> = picker.FileTypeChoices()?;

When I call map.Insert(K, V), the V parameter is expected to be Param<IVector>. I tried the following:

let vector_view: IVectorView<HSTRING> = rust_vec.try_into()?;
map.insert(&HSTRING::from("Example"), &vector_view)?;

But it didn't work. I also searched for a solution but couldn't find one. Could you please guide me on how to create an IVector for map.Insert(K, V)?

kennykerr commented 3 weeks ago

Ah, well https://github.com/microsoft/windows-rs/releases/0.46.0 added stock collection implementations but only for the "view" collection interfaces. I still need to add stock collection implementations for the read-write collection interfaces.

kennykerr commented 3 weeks ago

The "Windows.Storage" APIs do however have a number of problems. You may be better off using the older and more reliable COM APIs that I wrote about here:

https://asp-blogs.azurewebsites.net/kennykerr/Windows-Vista-for-Developers-_1320_-Part-6-_1320_-The-New-File-Dialogs

Here's a simple example:

use windows::{core::*, Win32::System::Com::*, Win32::UI::Shell::Common::*, Win32::UI::Shell::*};

fn main() -> Result<()> {
    unsafe {
        CoIncrementMTAUsage()?;

        let dialog: IFileSaveDialog = CoCreateInstance(&FileSaveDialog, None, CLSCTX_ALL)?;

        dialog.SetFileTypes(&[
            COMDLG_FILTERSPEC {
                pszName: w!("Text files"),
                pszSpec: w!("*.txt"),
            },
            COMDLG_FILTERSPEC {
                pszName: w!("All files"),
                pszSpec: w!("*.*"),
            },
        ])?;

        dialog.Show(None)?;
        Ok(())
    }
}
kennykerr commented 3 weeks ago

Here's a complete example: #3226

JunkuiZhang commented 3 weeks ago

Thank you so much for your advice. Actually, I’m contributing to an open-source project called Zed, where I mainly work on the Windows implementation. The code for opening the save file dialog that I implemented is using IFileSaveDialog which you suggest.

However, I've encountered a particularly baffling bug recently, and I’m completely stumped. That’s why I started experimenting with the FilePicker series of APIs in winrt. The bug occurs when my Windows input method is set to Microsoft Pinyin, and the dialog created by IFileSaveDialog freezes. It doesn’t respond to any keyboard or mouse input, yet the caret in the text box for the file name within the dialog still blinks, which suggests that the dialog isn't entirely unresponsive, but just isn't responding to any input. My guess is that, for some reason, the IME window is capturing all keyboard and mouse input.

The reproduce steps:

Here is the relevant code:

// `CoInit` is called when app launched
fn file_save_dialog(directory: PathBuf) -> Result<Option<PathBuf>> {
    let dialog: IFileSaveDialog = unsafe { CoCreateInstance(&FileSaveDialog, None, CLSCTX_ALL)? };
    if let Some(full_path) = directory.canonicalize().log_err() {
        let full_path = full_path.to_string_lossy().to_string();
        if !full_path.is_empty() {
            let path_item: IShellItem =
                unsafe { SHCreateItemFromParsingName(&HSTRING::from(&full_path), None)? };
            unsafe { dialog.SetFolder(&path_item).log_err() };
        }
    }
    unsafe {
        if dialog.Show(None).is_err() {
            // User cancelled
            return Ok(None);
        }
    }
    let shell_item = unsafe { dialog.GetResult()? };
    let file_path_string = unsafe { shell_item.GetDisplayName(SIGDN_DESKTOPABSOLUTEPARSING)?.to_string()? };
    Ok(Some(PathBuf::from(file_path_string)))
}

The video, when the bottom right corner shows ENG, it indicates that the IME is turned off. When it shows 中 拼, it means that Microsoft Pinyin is active.

https://github.com/user-attachments/assets/9ff23717-180d-4b35-acc6-7f392c7d75ac

JunkuiZhang commented 3 weeks ago

It has also been reported that older versions of the Pinyin input method do not have this issue. And I have tried every possible solution I could think of, such as setting the owner window for the dialog, adjusting options, and more. I've also searched online extensively but haven't found a viable solution.

Additionally, I reviewed the code from Chrome, but it doesn't differ significantly from my current implementation. Since Chrome and VSCode don't seem to have this issue, I suspect they might have applied a fix elsewhere. Further investigation is still needed.

I wanted to provide this bit of background information, along with some of my frustrations. Really appreciate your help and advice, thank you!

riverar commented 3 weeks ago

@JunkuiZhang Hm, I can't reproduce this with Pinyin input method enabled. Can you reproduce this with the sample @kennykerr linked to? Can you also share what OS version you're running in the video above? Is the window usable if it appears outside the bounds of the Zed main window? (You can shift-right-click on the taskbar icon > move > [keyboard keys] to shift it around.)

JunkuiZhang commented 3 weeks ago

@JunkuiZhang Hm, I can't reproduce this with Pinyin input method enabled. Can you reproduce this with the sample @kennykerr linked to?

@kennykerr's example works perfectly on my machine, dose not have this bug. (I suspect it might be related to the message loop?).

Can you also share what OS version you're running in the video above?

My Windows version is Windows 11 Home Chinese Edition.

Screenshot 2024-08-28 014533

Is the window usable if it appears outside the bounds of the Zed main window? (You can shift-right-click on the taskbar icon > move > [keyboard keys] to shift it around.)

The save file dialog still freezes even when it doesn't overlap with the Zed window, video:

https://github.com/user-attachments/assets/efecf7b4-5073-4e9f-81cf-8ab7471d868b

BTW, it is odd that the open file dialog (IFileOpenDialog) doesn't have this issue.

riverar commented 3 weeks ago

@JunkuiZhang I can reproduce this on 27686.rs_prerelease.240809-2254, but only with the new Microsoft Pinyin candidate window enabled (which is on by default). If I turn it off and fallback to the old version, Zed works as expected.

I can't reproduce this in the Rust sample, so more investigation is needed.

image (The white text is a Windows bug.)

JunkuiZhang commented 3 weeks ago

@riverar Sorry for the late reply. Since I'm in China, it was already 5 a.m. here when you sent your last message, so I was probably dreaming at the time.

Zed has several bugs on Windows related to the message loop, not just this one. For example, consider the following code:

loop {
    while PeekMessageW(&mut msg, None, 0, 0, PM_REMOVE) {
        TranslateMessage(...);
        DispatchMessageW(...);
    }
    window.draw();
}

In this case, if vertical synchronization is enabled, the window should render at a steady 60fps. However, in Zed, even with vertical sync enabled, the rendering rate skyrockets uncontrollably, regardless of whether the rendering backend is Vulkan, DX11, or DX12.

@JunkuiZhang I can reproduce this on 27686.rs_prerelease.240809-2254, but only with the new Microsoft Pinyin candidate window enabled (which is on by default). If I turn it off and fallback to the old version, Zed works as expected.

I can't reproduce this in the Rust sample, so more investigation is needed.

image (The white text is a Windows bug.)

Yes, if I turn on the compatibility button, IFileSaveDialog works fine. The strangest part is that this bug only occurs when IFileSaveDialog is open, while IFileOpenDialog is unaffected.

riverar commented 3 weeks ago

@jonwis Anyone working on IME we can route to? I filled out the user-initiated feedback survey that appeared when switching back to the legacy IME, but that likely went into a black hole.

JunkuiZhang commented 1 week ago

@riverar I sincerely apologize for the disturbance. After conducting some tests, I discovered that the issue is caused by the DispatcherQueue. I have created a minimal reproducible repository to demonstrate the problem.

To reproduce the issue: make sure the Pinyin input method is enabled, then run the project using cargo run. Press any key to open the FileSaveDialog and observe how it freezes.

Specifically, Zed currently uses DispatcherQueue to execute code on the main thread, and opening the FileSaveDialog is dispatched to the main thread via DispatcherQueue::TryEnqueue.

My question might be a bit naive, sorry! Is that using DispatcherQueue in this way is incorrect? If so, how can I change it?

kennykerr commented 1 week ago

Not sure if this will help, but typically you need to call CreateDispatcherQueueController early in the main function as it may be used by the UI stack. You can then later get a hold of the dispatcher queue using GetForCurrentThread.

JunkuiZhang commented 1 week ago

Not sure if this will help, but typically you need to call CreateDispatcherQueueController early in the main function as it may be used by the UI stack.

Really thanks for your advices! In zed we call CreateDispatcherQueueController at the first line, so I guess this dose not help.

You can then later get a hold of the dispatcher queue using GetForCurrentThread.

Are you suggesting using DispatcherQueue::GetForCurrentThread()? I had tried using this function earlier, still no luck.

JunkuiZhang commented 1 week ago

After changing to call CreateDispatcherQueueController early and use DispatcherQueue::GetForCurrentThread(), I can still reproduce this bug. The repo.

kennykerr commented 1 week ago

I wasn't able to reproduce it, but I know nothing of Chinese input. Perhaps @oldnewthing has observed this before?

JunkuiZhang commented 1 week ago

I wasn't able to reproduce it, but I know nothing of Chinese input. Perhaps @oldnewthing has observed this before?

The key thing here is turning on Pinyin input method, and as @riverar said, this only happens with the new Microsoft Pinyin candidate window enabled (which is on by default). If it is turned off and fallback to the old version, the bug is gone.

https://github.com/user-attachments/assets/83cf3b39-eb79-4144-8ff1-6f08032471e1

JunkuiZhang commented 1 week ago

BTW, really thank you for the help!

kennykerr commented 1 week ago

Ah, yes that repros! 🙂

I will say it hangs for some seconds but eventually becomes responsive. I'm not sure what's going on there. Will try to find somebody who does.

kennykerr commented 1 week ago

Minimal repro for reference.

[dependencies.windows]
version = "0.58"
features = [
    "System",
    "Win32_Graphics_Gdi",
    "Win32_System_Com",
    "Win32_System_LibraryLoader",
    "Win32_System_WinRT",
    "Win32_UI_Shell",
    "Win32_UI_WindowsAndMessaging",
]
use windows::{
    core::*, System::*, Win32::Foundation::*, Win32::Graphics::Gdi::*, Win32::System::Com::*,
    Win32::System::LibraryLoader::*, Win32::System::WinRT::*, Win32::UI::Shell::*,
    Win32::UI::WindowsAndMessaging::*,
};

fn main() -> Result<()> {
    unsafe {
        CoInitialize(None).ok()?;

        let options = DispatcherQueueOptions {
            dwSize: std::mem::size_of::<DispatcherQueueOptions>() as u32,
            threadType: DQTYPE_THREAD_CURRENT,
            apartmentType: DQTAT_COM_NONE,
        };
        let _controller = CreateDispatcherQueueController(options)?;

        let instance = GetModuleHandleA(None)?;
        let window_class = s!("window");

        let wc = WNDCLASSA {
            hCursor: LoadCursorW(None, IDC_ARROW)?,
            hInstance: instance.into(),
            lpszClassName: window_class,
            style: CS_HREDRAW | CS_VREDRAW,
            lpfnWndProc: Some(wndproc),
            ..Default::default()
        };

        RegisterClassA(&wc);

        CreateWindowExA(
            WINDOW_EX_STYLE::default(),
            window_class,
            s!("This is a sample window"),
            WS_OVERLAPPEDWINDOW | WS_VISIBLE,
            CW_USEDEFAULT,
            CW_USEDEFAULT,
            CW_USEDEFAULT,
            CW_USEDEFAULT,
            None,
            None,
            instance,
            None,
        )?;

        let mut message = MSG::default();

        while GetMessageA(&mut message, None, 0, 0).into() {
            DispatchMessageA(&message);
        }

        Ok(())
    }
}

fn show_dialog() -> Result<()> {
    let handler = DispatcherQueueHandler::new(move || unsafe {
        let dialog: IFileSaveDialog = CoCreateInstance(&FileSaveDialog, None, CLSCTX_ALL)?;
        _ = dialog.Show(None);
        Ok(())
    });
    let queue = DispatcherQueue::GetForCurrentThread()?;
    queue.TryEnqueue(&handler)?;
    Ok(())
}

extern "system" fn wndproc(window: HWND, message: u32, wparam: WPARAM, lparam: LPARAM) -> LRESULT {
    unsafe {
        match message {
            WM_PAINT => {
                _ = ValidateRect(window, None);
                LRESULT(0)
            }
            WM_KEYDOWN => {
                show_dialog().unwrap();
                LRESULT(0)
            }
            WM_DESTROY => {
                PostQuitMessage(0);
                LRESULT(0)
            }
            _ => DefWindowProcA(window, message, wparam, lparam),
        }
    }
}
JunkuiZhang commented 1 week ago

I will say it hangs for some seconds but eventually becomes responsive.

On my machine, the FileSaveDialog is always unresponsive. More precisely, it doesn't behave as a typical freeze or unresponsive. As you can see from the video I uploaded earlier, the caret in the input field is still blinking, but the dialog simply doesn't respond to any keyboard or mouse input. This led me to suspect that the IME window might be "hijacking" all keyboard & mouse inputs.

I'm not sure what's going on there. Will try to find somebody who does.

It would be wonderful if you could find an expert to look into this issue. I'm looking forward to hearing some good news!

kennykerr commented 1 week ago

Preliminary investigation seems to indicate that this is a known issue with the dispatcher queue - if possible you should avoid using it.

JunkuiZhang commented 1 week ago

Preliminary investigation seems to indicate that this is a known issue with the dispatcher queue - if possible you should avoid using it.

Oh no... This is unfortunate news. Is there a similar API in Win32 or WinRT that serves the same function as DispatcherQueue? I know it's possible to use an Event to send messages to the message loop for executing similar functionality, but that approach feels quite inelegant and ugly.

kennykerr commented 1 week ago

You could write a simple closure queue that you drain from the message loop.

JunkuiZhang commented 1 week ago

You could write a simple closure queue that you drain from the message loop.

You mean some code like this?

// Dispatcher
fn dispatch_on_main() {
    sender.send(runnable);
    SetEvent(dispatch_event);
}

// UI thread
loop {
    let wait_result = unsafe {
        MsgWaitForMultipleObjects(Some(&[dispatch_event]), false, INFINITE, QS_ALLINPUT)
    };
    match wait_result {
        0 => {
            for runnable in receiver.drain() { runnable.run(); }
        }
        1 => {
            // Message loop here
        }
    }
}
kennykerr commented 1 week ago

It's worth a shot and you have one less dependency. 😊

JunkuiZhang commented 1 week ago

It's worth a shot and you have one less dependency. 😊

Actually, this was the original implementation. However, we later switched to using DispatcherQueue to more closely align the implementation on macOS, on which we use DispatchQueue. But, it seems that reverting the changes is necessary for fixing this bug. I plan to submit a PR to revert the previous changes made to DispatcherQueue. Would you mind if I @ you in that PR? I will briefly introduce you as the author and expert of winRT and windows-rs, and express my gratitude for your assistancce.

kennykerr commented 6 days ago

Happy to look, although I have no particular expertise in IME or this bug. 🤷

JunkuiZhang commented 6 days ago

Happy to look, although I have no particular expertise in IME or this bug. 🤷

No worries, you are the expert in every sense to me!