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

File dialogs do not block application UI #90

Closed
Univstar opened this issue Mar 27, 2023 · 18 comments · Fixed by #136
Closed

File dialogs do not block application UI #90

Univstar opened this issue Mar 27, 2023 · 18 comments · Fixed by #136
Milestone

Comments

@Univstar
Copy link

I am compiling and testing nfd-ext on Win 10/11 platforms. When a file dialog is shown, users can still interface with the background application, which is not expected. How can I fix it?

@btzy
Copy link
Owner

btzy commented Mar 27, 2023

This is not expected, because the dialog is meant to be modal. Please provide a reproduceable example - at this point, it seems equally likely that your code has a bug.

@Univstar
Copy link
Author

Thanks. I will provide a reproduceable code snippet here. The screen recording on my computer is also attached.

#include <GLFW/glfw3.h>
#include <nfd.h>

#include <cstdio>
#include <cstdlib>

int main(void) {
	GLFWwindow* window;

	glfwInit();
	NFD_Init();

	window = glfwCreateWindow(640, 480, "Hello World", NULL, NULL);
	glfwMakeContextCurrent(window);

	glfwSetMouseButtonCallback(window, [](GLFWwindow *window, int button, int action, int mods) {
		if (button != GLFW_MOUSE_BUTTON_LEFT || action != GLFW_PRESS) return;

		nfdchar_t *outPath;
		nfdfilteritem_t filterItem[2] = { { "Source code", "c,cpp,cc" }, { "Headers", "h,hpp" } };
		nfdresult_t result = NFD_OpenDialog(&outPath, filterItem, 2, NULL);
		if (result == NFD_OKAY) {
			std::puts("Success!");
			std::puts(outPath);
			NFD_FreePath(outPath);
		} else if (result == NFD_CANCEL) {
			std::puts("User pressed cancel.");
		} else {
			std::printf("Error: %s\n", NFD_GetError());
		}
	});

	while (!glfwWindowShouldClose(window)) {
		glfwSwapBuffers(window);
		glfwPollEvents();
	}

	NFD_Quit();
	glfwTerminate();
	return 0;
}
bug-1.mp4

@btzy
Copy link
Owner

btzy commented Mar 29, 2023

Can you check if it is writing "Success!" or "User pressed cancel." or "Error: ..." to stdout? If it isn't, then it should be very likely that NFD_OpenDialog has not yet returned, and it is something in GLFW that spawns another thread to handle the callback.

GLFW documentation seems to suggest that the callback will be called on the main thread though, so I'm not sure why it is doing that.

Can you try maybe printing out some thread ID at the top of the callback function, and also just before glfwPollEvents()? That would confirm whether GLFW is spawning another thread for the callback.

@Univstar
Copy link
Author

Sure. It turns out that "Success!" or "User pressed cancel." or "Error: ..." can be successfully written, and the thread ID is always the same.

I am afraid that the problem does not result from multiple threads. The same problem is observed in the original NFD, but it is not observed in the lib tinyfiledialogs.

@btzy
Copy link
Owner

btzy commented Mar 30, 2023

Which one is written? Is it success, cancel, or error that is printed for each dialog? Also, if it is error, can you tell me what NFD_GetError() returns?

@btzy
Copy link
Owner

btzy commented Mar 30, 2023

I will probably take a look at this at some point, but maybe you could try asking the maintainers of some of the other projects that use GLFW and NFDe together. I have a list here: https://github.com/btzy/nativefiledialog-extended/wiki/Native-File-Dialog-Extended-in-the-wild

@Univstar
Copy link
Author

It was 'success'. It seems that besides the non-blocking issue, every thing is correct.

OK, I will study on these projects.

@Univstar
Copy link
Author

@btzy Dear maintainer, I have looked into nfd_win.cpp and got to the root of the problem. In Line 375, 446, and 515, the code calls the member function Show(hwndOwner) of IModalWindow interface, with hwndOwner set as nullptr. However, this parameter determines the handle of the owner window. If it is not present, Show() does not know which window the dialog belongs to. I tried to fill in the window handle and the problem can be addressed.

@btzy
Copy link
Owner

btzy commented Mar 31, 2023

Thanks for investigating. I'm actually still confused by your statement

It was 'success'. It seems that besides the non-blocking issue, every thing is correct.

when read together with

thread ID is always the same.

If the thread ID of both NFD_OpenDialog calls are the same, then doesn't that mean that one call must finish before the other one starts? And if that is so, then the first dialog can't possibly return success since you have not yet selected a file?

