Rawtherapee: SaveAs Dialog shows TIFF and JPEG options when PNG (16-bit) is selected

[heckflosse]

This patch fixes it

diff --git a/rtgui/saveformatpanel.cc b/rtgui/saveformatpanel.cc
index 2a6d5cad1..755cb9239 100644
--- a/rtgui/saveformatpanel.cc
+++ b/rtgui/saveformatpanel.cc
@@ -182,7 +182,7 @@ void SaveFormatPanel::formatChanged ()

     int act = format->get_active_row_number();

-    if (act < 0 || act > 5) {
+    if (act < 0 || act > 6) {
         return;
     }

[heckflosse]

Same bug is in queue. Because the widget is used in saveas and queue, the patch fixes both bugs.

[Benitoite]

@heckflosse thanks for the patch. Save format panel working ok on mac now. Here's a pull request. https://github.com/Beep6581/RawTherapee/pull/4791

[Floessie]

Thanks, Ingo! Merged in ea992fc91f32c09e5f2c89d4d56c1472ade4ab6f.

[Beep6581]

This part has a tendency to get broken - #4463

[heckflosse]

I will post a better fix today.

[Floessie]

@Beep6581 @heckflosse Having an enum class SaveFormat and a switch (SaveFormat(act)) without default: here (plus a bool proceed for the good cases), would make things much harder to forget.

[heckflosse]

@Floessie Here's a patch I made before your enum suggestion:

diff --git a/rtgui/saveformatpanel.cc b/rtgui/saveformatpanel.cc
index 755cb9239..dc8638d63 100644
--- a/rtgui/saveformatpanel.cc
+++ b/rtgui/saveformatpanel.cc
@@ -45,14 +45,6 @@ SaveFormatPanel::SaveFormatPanel () : listener (nullptr)
     format->append ("PNG (8-bit)");
     format->append ("PNG (16-bit)");

-    fstr[0] = "jpg";
-    fstr[1] = "tif";
-    fstr[2] = "tif";
-    fstr[3] = "tif";
-    fstr[4] = "tif";
-    fstr[5] = "png";
-    fstr[6] = "png";
-
     hb1->attach (*flab, 0, 0, 1, 1);
     hb1->attach (*format, 1, 0, 1, 1);
     hb1->show_all();
@@ -152,28 +144,31 @@ SaveFormat SaveFormatPanel::getFormat ()
     SaveFormat sf;

     int sel = format->get_active_row_number();
-    sf.format = fstr[sel];
-
-    if (sel == 6) {
-        sf.pngBits = 16;
-    } else {
-        sf.pngBits = 8;
-    }

-    if (sel == 2 || sel == 3) {
-        sf.tiffBits = 16;
-    } else if (sel == 4) {
-        sf.tiffBits = 32;
-    } else {
-        sf.tiffBits = 8;
+    if (sel >= 0 && sel < static_cast<int>(fileType.size())) {
+        sf.format = fileType.at(sel);
+
+        if (sel == 6) {
+            sf.pngBits = 16;
+        } else {
+            sf.pngBits = 8;
+        }
+
+        if (sel == 2 || sel == 3) {
+            sf.tiffBits = 16;
+        } else if (sel == 4) {
+            sf.tiffBits = 32;
+        } else {
+            sf.tiffBits = 8;
+        }
+
+        sf.tiffFloat = sel == 4 || sel == 3;
+
+        sf.jpegQuality      = (int) jpegQual->getValue ();
+        sf.jpegSubSamp      = jpegSubSamp->get_active_row_number() + 1;
+        sf.tiffUncompressed = tiffUncompressed->get_active();
+        sf.saveParams       = savesPP->get_active ();
     }
-
-    sf.tiffFloat = sel == 4 || sel == 3;
-
-    sf.jpegQuality      = (int) jpegQual->getValue ();
-    sf.jpegSubSamp      = jpegSubSamp->get_active_row_number() + 1;
-    sf.tiffUncompressed = tiffUncompressed->get_active();
-    sf.saveParams       = savesPP->get_active ();
     return sf;
 }

@@ -182,11 +177,11 @@ void SaveFormatPanel::formatChanged ()

     int act = format->get_active_row_number();

-    if (act < 0 || act > 6) {
+    if (act < 0 || act >= static_cast<int>(fileType.size())) {
         return;
     }

-    Glib::ustring fr = fstr[act];
+    Glib::ustring fr = fileType.at(act);

     if (fr == "jpg") {
         jpegOpts->show_all();
@@ -214,6 +209,6 @@ void SaveFormatPanel::adjusterChanged (Adjuster* a, double newval)
     }

     if (listener) {
-        listener->formatChanged (fstr[act]);
+        listener->formatChanged (fileType.at(act));
     }
 }
diff --git a/rtgui/saveformatpanel.h b/rtgui/saveformatpanel.h
index 788593d9e..70e1cc9c2 100644
--- a/rtgui/saveformatpanel.h
+++ b/rtgui/saveformatpanel.h
@@ -44,7 +44,7 @@ protected:
     Gtk::Grid*          jpegOpts;
     Gtk::Label*         jpegSubSampLabel;
     FormatChangeListener* listener;
-    Glib::ustring       fstr[7];
+    const std::array<Glib::ustring, 7> fileType {"jpg", "tif", "tif", "tif", "tif", "png", "png"};
     Gtk::CheckButton*   savesPP;

[Floessie]

I love the static_cast<int>, but not so much the in-header-initialization, because each time we extend the list, all depending .cc files have to be recompiled. Without enum you still have "magic" numbers like in if (sel == 2 || sel == 3)...

How about storing template sfs in the array? Then everything should fall into place.

[heckflosse]

@Floessie It may be easier if you provide some lines of code or a patch ;-)

[Floessie]

@heckflosse Sure, Ingo, but not today. Anyway, I'll assign myself to this issue and push something based on your patch within the next days.

[Floessie]

@heckflosse Here's a little bedtime treat. That could be further consolidated, but not today (this time for real).

[Floessie]

Continued in #4800.