From 36aad4d1c6509ba982d8e322ffca842248301a54 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Sat, 18 Jan 2020 14:37:49 -0500 Subject: [PATCH] Don't hard-code the exhaustive list of sub-view types... ... in Wave track context menu and SetTrackVisualsCommand Instead, discover them through a registry. This eliminates some duplication of string constants and prepares for non-intrusive generalization to more kinds of sub-views. This makes the command agnostic about which subview types are known, but the context menu still has special case treatment for Spectrogram Settings and Wave Colors. --- include/audacity/ComponentInterface.h | 3 +- src/commands/SetTrackInfoCommand.cpp | 44 ++++----- .../wavetrack/ui/SpectrumView.cpp | 5 ++ .../wavetrack/ui/WaveTrackControls.cpp | 90 ++++++++++++------- .../wavetrack/ui/WaveTrackView.cpp | 47 ++++++++++ .../wavetrack/ui/WaveTrackView.h | 23 +++++ .../wavetrack/ui/WaveformView.cpp | 5 ++ 7 files changed, 164 insertions(+), 53 deletions(-) diff --git a/include/audacity/ComponentInterface.h b/include/audacity/ComponentInterface.h index 6a5fd0eb3..149ecf8e2 100644 --- a/include/audacity/ComponentInterface.h +++ b/include/audacity/ComponentInterface.h @@ -87,9 +87,10 @@ public: const wxString &Internal() const { return mInternal; } const TranslatableString &Msgid() const { return mMsgid; } + const TranslatableString Stripped() const { return mMsgid.Stripped(); } const wxString Translation() const { return mMsgid.Translation(); } const wxString StrippedTranslation() const - { return mMsgid.Stripped().Translation(); } + { return Stripped().Translation(); } bool empty() const { return mInternal.empty(); } diff --git a/src/commands/SetTrackInfoCommand.cpp b/src/commands/SetTrackInfoCommand.cpp index 3f2b2b973..61aed48ea 100644 --- a/src/commands/SetTrackInfoCommand.cpp +++ b/src/commands/SetTrackInfoCommand.cpp @@ -248,20 +248,6 @@ static const EnumValueSymbol kColourStrings[nColours] = }; -enum kDisplayTypes -{ - kWaveform, - kSpectrogram, - nDisplayTypes -}; - -static const EnumValueSymbol kDisplayTypeStrings[nDisplayTypes] = -{ - // These are acceptable dual purpose internal/visible names - { XO("Waveform") }, - { XO("Spectrogram") }, -}; - enum kScaleTypes { kLinear, @@ -292,10 +278,22 @@ static const EnumValueSymbol kZoomTypeStrings[nZoomTypes] = { XO("HalfWave") }, }; +static EnumValueSymbols DiscoverSubViewTypes() +{ + const auto &types = WaveTrackSubView::AllTypes(); + return transform_container< EnumValueSymbols >( + types, std::mem_fn( &WaveTrackSubView::Type::name ) ); +} + bool SetTrackVisualsCommand::DefineParams( ShuttleParams & S ){ SetTrackBase::DefineParams( S ); S.OptionalN( bHasHeight ).Define( mHeight, wxT("Height"), 120, 44, 2000 ); - S.OptionalN( bHasDisplayType ).DefineEnum( mDisplayType, wxT("Display"), kWaveform, kDisplayTypeStrings, nDisplayTypes ); + + { + auto symbols = DiscoverSubViewTypes(); + S.OptionalN( bHasDisplayType ).DefineEnum( mDisplayType, wxT("Display"), 0, symbols.data(), symbols.size() ); + } + S.OptionalN( bHasScaleType ).DefineEnum( mScaleType, wxT("Scale"), kLinear, kScaleTypeStrings, nScaleTypes ); S.OptionalN( bHasColour ).DefineEnum( mColour, wxT("Color"), kColour0, kColourStrings, nColours ); S.OptionalN( bHasVZoom ).DefineEnum( mVZoom, wxT("VZoom"), kReset, kZoomTypeStrings, nZoomTypes ); @@ -318,8 +316,15 @@ void SetTrackVisualsCommand::PopulateOrExchange(ShuttleGui & S) S.Optional( bHasHeight ).TieNumericTextBox( XO("Height:"), mHeight ); S.Optional( bHasColour ).TieChoice( XO("Color:"), mColour, Msgids( kColourStrings, nColours ) ); - S.Optional( bHasDisplayType ).TieChoice( XO("Display:"), mDisplayType, - Msgids( kDisplayTypeStrings, nDisplayTypes ) ); + + { + auto symbols = DiscoverSubViewTypes(); + auto typeNames = transform_container( + symbols, std::mem_fn( &EnumValueSymbol::Stripped ) ); + S.Optional( bHasDisplayType ).TieChoice( XO("Display:"), mDisplayType, + typeNames ); + } + S.Optional( bHasScaleType ).TieChoice( XO("Scale:"), mScaleType, Msgids( kScaleTypeStrings, nScaleTypes ) ); S.Optional( bHasVZoom ).TieChoice( XO("VZoom:"), mVZoom, @@ -355,10 +360,7 @@ bool SetTrackVisualsCommand::ApplyInner(const CommandContext & context, Track * if( wt && bHasDisplayType ) WaveTrackView::Get( *wt ).SetDisplay( - (mDisplayType == kWaveform) ? - WaveTrackViewConstants::Waveform - : WaveTrackViewConstants::Spectrum - ); + WaveTrackSubView::AllTypes()[ mDisplayType ].id ); if( wt && bHasScaleType ) wt->GetIndependentWaveformSettings().scaleType = (mScaleType==kLinear) ? diff --git a/src/tracks/playabletrack/wavetrack/ui/SpectrumView.cpp b/src/tracks/playabletrack/wavetrack/ui/SpectrumView.cpp index 88f1736cc..35a9f2a40 100644 --- a/src/tracks/playabletrack/wavetrack/ui/SpectrumView.cpp +++ b/src/tracks/playabletrack/wavetrack/ui/SpectrumView.cpp @@ -30,6 +30,11 @@ Paul Licameli split from WaveTrackView.cpp #include #include +static WaveTrackSubView::RegisteredType reg{ { + WaveTrackViewConstants::Spectrum, + { wxT("Spectrogram"), XO("&Spectrogram") } +} }; + SpectrumView::~SpectrumView() = default; bool SpectrumView::IsSpectral() const diff --git a/src/tracks/playabletrack/wavetrack/ui/WaveTrackControls.cpp b/src/tracks/playabletrack/wavetrack/ui/WaveTrackControls.cpp index 7a4274b6b..e82f9181c 100644 --- a/src/tracks/playabletrack/wavetrack/ui/WaveTrackControls.cpp +++ b/src/tracks/playabletrack/wavetrack/ui/WaveTrackControls.cpp @@ -104,6 +104,8 @@ std::vector WaveTrackControls::HitTest } enum { + reserveDisplays = 100, + OnRate8ID = 30000, // <--- OnRate11ID, // | OnRate16ID, // | @@ -123,8 +125,9 @@ enum { OnFloatID, // <--- OnMultiViewID, - OnWaveformID, - OnSpectrumID, + + OnSetDisplayId, lastDisplayId = (OnSetDisplayId + reserveDisplays - 1), + OnSpectrogramSettingsID, OnChannelLeftID, @@ -620,6 +623,16 @@ void WaveTrackMenuTable::InitUserData(void *pUserData) mpData = static_cast(pUserData); } +static WaveTrackSubView::Types AllTypes() +{ + auto result = WaveTrackSubView::AllTypes(); + if ( result.size() > reserveDisplays ) { + wxASSERT( false ); + result.resize( reserveDisplays ); + } + return result; +} + void WaveTrackMenuTable::InitMenu(Menu *pMenu) { WaveTrack *const pTrack = static_cast(mpData->pTrack); @@ -631,16 +644,40 @@ void WaveTrackMenuTable::InitMenu(Menu *pMenu) if (multiView) checkedIds.push_back( OnMultiViewID ); + bool hasSpectrum = false; int uniqueDisplay = 0; + { + // Find the set of type ids of the shown displays, disregarding their + // top-to-bottom arrangement + auto displays = view.GetDisplays(); + const auto end = displays.end(); + auto iter = displays.begin(); + std::sort( iter, end ); - const auto displays = view.GetDisplays(); - for ( auto display : displays ) { - auto id = (display == WaveTrackViewConstants::Waveform) - ? OnWaveformID - : OnSpectrumID; - checkedIds.push_back( id ); - if ( displays.size() == 1 ) - uniqueDisplay = id; + // Check the corresponding menu items, and decide which if any has + // the unique check + int displayId = OnSetDisplayId; + int nDisplays = 0; + for ( const auto &type : AllTypes() ) { + if ( iter != end && *iter == type.id ) { + checkedIds.push_back( displayId ); + uniqueDisplay = displayId; + ++iter; + ++nDisplays; + + // Unfortunately this special knowledge of the Spectrum view type + // remains. In future, either let a registry system insert this + // menu item, or (better) move it to the context menu of the + // Spectrum vertical ruler. + // (But the latter won't be satisfactory without a means to + // open that other context menu with keystrokes only, and that + // would require some notion of a focused sub-view.) + hasSpectrum = ( type.id == WaveTrackViewConstants::Spectrum ); + } + ++displayId; + } + if ( nDisplays > 1 ) + uniqueDisplay = 0; } if ( multiView && uniqueDisplay ) @@ -652,9 +689,6 @@ void WaveTrackMenuTable::InitMenu(Menu *pMenu) // We can't change them on the fly yet anyway. auto gAudioIO = AudioIOBase::Get(); const bool bAudioBusy = gAudioIO->IsBusy(); - bool hasSpectrum = - make_iterator_range( displays.begin(), displays.end() ) - .contains( WaveTrackViewConstants::Spectrum ); pMenu->Enable(OnSpectrogramSettingsID, hasSpectrum && !bAudioBusy); AudacityProject *const project = &mpData->project; @@ -733,13 +767,14 @@ BEGIN_POPUP_MENU(WaveTrackMenuTable) if ( WaveTrackSubViews::slots() > 1 ) POPUP_MENU_CHECK_ITEM(OnMultiViewID, XO("&Multi-view"), OnMultiView) - if ( view.GetMultiView() ) { - POPUP_MENU_CHECK_ITEM(OnWaveformID, XO("Wa&veform"), OnSetDisplay) - POPUP_MENU_CHECK_ITEM(OnSpectrumID, XO("&Spectrogram"), OnSetDisplay) - } - else { - POPUP_MENU_RADIO_ITEM(OnWaveformID, XO("Wa&veform"), OnSetDisplay) - POPUP_MENU_RADIO_ITEM(OnSpectrumID, XO("&Spectrogram"), OnSetDisplay) + int id = OnSetDisplayId; + for ( const auto &type : AllTypes() ) { + if ( view.GetMultiView() ) { + POPUP_MENU_CHECK_ITEM(id++, type.name.Msgid(), OnSetDisplay) + } + else { + POPUP_MENU_RADIO_ITEM(id++, type.name.Msgid(), OnSetDisplay) + } } POPUP_MENU_ITEM(OnSpectrogramSettingsID, XO("S&pectrogram Settings..."), OnSpectrogramSettings) @@ -799,19 +834,12 @@ void WaveTrackMenuTable::OnMultiView(wxCommandEvent & event) /// Set the Display mode based on the menu choice in the Track Menu. void WaveTrackMenuTable::OnSetDisplay(wxCommandEvent & event) { - using namespace WaveTrackViewConstants; int idInt = event.GetId(); - wxASSERT(idInt >= OnWaveformID && idInt <= OnSpectrumID); + wxASSERT(idInt >= OnSetDisplayId && + idInt <= lastDisplayId); const auto pTrack = static_cast(mpData->pTrack); - WaveTrackView::Display id; - switch (idInt) { - default: - case OnWaveformID: - id = Waveform; break; - case OnSpectrumID: - id = Spectrum; break; - } + auto id = AllTypes()[ idInt - OnSetDisplayId ].id; auto &view = WaveTrackView::Get( *pTrack ); if ( view.GetMultiView() ) { @@ -887,7 +915,7 @@ void WaveTrackMenuTable::OnSpectrogramSettings(wxCommandEvent &) // factories.push_back(WaveformPrefsFactory( pTrack )); factories.push_back(SpectrumPrefsFactory( pTrack )); const int page = - // (pTrack->GetDisplay() == WaveTrack::Spectrum) ? 1 : + // (pTrack->GetDisplay() == WaveTrackViewConstants::Spectrum) ? 1 : 0; auto title = XO("%s:").Format( pTrack->GetName() ); diff --git a/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.cpp b/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.cpp index bcfa33a67..d4cabd999 100644 --- a/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.cpp +++ b/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.cpp @@ -40,6 +40,53 @@ Paul Licameli split from TrackPanel.cpp #include "../../../ui/ButtonHandle.h" #include "../../../../TrackInfo.h" +namespace { + class Registry { + public: + using Type = WaveTrackSubView::Type; + using Types = std::vector< Type >; + + void Append( Type type ) + { + types.emplace_back( std::move( type ) ); + sorted = false; + } + + Types &Get() + { + if ( !sorted ) { + auto begin = types.begin(), end = types.end(); + std::sort( begin, end ); + // We don't want duplicate ids! + wxASSERT( end == std::adjacent_find( begin, end ) ); + sorted = true; + } + return types; + } + + private: + Types types; + bool sorted = false; + }; + + Registry &GetRegistry() + { + static Registry result; + return result; + } +} + +WaveTrackSubView::RegisteredType::RegisteredType( Type type ) +{ + GetRegistry().Append( std::move( type ) ); +} + +// static +auto WaveTrackSubView::AllTypes() -> const Types & +{ + return GetRegistry().Get(); +} + namespace { using WaveTrackSubViewPtrs = std::vector< std::shared_ptr< WaveTrackSubView > >; diff --git a/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.h b/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.h index 6d0e3fbf2..95006b938 100644 --- a/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.h +++ b/src/tracks/playabletrack/wavetrack/ui/WaveTrackView.h @@ -13,16 +13,39 @@ Paul Licameli split from class WaveTrack #include "../../../ui/CommonTrackView.h" #include "../../../../ClientData.h" +#include "audacity/ComponentInterface.h" namespace WaveTrackViewConstants{ enum Display : int; } class CutlineHandle; +class TranslatableString; class WaveTrack; class WaveTrackView; class WaveTrackSubView : public CommonTrackView { public: + using Display = WaveTrackViewConstants::Display; + struct Type { + // Identifies the type session-wide, and determines relative position in + // menus listing all types + Display id; + // The translation is suitable for the track control panel drop-down, + // and it may contain a menu accelerator + EnumValueSymbol name; + + bool operator < ( const Type &other ) const { return id < other.id; } + bool operator == ( const Type &other ) const { return id == other.id; } + }; + using Types = std::vector< Type >; + + // Typically a file scope statically constructed object + struct RegisteredType { + RegisteredType( Type type ); + }; + + // Discover all registered types + static const Types &AllTypes(); explicit WaveTrackSubView( WaveTrackView &waveTrackView ); diff --git a/src/tracks/playabletrack/wavetrack/ui/WaveformView.cpp b/src/tracks/playabletrack/wavetrack/ui/WaveformView.cpp index abdbfb91c..a78f785d1 100644 --- a/src/tracks/playabletrack/wavetrack/ui/WaveformView.cpp +++ b/src/tracks/playabletrack/wavetrack/ui/WaveformView.cpp @@ -36,6 +36,11 @@ Paul Licameli split from WaveTrackView.cpp #include #include +static WaveTrackSubView::RegisteredType reg{ { + WaveTrackViewConstants::Waveform, + { wxT("Waveform"), XO("Wa&veform") } +} }; + WaveformView::~WaveformView() = default; std::vector WaveformView::DetailedHitTest(