If possible, could you show me where you printed the thread IDs, and a video of what the log looks like?

@Univstar
Copy link
Author

Univstar commented Apr 1, 2023

I think that this is because the Windows API simply calls Window kernel functions. Although API itself is run on the main thread of the user process, if not specified, the kernel does not guarantee to have any synchronization properties.

@Univstar
Copy link
Author

Univstar commented Apr 1, 2023

I printed the thread IDs as follows:

#include <GLFW/glfw3.h>
#include <nfd.h>

#include <thread>
#include <iostream>

#include <cstdio>
#include <cstdint>
#include <cstdlib>

int main(void) {
	GLFWwindow* window;

	glfwInit();
	NFD_Init();

	window = glfwCreateWindow(640, 480, "Hello World", NULL, NULL);
	glfwMakeContextCurrent(window);

	glfwSetMouseButtonCallback(window, [](GLFWwindow *window, int button, int action, int mods) {
		if (button != GLFW_MOUSE_BUTTON_LEFT || action != GLFW_PRESS) return;
		std::cout << std::this_thread::get_id() << std::endl;

		nfdchar_t *outPath;
		nfdfilteritem_t filterItem[2] = { { "Source code", "c,cpp,cc" }, { "Headers", "h,hpp" } };
		nfdresult_t result = NFD_OpenDialog(&outPath, filterItem, 2, NULL);
		if (result == NFD_OKAY) {
			std::puts("Success!");
			std::puts(outPath);
			NFD_FreePath(outPath);
		} else if (result == NFD_CANCEL) {
			std::puts("User pressed cancel.");
		} else {
			std::printf("Error: %s\n", NFD_GetError());
		}
	});

	std::cout << std::this_thread::get_id() << std::endl;
	while (!glfwWindowShouldClose(window)) {
		glfwSwapBuffers(window);
		glfwPollEvents();
	}

	NFD_Quit();
	glfwTerminate();
	return 0;
}

I can open multiple file dialogs by clicking the window content. If I close these dialogs one by one, the log looks like

User pressed cancel.
User pressed cancel.
User pressed cancel.

@btzy
Copy link
Owner

btzy commented Apr 1, 2023

Can you show me a log that contains both the printed thread IDs and "User pressed cancel." in the same log? It can't possibly be using the same thread all the time. Also, you should use GetCurrentThreadId() instead of std::this_thread::get_id(), because std::this_thread::get_id() might not work if the thread was not created using std::thread.

@Univstar
Copy link
Author

Univstar commented Apr 2, 2023

Code snippet:

#include <GLFW/glfw3.h>
#include <nfd.h>

#include <thread>
#include <iostream>

#include <cstdio>
#include <cstdint>
#include <cstdlib>

#include <Windows.h>

int main(void) {
	GLFWwindow* window;

	glfwInit();
	NFD_Init();

	window = glfwCreateWindow(640, 480, "Hello World", NULL, NULL);
	glfwMakeContextCurrent(window);

	glfwSetMouseButtonCallback(window, [](GLFWwindow *window, int button, int action, int mods) {
		if (button != GLFW_MOUSE_BUTTON_LEFT || action != GLFW_PRESS) return;

		nfdchar_t *outPath;
		nfdfilteritem_t filterItem[2] = { { "Source code", "c,cpp,cc" }, { "Headers", "h,hpp" } };

		std::printf("Callback Thread: %u\n",  GetCurrentThreadId());
		nfdresult_t result = NFD_OpenDialog(&outPath, filterItem, 2, NULL);
		if (result == NFD_OKAY) {
			std::puts("Success!");
			std::puts(outPath);
			NFD_FreePath(outPath);
		} else if (result == NFD_CANCEL) {
			std::puts("User pressed cancel.");
		} else {
			std::printf("Error: %s\n", NFD_GetError());
		}
	});

	std::printf("Loop Thread: %u\n",  GetCurrentThreadId());
	while (!glfwWindowShouldClose(window)) {
		glfwSwapBuffers(window);
		glfwPollEvents();
	}

	NFD_Quit();
	glfwTerminate();
	return 0;
}

Console log:

Loop Thread: 11972
Callback Thread: 11972
Callback Thread: 11972
User pressed cancel.
User pressed cancel.

