From a577af1ca656e1f7060cbe47ce7ac92162e87386 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 6 Dec 2022 01:44:22 +0100 Subject: [PATCH] Initialize atoms used in wxGTK wxDataObject on demand This allows to avoid having to call PrepareFormats() manually which was not only verbose, but also error-prone as proved by the fact that wxURLDataObject used wxTextURIListDataObject which used g_fileAtom in its ctor without calling PrepareFormats() first and meant that this class didn't work correctly unless some other wxDataObject had been created before it. --- include/wx/gtk/dataform.h | 2 - src/gtk/clipbrd.cpp | 19 +++++---- src/gtk/dataobj.cpp | 83 +++++++++++++++++++-------------------- 3 files changed, 51 insertions(+), 53 deletions(-) diff --git a/include/wx/gtk/dataform.h b/include/wx/gtk/dataform.h index e248653eea..c5b513786f 100644 --- a/include/wx/gtk/dataform.h +++ b/include/wx/gtk/dataform.h @@ -70,8 +70,6 @@ private: wxDataFormatId m_type; NativeFormat m_format; - - void PrepareFormats(); }; #endif // _WX_GTK_DATAFORM_H diff --git a/src/gtk/clipbrd.cpp b/src/gtk/clipbrd.cpp index 099b145862..a3be262f7e 100644 --- a/src/gtk/clipbrd.cpp +++ b/src/gtk/clipbrd.cpp @@ -44,7 +44,8 @@ typedef wxScopedArray wxDataFormatArray; static GdkAtom g_targetsAtom = nullptr; static GdkAtom g_timestampAtom = nullptr; -extern GdkAtom g_altTextAtom; +// This is defined in src/gtk/dataobj.cpp. +extern GdkAtom wxGetAltTextAtom(); // the trace mask we use with wxLogTrace() - call // wxLog::AddTraceMask(TRACE_CLIPBOARD) to enable the trace messages from here @@ -304,7 +305,7 @@ selection_handler( GtkWidget *WXUNUSED(widget), if ( !size ) return; - wxLogTrace(TRACE_CLIPBOARD, "Valid clipboard data found"); + wxLogTrace(TRACE_CLIPBOARD, "Valid clipboard data of size %d found", size); wxCharBuffer buf(size - 1); // it adds 1 internally (for NUL) @@ -337,16 +338,18 @@ void wxClipboard::GTKOnSelectionReceived(const GtkSelectionData& sel) { wxCHECK_RET( m_receivedData, wxT("should be inside GetData()") ); - const wxDataFormat format(gtk_selection_data_get_target(const_cast(&sel))); - wxLogTrace(TRACE_CLIPBOARD, wxT("Received selection %s"), - format.GetId()); + GtkSelectionData* const gsel = const_cast(&sel); + + const wxDataFormat format(gtk_selection_data_get_target(gsel)); + wxLogTrace(TRACE_CLIPBOARD, wxT("Received selection %s, len=%d"), + format.GetId(), gtk_selection_data_get_length(gsel)); if ( !m_receivedData->IsSupportedFormat(format, wxDataObject::Set) ) return; m_receivedData->SetData(format, - gtk_selection_data_get_length(const_cast(&sel)), - gtk_selection_data_get_data(const_cast(&sel))); + gtk_selection_data_get_length(gsel), + gtk_selection_data_get_data(gsel)); m_formatSupported = true; } @@ -688,7 +691,7 @@ bool wxClipboard::IsSupported( const wxDataFormat& format ) if ( format == wxDF_UNICODETEXT ) { // also with plain STRING format - return DoIsSupported(g_altTextAtom); + return DoIsSupported(wxGetAltTextAtom()); } return false; diff --git a/src/gtk/dataobj.cpp b/src/gtk/dataobj.cpp index 7589bca170..f5a0a43b68 100644 --- a/src/gtk/dataobj.cpp +++ b/src/gtk/dataobj.cpp @@ -25,14 +25,48 @@ #include "wx/gtk/private.h" //------------------------------------------------------------------------- -// global data +// module data //------------------------------------------------------------------------- -GdkAtom g_textAtom = nullptr; -GdkAtom g_altTextAtom = nullptr; -GdkAtom g_pngAtom = nullptr; -GdkAtom g_fileAtom = nullptr; -GdkAtom g_htmlAtom = nullptr; +namespace +{ + +// Atom initialized on first access. +// +// Note that this is more than just an optimization, as we have to delay +// calling gdk_atom_intern() until after GDK initialization. +class wxGdkAtom +{ +public: + // Name is literal, so we don't copy it but just store the pointer. + wxGdkAtom(const char* name) : m_name{name} {} + + wxGdkAtom(const wxGdkAtom&) = delete; + wxGdkAtom& operator=(const wxGdkAtom&) = delete; + + operator GdkAtom() + { + if ( !m_atom ) + m_atom = gdk_atom_intern(m_name, FALSE); + + return m_atom; + } + +private: + const char* const m_name; + GdkAtom m_atom = nullptr; +}; + +wxGdkAtom g_textAtom {"UTF8_STRING"}; +wxGdkAtom g_altTextAtom {"STRING"}; +wxGdkAtom g_pngAtom {"image/png"}; +wxGdkAtom g_fileAtom {"text/uri-list"}; +wxGdkAtom g_htmlAtom {"text/html"}; + +} // anonymous namespace + +// This is used in src/gtk/clipbrd.cpp +extern GdkAtom wxGetAltTextAtom() { return g_altTextAtom; } //------------------------------------------------------------------------- // wxDataFormat @@ -40,40 +74,27 @@ GdkAtom g_htmlAtom = nullptr; wxDataFormat::wxDataFormat() { - // do *not* call PrepareFormats() from here for 2 reasons: - // - // 1. we will have time to do it later because some other Set function - // must be called before we really need them - // - // 2. doing so prevents us from declaring global wxDataFormats because - // calling PrepareFormats (and thus gdk_atom_intern) before GDK is - // initialised will result in a crash m_type = wxDF_INVALID; m_format = (GdkAtom) nullptr; } wxDataFormat::wxDataFormat( wxDataFormatId type ) { - PrepareFormats(); SetType( type ); } void wxDataFormat::InitFromString( const wxString &id ) { - PrepareFormats(); SetId( id ); } wxDataFormat::wxDataFormat( NativeFormat format ) { - PrepareFormats(); SetId( format ); } void wxDataFormat::SetType( wxDataFormatId type ) { - PrepareFormats(); - m_type = type; if (m_type == wxDF_UNICODETEXT) @@ -108,7 +129,6 @@ wxString wxDataFormat::GetId() const void wxDataFormat::SetId( NativeFormat format ) { - PrepareFormats(); m_format = format; if (m_format == g_textAtom) @@ -131,33 +151,10 @@ void wxDataFormat::SetId( NativeFormat format ) void wxDataFormat::SetId( const wxString& id ) { - PrepareFormats(); m_type = wxDF_PRIVATE; m_format = gdk_atom_intern( id.ToAscii(), FALSE ); } -void wxDataFormat::PrepareFormats() -{ - // VZ: GNOME included in RedHat 6.1 uses the MIME types below and not the - // atoms STRING and file:ALL as the old code was, but normal X apps - // use STRING for text selection when transferring the data via - // clipboard, for example, so do use STRING for now (GNOME apps will - // probably support STRING as well for compatibility anyhow), but use - // text/uri-list for file dnd because compatibility is not important - // here (with whom?) - if (!g_textAtom) - { - g_textAtom = gdk_atom_intern( "UTF8_STRING", FALSE ); - g_altTextAtom = gdk_atom_intern( "STRING", FALSE ); - } - if (!g_pngAtom) - g_pngAtom = gdk_atom_intern( "image/png", FALSE ); - if (!g_fileAtom) - g_fileAtom = gdk_atom_intern( "text/uri-list", FALSE ); - if (!g_htmlAtom) - g_htmlAtom = gdk_atom_intern( "text/html", FALSE ); -} - //------------------------------------------------------------------------- // wxDataObject //-------------------------------------------------------------------------