Skip to content

Remove instrument track window caching to fix related bugs #5808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PhysSong merged 3 commits into from
Dec 1, 2020
Merged

Remove instrument track window caching to fix related bugs #5808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d797df5
Remove instrument track window caching to fix related bugs
PhysSong Nov 8, 2020
1dd97d4
Remove QQueue leftovers
PhysSong Nov 30, 2020
85a70d1
Remove unnecessary Qt::QueuedConnection
PhysSong Nov 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions include/InstrumentTrack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@


class QLineEdit;
template<class T> class QQueue;
class InstrumentFunctionArpeggioView;
class InstrumentFunctionNoteStackingView;
class EffectRackView;
Expand Down Expand Up @@ -324,10 +323,6 @@ class InstrumentTrackView : public TrackView
return m_midiMenu;
}

void freeInstrumentTrackWindow();

static void cleanupWindowCache();

// Create a menu for assigning/creating channels for this track
QMenu * createFxMenu( QString title, QString newFxLabel ) override;

Expand All @@ -349,12 +344,12 @@ private slots:
void assignFxLine( int channelIndex );
void createFxLine();

void handleConfigChange(QString cls, QString attr, QString value);


private:
InstrumentTrackWindow * m_window;

static QQueue<InstrumentTrackWindow *> s_windowCache;

// widgets in track-settings-widget
TrackLabelButton * m_tlb;
Knob * m_volumeKnob;
Expand Down Expand Up @@ -482,6 +477,7 @@ protected slots:
PianoView * m_pianoView;

friend class InstrumentView;
friend class InstrumentTrackView;

} ;

Expand Down
2 changes: 0 additions & 2 deletions src/core/Song.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -884,8 +884,6 @@ void Song::clearProject()
Engine::projectJournal()->clearJournal();

Engine::projectJournal()->setJournalling( true );

InstrumentTrackView::cleanupWindowCache();
}


Expand Down
1 change: 0 additions & 1 deletion src/gui/GuiApplication.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ GuiApplication::GuiApplication()

GuiApplication::~GuiApplication()
{
InstrumentTrackView::cleanupWindowCache();
s_instance = nullptr;
}

Expand Down
107 changes: 22 additions & 85 deletions src/tracks/InstrumentTrack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "InstrumentTrack.h"

#include <QDir>
#include <QQueue>
#include <QApplication>
#include <QCloseEvent>
#include <QLabel>
Expand Down Expand Up @@ -79,7 +78,6 @@
const int INSTRUMENT_WIDTH = 254;
const int INSTRUMENT_HEIGHT = INSTRUMENT_WIDTH;
const int PIANO_HEIGHT = 80;
const int INSTRUMENT_WINDOW_CACHE_SIZE = 8;


// #### IT:
Expand Down Expand Up @@ -951,9 +949,6 @@ void InstrumentTrack::autoAssignMidiDevice(bool assign)
// #### ITV:


QQueue<InstrumentTrackWindow *> InstrumentTrackView::s_windowCache;



InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerView* tcv ) :
TrackView( _it, tcv ),
Expand All @@ -975,6 +970,9 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV
connect( _it, SIGNAL( nameChanged() ),
m_tlb, SLOT( update() ) );

connect(ConfigManager::inst(), SIGNAL(valueChanged(QString, QString, QString)),
this, SLOT(handleConfigChange(QString, QString, QString)));

