Hack to fix wxChoice heights on KDE (#2343).

8 files changed, 195 insertions, 57 deletions

diff --git a/src/wx/dcp_panel.cc b/src/wx/dcp_panel.cc
index 895aed9c9..ea1dd6d61 100644
--- a/src/wx/dcp_panel.cc
+++ b/src/wx/dcp_panel.cc
@@ -24,6 +24,7 @@
 #include "check_box.h"
 #include "dcp_panel.h"
 #include "dcpomatic_button.h"
+#include "dcpomatic_choice.h"
 #include "dcpomatic_spin_ctrl.h"
 #include "focus_manager.h"
 #include "interop_metadata_dialog.h"
@@ -101,7 +102,7 @@ DCPPanel::DCPPanel (wxNotebook* n, shared_ptr<Film> film, weak_ptr<FilmViewer> v
 	_edit_audio_language = new Button (_panel, _("Edit..."));
 
 	_dcp_content_type_label = create_label (_panel, _("Content Type"), true);
-	_dcp_content_type = new wxChoice (_panel, wxID_ANY);
+	_dcp_content_type = new Choice(_panel);
 
 	_encrypted = new CheckBox (_panel, _("Encrypted"));
 
@@ -110,14 +111,14 @@ DCPPanel::DCPPanel (wxNotebook* n, shared_ptr<Film> film, weak_ptr<FilmViewer> v
         size.SetHeight (-1);
 
 	_reels_label = create_label (_panel, _("Reels"), true);
-	_reel_type = new wxChoice (_panel, wxID_ANY);
+	_reel_type = new Choice(_panel);
 
 	_reel_length_label = create_label (_panel, _("Reel length"), true);
 	_reel_length = new SpinCtrl (_panel, DCPOMATIC_SPIN_CTRL_WIDTH);
 	_reel_length_gb_label = create_label (_panel, _("GB"), false);
 
 	_standard_label = create_label (_panel, _("Standard"), true);
-	_standard = new wxChoice (_panel, wxID_ANY);
+	_standard = new Choice(_panel);
 
 	_markers = new Button (_panel, _("Markers..."));
 	_metadata = new Button (_panel, _("Metadata..."));
@@ -142,18 +143,18 @@ DCPPanel::DCPPanel (wxNotebook* n, shared_ptr<Film> film, weak_ptr<FilmViewer> v
 	_edit_audio_language->Bind   (wxEVT_BUTTON,   boost::bind(&DCPPanel::edit_audio_language_clicked, this));
 
 	for (auto i: DCPContentType::all()) {
-		_dcp_content_type->Append (std_to_wx(i->pretty_name()));
+		_dcp_content_type->add(i->pretty_name());
 	}
 
-	_reel_type->Append (_("Single reel"));
-	_reel_type->Append (_("Split by video content"));
+	_reel_type->add(_("Single reel"));
+	_reel_type->add(_("Split by video content"));
 	/// TRANSLATORS: translate the word "Custom" here; do not include the "Reel|" prefix
-	_reel_type->Append (S_("Reel|Custom"));
+	_reel_type->add(S_("Reel|Custom"));
 
 	_reel_length->SetRange (1, 64);
 
-	_standard->Append (_("SMPTE"));
-	_standard->Append (_("Interop"));
+	_standard->add(_("SMPTE"));
+	_standard->add(_("Interop"));
 
 	Config::instance()->Changed.connect (boost::bind(&DCPPanel::config_changed, this, _1));
 
@@ -265,13 +266,13 @@ DCPPanel::encrypted_toggled ()
 void
 DCPPanel::frame_rate_choice_changed ()
 {
-	if (!_film) {
+	if (!_film || !_frame_rate_choice->get()) {
 		return;
 	}
 
 	_film->set_video_frame_rate (
 		boost::lexical_cast<int>(
-			wx_to_std(_frame_rate_choice->GetString(_frame_rate_choice->GetSelection()))
+			wx_to_std(_frame_rate_choice->GetString(*_frame_rate_choice->get()))
 			),
 		true
 		);
@@ -297,29 +298,29 @@ DCPPanel::audio_channels_changed ()
 		return;
 	}
 
-	_film->set_audio_channels (locale_convert<int>(string_client_data(_audio_channels->GetClientObject(_audio_channels->GetSelection()))));
+	_film->set_audio_channels(locale_convert<int>(string_client_data(_audio_channels->GetClientObject(*_audio_channels->get()))));
 }
 
 
 void
 DCPPanel::resolution_changed ()
 {
-	if (!_film) {
+	if (!_film || !_resolution->get()) {
 		return;
 	}
 
-	_film->set_resolution (_resolution->GetSelection() == 0 ? Resolution::TWO_K : Resolution::FOUR_K);
+	_film->set_resolution(*_resolution->get() == 0 ? Resolution::TWO_K : Resolution::FOUR_K);
 }
 
 
 void
 DCPPanel::standard_changed ()
 {
-	if (!_film) {
+	if (!_film || !_standard->get()) {
 		return;
 	}
 
-	_film->set_interop (_standard->GetSelection() == 1);
+	_film->set_interop(*_standard->get() == 1);
 
 }
 
@@ -557,11 +558,11 @@ DCPPanel::container_changed ()
 		return;
 	}
 
-	int const n = _container->GetSelection ();
-	if (n >= 0) {
+	auto const n = _container->get();
+	if (n) {
 		auto ratios = Ratio::containers ();
-		DCPOMATIC_ASSERT (n < int(ratios.size()));
-		_film->set_container (ratios[n]);
+		DCPOMATIC_ASSERT(*n < int(ratios.size()));
+		_film->set_container(ratios[*n]);
 	}
 }
 
@@ -574,9 +575,9 @@ DCPPanel::dcp_content_type_changed ()
 		return;
 	}
 
-	int const n = _dcp_content_type->GetSelection ();
-	if (n != wxNOT_FOUND) {
-		_film->set_dcp_content_type (DCPContentType::from_index(n));
+	auto n = _dcp_content_type->get();
+	if (n) {
+		_film->set_dcp_content_type(DCPContentType::from_index(*n));
 	}
 }
 
@@ -777,14 +778,14 @@ DCPPanel::make_video_panel ()
 	panel->SetSizer (sizer);
 
 	_container_label = create_label (panel, _("Container"), true);
-	_container = new wxChoice (panel, wxID_ANY);
+	_container = new Choice(panel);
 	_container_size = new StaticText (panel, wxT (""));
 
 	_resolution_label = create_label (panel, _("Resolution"), true);
-	_resolution = new wxChoice (panel, wxID_ANY);
+	_resolution = new Choice(panel);
 
 	_frame_rate_label = create_label (panel, _("Frame Rate"), true);
-	_frame_rate_choice = new wxChoice (panel, wxID_ANY);
+	_frame_rate_choice = new Choice(panel);
 	_frame_rate_spin = new SpinCtrl (panel, DCPOMATIC_SPIN_CTRL_WIDTH);
 	setup_frame_rate_widget ();
 	_best_frame_rate = new Button (panel, _("Use best"));
@@ -809,18 +810,18 @@ DCPPanel::make_video_panel ()
 	_reencode_j2k->Bind      (wxEVT_CHECKBOX, boost::bind(&DCPPanel::reencode_j2k_changed, this));
 
 	for (auto i: Ratio::containers()) {
-		_container->Append (std_to_wx(i->container_nickname()));
+		_container->add(i->container_nickname());
 	}
 
 	for (auto i: Config::instance()->allowed_dcp_frame_rates()) {
-		_frame_rate_choice->Append (std_to_wx (boost::lexical_cast<string> (i)));
+		_frame_rate_choice->add(boost::lexical_cast<string>(i));
 	}
 
 	_j2k_bandwidth->SetRange (1, Config::instance()->maximum_j2k_bandwidth() / 1000000);
 	_frame_rate_spin->SetRange (1, 480);
 
-	_resolution->Append (_("2K"));
-	_resolution->Append (_("4K"));
+	_resolution->add(_("2K"));
+	_resolution->add(_("4K"));
 
 	add_video_panel_to_grid ();
 
@@ -895,7 +896,7 @@ DCPPanel::make_audio_panel ()
 	panel->SetSizer (_audio_panel_sizer);
 
 	_channels_label = create_label (panel, _("Channels"), true);
-	_audio_channels = new wxChoice (panel, wxID_ANY);
+	_audio_channels = new Choice(panel);
 	setup_audio_channels_choice (_audio_channels, minimum_allowed_audio_channels ());
 
 	if (Config::instance()->allow_96khz_audio()) {
@@ -904,7 +905,7 @@ DCPPanel::make_audio_panel ()
 	}
 
 	_processor_label = create_label (panel, _("Processor"), true);
-	_audio_processor = new wxChoice (panel, wxID_ANY);
+	_audio_processor = new Choice(panel);
 	add_audio_processors ();
 
 	_show_audio = new Button (panel, _("Show graph of audio levels..."));
@@ -962,11 +963,11 @@ DCPPanel::copy_isdcf_name_button_clicked ()
 void
 DCPPanel::audio_processor_changed ()
 {
-	if (!_film) {
+	if (!_film || !_audio_processor->get()) {
 		return;
 	}
 
-	auto const s = string_client_data (_audio_processor->GetClientObject (_audio_processor->GetSelection ()));
+	auto const s = string_client_data(_audio_processor->GetClientObject(*_audio_processor->get()));
 	_film->set_audio_processor (AudioProcessor::from_id (s));
 }
 
@@ -991,11 +992,11 @@ DCPPanel::show_audio_clicked ()
 void
 DCPPanel::reel_type_changed ()
 {
-	if (!_film) {
+	if (!_film || !_reel_type->get()) {
 		return;
 	}
 
-	_film->set_reel_type (static_cast<ReelType>(_reel_type->GetSelection()));
+	_film->set_reel_type(static_cast<ReelType>(*_reel_type->get()));
 }
 
 
@@ -1015,7 +1016,7 @@ DCPPanel::add_audio_processors ()
 {
 	_audio_processor->Append (_("None"), new wxStringClientData(N_("none")));
 	for (auto ap: AudioProcessor::visible()) {
-		_audio_processor->Append (std_to_wx(ap->name()), new wxStringClientData(std_to_wx(ap->id())));
+		_audio_processor->add(std_to_wx(ap->name()), new wxStringClientData(std_to_wx(ap->id())));
 	}
 	_audio_panel_sizer->Layout();
 }
diff --git a/src/wx/dcp_panel.h b/src/wx/dcp_panel.h
index 37bc20aa3..470f98fc8 100644
--- a/src/wx/dcp_panel.h
+++ b/src/wx/dcp_panel.h
@@ -37,13 +37,13 @@ class wxSizer;
 class wxGridBagSizer;
 
 class AudioDialog;
-class MarkersDialog;
-class InteropMetadataDialog;
-class SMPTEMetadataDialog;
+class Choice;
 class Film;
 class FilmViewer;
+class InteropMetadataDialog;
+class MarkersDialog;
 class Ratio;
-
+class SMPTEMetadataDialog;
 
 class DCPPanel
 {
@@ -123,35 +123,35 @@ private:
 	wxStaticText* _audio_language = nullptr;
 	Button* _edit_audio_language = nullptr;
 	wxStaticText* _container_label;
-	wxChoice* _container;
+	Choice* _container;
 	wxStaticText* _container_size;
 	wxButton* _copy_isdcf_name_button;
 	wxStaticText* _j2k_bandwidth_label;
 	wxStaticText* _mbits_label;
  	wxSpinCtrl* _j2k_bandwidth;
 	wxStaticText* _dcp_content_type_label;
-	wxChoice* _dcp_content_type;
+	Choice* _dcp_content_type;
 	wxStaticText* _frame_rate_label;
-	wxChoice* _frame_rate_choice;
+	Choice* _frame_rate_choice;
 	wxSpinCtrl* _frame_rate_spin;
 	wxSizer* _frame_rate_sizer;
 	wxStaticText* _channels_label;
-	wxChoice* _audio_channels;
+	Choice* _audio_channels;
 	wxStaticText* _audio_sample_rate_label = nullptr;
 	wxChoice* _audio_sample_rate = nullptr;
 	wxStaticText* _processor_label;
-	wxChoice* _audio_processor;
+	Choice* _audio_processor;
 	wxButton* _show_audio;
 	wxButton* _best_frame_rate;
 	wxCheckBox* _three_d;
 	wxCheckBox* _reencode_j2k;
 	wxStaticText* _resolution_label;
-	wxChoice* _resolution;
+	Choice* _resolution;
 	wxStaticText* _standard_label;
-	wxChoice* _standard;
+	Choice* _standard;
 	wxCheckBox* _encrypted;
 	wxStaticText* _reels_label;
-	wxChoice* _reel_type;
+	Choice* _reel_type;
 	wxStaticText* _reel_length_label;
 	wxStaticText* _reel_length_gb_label;
 	wxSpinCtrl* _reel_length;
diff --git a/src/wx/dcpomatic_choice.cc b/src/wx/dcpomatic_choice.cc
new file mode 100644
index 000000000..168d76430
--- /dev/null
+++ b/src/wx/dcpomatic_choice.cc
@@ -0,0 +1,90 @@
+/*
+    Copyright (C) 2022 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+
+#include "dcpomatic_choice.h"
+#include "wx_util.h"
+
+
+using std::string;
+using boost::optional;
+
+
+Choice::Choice(wxWindow* parent)
+	: wxChoice(parent, wxID_ANY)
+{
+	/* This hack works around a problem where the height of the wxChoice would be
+	 * too small on KDE.  This added empty string will be removed in the first
+	 * call to add().
+	 */
+	Append("");
+	set(0);
+}
+
+
+void
+Choice::add(string const& entry)
+{
+	add(std_to_wx(entry));
+}
+
+
+void
+Choice::add(wxString const& entry)
+{
+	if (_needs_clearing) {
+		Clear();
+		_needs_clearing = false;
+	}
+
+	Append(entry);
+}
+
+
+void
+Choice::add(wxString const& entry, wxClientData* data)
+{
+	if (_needs_clearing) {
+		Clear();
+		_needs_clearing = false;
+	}
+
+	Append(entry, data);
+}
+
+
+void
+Choice::set(int index)
+{
+	SetSelection(index);
+}
+
+
+optional<int>
+Choice::get() const
+{
+	auto const sel = GetSelection();
+	if (sel == wxNOT_FOUND) {
+		return {};
+	}
+
+	return sel;
+}
+
diff --git a/src/wx/dcpomatic_choice.h b/src/wx/dcpomatic_choice.h
new file mode 100644
index 000000000..3a5987ead
--- /dev/null
+++ b/src/wx/dcpomatic_choice.h
@@ -0,0 +1,43 @@
+/*
+    Copyright (C) 2022 Carl Hetherington <cth@carlh.net>
+
+    This file is part of DCP-o-matic.
+
+    DCP-o-matic is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or
+    (at your option) any later version.
+
+    DCP-o-matic is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with DCP-o-matic.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+
+#include <dcp/warnings.h>
+LIBDCP_DISABLE_WARNINGS
+#include <wx/wx.h>
+LIBDCP_ENABLE_WARNINGS
+#include <boost/optional.hpp>
+
+
+class Choice : public wxChoice
+{
+public:
+	Choice(wxWindow* parent);
+
+	void add(wxString const& entry);
+	void add(wxString const& entry, wxClientData* data);
+	void add(std::string const& entry);
+	void set(int index);
+	boost::optional<int> get() const;
+
+private:
+	bool _needs_clearing = true;
+};
+
diff --git a/src/wx/metadata_dialog.cc b/src/wx/metadata_dialog.cc
index 7e0d304f8..c6f805ed6 100644
--- a/src/wx/metadata_dialog.cc
+++ b/src/wx/metadata_dialog.cc
@@ -20,6 +20,7 @@
 
 
 #include "dcpomatic_button.h"
+#include "dcpomatic_choice.h"
 #include "editable_list.h"
 #include "full_language_tag_dialog.h"
 #include "language_tag_widget.h"
@@ -303,14 +304,13 @@ MetadataDialog::setup_advanced (wxPanel* panel, wxSizer* sizer)
 		_luminance_value->SetDigits (1);
 		_luminance_value->SetIncrement (0.1);
 		s->Add (_luminance_value, 0);
-		_luminance_unit = new wxChoice (panel, wxID_ANY);
+		_luminance_unit = new Choice(panel);
 		s->Add (_luminance_unit, 0, wxLEFT, DCPOMATIC_SIZER_X_GAP);
 		sizer->Add (s, 1, wxEXPAND);
 	}
 
-	_luminance_unit->Append (_("candela per m²"));
-	_luminance_unit->Append (_("foot lambert"));
-
+	_luminance_unit->add(_("candela per m²"));
+	_luminance_unit->add(_("foot lambert"));
 }
 
 
@@ -415,7 +415,8 @@ void
 MetadataDialog::luminance_changed ()
 {
 	dcp::Luminance::Unit unit;
-	switch (_luminance_unit->GetSelection()) {
+	DCPOMATIC_ASSERT(_luminance_unit->get());
+	switch (*_luminance_unit->get()) {
 	case 0:
 		unit = dcp::Luminance::Unit::CANDELA_PER_SQUARE_METRE;
 		break;
diff --git a/src/wx/metadata_dialog.h b/src/wx/metadata_dialog.h
index ea7233cbb..29d8b39cc 100644
--- a/src/wx/metadata_dialog.h
+++ b/src/wx/metadata_dialog.h
@@ -35,9 +35,10 @@ LIBDCP_ENABLE_WARNINGS
 
 
 class Button;
+class Choice;
+class LanguageTagWidget;
 class RatingDialog;
 class wxSpinCtrlDouble;
-class LanguageTagWidget;
 
 
 class MetadataDialog : public wxDialog, public WeakFilm
@@ -96,7 +97,7 @@ private:
 	wxCheckBox* _two_d_version_of_three_d;
 	wxCheckBox* _enable_luminance;
 	wxSpinCtrlDouble* _luminance_value;
-	wxChoice* _luminance_unit;
+	Choice* _luminance_unit;
 
 	boost::signals2::scoped_connection _film_changed_connection;
 };
diff --git a/src/wx/wscript b/src/wx/wscript
index 236e99f97..ba51aec57 100644
--- a/src/wx/wscript
+++ b/src/wx/wscript
@@ -56,6 +56,7 @@ sources = """
           dcp_panel.cc
           dcp_text_track_dialog.cc
           dcpomatic_button.cc
+          dcpomatic_choice.cc
           dcpomatic_spin_ctrl.cc
           dir_picker_ctrl.cc
           disk_warning_dialog.cc
diff --git a/src/wx/wx_util.cc b/src/wx/wx_util.cc
index d02eb3ca5..9d9d4c599 100644
--- a/src/wx/wx_util.cc
+++ b/src/wx/wx_util.cc
@@ -304,7 +304,7 @@ checked_set (wxChoice* widget, vector<pair<string, string>> items)
                current.push_back (
                        make_pair(
                                wx_to_std(widget->GetString(i)),
-                               string_client_data(widget->GetClientObject(i))
+                               widget->GetClientData() ? string_client_data(widget->GetClientObject(i)) : ""
                                )
                        );
        }
@@ -724,3 +724,4 @@ report_config_load_failure(wxWindow* parent, Config::LoadFailure what)
 		break;
 	}
 }
+