SDL icon indicating copy to clipboard operation
SDL copied to clipboard

`SDL_ShowOpenFileDialog` immediately changes working directory to the directory being browsed

Open AntTheAlchemist opened this issue 1 year ago • 4 comments

For example on Windows, SDL_LoadFile (if no path is specified) will look in the directory .exe was run. But, upon calling SDL_ShowOpenFileDialog, SDL_LoadFile (and probably others; but I haven't tested) looks in the directory currently being browsed by the dialog. I don't think this is intended behaviour.

AntTheAlchemist avatar Nov 22 '24 01:11 AntTheAlchemist

No, this is not intended.

slouken avatar Nov 22 '24 17:11 slouken

#9406 was meant to fix this; I'll give it a try on my Windows whenever I can.

Semphriss avatar Nov 22 '24 17:11 Semphriss

What build of SDL are you using? I've tried with the latest on main, and it seems to keep the same working directory with all three dialogs functions.

To test it, I've modified test/testdialog.c with this:

diff --git a/test/testdialog.c b/test/testdialog.c
index 7561d7e67..0035e5369 100644
--- a/test/testdialog.c
+++ b/test/testdialog.c
@@ -15,6 +15,9 @@
 #include <SDL3/SDL_main.h>
 #include <SDL3/SDL_test.h>

+#include <unistd.h>
+#include <stdlib.h>
+
 const SDL_DialogFileFilter filters[3] = {
     { "All files", "*" },
     { "JPG images", "jpg;jpeg" },
@@ -22,6 +25,10 @@ const SDL_DialogFileFilter filters[3] = {
 };

 static void SDLCALL callback(void* userdata, const char* const* files, int filter) {
+    char *cwd = getcwd(NULL, 0);
+    SDL_Log("CWD: %s\n", cwd);
+    free(cwd);
+
     if (files) {
         const char* filter_name = "(filter fetching unsupported)";

I've opened and saved files and folders in multiple directories, and the printed working directory remained the same.

If the problem also happens with my patch and the latest commit on the main branch, which build of Windows are you using?

Semphriss avatar Nov 23 '24 18:11 Semphriss

Latest SDL from a few days ago. Let me grab the very latest SDL and do some more digging and compare with getcwd(). Bare with.

Edition	Windows 10 Pro
Version	22H2
Installed on	‎01/‎08/‎2024
OS build	19045.5198
Experience	Windows Feature Experience Pack 1000.19060.1000.0

AntTheAlchemist avatar Nov 23 '24 23:11 AntTheAlchemist

I'm using the latest source, and it's still changing the current directory. This example will set the window title to the current directory, you can see it changing as you navigate in the dialog.

[Update: improved example code]

#define SDL_MAIN_USE_CALLBACKS 1
#include <SDL3/SDL_main.h>
#include <SDL3/SDL.h>

SDL_AppResult SDL_AppInit(void** win, int, char**) {
	SDL_Init(SDL_INIT_EVENTS);
	*win = (void*) SDL_CreateWindow(nullptr, 1000, 500, 0);
	return SDL_APP_CONTINUE;
}

SDL_AppResult SDL_AppIterate(void* win) {
	SDL_SetWindowTitle((SDL_Window*)win, SDL_GetCurrentDirectory());
	SDL_Delay(1);
	return SDL_APP_CONTINUE;
}

static void SDLCALL Callback(void* userdata, const char* const* files, int filter) { }

SDL_AppResult SDL_AppEvent(void* win, SDL_Event* event) {
	switch (event->type) {
	case SDL_EVENT_KEY_UP:
		SDL_ShowOpenFileDialog(Callback, nullptr, (SDL_Window*)win, nullptr, 0, nullptr, false);
		break;
	case SDL_EVENT_QUIT:
		return SDL_APP_SUCCESS;
	}
	return SDL_APP_CONTINUE;
}

void SDL_AppQuit(void*, SDL_AppResult result) { }

AntTheAlchemist avatar Dec 16 '24 09:12 AntTheAlchemist

If the problem also happens with my patch and the latest commit on the main branch, which build of Windows are you using?

Your getcwd was only being called after the dialog returns, so it wasn't a good enough test.

AntTheAlchemist avatar Dec 16 '24 10:12 AntTheAlchemist

Leaving a comment here to remind myself to fix this as well, for some reason I hadn't received the notifications.

Semphriss avatar Dec 18 '24 04:12 Semphriss

So, I have some bad news...

It turns out that the dialog will always change the current directory while it is being used; the OFN_NOCHANGEDIR option only makes it restore the original working directory after opening... which probably makes it even worse if another thread changes the working directory while the dialog is open and expects to preserve that change.

The advice I've seen isn't particularly helpful, recommending as the best option to "remove dependencies on current directory in your code". The most workable option would require using a third-party library to override SetCurrentDirectory with a no-op, assuming that is enough, and keeping in mind this would also prevent app code from doing the same, if it uses Win32 directly.

There doesn't seem to be a way for SDL to fix this conveniently, if at all, unfortunately.

Semphriss avatar Dec 19 '24 02:12 Semphriss

Not depending on the current directory isn't a deal breaker (we've lived without it up to a month ago anyway), so I think it's reasonable to just keep it how it is and document this behaviour in SDL_GetCurrentDirectory() and perhaps all the file dialog functions.

I wonder if Windows is the only platform that does this?

AntTheAlchemist avatar Dec 19 '24 07:12 AntTheAlchemist

Using the IFileDialog interface, I don't see this behavior.

I adapted this example to log the path changes: it now loops in a thread and I added option 6 for the old-style file dialogs.

Using IFileDialog for file dialogs
#include <iostream>
#include <windows.h>
#include <shobjidl.h>
#include <stdio.h>
#include <string>
#include <vector>

static void trace_working_directory() {
    static char working_directory[512];
    char buffer[256];
    GetCurrentDirectoryA(sizeof(buffer), buffer);
    if (strcmp(buffer, working_directory)) {
        printf("Working directory: %s\n", buffer);
        strcpy(working_directory, buffer);
    }
}

/**
 * @brief Open a dialog to select item(s) or folder(s).
 * @param paths Specifies the reference to the string vector that will receive the file or folder path(s). [IN]
 * @param selectFolder Specifies whether to select folder(s) rather than file(s). (optional)
 * @param multiSelect Specifies whether to allow the user to select multiple items. (optional)
 * @note If no item(s) were selected, the function still returns true, and the given vector is unmodified.
 * @note `<windows.h>`, `<string>`, `<vector>`, `<shobjidl.h>`
 * @return Returns true if all the operations are successfully performed, false otherwise.
 */
bool OpenFileDialog(std::vector<std::wstring> &paths, bool selectFolder = false, bool multiSelect = false)
{
    IFileOpenDialog *p_file_open = nullptr;
    bool are_all_operation_success = false;
    while (!are_all_operation_success)
    {
        HRESULT hr = CoCreateInstance(CLSID_FileOpenDialog, NULL, CLSCTX_ALL,
                                      IID_IFileOpenDialog, reinterpret_cast<void **>(&p_file_open));
        if (FAILED(hr))
            break;

        if (selectFolder || multiSelect)
        {
            FILEOPENDIALOGOPTIONS options = 0;
            hr = p_file_open->GetOptions(&options);
            if (FAILED(hr))
                break;

            if (selectFolder)
                options |= FOS_PICKFOLDERS;
            if (multiSelect)
                options |= FOS_ALLOWMULTISELECT;

            hr = p_file_open->SetOptions(options);
            if (FAILED(hr))
                break;
        }

        hr = p_file_open->Show(NULL);
        if (hr == HRESULT_FROM_WIN32(ERROR_CANCELLED)) // No items were selected.
        {
            are_all_operation_success = true;
            break;
        }
        else if (FAILED(hr))
            break;

        IShellItemArray *p_items;
        hr = p_file_open->GetResults(&p_items);
        if (FAILED(hr))
            break;
        DWORD total_items = 0;
        hr = p_items->GetCount(&total_items);
        if (FAILED(hr))
            break;

        for (int i = 0; i < static_cast<int>(total_items); ++i)
        {
            IShellItem *p_item;
            p_items->GetItemAt(i, &p_item);
            if (SUCCEEDED(hr))
            {
                PWSTR path;
                hr = p_item->GetDisplayName(SIGDN_FILESYSPATH, &path);
                if (SUCCEEDED(hr))
                {
                    paths.push_back(path);
                    CoTaskMemFree(path);
                }
                p_item->Release();
            }
        }

        p_items->Release();
        are_all_operation_success = true;
    }

    if (p_file_open)
        p_file_open->Release();
    return are_all_operation_success;
}

/**
 * @brief Open a dialog to save an item.
 * @param path Specifies the reference to the string that will receive the target save path. [IN]
 * @param defaultFileName Specifies the default save file name. (optional)
 * @param pFilterInfo Specifies the pointer to the pair that contains filter information. (optional)
 * @note If no path was selected, the function still returns true, and the given string is unmodified.
 * @note `<windows.h>`, `<string>`, `<vector>`, `<shobjidl.h>`
 * @return Returns true if all the operations are successfully performed, false otherwise.
 */
bool SaveFileDialog(std::wstring &path, std::wstring defaultFileName = L"", std::pair<COMDLG_FILTERSPEC *, int> *pFilterInfo = nullptr)
{
    IFileSaveDialog *p_file_save = nullptr;
    bool are_all_operation_success = false;
    while (!are_all_operation_success)
    {
        HRESULT hr = CoCreateInstance(CLSID_FileSaveDialog, NULL, CLSCTX_ALL,
                                      IID_IFileSaveDialog, reinterpret_cast<void **>(&p_file_save));
        if (FAILED(hr))
            break;

        if (!pFilterInfo)
        {
            COMDLG_FILTERSPEC save_filter[1];
            save_filter[0].pszName = L"All files";
            save_filter[0].pszSpec = L"*.*";
            hr = p_file_save->SetFileTypes(1, save_filter);
            if (FAILED(hr))
                break;
            hr = p_file_save->SetFileTypeIndex(1);
            if (FAILED(hr))
                break;
        }
        else
        {
            hr = p_file_save->SetFileTypes(pFilterInfo->second, pFilterInfo->first);
            if (FAILED(hr))
                break;
            hr = p_file_save->SetFileTypeIndex(1);
            if (FAILED(hr))
                break;
        }

        if (!defaultFileName.empty())
        {
            hr = p_file_save->SetFileName(defaultFileName.c_str());
            if (FAILED(hr))
                break;
        }

        hr = p_file_save->Show(NULL);
        if (hr == HRESULT_FROM_WIN32(ERROR_CANCELLED)) // No item was selected.
        {
            are_all_operation_success = true;
            break;
        }
        else if (FAILED(hr))
            break;

        IShellItem *p_item;
        hr = p_file_save->GetResult(&p_item);
        if (FAILED(hr))
            break;

        PWSTR item_path;
        hr = p_item->GetDisplayName(SIGDN_FILESYSPATH, &item_path);
        if (FAILED(hr))
            break;
        path = item_path;
        CoTaskMemFree(item_path);
        p_item->Release();

        are_all_operation_success = true;
    }

    if (p_file_save)
        p_file_save->Release();
    return are_all_operation_success;
}


#include <SDL3/SDL.h>
static SDL_AtomicInt finished;

static int SDLCALL run_thread(void *)
{
    HRESULT hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE);
    if (FAILED(hr))
    {
        std::cout << "Failed to initialize COM library.\n";
        return -1;
    }
    while (!SDL_GetAtomicInt(&finished)) {
        // Select an example.
        std::cout << "1.    Select an item.\n";
        std::cout << "2.    Select a folder.\n";
        std::cout << "3.    Select multiple items.\n";
        std::cout << "4.    Save an item.\n";
        std::cout << "5.    Save an item with filters.\n";
        std::cout << "6.    Select an item (old-style).\n";
        std::cout << "666.  Quit.\n";
        std::cout << "Select an example: ";
        int choice = 0;
        std::cin >> choice;
        switch (choice) {
            // QUit
            case 666:
                SDL_SetAtomicInt(&finished, 1);
                break;
            case 6:
                // Example: Open file old style
                {
                    char path_buffer[1024] = {};
                    OPENFILENAMEA parms = {};
                    parms.lpstrFilter = "All\0*.*\0Text\0*.TXT\0";
                    parms.nFilterIndex = 1;
                    parms.lStructSize = sizeof(parms);
                    parms.lpstrFile = path_buffer;
                    parms.nMaxFile = sizeof(path_buffer);
                    parms.Flags = OFN_PATHMUSTEXIST | OFN_FILEMUSTEXIST |OFN_NOCHANGEDIR | OFN_LONGNAMES;
                    GetOpenFileNameA(&parms);
                }
                break;
                // Example: Select an item.
            case 1: {
                std::vector<std::wstring> paths;
                if (OpenFileDialog(paths)) {
                    if (!paths.empty()) {
                        std::cout << "Total items: " << paths.size() << "\n";
                        for (int i = 0; i < static_cast<int>(paths.size()); ++i)
                            std::wcout << L"Path " << std::to_wstring(i + 1) << L": " << paths[i] << L"\n";
                    } else
                        std::cout << "No item were selected.\n";
                }

                break;
            }
                // Example: Select a folder.
            case 2: {
                std::vector<std::wstring> paths;
                if (OpenFileDialog(paths, true)) {
                    if (!paths.empty()) {
                        std::cout << "Total items: " << paths.size() << "\n";
                        for (int i = 0; i < static_cast<int>(paths.size()); ++i)
                            std::wcout << L"Path " << std::to_wstring(i + 1) << L": " << paths[i] << L"\n";
                    } else
                        std::cout << "No item were selected.\n";
                }

                break;
            }
                // Example: Select multiple items.
            case 3: {
                std::vector<std::wstring> paths;
                if (OpenFileDialog(paths, false, true)) {
                    if (!paths.empty()) {
                        std::cout << "Total items: " << paths.size() << "\n";
                        for (int i = 0; i < static_cast<int>(paths.size()); ++i)
                            std::wcout << L"Path " << std::to_wstring(i + 1) << L": " << paths[i] << L"\n";
                    } else
                        std::cout << "No item were selected.\n";
                }

                break;
            }
                // Example: Save an item.
            case 4: {
                std::wstring path = L"";
                if (SaveFileDialog(path, L"Some file.txt")) {
                    if (!path.empty()) {
                        std::wcout << L"Selected save path:  " << path << L"\n";
                    } else
                        std::cout << "No item were selected.\n";
                }

                break;
            }
                // Example: Save an item with filters.
            case 5: {
                std::wstring path = L"";
                const unsigned int total_filters = 3;
                COMDLG_FILTERSPEC filters[total_filters];
                filters[0].pszName = L"All files. (*.*)";
                filters[0].pszSpec = L"*.*";
                filters[1].pszName = L"Image files. (.bmp, .jpg, .png)";
                filters[1].pszSpec = L"*.bmp;*.jpg;*.png";
                filters[2].pszName = L"Specific file. (unique_file.txt)";
                filters[2].pszSpec = L"unique_file.txt";
                std::pair<COMDLG_FILTERSPEC *, int> filter_info = std::make_pair<COMDLG_FILTERSPEC *, int>(filters,
                                                                                                           total_filters);
                if (SaveFileDialog(path, L"", &filter_info)) {
                    if (!path.empty()) {
                        std::wcout << L"Selected save path: " << path << L"\n";
                    } else
                        std::cout << "No item were selected.\n";
                }

                break;
            }
        }
    }

    CoUninitialize();
    SDL_SetAtomicInt(&finished, 1);

    return 0;
}

int main() {
    SDL_Init(0);
    SDL_Thread *t = SDL_CreateThread(run_thread, "dialog_thread", NULL);
    while (!SDL_GetAtomicInt(&finished)) {
        trace_working_directory();
        Sleep(100);
    }
    SDL_WaitThread(t, NULL);
    SDL_Quit();
}

madebr avatar Dec 19 '24 09:12 madebr