jcelaya / hdrmerge

HDR exposure merging
http://jcelaya.github.io/hdrmerge/
Other
363 stars 78 forks source link

Open and Save dialogs have no sidebar URLs #135

Closed Hombre57 closed 6 years ago

Hombre57 commented 6 years ago

As you can see below, there's no side URLs in the Open and Save dialog. Even if you disable the code that adds an entry to this list, it's still empty.

hdrm-no-side-urls

@heckflosse Do you confirm on your side ? @Beep6581 is it the same on Linux ?

Floessie commented 6 years ago

@Hombre57 Same on Debian 9.4 (XFCE).

Beep6581 commented 6 years ago

Same in Sabayon, continuous-5-g00bda2f.

Beep6581 commented 6 years ago

Qt4 load: imgur-2018_05_01-14 40 28

Qt4 save: imgur-2018_05_01-14 40 53

Beep6581 commented 6 years ago

http://doc.qt.io/qt-5/qfiledialog.html#getOpenFileNames http://doc.qt.io/qt-5/qfiledialog.html#Option-enum

diff --git a/src/LoadOptionsDialog.cpp b/src/LoadOptionsDialog.cpp
index 1751e8b..3e9370d 100644
--- a/src/LoadOptionsDialog.cpp
+++ b/src/LoadOptionsDialog.cpp
@@ -140,7 +140,7 @@ void LoadOptionsDialog::addFiles() {
     ")"));
     QStringList files = QFileDialog::getOpenFileNames(this, tr("Open raw images"),
         lastDirSetting.isNull() ? QDir::currentPath() : QDir(lastDirSetting.toString()).absolutePath(),
-        filter, NULL, QFileDialog::DontUseNativeDialog);
+        filter, Q_NULLPTR, Q_NULLPTR);
     if (!files.empty()) {
         QString lastDir = QFileInfo(files.front()).absolutePath();
         settings.setValue("lastOpenDirectory", lastDir);
diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp
index 4cb219d..8ad7d79 100644
--- a/src/MainWindow.cpp
+++ b/src/MainWindow.cpp
@@ -351,7 +351,6 @@ void MainWindow::saveResult() {
         urlsBak = saveDialog.sidebarUrls();
         urls << urlsBak << QUrl::fromLocalFile(io.getInputPath());
         saveDialog.setSidebarUrls(urls);
-        saveDialog.setOptions(QFileDialog::DontUseNativeDialog);

         if (saveDialog.exec()) {
             QString file = saveDialog.selectedFiles().front();

Qt5 load: imgur-2018_05_01-14 52 36

heckflosse commented 6 years ago

@Hombre57

Do you confirm on your side

I never built hdrmerge on windows

heckflosse commented 6 years ago

Master branch on Manjaro

grafik

grafik

gaaned92 commented 6 years ago

@Beep6581 Thanks for the patch

screen-2018-06-09_10-49-52

HDRmerge_master_continuous-22-g757f3c4_release_64.zip

uploaded at https://keybase.pub/gaaned92/LHDR64/

Beep6581 commented 6 years ago

@Hombre57 do you remember what was the issue with the patch above?

Hombre57 commented 6 years ago

@Beep6581 IIRC, on Win7, clicking on "Open" didn't closed the file selector. I've updated my MSYS2 toolchain, and it works fine now, though we end up with Windows' file selector, where we can't add entry in the url side bar. :+1: to commit

Beep6581 commented 6 years ago

Hopefully someone who knows Qt comes along and helps out. https://discuss.pixls.us/t/do-you-know-your-way-around-qt-help-needed/8027

gaaned92 commented 6 years ago

@Hombre57 could you explain please what you mean by

though we end up with Windows' file selector, where we can't add entry in the url side bar. as I don't notice something missing for windows10?

Beep6581 commented 6 years ago

@gaaned92 for example we can't add the current photo's folder to the sidebar when using the save dialog.

Beep6581 commented 6 years ago

@Hombre57 @Floessie could you try this patch on top of latest master?

diff --git a/src/LoadOptionsDialog.cpp b/src/LoadOptionsDialog.cpp
index 3e9370d..1751e8b 100644
--- a/src/LoadOptionsDialog.cpp
+++ b/src/LoadOptionsDialog.cpp
@@ -140,7 +140,7 @@ void LoadOptionsDialog::addFiles() {
     ")"));
     QStringList files = QFileDialog::getOpenFileNames(this, tr("Open raw images"),
         lastDirSetting.isNull() ? QDir::currentPath() : QDir(lastDirSetting.toString()).absolutePath(),
-        filter, Q_NULLPTR, Q_NULLPTR);
+        filter, NULL, QFileDialog::DontUseNativeDialog);
     if (!files.empty()) {
         QString lastDir = QFileInfo(files.front()).absolutePath();
         settings.setValue("lastOpenDirectory", lastDir);
diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp
index ae23a9f..90ffdd7 100644
--- a/src/MainWindow.cpp
+++ b/src/MainWindow.cpp
@@ -344,6 +344,7 @@ void MainWindow::saveResult() {
         }

         QFileDialog saveDialog(this, tr("Save DNG file"), name, tr("Digital Negatives (*.dng)"));
+        saveDialog.setOptions(QFileDialog::DontUseNativeDialog);
         saveDialog.setAcceptMode(QFileDialog::AcceptSave);
         saveDialog.setFileMode(QFileDialog::AnyFile);
         saveDialog.setConfirmOverwrite(true);

Create a HDR DNG using raw files from an unusual location, like ~/foo/bar<1-3>.raw or c:\not my documents\potatos\bar<1-3>.raw Ctrl+s to save, and see if ~/foo/ or c:\not my documents\potatos\ are in the sidebar.

gaaned92 commented 6 years ago
Create a HDR DNG using raw files from an unusual location, like ~/foo/bar<1-3>.raw or c:\not my documents\potatos\bar<1-3>.raw
Ctrl+s to save, and see if ~/foo/ or c:\not my documents\potatos\ are in the sidebar.

W10: it is already like that without patch!

Beep6581 commented 6 years ago

I looked into this a bit deeper, and as far as I understand it, the QFileDialog can run in two modes:

diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp
index 90ffdd7..fc4acd1 100644
--- a/src/MainWindow.cpp
+++ b/src/MainWindow.cpp
@@ -344,13 +344,18 @@ void MainWindow::saveResult() {
         }

         QFileDialog saveDialog(this, tr("Save DNG file"), name, tr("Digital Negatives (*.dng)"));
-        saveDialog.setOptions(QFileDialog::DontUseNativeDialog);
+        saveDialog.setOptions(QFileDialog::DontUseNativeDialog); // Keep before setSidebarUrls due to bug in old Qt5 versions
         saveDialog.setAcceptMode(QFileDialog::AcceptSave);
         saveDialog.setFileMode(QFileDialog::AnyFile);
         saveDialog.setConfirmOverwrite(true);
-        QList<QUrl> urls, urlsBak;
-        urlsBak = saveDialog.sidebarUrls();
-        urls << urlsBak << QUrl::fromLocalFile(io.getInputPath());
+
+        QList<QUrl> urls;
+        urls.append(QUrl::fromLocalFile(io.getInputPath()));
+        urls.append(QUrl::fromLocalFile(QStandardPaths::writableLocation(QStandardPaths::HomeLocation)));
+        urls.append(QUrl::fromLocalFile(QStandardPaths::writableLocation(QStandardPaths::DesktopLocation)));
+        urls.append(QUrl::fromLocalFile(QStandardPaths::writableLocation(QStandardPaths::DocumentsLocation)));
+        urls.append(QUrl::fromLocalFile(QStandardPaths::writableLocation(QStandardPaths::PicturesLocation)));
+        urls.append(QUrl::fromLocalFile(QStandardPaths::writableLocation(QStandardPaths::TempLocation)));
         saveDialog.setSidebarUrls(urls);

         if (saveDialog.exec()) {
@@ -372,7 +377,6 @@ void MainWindow::saveResult() {
                     QApplication::instance()->processEvents(QEventLoop::ExcludeUserInputEvents);
             }
         }
-        saveDialog.setSidebarUrls(urlsBak);
     }
     setToolFromKey();
 }

For anyone who wants to try, branch master uses the native dialog, and branch qfiledialog-notnative uses the non-native one.

The default location in both save dialogs is the last-saved-to location.

Which mode do you prefer for the save dialog @Hombre57 @heckflosse @gaaned92, native or non-native? And which for the load dialog?

Beep6581 commented 6 years ago

Both suite my needs so I don't really have a favorite, but if I had to choose I'd vote for non-native, where the source folder appears at the top.

gaaned92 commented 6 years ago

@Beep6581 on W10

So I vote for Master branch Thank you for improvment.

Beep6581 commented 6 years ago

@gaaned92 @Hombre57 and others, when testing, please show a screenshot of the Save window as it looks like when using master or qfiledialog-notnative, and state your OS (and window manager if Linux). When using master, make sure you're on commit ba22dc8 (or newer). Make sure that your source photos are in a non-standard location, for example create c:\blah and put the source photos into that.

Hombre57 commented 6 years ago

@Beep6581 After some search in the doc and on the web, I think it's normal that the side bar urls is empty. It's its default state. I'm making a patch to add standard urls. Will come back shortly...

Hombre57 commented 6 years ago

@Beep6581 Sooo..... you already RTFM and produced a patch... :sweat:

See my side-bar-urls branch, in PR #147, it adds standard locations in non-native mode QFileDialog, including valid mounted device.

Beep6581 commented 6 years ago

The current implementation adds the following URLs to the sidebar which should not be there (tested in Sabayon):

If anyone knows how to fix that, a patch would be welcome.

Hombre57 commented 6 years ago

@Beep6581 By looking at the mount list you pasted on IRC, efi seem to be your Windows partition and 1000 your temp mount.

Beep6581 commented 6 years ago

@Hombre57 yes,

and none of those locations folders appear in any other program.

I also noticed that custom sidebar locations are not saved between HDRMerge sessions, that would be nice to have.