From e0c88b1e53244b72e01b153b4b10edb78d0e7091 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Thu, 18 Feb 2016 19:09:10 -0500 Subject: [PATCH 1/2] Remove needless indirection for more little structures, in CommandManager... ... Also use std::shared_ptr for functors to simplify resource management --- src/Menus.cpp | 6 +-- src/commands/CommandManager.cpp | 96 ++++++++++++--------------------- src/commands/CommandManager.h | 38 ++++++++----- 3 files changed, 60 insertions(+), 80 deletions(-) diff --git a/src/Menus.cpp b/src/Menus.cpp index b91b38375..10bb01015 100644 --- a/src/Menus.cpp +++ b/src/Menus.cpp @@ -198,9 +198,9 @@ void AudacityProjectCommandFunctor::operator()(int index, const wxEvent * evt) (mProject->*(mCommandFunction)) (); } -#define FN(X) new AudacityProjectCommandFunctor(this, &AudacityProject:: X ) -#define FNI(X, I) new AudacityProjectCommandFunctor(this, &AudacityProject:: X, I) -#define FNS(X, S) new AudacityProjectCommandFunctor(this, &AudacityProject:: X, S) +#define FN(X) CommandFunctorPointer{safenew AudacityProjectCommandFunctor(this, &AudacityProject:: X )} +#define FNI(X, I) CommandFunctorPointer{safenew AudacityProjectCommandFunctor(this, &AudacityProject:: X, I)} +#define FNS(X, S) CommandFunctorPointer{safenew AudacityProjectCommandFunctor(this, &AudacityProject:: X, S)} // // Effects menu arrays diff --git a/src/commands/CommandManager.cpp b/src/commands/CommandManager.cpp index 156238007..4ae76ba9a 100644 --- a/src/commands/CommandManager.cpp +++ b/src/commands/CommandManager.cpp @@ -413,29 +413,12 @@ CommandManager::~CommandManager() void CommandManager::PurgeData() { - // Delete callback functors BEFORE clearing mCommandList! - // These are the items created within 'FN()' - size_t i; - CommandFunctor * pCallback = NULL; - for(i=0; icallback != pCallback ) - { - pCallback = tmpEntry->callback; - delete pCallback; - } - } - // mCommandList contains pointers to CommandListEntrys - // mMenuBarList contains pointers to MenuBarListEntrys. - // mSubMenuList contains pointers to SubMenuListEntrys + // mMenuBarList contains MenuBarListEntrys. + // mSubMenuList contains SubMenuListEntrys WX_CLEAR_ARRAY( mCommandList ); - WX_CLEAR_ARRAY( mMenuBarList ); - WX_CLEAR_ARRAY( mSubMenuList ); + mMenuBarList.clear(); + mSubMenuList.clear(); mCommandNameHash.clear(); mCommandKeyHash.clear(); @@ -458,14 +441,10 @@ wxMenuBar *CommandManager::AddMenuBar(const wxString & sMenu) if (menuBar) return menuBar; - MenuBarListEntry *tmpEntry = new MenuBarListEntry; + const auto result = new wxMenuBar{}; + mMenuBarList.emplace_back(sMenu, result); - tmpEntry->menubar = new wxMenuBar(); - tmpEntry->name = sMenu; - - mMenuBarList.Add(tmpEntry); - - return tmpEntry->menubar; + return result; } @@ -474,10 +453,10 @@ wxMenuBar *CommandManager::AddMenuBar(const wxString & sMenu) /// wxMenuBar * CommandManager::GetMenuBar(const wxString & sMenu) const { - for(unsigned int i = 0; i < mMenuBarList.GetCount(); i++) + for (const auto &entry : mMenuBarList) { - if(!mMenuBarList[i]->name.Cmp(sMenu)) - return mMenuBarList[i]->menubar; + if(!entry.name.Cmp(sMenu)) + return entry.menubar; } return NULL; @@ -489,10 +468,10 @@ wxMenuBar * CommandManager::GetMenuBar(const wxString & sMenu) const /// last on in the mMenuBarList. wxMenuBar * CommandManager::CurrentMenuBar() const { - if(mMenuBarList.IsEmpty()) + if(mMenuBarList.empty()) return NULL; - return mMenuBarList[mMenuBarList.GetCount()-1]->menubar; + return mMenuBarList.back().menubar; } @@ -527,15 +506,10 @@ void CommandManager::EndMenu() /// the function's argument. wxMenu* CommandManager::BeginSubMenu(const wxString & tName) { - SubMenuListEntry *tmpEntry = new SubMenuListEntry; - - tmpEntry->menu = new wxMenu(); - tmpEntry->name = tName; - - mSubMenuList.Add(tmpEntry); + const auto result = new wxMenu{}; + mSubMenuList.emplace_back(tName, result); mbSeparatorAllowed = false; - - return(tmpEntry->menu); + return result; } @@ -545,19 +519,15 @@ wxMenu* CommandManager::BeginSubMenu(const wxString & tName) /// after BeginSubMenu() is called but before EndSubMenu() is called. void CommandManager::EndSubMenu() { - size_t submenu_count = mSubMenuList.GetCount()-1; - //Save the submenu's information - SubMenuListEntry *tmpSubMenu = mSubMenuList[submenu_count]; + SubMenuListEntry tmpSubMenu = mSubMenuList.back(); //Pop off the NEW submenu so CurrentMenu returns the parent of the submenu - mSubMenuList.RemoveAt(submenu_count); + mSubMenuList.pop_back(); //Add the submenu to the current menu - CurrentMenu()->Append(0, tmpSubMenu->name, tmpSubMenu->menu, tmpSubMenu->name); + CurrentMenu()->Append(0, tmpSubMenu.name, tmpSubMenu.menu, tmpSubMenu.name); mbSeparatorAllowed = true; - - delete tmpSubMenu; } @@ -566,10 +536,10 @@ void CommandManager::EndSubMenu() /// end of the mSubMenuList (or NULL, if it doesn't exist). wxMenu * CommandManager::CurrentSubMenu() const { - if(mSubMenuList.IsEmpty()) + if(mSubMenuList.empty()) return NULL; - return mSubMenuList[mSubMenuList.GetCount()-1]->menu; + return mSubMenuList.back().menu; } /// @@ -596,7 +566,7 @@ wxMenu * CommandManager::CurrentMenu() const /// given functor will be called void CommandManager::InsertItem(const wxString & name, const wxString & label_in, - CommandFunctor *callback, + const CommandFunctorPointer &callback, const wxString & after, int checkmark) { @@ -665,7 +635,7 @@ void CommandManager::InsertItem(const wxString & name, void CommandManager::AddCheck(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, int checkmark) { AddItem(name, label, callback, wxT(""), (unsigned int)NoFlagsSpecifed, (unsigned int)NoFlagsSpecifed, checkmark); @@ -673,7 +643,7 @@ void CommandManager::AddCheck(const wxChar *name, void CommandManager::AddCheck(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, int checkmark, unsigned int flags, unsigned int mask) @@ -683,7 +653,7 @@ void CommandManager::AddCheck(const wxChar *name, void CommandManager::AddItem(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, unsigned int flags, unsigned int mask) { @@ -692,7 +662,7 @@ void CommandManager::AddItem(const wxChar *name, void CommandManager::AddItem(const wxChar *name, const wxChar *label_in, - CommandFunctor *callback, + const CommandFunctorPointer &callback, const wxChar *accel, unsigned int flags, unsigned int mask, @@ -726,7 +696,7 @@ void CommandManager::AddItem(const wxChar *name, /// all of the items at once. void CommandManager::AddItemList(const wxString & name, const wxArrayString & labels, - CommandFunctor *callback) + const CommandFunctorPointer &callback) { for (size_t i = 0, cnt = labels.GetCount(); i < cnt; i++) { CommandListEntry *entry = NewIdentifier(name, @@ -746,7 +716,7 @@ void CommandManager::AddItemList(const wxString & name, /// given function pointer will be called (via the CommandManagerListener) void CommandManager::AddCommand(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, unsigned int flags, unsigned int mask) { @@ -755,7 +725,7 @@ void CommandManager::AddCommand(const wxChar *name, void CommandManager::AddCommand(const wxChar *name, const wxChar *label_in, - CommandFunctor *callback, + const CommandFunctorPointer &callback, const wxChar *accel, unsigned int flags, unsigned int mask) @@ -769,7 +739,7 @@ void CommandManager::AddCommand(const wxChar *name, void CommandManager::AddGlobalCommand(const wxChar *name, const wxChar *label_in, - CommandFunctor *callback, + const CommandFunctorPointer &callback, const wxChar *accel) { CommandListEntry *entry = NewIdentifier(name, label_in, accel, NULL, callback, false, 0, 0); @@ -806,7 +776,7 @@ int CommandManager::NextIdentifier(int ID) CommandListEntry *CommandManager::NewIdentifier(const wxString & name, const wxString & label, wxMenu *menu, - CommandFunctor *callback, + const CommandFunctorPointer &callback, bool multi, int index, int count) @@ -825,7 +795,7 @@ CommandListEntry *CommandManager::NewIdentifier(const wxString & name, const wxString & label, const wxString & accel, wxMenu *menu, - CommandFunctor *callback, + const CommandFunctorPointer &callback, bool multi, int index, int count) @@ -833,8 +803,8 @@ CommandListEntry *CommandManager::NewIdentifier(const wxString & name, CommandListEntry *entry = new CommandListEntry; wxString labelPrefix; - if (!mSubMenuList.IsEmpty()) { - labelPrefix = mSubMenuList[mSubMenuList.GetCount() - 1]->name; + if (!mSubMenuList.empty()) { + labelPrefix = mSubMenuList.back().name; } // wxMac 2.5 and higher will do special things with the diff --git a/src/commands/CommandManager.h b/src/commands/CommandManager.h index bbeaa6d2f..c964ece22 100644 --- a/src/commands/CommandManager.h +++ b/src/commands/CommandManager.h @@ -34,16 +34,26 @@ public: struct MenuBarListEntry { + MenuBarListEntry(const wxString &name_, wxMenuBar *menubar_) + : name(name_), menubar(menubar_) + {} + wxString name; wxMenuBar *menubar; }; struct SubMenuListEntry { + SubMenuListEntry(const wxString &name_, wxMenu *menu_) + : name(name_), menu(menu_) + {} + wxString name; wxMenu *menu; }; +using CommandFunctorPointer = std::shared_ptr ; + struct CommandListEntry { int id; @@ -54,7 +64,7 @@ struct CommandListEntry wxString labelPrefix; wxString labelTop; wxMenu *menu; - CommandFunctor *callback; + CommandFunctorPointer callback; bool multi; int index; int count; @@ -66,8 +76,8 @@ struct CommandListEntry wxUint32 mask; }; -WX_DEFINE_USER_EXPORTED_ARRAY(MenuBarListEntry *, MenuBarList, class AUDACITY_DLL_API); -WX_DEFINE_USER_EXPORTED_ARRAY(SubMenuListEntry *, SubMenuList, class AUDACITY_DLL_API); +using MenuBarList = std::vector < MenuBarListEntry >; +using SubMenuList = std::vector < SubMenuListEntry >; WX_DEFINE_USER_EXPORTED_ARRAY(CommandListEntry *, CommandList, class AUDACITY_DLL_API); WX_DECLARE_STRING_HASH_MAP_WITH_DECL(CommandListEntry *, CommandNameHash, class AUDACITY_DLL_API); @@ -105,35 +115,35 @@ class AUDACITY_DLL_API CommandManager: public XMLTagHandler void InsertItem(const wxString & name, const wxString & label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, const wxString & after, int checkmark = -1); void AddItemList(const wxString & name, const wxArrayString & labels, - CommandFunctor *callback); + const CommandFunctorPointer &callback); void AddCheck(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, int checkmark = 0); void AddCheck(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, int checkmark, unsigned int flags, unsigned int mask); void AddItem(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, unsigned int flags = NoFlagsSpecifed, unsigned int mask = NoFlagsSpecifed); void AddItem(const wxChar *name, const wxChar *label_in, - CommandFunctor *callback, + const CommandFunctorPointer &callback, const wxChar *accel, unsigned int flags = NoFlagsSpecifed, unsigned int mask = NoFlagsSpecifed, @@ -145,20 +155,20 @@ class AUDACITY_DLL_API CommandManager: public XMLTagHandler // keyboard shortcut. void AddCommand(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, unsigned int flags = NoFlagsSpecifed, unsigned int mask = NoFlagsSpecifed); void AddCommand(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, const wxChar *accel, unsigned int flags = NoFlagsSpecifed, unsigned int mask = NoFlagsSpecifed); void AddGlobalCommand(const wxChar *name, const wxChar *label, - CommandFunctor *callback, + const CommandFunctorPointer &callback, const wxChar *accel); // // Command masks @@ -242,7 +252,7 @@ protected: CommandListEntry *NewIdentifier(const wxString & name, const wxString & label, wxMenu *menu, - CommandFunctor *callback, + const CommandFunctorPointer &callback, bool multi, int index, int count); @@ -250,7 +260,7 @@ protected: const wxString & label, const wxString & accel, wxMenu *menu, - CommandFunctor *callback, + const CommandFunctorPointer &callback, bool multi, int index, int count); From 1758f85451ed92215e2fdf2d67e73cd63634c190 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Tue, 16 Feb 2016 09:58:59 -0500 Subject: [PATCH 2/2] No extra indirection accessing SnapPoint --- src/Snap.cpp | 33 +++++++++++++-------------------- src/Snap.h | 9 +++++---- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/src/Snap.cpp b/src/Snap.cpp index c3c880d78..f4716a95b 100644 --- a/src/Snap.cpp +++ b/src/Snap.cpp @@ -21,9 +21,9 @@ WX_DEFINE_USER_EXPORTED_OBJARRAY(TrackClipArray); -static int CompareSnapPoints(SnapPoint *s1, SnapPoint *s2) +inline bool operator < (SnapPoint s1, SnapPoint s2) { - return (s1->t - s2->t > 0? 1 : -1); + return s1.t < s2.t; } SnapManager::SnapManager(TrackList *tracks, @@ -32,8 +32,7 @@ SnapManager::SnapManager(TrackList *tracks, const TrackArray *trackExclusions, bool noTimeSnap, int pixelTolerance) -: mConverter(NumericConverter::TIME), - mSnapPoints(CompareSnapPoints) +: mConverter(NumericConverter::TIME) { mTracks = tracks; mZoomInfo = zoomInfo; @@ -57,10 +56,6 @@ SnapManager::SnapManager(TrackList *tracks, SnapManager::~SnapManager() { - for (size_t i = 0, cnt = mSnapPoints.GetCount(); i < cnt; ++i) - { - delete mSnapPoints[i]; - } } void SnapManager::Reinit() @@ -80,12 +75,7 @@ void SnapManager::Reinit() mRate = rate; mFormat = format; - // Clear snap points - for (size_t i = 0, cnt = mSnapPoints.GetCount(); i < cnt; ++i) - { - delete mSnapPoints[i]; - } - mSnapPoints.Clear(); + mSnapPoints.clear(); // Grab time-snapping prefs (unless otherwise requested) mSnapToTime = false; @@ -99,7 +89,7 @@ void SnapManager::Reinit() } // Add a SnapPoint at t=0 - mSnapPoints.Add(new SnapPoint(0.0, NULL)); + mSnapPoints.push_back(SnapPoint{}); TrackListIterator iter(mTracks); for (Track *track = iter.First(); track; track = iter.Next()) @@ -162,6 +152,9 @@ void SnapManager::Reinit() } #endif } + + // Sort all by time + std::sort(mSnapPoints.begin(), mSnapPoints.end()); } // Adds to mSnapPoints, filtering by TimeConverter @@ -174,14 +167,14 @@ void SnapManager::CondListAdd(double t, Track *track) if (!mSnapToTime || mConverter.GetValue() == t) { - mSnapPoints.Add(new SnapPoint(t, track)); + mSnapPoints.push_back(SnapPoint{ t, track }); } } // Return the time of the SnapPoint at a given index double SnapManager::Get(size_t index) { - return mSnapPoints[index]->t; + return mSnapPoints[index].t; } // Returns the difference in time between t and the point at a given index @@ -213,7 +206,7 @@ size_t SnapManager::Find(double t, size_t i0, size_t i1) // Find the SnapPoint nearest to time t size_t SnapManager::Find(double t) { - size_t cnt = mSnapPoints.GetCount(); + size_t cnt = mSnapPoints.size(); size_t index = Find(t, 0, cnt); // At this point, either index is the closest, or the next one @@ -242,7 +235,7 @@ bool SnapManager::SnapToPoints(Track *currentTrack, { *outT = t; - size_t cnt = mSnapPoints.GetCount(); + size_t cnt = mSnapPoints.size(); if (cnt == 0) { return false; @@ -284,7 +277,7 @@ bool SnapManager::SnapToPoints(Track *currentTrack, size_t countInThisTrack = 0; for (i = left; i <= right; ++i) { - if (mSnapPoints[i]->track == currentTrack) + if (mSnapPoints[i].track == currentTrack) { indexInThisTrack = i; countInThisTrack++; diff --git a/src/Snap.h b/src/Snap.h index dd3a8f64d..1d5f80181 100644 --- a/src/Snap.h +++ b/src/Snap.h @@ -57,16 +57,17 @@ const int kPixelTolerance = 4; class SnapPoint { public: - SnapPoint(double t, Track *track) + explicit + SnapPoint(double t_ = 0.0, Track *track_ = nullptr) + : t(t_), track(track_) { - this->t = t; - this->track = track; } + double t; Track *track; }; -WX_DEFINE_SORTED_ARRAY(SnapPoint *, SnapPointArray); +using SnapPointArray = std::vector < SnapPoint > ; class SnapManager {