summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCarl Hetherington <cth@carlh.net>2025-02-25 01:53:48 +0100
committerCarl Hetherington <cth@carlh.net>2025-02-25 13:04:08 +0100
commit6d3086dd78a35bdc179a6579e23f8b5816347dbf (patch)
treea47facfec259302afd6e0e690b748ad9f655548e
parent91c171b65e7329b90ac77442fd35b165bde70b26 (diff)
Fix misunderstanding of wxDialog lifetime handling.
Broken by d0308d53dd9f4d036d8c5fe8023920fcdfd43f39 wxDialog can be stack allocated if opened with ShowModal(), but not with Show(). Go back to wx_ptr for those that are opened with Show().
-rw-r--r--src/tools/dcpomatic.cc16
-rw-r--r--src/tools/dcpomatic_player.cc5
-rw-r--r--src/wx/audio_panel.cc2
-rw-r--r--src/wx/audio_panel.h5
-rw-r--r--src/wx/content_menu.cc2
-rw-r--r--src/wx/content_menu.h3
-rw-r--r--src/wx/content_panel.cc2
-rw-r--r--src/wx/content_panel.h3
-rw-r--r--src/wx/dcp_panel.cc10
-rw-r--r--src/wx/dcp_panel.h20
-rw-r--r--src/wx/dcp_timeline.cc2
-rw-r--r--src/wx/text_panel.cc6
-rw-r--r--src/wx/text_panel.h14
-rw-r--r--src/wx/wx_ptr.h117
14 files changed, 163 insertions, 44 deletions
diff --git a/src/tools/dcpomatic.cc b/src/tools/dcpomatic.cc
index 7048781e3..3d9d571af 100644
--- a/src/tools/dcpomatic.cc
+++ b/src/tools/dcpomatic.cc
@@ -876,7 +876,7 @@ private:
return;
}
- _kdm_dialog.emplace(this, _film);
+ _kdm_dialog.reset(this, _film);
_kdm_dialog->Show ();
}
@@ -886,7 +886,7 @@ private:
return;
}
- _dkdm_dialog.emplace(this, _film);
+ _dkdm_dialog.reset(this, _film);
_dkdm_dialog->Show ();
}
@@ -1083,7 +1083,7 @@ private:
void view_video_waveform ()
{
if (!_video_waveform_dialog) {
- _video_waveform_dialog.emplace(this, _film, _film_viewer);
+ _video_waveform_dialog.reset(this, _film, _film_viewer);
}
_video_waveform_dialog->Show ();
@@ -1130,7 +1130,7 @@ private:
void tools_manage_templates ()
{
if (!_templates_dialog) {
- _templates_dialog.emplace(this);
+ _templates_dialog.reset(this);
}
_templates_dialog->Show ();
@@ -1578,15 +1578,15 @@ private:
wxPanel* _right_panel;
FilmViewer _film_viewer;
StandardControls* _controls;
- boost::optional<VideoWaveformDialog> _video_waveform_dialog;
+ wx_ptr<VideoWaveformDialog> _video_waveform_dialog;
SystemInformationDialog* _system_information_dialog = nullptr;
DCPReferencingDialog* _dcp_referencing_dialog = nullptr;
HintsDialog* _hints_dialog = nullptr;
ServersListDialog* _servers_list_dialog = nullptr;
wxPreferencesEditor* _config_dialog = nullptr;
- boost::optional<KDMDialog> _kdm_dialog;
- boost::optional<DKDMDialog> _dkdm_dialog;
- boost::optional<TemplatesDialog> _templates_dialog;
+ wx_ptr<KDMDialog> _kdm_dialog;
+ wx_ptr<DKDMDialog> _dkdm_dialog;
+ wx_ptr<TemplatesDialog> _templates_dialog;
wxMenu* _file_menu = nullptr;
shared_ptr<Film> _film;
int _history_items = 0;
diff --git a/src/tools/dcpomatic_player.cc b/src/tools/dcpomatic_player.cc
index b0a791633..c5242c310 100644
--- a/src/tools/dcpomatic_player.cc
+++ b/src/tools/dcpomatic_player.cc
@@ -33,6 +33,7 @@
#include "wx/timer_display.h"
#include "wx/update_dialog.h"
#include "wx/verify_dcp_dialog.h"
+#include "wx/wx_ptr.h"
#include "wx/wx_signal_manager.h"
#include "wx/wx_util.h"
#include "wx/wx_variant.h"
@@ -1014,7 +1015,7 @@ private:
void tools_system_information ()
{
if (!_system_information_dialog) {
- _system_information_dialog.emplace(this, _viewer);
+ _system_information_dialog.reset(this, _viewer);
}
_system_information_dialog->Show ();
@@ -1234,7 +1235,7 @@ private:
wxMenuItem* _history_separator = nullptr;
FilmViewer _viewer;
Controls* _controls;
- boost::optional<SystemInformationDialog> _system_information_dialog;
+ wx_ptr<SystemInformationDialog> _system_information_dialog;
std::shared_ptr<Film> _film;
boost::signals2::scoped_connection _config_changed_connection;
boost::signals2::scoped_connection _examine_job_connection;
diff --git a/src/wx/audio_panel.cc b/src/wx/audio_panel.cc
index d6a4ca130..1418f1ff9 100644
--- a/src/wx/audio_panel.cc
+++ b/src/wx/audio_panel.cc
@@ -400,7 +400,7 @@ AudioPanel::show_clicked ()
return;
}
- _audio_dialog.emplace(this, _parent->film(), _parent->film_viewer(), ac.front());
+ _audio_dialog.reset(this, _parent->film(), _parent->film_viewer(), ac.front());
_audio_dialog->Show ();
}
diff --git a/src/wx/audio_panel.h b/src/wx/audio_panel.h
index 56552992d..813246652 100644
--- a/src/wx/audio_panel.h
+++ b/src/wx/audio_panel.h
@@ -19,13 +19,14 @@
*/
-#include "audio_dialog.h"
#include "content_sub_panel.h"
#include "content_widget.h"
#include "timecode.h"
+#include "wx_ptr.h"
#include "lib/audio_mapping.h"
+class AudioDialog;
class AudioMappingView;
class CheckBox;
class LanguageTagWidget;
@@ -76,7 +77,7 @@ private:
CheckBox* _use_same_fades_as_video;
AudioMappingView* _mapping;
wxStaticText* _description;
- boost::optional<AudioDialog> _audio_dialog;
+ wx_ptr<AudioDialog> _audio_dialog;
boost::signals2::scoped_connection _mapping_connection;
boost::signals2::scoped_connection _active_jobs_connection;
diff --git a/src/wx/content_menu.cc b/src/wx/content_menu.cc
index 1106a10fc..e2dfdaf97 100644
--- a/src/wx/content_menu.cc
+++ b/src/wx/content_menu.cc
@@ -570,7 +570,7 @@ ContentMenu::auto_crop ()
auto const crop = guess_crop_for_content ();
update_viewer (crop);
- _auto_crop_dialog.emplace(_parent, crop);
+ _auto_crop_dialog.reset(_parent, crop);
_auto_crop_dialog->Show ();
/* Update the dialog and view when the crop threshold changes */
diff --git a/src/wx/content_menu.h b/src/wx/content_menu.h
index a9348fe29..e7f095390 100644
--- a/src/wx/content_menu.h
+++ b/src/wx/content_menu.h
@@ -25,6 +25,7 @@
#include "auto_crop_dialog.h"
#include "timeline_content_view.h"
+#include "wx_ptr.h"
#include "lib/types.h"
#include <dcp/warnings.h>
LIBDCP_DISABLE_WARNINGS
@@ -89,7 +90,7 @@ private:
wxMenuItem* _set_dcp_markers;
wxMenuItem* _remove;
- boost::optional<AutoCropDialog> _auto_crop_dialog;
+ wx_ptr<AutoCropDialog> _auto_crop_dialog;
boost::signals2::scoped_connection _auto_crop_config_connection;
boost::signals2::scoped_connection _auto_crop_viewer_connection;
};
diff --git a/src/wx/content_panel.cc b/src/wx/content_panel.cc
index bb8acf664..8938ef0fd 100644
--- a/src/wx/content_panel.cc
+++ b/src/wx/content_panel.cc
@@ -733,7 +733,7 @@ ContentPanel::timeline_clicked ()
return;
}
- _timeline_dialog.emplace(this, _film, _film_viewer);
+ _timeline_dialog.reset(this, _film, _film_viewer);
_timeline_dialog->set_selection (selected());
_timeline_dialog->Show ();
}
diff --git a/src/wx/content_panel.h b/src/wx/content_panel.h
index 18d740828..f99d518a2 100644
--- a/src/wx/content_panel.h
+++ b/src/wx/content_panel.h
@@ -20,7 +20,6 @@
#include "content_menu.h"
-#include "content_timeline_dialog.h"
#include "lib/enum_indexed_vector.h"
#include "lib/film_property.h"
#include "lib/text_type.h"
@@ -134,7 +133,7 @@ private:
EnumIndexedVector<TextPanel*, TextType> _text_panel;
TimingPanel* _timing_panel;
ContentMenu* _menu;
- boost::optional<ContentTimelineDialog> _timeline_dialog;
+ wx_ptr<ContentTimelineDialog> _timeline_dialog;
wxNotebook* _parent;
wxWindow* _last_selected_tab = nullptr;
diff --git a/src/wx/dcp_panel.cc b/src/wx/dcp_panel.cc
index 3d28a3ae5..0f0edc971 100644
--- a/src/wx/dcp_panel.cc
+++ b/src/wx/dcp_panel.cc
@@ -339,7 +339,7 @@ DCPPanel::resolution_changed ()
void
DCPPanel::markers_clicked ()
{
- _markers_dialog.emplace(_panel, _film, _viewer);
+ _markers_dialog.reset(_panel, _film, _viewer);
_markers_dialog->Show();
}
@@ -348,11 +348,11 @@ void
DCPPanel::metadata_clicked ()
{
if (_film->interop()) {
- _interop_metadata_dialog.emplace(_panel, _film);
+ _interop_metadata_dialog.reset(_panel, _film);
_interop_metadata_dialog->setup ();
_interop_metadata_dialog->Show ();
} else {
- _smpte_metadata_dialog.emplace(_panel, _film);
+ _smpte_metadata_dialog.reset(_panel, _film);
_smpte_metadata_dialog->setup ();
_smpte_metadata_dialog->Show ();
}
@@ -362,7 +362,7 @@ DCPPanel::metadata_clicked ()
void
DCPPanel::reels_clicked()
{
- _dcp_timeline.emplace(_panel, _film);
+ _dcp_timeline.reset(_panel, _film);
_dcp_timeline->Show();
}
@@ -1029,7 +1029,7 @@ DCPPanel::show_audio_clicked ()
return;
}
- _audio_dialog.emplace(_panel, _film, _viewer);
+ _audio_dialog.reset(_panel, _film, _viewer);
_audio_dialog->Show();
}
diff --git a/src/wx/dcp_panel.h b/src/wx/dcp_panel.h
index 25b7d959d..4988922a8 100644
--- a/src/wx/dcp_panel.h
+++ b/src/wx/dcp_panel.h
@@ -19,11 +19,7 @@
*/
-#include "audio_dialog.h"
-#include "dcp_timeline_dialog.h"
-#include "interop_metadata_dialog.h"
-#include "markers_dialog.h"
-#include "smpte_metadata_dialog.h"
+#include "wx_ptr.h"
#include "lib/config.h"
#include "lib/film_property.h"
@@ -41,11 +37,15 @@ class wxSpinCtrl;
class wxSizer;
class wxGridBagSizer;
+class AudioDialog;
class Choice;
+class DCPTimelineDialog;
class Film;
class FilmViewer;
class InteropMetadataDialog;
+class MarkersDialog;
class Ratio;
+class SMPTEMetadataDialog;
class DCPPanel
{
@@ -158,11 +158,11 @@ private:
Button* _reels;
wxSizer* _audio_panel_sizer;
- boost::optional<AudioDialog> _audio_dialog;
- boost::optional<MarkersDialog> _markers_dialog;
- boost::optional<InteropMetadataDialog> _interop_metadata_dialog;
- boost::optional<SMPTEMetadataDialog> _smpte_metadata_dialog;
- boost::optional<DCPTimelineDialog> _dcp_timeline;
+ wx_ptr<AudioDialog> _audio_dialog;
+ wx_ptr<MarkersDialog> _markers_dialog;
+ wx_ptr<InteropMetadataDialog> _interop_metadata_dialog;
+ wx_ptr<SMPTEMetadataDialog> _smpte_metadata_dialog;
+ wx_ptr<DCPTimelineDialog> _dcp_timeline;
std::shared_ptr<Film> _film;
FilmViewer& _viewer;
diff --git a/src/wx/dcp_timeline.cc b/src/wx/dcp_timeline.cc
index 85119dc34..a51c18738 100644
--- a/src/wx/dcp_timeline.cc
+++ b/src/wx/dcp_timeline.cc
@@ -150,7 +150,7 @@ DCPTimeline::DCPTimeline(wxWindow* parent, shared_ptr<Film> film)
_reel_detail->SetSizer(_reel_detail_sizer);
auto sizer = new wxBoxSizer(wxVERTICAL);
- sizer->Add(_reel_settings, 0);
+ sizer->Add(_reel_settings, 0, wxBOTTOM, DCPOMATIC_SIZER_GAP);
sizer->Add(_canvas, 0, wxEXPAND);
sizer->Add(_reel_detail, 1, wxEXPAND | wxALL, DCPOMATIC_DIALOG_BORDER);
SetSizer(sizer);
diff --git a/src/wx/text_panel.cc b/src/wx/text_panel.cc
index ddb5bf56d..f2055a542 100644
--- a/src/wx/text_panel.cc
+++ b/src/wx/text_panel.cc
@@ -689,7 +689,7 @@ TextPanel::text_view_clicked ()
auto decoder = decoder_factory (_parent->film(), c.front(), false, false, shared_ptr<Decoder>());
if (decoder) {
- _text_view.emplace(this, _parent->film(), c.front(), c.front()->text_of_original_type(_original_type), decoder, _parent->film_viewer());
+ _text_view.reset(this, _parent->film(), c.front(), c.front()->text_of_original_type(_original_type), decoder, _parent->film_viewer());
_text_view->show();
}
}
@@ -701,8 +701,8 @@ TextPanel::fonts_dialog_clicked ()
auto c = _parent->selected_text ();
DCPOMATIC_ASSERT (c.size() == 1);
- _fonts_dialog.emplace(this, c.front(), c.front()->text_of_original_type(_original_type));
- _fonts_dialog->Show();
+ _fonts_dialog.reset(this, c.front(), c.front()->text_of_original_type(_original_type));
+ _fonts_dialog->Show ();
}
diff --git a/src/wx/text_panel.h b/src/wx/text_panel.h
index e89b9a54d..3c6e183dc 100644
--- a/src/wx/text_panel.h
+++ b/src/wx/text_panel.h
@@ -19,19 +19,19 @@
*/
-#include "content_sub_panel.h"
-#include "fonts_dialog.h"
-#include "text_view.h"
#include "lib/job.h"
+#include "content_sub_panel.h"
+#include "wx_ptr.h"
class CheckBox;
class Choice;
+class wxSpinCtrl;
class LanguageTagWidget;
+class TextView;
+class FontsDialog;
class SpinCtrl;
class SubtitleAnalysis;
-class TextView;
-class wxSpinCtrl;
class TextPanel : public ContentSubPanel
@@ -100,9 +100,9 @@ private:
wxStaticText* _stream_label;
wxChoice* _stream;
wxButton* _text_view_button;
- boost::optional<TextView> _text_view;
+ wx_ptr<TextView> _text_view;
wxButton* _fonts_dialog_button;
- boost::optional<FontsDialog> _fonts_dialog;
+ wx_ptr<FontsDialog> _fonts_dialog;
wxButton* _appearance_dialog_button;
TextType _original_type;
wxStaticText* _language_label = nullptr;
diff --git a/src/wx/wx_ptr.h b/src/wx/wx_ptr.h
new file mode 100644
index 000000000..fcca8b18b
--- /dev/null
+++ b/src/wx/wx_ptr.h
@@ -0,0 +1,117 @@
+/*
+ Copyright (C) 2023 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/>.
+
+*/
+
+
+#ifndef DCPOMATIC_WX_PTR_H
+#define DCPOMATIC_WX_PTR_H
+
+
+#include "lib/dcpomatic_assert.h"
+#include <utility>
+
+
+template <class T>
+class wx_ptr
+{
+public:
+ wx_ptr() {}
+
+ explicit wx_ptr(T* wx)
+ : _wx(wx)
+ {}
+
+ wx_ptr(wx_ptr&) = delete;
+ wx_ptr& operator=(wx_ptr&) = delete;
+
+ wx_ptr(wx_ptr&& other)
+ {
+ _wx = other._wx;
+ other._wx = nullptr;
+ }
+
+ wx_ptr& operator=(wx_ptr&& other)
+ {
+ if (this != &other) {
+ _wx = other._wx;
+ other._wx = nullptr;
+ }
+ return *this;
+ }
+
+ ~wx_ptr()
+ {
+ if (_wx) {
+ _wx->Destroy();
+ }
+ }
+
+ wx_ptr& operator=(T* ptr)
+ {
+ if (_wx) {
+ _wx->Destroy();
+ }
+ _wx = ptr;
+ return *this;
+ }
+
+ T* operator->()
+ {
+ DCPOMATIC_ASSERT(_wx);
+ return _wx;
+ }
+
+ operator bool() const
+ {
+ return _wx != nullptr;
+ }
+
+ void reset()
+ {
+ if (_wx) {
+ _wx->Destroy();
+ _wx = nullptr;
+ }
+ }
+
+ template <typename... Args>
+ void reset(Args&&... args)
+ {
+ if (_wx) {
+ _wx->Destroy();
+ _wx = nullptr;
+ }
+ _wx = new T(std::forward<Args>(args)...);
+ }
+
+private:
+ T* _wx = nullptr;
+};
+
+
+
+template <class T, typename... Args>
+wx_ptr<T>
+make_wx(Args... args)
+{
+ return wx_ptr<T>(new T(std::forward<Args>(args)...));
+}
+
+
+#endif