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

Missing SetFileTypeChoices method for FileSavePicker #3225

Closed
JunkuiZhang opened this issue Aug 27, 2024 · 34 comments · Fixed by #3226
Closed

Missing SetFileTypeChoices method for FileSavePicker #3225

JunkuiZhang opened this issue Aug 27, 2024 · 34 comments · Fixed by #3226
Labels
question Further information is requested

Comments

@JunkuiZhang
Copy link

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

@JunkuiZhang JunkuiZhang added the bug Something isn't working label Aug 27, 2024
@kennykerr
Copy link
Collaborator

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

#[cfg(feature = "Foundation_Collections")]
pub fn FileTypeChoices(&self) -> windows_core::Result<super::super::Foundation::Collections::IMap<windows_core::HSTRING, super::super::Foundation::Collections::IVector<windows_core::HSTRING>>> {

@kennykerr kennykerr added question Further information is requested and removed bug Something isn't working labels Aug 27, 2024
@JunkuiZhang
Copy link
Author

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

That is what the sample does in the docs.

@JunkuiZhang
Copy link
Author

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

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

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

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

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

Here's a complete example: #3226

@JunkuiZhang
Copy link
Author

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:

  • Switch to Pinyin
  • Open the IFileSaveDialog
  • It freezes

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.

Screen.Recording.2024-08-28.004352.mp4

@JunkuiZhang
Copy link
Author

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

riverar commented Aug 27, 2024

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

@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:

Screen.Recording.2024-08-28.014950.mp4

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

@riverar
Copy link
Collaborator

riverar commented Aug 27, 2024

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

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

riverar commented Aug 29, 2024

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

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

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

JunkuiZhang commented Sep 12, 2024

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

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

@kennykerr
Copy link
Collaborator

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

@JunkuiZhang
Copy link
Author

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.

Screen.Recording.2024-09-12.234341.mp4

@JunkuiZhang
Copy link
Author

BTW, really thank you for the help!

@kennykerr
Copy link
Collaborator

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

kennykerr commented Sep 12, 2024

Minimal repro for reference.

  • Note that this repros only if Chinese is the default keyboard layout.
  • This "shows" the file dialog from a DispatcherQueue callback. If you show the dialog directly from the wndproc then it works fine. Either way works fine for English keyboard layout.
[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
Copy link
Author

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

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

@JunkuiZhang
Copy link
Author

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

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

@JunkuiZhang
Copy link
Author

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

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

@JunkuiZhang
Copy link
Author

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

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

@JunkuiZhang
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants