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

diff --git a/src/wx/dcp_panel.cc b/src/wx/dcp_panel.cc
index 895aed9c9cc98e36b47eb5ecc9e1665d2c309d3e..ea1dd6d612ab9221014eef9e02e23e25edca1dcf 100644 (file)
--- 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 37bc20aa39f68b25a6b085fe5e7559a3a022819b..470f98fc8b54afbd1bac1f2b0d4fa6983d107ccb 100644 (file)
--- 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 (file)
index 0000000..168d764
--- /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 (file)
index 0000000..3a5987e
--- /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 7e0d304f86f9541dfb7b62495f446e86de314a94..c6f805ed6ed57751f7ad138ce6192d5d91a037bb 100644 (file)
--- 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 ea7233cbb1f3b12bfd6086abc78ce3dde33fc493..29d8b39cce6f1250af7a84f6d5330c74380b8eae 100644 (file)
--- 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 236e99f975f091f5befe60123222c4ff9317740c..ba51aec571db9cf905cad55675a05daaa6c32835 100644 (file)
--- 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 d02eb3ca5d3b3e68a6f5d26b87e1fd19ac14480d..9d9d4c599ff2862ad164e07931facfedbba6b5de 100644 (file)
--- 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;
        }
 }
+