From 75627a15f9fc23fe0a652175406c79dafd879e46 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Fri, 26 May 2023 10:52:35 +0200 Subject: [PATCH 1/5] Fix automation crash (#6711) Fix a crash that occurs if the zooming or snapping model of the Song Editor is used for automation. The crash is caused by a failed static_cast to a Model* in Model::parentModel(). The cast fails because in the constructor of SongEditor the SongEditor is set as the parent of the zooming and snapping model: m_zoomingModel->setParent(this); m_snappingModel->setParent(this); This commit is rather a "band aid" fix because it only fixes the crash by changing the static_cast to a dynamic_cast. However this means that the name of the automation clip is initially empty. A better solution would be to not use the Qt parent/child relationships to implement the model hierarchies. --- include/Model.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/Model.h b/include/Model.h index 3e304297f..cc3079796 100644 --- a/include/Model.h +++ b/include/Model.h @@ -54,7 +54,7 @@ public: Model* parentModel() const { - return static_cast( parent() ); + return dynamic_cast( parent() ); } virtual QString displayName() const From 43319a8d793aca70b7a06090d24ac5dfb834910f Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Fri, 26 May 2023 11:11:45 +0200 Subject: [PATCH 2/5] Move implementations of Model into cpp file (#6711) Move the implementation of the Model methods into Model.cpp so that recompiles after changes are much quicker. Make Model:: isDefaultConstructed const. --- include/Model.h | 29 ++++++----------------------- src/core/Model.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/include/Model.h b/include/Model.h index cc3079796..6db8f9f30 100644 --- a/include/Model.h +++ b/include/Model.h @@ -37,35 +37,18 @@ class LMMS_EXPORT Model : public QObject { Q_OBJECT public: - Model( Model * _parent, QString _display_name = QString(), - bool _default_constructed = false ) : - QObject( _parent ), - m_displayName( _display_name ), - m_defaultConstructed( _default_constructed ) - { - } + Model(Model * _parent, QString _display_name = QString(), + bool _default_constructed = false ); ~Model() override = default; - bool isDefaultConstructed() - { - return m_defaultConstructed; - } + bool isDefaultConstructed() const; - Model* parentModel() const - { - return dynamic_cast( parent() ); - } + Model* parentModel() const; - virtual QString displayName() const - { - return m_displayName; - } + virtual QString displayName() const; - virtual void setDisplayName( const QString& displayName ) - { - m_displayName = displayName; - } + virtual void setDisplayName(const QString& displayName); virtual QString fullDisplayName() const; diff --git a/src/core/Model.cpp b/src/core/Model.cpp index dd277bc4e..83602bd08 100644 --- a/src/core/Model.cpp +++ b/src/core/Model.cpp @@ -27,6 +27,33 @@ namespace lmms { +Model::Model(Model * _parent, QString _display_name, bool _default_constructed) : + QObject( _parent ), + m_displayName( _display_name ), + m_defaultConstructed( _default_constructed ) +{ +} + +bool Model::isDefaultConstructed() const +{ + return m_defaultConstructed; +} + +Model* Model::parentModel() const +{ + return dynamic_cast( parent() ); +} + +QString Model::displayName() const +{ + return m_displayName; +} + +void Model::setDisplayName( const QString& displayName ) +{ + m_displayName = displayName; +} + QString Model::fullDisplayName() const { const QString & n = displayName(); From fda938fccaf543a480561c7c4e759731690ee299 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Fri, 2 Jun 2023 20:31:38 +0200 Subject: [PATCH 3/5] Code review changes (#6711) Remove underscores and whitespace in the Model files. --- include/Model.h | 4 ++-- src/core/Model.cpp | 20 ++++++++++++-------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/include/Model.h b/include/Model.h index 6db8f9f30..e481fbdf1 100644 --- a/include/Model.h +++ b/include/Model.h @@ -37,8 +37,8 @@ class LMMS_EXPORT Model : public QObject { Q_OBJECT public: - Model(Model * _parent, QString _display_name = QString(), - bool _default_constructed = false ); + Model(Model * parent, QString displayName = QString(), + bool defaultConstructed = false ); ~Model() override = default; diff --git a/src/core/Model.cpp b/src/core/Model.cpp index 83602bd08..c59887702 100644 --- a/src/core/Model.cpp +++ b/src/core/Model.cpp @@ -27,10 +27,10 @@ namespace lmms { -Model::Model(Model * _parent, QString _display_name, bool _default_constructed) : - QObject( _parent ), - m_displayName( _display_name ), - m_defaultConstructed( _default_constructed ) +Model::Model(Model * parent, QString displayName, bool defaultConstructed) : + QObject(parent), + m_displayName(displayName), + m_defaultConstructed(defaultConstructed) { } @@ -41,7 +41,7 @@ bool Model::isDefaultConstructed() const Model* Model::parentModel() const { - return dynamic_cast( parent() ); + return dynamic_cast(parent()); } QString Model::displayName() const @@ -57,19 +57,23 @@ void Model::setDisplayName( const QString& displayName ) QString Model::fullDisplayName() const { const QString & n = displayName(); - if( parentModel() ) + + if(parentModel()) { const QString p = parentModel()->fullDisplayName(); - if( n.isEmpty() && p.isEmpty() ) + + if(n.isEmpty() && p.isEmpty()) { return QString(); } - else if( p.isEmpty() ) + else if(p.isEmpty()) { return n; } + return p + ">" + n; } + return n; } From 9fe7fbd69eabc98f6fe63ef4c4b018fae09442f0 Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Fri, 2 Jun 2023 20:45:58 +0200 Subject: [PATCH 4/5] Simplify logic of Model::fullDisplayName (#6711) Simplify the logic of the method Model::fullDisplayName. Please note that this shows that with the current logic if the parent model has a non-empty name like "parentmodel" and the name of the child model is empty the result is the name "parentmodel >" which might not be what's wanted. --- src/core/Model.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/core/Model.cpp b/src/core/Model.cpp index c59887702..7a7919bb8 100644 --- a/src/core/Model.cpp +++ b/src/core/Model.cpp @@ -56,22 +56,16 @@ void Model::setDisplayName( const QString& displayName ) QString Model::fullDisplayName() const { - const QString & n = displayName(); + const QString n = displayName(); if(parentModel()) { const QString p = parentModel()->fullDisplayName(); - if(n.isEmpty() && p.isEmpty()) + if (!p.isEmpty()) { - return QString(); + return p + ">" + n; } - else if(p.isEmpty()) - { - return n; - } - - return p + ">" + n; } return n; From 0ebc18941b934893f82e322b8bd09efea139b05d Mon Sep 17 00:00:00 2001 From: Michael Gregorius Date: Fri, 2 Jun 2023 22:17:03 +0200 Subject: [PATCH 5/5] More whitespace adjustments (#6711) More whitespace adjustments as proposed in the code review. --- include/Model.h | 4 ++-- src/core/Model.cpp | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/Model.h b/include/Model.h index e481fbdf1..ce7b751e7 100644 --- a/include/Model.h +++ b/include/Model.h @@ -37,8 +37,8 @@ class LMMS_EXPORT Model : public QObject { Q_OBJECT public: - Model(Model * parent, QString displayName = QString(), - bool defaultConstructed = false ); + Model(Model* parent, QString displayName = QString(), + bool defaultConstructed = false); ~Model() override = default; diff --git a/src/core/Model.cpp b/src/core/Model.cpp index 7a7919bb8..634e2fbed 100644 --- a/src/core/Model.cpp +++ b/src/core/Model.cpp @@ -27,7 +27,7 @@ namespace lmms { -Model::Model(Model * parent, QString displayName, bool defaultConstructed) : +Model::Model(Model* parent, QString displayName, bool defaultConstructed) : QObject(parent), m_displayName(displayName), m_defaultConstructed(defaultConstructed) @@ -41,7 +41,7 @@ bool Model::isDefaultConstructed() const Model* Model::parentModel() const { - return dynamic_cast(parent()); + return dynamic_cast(parent()); } QString Model::displayName() const @@ -49,7 +49,7 @@ QString Model::displayName() const return m_displayName; } -void Model::setDisplayName( const QString& displayName ) +void Model::setDisplayName(const QString& displayName) { m_displayName = displayName; } @@ -58,7 +58,7 @@ QString Model::fullDisplayName() const { const QString n = displayName(); - if(parentModel()) + if (parentModel()) { const QString p = parentModel()->fullDisplayName();