// creation of widgets for track-settings-widget
int widgetWidth;
if( ConfigManager::inst()->value( "ui",
Expand Down Expand Up @@ -1066,7 +1064,8 @@ InstrumentTrackView::InstrumentTrackView( InstrumentTrack * _it, TrackContainerV

InstrumentTrackView::~InstrumentTrackView()
{
freeInstrumentTrackWindow();
delete m_window;
Copy link
Contributor

@JohannesLorenz JohannesLorenz Nov 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using unique_ptr instead?

Copy link
Member Author

@PhysSong PhysSong Nov 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's out of scope, but it's doable.

m_window = nullptr;

delete model()->m_midiPort.m_readablePortsMenu;
delete model()->m_midiPort.m_writablePortsMenu;
Expand Down Expand Up @@ -1119,89 +1118,26 @@ void InstrumentTrackView::assignFxLine(int channelIndex)



// TODO: Add windows to free list on freeInstrumentTrackWindow.
// But, don't NULL m_window or disconnect signals. This will allow windows
// that are being show/hidden frequently to stay connected.
void InstrumentTrackView::freeInstrumentTrackWindow()
InstrumentTrackWindow * InstrumentTrackView::getInstrumentTrackWindow()
{
if( m_window != NULL )
if (!m_window)
{
m_lastPos = m_window->parentWidget()->pos();

if( ConfigManager::inst()->value( "ui",
"oneinstrumenttrackwindow" ).toInt() ||
s_windowCache.count() < INSTRUMENT_WINDOW_CACHE_SIZE )
{
model()->setHook( NULL );
m_window->setInstrumentTrackView( NULL );
m_window->parentWidget()->hide();
m_window->updateInstrumentView();
s_windowCache << m_window;
}
else
{
delete m_window;
}

m_window = NULL;
m_window = new InstrumentTrackWindow(this);
}
}




void InstrumentTrackView::cleanupWindowCache()
{
while( !s_windowCache.isEmpty() )
{
delete s_windowCache.dequeue();
}
return m_window;
}




InstrumentTrackWindow * InstrumentTrackView::getInstrumentTrackWindow()
void InstrumentTrackView::handleConfigChange(QString cls, QString attr, QString value)
{
if( m_window != NULL )
{
}
else if( !s_windowCache.isEmpty() )
// When one instrument track window mode is turned on,
// close windows except last opened one.
if (cls == "ui" && attr == "oneinstrumenttrackwindow" && value.toInt())
{
m_window = s_windowCache.dequeue();

m_window->setInstrumentTrackView( this );
m_window->setModel( model() );
m_window->updateInstrumentView();
model()->setHook( m_window );

if( ConfigManager::inst()->
value( "ui", "oneinstrumenttrackwindow" ).toInt() )
{
s_windowCache << m_window;
}
else if( m_lastPos.x() > 0 || m_lastPos.y() > 0 )
{
m_window->parentWidget()->move( m_lastPos );
}
m_tlb->setChecked(m_window && m_window == topLevelInstrumentTrackWindow());
}
else
{
m_window = new InstrumentTrackWindow( this );
if( ConfigManager::inst()->
value( "ui", "oneinstrumenttrackwindow" ).toInt() )
{
// first time, an InstrumentTrackWindow is opened
s_windowCache << m_window;
}
}

return m_window;
}




void InstrumentTrackView::dragEnterEvent( QDragEnterEvent * _dee )
{
InstrumentTrackWindow::dragEnterEventGeneric( _dee );
Expand All @@ -1225,12 +1161,15 @@ void InstrumentTrackView::dropEvent( QDropEvent * _de )

void InstrumentTrackView::toggleInstrumentWindow( bool _on )
{
getInstrumentTrackWindow()->toggleVisibility( _on );

if( !_on )
if (_on && ConfigManager::inst()->value("ui", "oneinstrumenttrackwindow").toInt())
{
freeInstrumentTrackWindow();
if (topLevelInstrumentTrackWindow())
{
topLevelInstrumentTrackWindow()->m_itv->m_tlb->setChecked(false);
}
}

getInstrumentTrackWindow()->toggleVisibility( _on );
}


Expand Down Expand Up @@ -1547,11 +1486,9 @@ InstrumentTrackWindow::InstrumentTrackWindow( InstrumentTrackView * _itv ) :

InstrumentTrackWindow::~InstrumentTrackWindow()
{
InstrumentTrackView::s_windowCache.removeAll( this );

delete m_instrumentView;

if( gui->mainWindow()->workspace() )
if (parentWidget())
Copy link
Member Author

@PhysSong PhysSong Nov 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to prevent crashes on closing.

{
parentWidget()->hide();
parentWidget()->deleteLater();
Expand Down