There is a new observation. If the sequence of operations is "open Dialog 1, open Dialog 2, close Dialog 2, close Dialog 1", then the logger outputs to the console just after each operation. However, if it is "open Dialog 1, open Dialog 2, close Dialog 1, close Dialog 2", the logger outputs the last two lines in one time after closing both the two dialogs.

@btzy
Copy link
Owner

btzy commented Apr 3, 2023

Thanks for the log and the code.

That's an interesting observation - perhaps there is some (nested) event loop running when the file dialog is open, and because we did not set the owner window, events from the owner window are not suppressed in the new event loop?

Maybe I should look into exposing some function to set the owner hwnd. There is also a similar limitation on Linux/GTK as well, mentioned in the readme:

GTK dialogs don't set the existing window as parent, so if users click the existing window while the dialog is open then the dialog will go behind it. GTK writes a warning to stdout or stderr about this.

The main issue is that the window handles are by nature OS-specific, so we'll end up with some OS-specific functions.

@btzy
Copy link
Owner

btzy commented Apr 3, 2023

I went to download ImHex (I know it uses my library) on Windows, and they have a similar issue where if I click the original window, the file dialog loses focus. However, their UI seems to be frozen until the dialog returns. I am not sure if they have extra code to pause events on the main window while the dialog is open.

@btzy
Copy link
Owner

btzy commented Apr 9, 2023

@iTrooz Do you know if ImHex does any special handling that blocks the UI on the main window while an NFD dialog is open? I'm trying to understand how pervasive the issues being reported here are. Specifically there are two issues that are being reported here:

  1. The file dialog goes under the main window if you click on the main window while the file dialog is open.
  2. The main window still processes events while the file dialog is open.

I've verified that (1) occurs on ImHex's latest Windows build (I haven't checked Linux but but it likely occurs there too), but I cannot reproduce (2). I'm wondering if ImHex is doing anything to prevent (2) from occurring.

@iTrooz
Copy link
Contributor

iTrooz commented Apr 9, 2023

Hey ! No, sorry, I'm not aware of ImHex doing such a thing

But actually, ImHex still processes events for me (Linux, with portals) Here i what happens:

  • I click on "Open file"
  • ImHex calls the blocking function NFD::OpenDialog() from the main (GUI) thread
  • All interactions during the time the file dialog is opened are not dispatched, since the thread is blocked by NFD::OpenDialog()
  • All interactions dispatch at the same time when the user closes the the dialog
Peek.2023-04-09.21-44.mp4

EDIT: btw here is where nfd is used: https://github.com/WerWolv/ImHex/blob/09f1b56964a8553689e6d4660f27a454ed4ba869/lib/libimhex/source/helpers/fs.cpp#L30

@btzy
Copy link
Owner

btzy commented Apr 10, 2023

Thanks for the clear description and the screen recording. I just tried what you did on Windows, and found the same behaviour as in your recording. I don't know much about ImGui, but I would hypothesise that ImGui is buffering OS events and only processing them in the outermost event loop.

I would consider not ignoring events on the main window to be an NFDe bug - the expected behaviour for all supported OSes is for events on the original window to be discarded while a modal window is open, and that the dialog stays on top of the parent window.

I think this should be solvable by passing the parent window handle to the dialog (though some additional investigation might be needed if multiple windows are rendered by the same event loop). The parent window is OS-specific (a HWND for Windows, "x11:<XID>" for Portal with X11, "wayland:HANDLE" for Portal with Wayland, and GTKWindow* for GTK).

As this seems to affect essentially all use cases of NFDe on the affected backends (Windows, Linux GTK, and Linux Portal), I am leaning towards doing the following (in order):

  • Postponing this until New API to set each property of the dialog separately #92 is implemented
  • Implementing platform-specific NFD_OpenFileDialog_SetParent_Windows(NFD_OpenFileDialog*, HWND) and NFD_OpenFileDialog_SetParent_Portal(NFD_OpenFileDialog*, const char*) APIs that will only exist with the correct backend (and maybe NFD_OpenFileDialog_SetParent_GTK(NFD_OpenFileDialog*, const char*) too, though I'm not sure how to give a GTK dialog a non-GTK parent, but it might be doable since that's what xdg-desktop-portal-gtk would have to do)
  • Adding an optional nfd_glfw.h header that contains an inline definition for NFD_OpenFileDialog_SetParent_GLFW(NFD_OpenFileDialog*, GLFWwindow*) (and perhaps another header with a similar function for SDL2) that retrieves the native window handle and calls (at most) one of the platform-specific APIs in the previous step

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants