cycfi / elements

Elements C++ GUI library
http://cycfi.github.io/elements/
3.15k stars 235 forks source link

browse for file/directory widget #44

Closed Xeverous closed 1 year ago

Xeverous commented 4 years ago

This issue is for tracking any decisions before implementing this widget.

djowel commented 4 years ago

This is best done using platform code; Probably outside the scope of the library? I'm not sure.

Xeverous commented 4 years ago

Not sure about other systems, but Windows has a set of predefined modal dialogs

https://docs.microsoft.com/en-us/windows/win32/dlgbox/dialog-box-types

You just call an API function and it creates a modal window managed by the OS, the function will return (with user data) when the window is closed.

djowel commented 4 years ago

All platforms have those. It might be good to abstract this in a cross-platform API, but I am not sure where to draw the line which elements should support. How this is triggered is App specific. It can be from a menu, a button, etc.

Xeverous commented 4 years ago

True, I was thinking more about providing such platform-abstracted API so one can easily create a custom widget and do something like w.on_click = os_modal_save_as().

djowel commented 4 years ago

True, I was thinking more about providing such platform-abstracted API so one can easily create a custom widget and do something like w.on_click = os_modal_save_as().

Agreed. Probably worth looking into.

Xeverous commented 4 years ago

I'm in the process of looking how other GUI libraries handle this and designing a common OS-agnostic API.

Windows and macOS interfaces are rather rich - what should I be looking at for unix systems? GTK+ 3 API?

Xeverous commented 4 years ago

I started making a sheet which compares major OS APIs and libraries (Windows, Mac, GTK+ 3, wxWidgets 3, Qt 5). Turns out all of them have very rich set of settings.

Core functionality like path filtering, default file/dir are present in all. For each of other/advanced options, it is present on most targets.

Both Qt and wxWidgets cover almost all and ignore specific ones on platforms on which they are not available. IMO this is a very good design.

The eventual interface in elements can be a free function that takes 1 parameter: options struct. This way:

djowel commented 4 years ago

Windows and macOS interfaces are rather rich - what should I be looking at for unix systems? GTK+ 3 API?

Yes GTK3 on Linux

djowel commented 4 years ago

The eventual interface in elements can be a free function that takes 1 parameter: options struct. This way:

  • for each option elements can provide a very reasonable default value
  • we can add more options later without any API breakages
  • Elements library users can do what they want with the options struct. file-save-as / file-open and directory-open share a lot of common settings and it would be good to store them in a common member struct. This way library users can easily implement consistent behavior.

Sounds good to me! I suggest starting basic. It's always easy to add APIs than to remove APIs, especially when the library gets established.

Xeverous commented 4 years ago

This is going to take some time, but I can share my WIP spreadsheet if you want.

djowel commented 4 years ago

This is going to take some time, but I can share my WIP spreadsheet if you want.

Google sheet will be good.

Xeverous commented 4 years ago

Actually, it's easier for me to work on the spreatsheet offline. I already switch too many browser tabs too often when reading various documentation. I can upload it here when I finish.

Xeverous commented 4 years ago

OS API notes.xlsx

These are all of my current notes on all 3 filesystem dialogs:

djowel commented 4 years ago

Wow, that's a lot! Nice work collecting that. So basically, as always: Start with a minimal subset that is available on all platforms.

(BTW. I wonder how you are able to test on MacOS, do you have a Mac you can work on?)

Xeverous commented 4 years ago

I'm not able to test it on mac. I don't even have Obj-C skills. All from the sheet is from API documentation. I will left Mac implementation for you.

djowel commented 4 years ago

I'm not able to test it on mac. I don't even have Obj-C skills. All from the sheet is from API documentation. I will left Mac implementation for you.

OK, make it work on Windows and Linux then, and I can take care of MacOS.

Xeverous commented 4 years ago

Implementation question: most (if not all) platforms require to hold or pass some platform-specific state to create any modal window.

Should we make the modal functions members of the window class or should we make the window class to return std::function objects from these functions?

Xeverous commented 4 years ago

I fkin love Microsoft...

They provide a sample usage code on https://docs.microsoft.com/en-us/previous-versions/windows/desktop/legacy/bb776913(v=vs.85)?redirectedfrom=MSDN but it is not a complete function.

Then this:

Note Several examples in this topic use the CDialogEventHandler_CreateInstance helper function to create an instance of the IFileDialogEvents implementation. To use this function in your own code, copy the source code for the CDialogEventHandler_CreateInstance function from the Common File Dialog Sample

The sample has its own page, except there is no code on it. Only 2 links:

The gallery link is basically 404 and the SDK installer can not finish it's job because it conflicts with already installed VC redistributable on my Windows (even if I select to install just the samples). MS docs say I need to uninstall existing redis first and then install them back after SDK is installed but I have no idea whether I will get correct versions back and whether something existing (which uses them) will broke. I just want the damn archive with sample code.

Fortunately there are good people that uploaded all of the samples to repos such as this one. It weights 300 MB and I have no idea what many of the directory names mean but fortunately a quick grep -rn "CDialogEventHandler_CreateInstance" . finds the thing I need.

Xeverous commented 4 years ago

Funny thing: while reading the docs, I was concerned if they are up to date since 1 function was provided only 1 argument while it was declared to take 2. I pasted the code to the IDE but nothing was underlined.

I'm first time doing anything complex in Microsoft native API and it turns out there are many nasty things in their macros. IID_PPV_ARGS(&pfd) is actually __mingw_uuidof<__typeof(**(&pfd))>(), IID_PPV_ARGS_Helper (&pfd).

djowel commented 4 years ago

Implementation question: most (if not all) platforms require to hold or pass some platform-specific state to create any modal window.

Should we make the modal functions members of the window class or should we make the window class to return std::function objects from these functions?

What does WX do? Keep in mind that when making plugins, you do not even have access to a window, which makes me wonder how this will all work out for that specific, and important use-case.

Xeverous commented 4 years ago

Both WX and Qt implement these dialogs as a separate, standalone classes.

In both librariers, constructors require a pointer to the parent window (possibly null pointer, but that is hardly ever the case - modal dialogs are practically always invoked from already existing window).

In both libraries, there is a member function that will show the dialog and establish a new event loop for the time of the modal window.

In both libraries, some member functions have preconditions. Violating them results in undefined behavior.

djowel commented 4 years ago

It should be standalone. Elements require components that do not require a parent window. It's one of the fundamental requirements. The Element 'window' and 'app' classes are actually optional and are never used when writing plugins.

djowel commented 4 years ago

It should be standalone. Elements require components that do not require a parent window. It's one of the fundamental requirements. The Element 'window' and 'app' classes are actually optional and are never used when writing plugins.

Having said that, the base_view has a pointer to the 'child-window' (in windows), widget (in gtk3) and nsview (in macos), so if you know what you are doing, it is possible to spawn the modal via the internal base_view platform specific handle.

Xeverous commented 4 years ago

Elements require components that do not require a parent window. It's one of the fundamental requirements.

The problem is, one can create multiple windows. And the modals on each system are window-blocking or application-blocking. I still haven't analyzed all of the dependencies on each system and what exactly is needed to create a modal.

I know that you want to have a completely disjoint types in the API, but it is not always possible. If there is any unavoidable coupling, IMO it is better to ilustrate it explictly in the API by making modals member functions of the window than to hide it with unaccessible, global variables.

Right now I'm writing OS-specific implementation and trying to satisfy the API, we will see where the problems arise.


Btw, 2 other things:

1) I think it would be useful to expose these functions on the public API (on Windows only)

https://github.com/cycfi/elements/blob/773b60b6fd759a3687f9d654d92555d953ef9f7c/lib/host/windows/base_view.cpp#L43-L62

They are not needed on other systems but they help a lot when doing any stuff on Windows.

2) Any progress on #23? I posted there a sample implementation.

Xeverous commented 4 years ago

I have a working Windows partial implementation.

Interesting observation: there is no need to pass any state, but if I set the parent window to nullptr then the dialog is not modal.

So (at least on Windows) the window object can be completely decoupled from modal function (no state required to pass). This actually opens 2 separate behaviors on the API. This is good, because the library user can then choose to open a mode-less dialog or pass a reference to specific elements::window instance.

Let's continue and see what is observed further.

Xeverous commented 4 years ago

You might want to look: 02f50e1

djowel commented 4 years ago

You might want to look: 02f50e1

auto scale = 1.0;

???

Are you testing on visual studio on a HDPI machine? It will give me a lot of work if you are not. I respect your decision to use gcc, but keep in mind that this is not the mainstream compiler on Windows.

djowel commented 4 years ago
  1. I think it would be useful to expose these functions on the public API (on Windows only)

Sure, but keep the header private. I usually do it in the same directory where the sources are and use include "some_util.hpp"

  1. Any progress on #23? I posted there a sample implementation.

No promises, but I'll see if I can give that top priority. Is it in the experimental branch already? If not please do.

Xeverous commented 4 years ago

I respect your decision to use gcc, but keep in mind that this is not the mainstream compiler on Windows.

Of course these changes are not intended for the actual commit. I always revert any patches like this one but this time the area where the work is being done is conflicting with the patch so I just commited it and will then revert it when the full PR is ready.

Also, I don't have any HDPI device for such testing. All of the commited scaling changes are irrelevant for the issue, they are just to be able to compile with MinGW which does not (yet) have this API implemented. I could actually contact MinGW runtime devs and check/ask why this API is not yet complete. It's not that much new. Initially I thought the thing is already done as I used a bit older installation and didn't wanted to bother you with MinGW ifdef PRs but now I see that it might take more time than I expected. I will check the situation - I know that many of the MinGW stuff is autogenerated so it is either simple overlook on their side or there is a bigger issue behind.

No promises, but I'll see if I can give that top priority. Is it in the experimental branch already? If not please do.

Good to read this, you seemed recently to kind of ignore this issue and I'm fighting with for a long time already. Pushed it and now the HEAD of experimental has the sample tab implementation (with a very bad drawing implementation). Also note that during experimenting I discovered #98 which might be related some bigger problems (eg weird behavior of stretching).

djowel commented 4 years ago

No promises, but I'll see if I can give that top priority. Is it in the experimental branch already? If not please do.

Good to read this, you seemed recently to kind of ignore this issue and I'm fighting with for a long time already. Pushed it and now the HEAD of experimental has the sample tab implementation (with a very bad drawing implementation). Also note that during experimenting I discovered #98 which might be related some bigger problems (eg weird behavior of stretching).

I am not ignoring the issue. There's just higher priority work already in the pipeline.

djowel commented 4 years ago

No promises, but I'll see if I can give that top priority. Is it in the experimental branch already? If not please do.

Good to read this, you seemed recently to kind of ignore this issue and I'm fighting with for a long time already. Pushed it and now the HEAD of experimental has the sample tab implementation (with a very bad drawing implementation). Also note that during experimenting I discovered #98 which might be related some bigger problems (eg weird behavior of stretching).

I am not ignoring the issue. There's just higher priority work already in the pipeline.

Right now, documentation is my highest priority. But as I said, I can step back a bit and give what you need some priority.

djowel commented 4 years ago

I respect your decision to use gcc, but keep in mind that this is not the mainstream compiler on Windows.

Of course these changes are not intended for the actual commit. I always revert any patches like this one but this time the area where the work is being done is conflicting with the patch so I just commited it and will then revert it when the full PR is ready.

Also, I don't have any HDPI device for such testing. All of the commited scaling changes are irrelevant for the issue, they are just to be able to compile with MinGW which does not (yet) have this API implemented. I could actually contact MinGW runtime devs and check/ask why this API is not yet complete. It's not that much new. Initially I thought the thing is already done as I used a bit older installation and didn't wanted to bother you with MinGW ifdef PRs but now I see that it might take more time than I expected. I will check the situation - I know that many of the MinGW stuff is autogenerated so it is either simple overlook on their side or there is a bigger issue behind.

That is fine, as long as you are testing on Visual Studio too. You can't ignore that compiler if you are serious on working on cross-platform libraries.

Xeverous commented 4 years ago

I have made a sample implementation for Windows and Linux and now want to iterate over the options and what to do with them. I'm interested in your opinion on each of them.

The core API:

https://github.com/Xeverous/elements/blob/5e6e6d74a130ca705492c0bd3c1f7157645939f4/lib/include/elements/window.hpp#L26-L196

Core notes:

Now, for the each option:

djowel commented 4 years ago

Wow, that is very comprehensive! I'll give a thorough reading and thinking as soon as I can (maybe replying to one item at a time -- i don't have enough time right now).

djowel commented 4 years ago
  • At least for Widnows and GTK, no changes were needed for event handling. I modified only window header and source files to do them (the rest modifications are only code deduplication / cleanup).

If the code requires window, then it will not be usable in plugins or any application where the client already has its own windows. Please bear in mind that elements should work together with other libraries, frameworks, plugin frameworks, etc. In many cases, you do not have control over the creation of windows and the app itself.

You might want to look at the base_view instead, which is the basis of all elements' components. The handle that base_view is given IS-A child-window (in windows parlance) and a widget (in gtk parlance) and a content-view (in MacOS parlance).

Xeverous commented 4 years ago

If the code requires window, then it will not be usable in plugins or any application where the client already has its own windows.

To open a dialog (that is modal), each platform requires its own window handle. We can't really bypass this requirement, unless the user wants a non-modal dialog. I haven't checked, but it seems that native_window_handle that is in the base_view class is exactly this thing. No other platform-related data is required, so I think it will be very good to offer extra overloads that take such native handle type as a parameter instead of elements window. I have seen such thing in many libraries, IIRC Qt, wxWidgets and even SFML - this is the standard approach (even std::thread::native_handle) so I think this will be a very good solution to offer both elements-window overloads and overloads that take native window handle.

djowel commented 4 years ago
  • Windows is free of localization problems. Everything that is not touched remains on system defaults. GTK is not so fortunate, and the implementation must provide some strings explicitly (eg open/save/cancel button labels). They recommed to use the same mechanism as GLib - that is, GNU gettext library (translations are already there, just use gettext to get them). Right now I hardcoded english strings but I think elements will need to take care of gettext initialization in the app so that the labels are properly localized.

The MacOS solution for such things is to place the strings in resource files (like the config thing again). That way, localization is a simple config edit. Localization is not limited to just the names of such dialogs elements, although yeah, common UI elements are already localized in MacOS. But localization involves the whole app! So here's another use-case for that json thing again.

At the very least, we assume that strings are UTF8. But it does not end with strings only, you'll also have to deal with other resources such as fonts, for example.

djowel commented 4 years ago
  • Extension filters: all platforms have some way of expressing desired file extensions. On file save, it's to choose desired file format. On file open, it's to filter the view to open files of type that match given application. Notetheless, *.* filter is used on many applications - eg Notepad++ has it to let user open any file as text if they really want to, such any-filter is usually the last of all filters. I strongly recommend to have such option, otherwise both GTK and Windows display and ugly empty dropdown. I have not yet implemented filters on any of Windows and GTK but they are very desired options on both these platforms. I need just time to think of an API to specify them and look into implementation of other GUI libraries.

That makes sense.

redtide commented 2 years ago

I would like to make some summary about a filedialog widget, which seems what is discussed in this issue.

TLDR: we could use an external native filedialog library and use it with a wrapper from Elements.

I would like also to point out that the issue title is a bit generic, there can be also widgets for browsing files to be embedded in the application, for example:

Another discussed (elsewhere) point was in the audio context: GTK3 cannot be used because ABI issues between host and plugins, ergo no GtkFileChooser directly (see below).

There are various solutions that implements, where possible, a native filedialog:

Most of them use native API for macOS and Windows, where for Linux/BSD the solution is a bit more complex and a simple implementation was by using external shell based programs like Zenity (GTK) or KDialog (Qt/KDE) (adopted also by VSTGUI), though a proper solution would be by using XDG Desktop Portal FileChooser, which was implemented recently by falktx in his DISTRHO framework.

djowel commented 2 years ago

TLDR: we could use an external native filedialog library and use it with a wrapper from Elements.

I think this makes sense.