From 94384622803ad1dddb5c35655eb09c8ca9bc64bd Mon Sep 17 00:00:00 2001 From: Klaus Basan Date: Thu, 14 Jun 2018 14:39:12 +0200 Subject: [PATCH] Optimizations for column formatters, use CVariant directly if a value remains const (no need to always contruct CVariant) --- src/blackgui/models/columnformatters.cpp | 52 +++++++++++++----------- src/blackgui/models/columnformatters.h | 48 ++++++++++++---------- 2 files changed, 55 insertions(+), 45 deletions(-) diff --git a/src/blackgui/models/columnformatters.cpp b/src/blackgui/models/columnformatters.cpp index beeb2746f..fe8e2d3af 100644 --- a/src/blackgui/models/columnformatters.cpp +++ b/src/blackgui/models/columnformatters.cpp @@ -21,6 +21,7 @@ #include #include #include +#include using namespace BlackMisc; using namespace BlackMisc::Aviation; @@ -145,6 +146,12 @@ namespace BlackGui return e; } + const CVariant &CDefaultFormatter::emptyPixmapVariant() + { + static const CVariant e = CVariant::from(QPixmap()); + return e; + } + CVariant CPixmapFormatter::displayRole(const CVariant &dataCVariant) const { Q_UNUSED(dataCVariant); @@ -216,12 +223,12 @@ namespace BlackGui // special treatment for some cases const CLength l = dataCVariant.value(); const bool valid = !l.isNull() && (l.isPositiveWithEpsilonConsidered() || l.isZeroEpsilonConsidered()); - return valid ? CPhysiqalQuantiyFormatter::displayRole(dataCVariant) : QStringLiteral(""); + return valid ? CPhysiqalQuantiyFormatter::displayRole(dataCVariant) : emptyStringVariant(); } else { Q_ASSERT_X(false, "CAirspaceDistanceFormatter::formatQVariant", "No CLength class"); - return QStringLiteral(""); + return emptyStringVariant(); } } @@ -229,7 +236,7 @@ namespace BlackGui { if (dataCVariant.canConvert()) { - // speical treatment for some cases + // special treatment for some cases const CFrequency f = dataCVariant.value(); if (CComSystem::isValidComFrequency(f)) { @@ -259,7 +266,7 @@ namespace BlackGui { if (dataCVariant.canConvert()) { return dataCVariant; } if (!dataCVariant.isValid()) { static const CVariant iv("invalid"); return iv; } - const QString s("Invalid type '%1'"); + static const QString s("Invalid type: '%1'"); return CVariant::from(s.arg(dataCVariant.typeName())); } @@ -273,8 +280,8 @@ namespace BlackGui { if (dataCVariant.canConvert()) { - bool v = dataCVariant.toBool(); - return v ? CVariant(m_trueName) : CVariant(m_falseName); + const bool v = dataCVariant.toBool(); + return v ? CVariant(m_trueNameVariant) : CVariant(m_falseNameVariant); } Q_ASSERT_X(false, "CBoolTextFormatter", "no boolean value"); return CVariant(); @@ -292,12 +299,11 @@ namespace BlackGui CBoolTextFormatter(alignment, onName, offName, rolesDecorationAndToolTip()) { // one time pixmap creation - CLedWidget *led = ledDefault(); + QScopedPointer led(createLedDefault()); led->setOn(true); - m_pixmapOnLed = led->asPixmap(); + m_pixmapOnLedVariant = CVariant::fromValue(led->asPixmap()); led->setOn(false); - m_pixmapOffLed = led->asPixmap(); - delete led; + m_pixmapOffLedVariant = CVariant::fromValue(led->asPixmap()); } CVariant CBoolLedFormatter::displayRole(const CVariant &dataCVariant) const @@ -311,8 +317,8 @@ namespace BlackGui { if (dataCVariant.canConvert()) { - bool v = dataCVariant.toBool(); - return CVariant::from(v ? m_pixmapOnLed : m_pixmapOffLed); + const bool v = dataCVariant.toBool(); + return v ? m_pixmapOnLedVariant : m_pixmapOffLedVariant; } Q_ASSERT_X(false, "CBoolLedFormatter", "no boolean value"); return CVariant(); @@ -331,11 +337,9 @@ namespace BlackGui { } CBoolIconFormatter::CBoolIconFormatter(const CIcon &onIcon, const CIcon &offIcon, const QString &onName, const QString &offName, int alignment) : - CBoolTextFormatter(alignment, onName, offName, rolesDecorationAndToolTip()), m_iconOn(onIcon), m_iconOff(offIcon) - { - m_iconOn.setDescriptiveText(onName); - m_iconOff.setDescriptiveText(offName); - } + CBoolTextFormatter(alignment, onName, offName, rolesDecorationAndToolTip()), + m_iconOnVariant(CVariant::fromValue(onIcon.toPixmap())), m_iconOffVariant(CVariant::fromValue(offIcon.toPixmap())) + { } CVariant CBoolIconFormatter::displayRole(const CVariant &dataCVariant) const { @@ -348,8 +352,8 @@ namespace BlackGui { if (dataCVariant.canConvert()) { - bool v = dataCVariant.toBool(); - return CVariant::from(v ? m_iconOn.toPixmap() : m_iconOff.toPixmap()); + const bool v = dataCVariant.toBool(); + return v ? m_iconOnVariant : m_iconOffVariant; } Q_ASSERT_X(false, "CBoolIconFormatter", "no boolean value"); return CVariant(); @@ -379,16 +383,16 @@ namespace BlackGui CVariant CColorFormatter::decorationRole(const CVariant &dataCVariant) const { - static const CVariant empty(CVariant::fromValue(QPixmap())); - CRgbColor rgbColor(dataCVariant.to()); - if (!rgbColor.isValid()) { return empty; } + const CRgbColor rgbColor(dataCVariant.to()); + if (!rgbColor.isValid()) { return emptyPixmapVariant(); } return CVariant::fromValue(rgbColor.toPixmap()); } CVariant CColorFormatter::tooltipRole(const CVariant &dataCVariant) const { - CRgbColor rgbColor(dataCVariant.to()); - if (!rgbColor.isValid()) { return ""; } + static const CVariant empty(CVariant::fromValue(QPixmap())); + const CRgbColor rgbColor(dataCVariant.to()); + if (!rgbColor.isValid()) { return emptyStringVariant(); } return rgbColor.hex(true); } diff --git a/src/blackgui/models/columnformatters.h b/src/blackgui/models/columnformatters.h index a508b4ba3..f1aef4c82 100644 --- a/src/blackgui/models/columnformatters.h +++ b/src/blackgui/models/columnformatters.h @@ -116,9 +116,12 @@ namespace BlackGui //! Empty string CVariant static const BlackMisc::CVariant &emptyStringVariant(); + //! Empty pixmap CVariant + static const BlackMisc::CVariant &emptyPixmapVariant(); + QList m_supportedRoles = roleDisplay(); //!< supports decoration roles - int m_alignment = -1; //!< alignment horizontal/vertically / Qt::Alignment - bool m_useI18n = true; //!< i18n? + int m_alignment = -1; //!< alignment horizontal/vertically / Qt::Alignment + bool m_useI18n = true; //!< i18n? }; //! Pixmap formatter @@ -164,7 +167,9 @@ namespace BlackGui public: //! Constructor CBoolTextFormatter(int alignment = alignDefault(), const QString &trueName = "true", const QString &falseName = "false", const QList &supportedRoles = roleDisplay()) : - CDefaultFormatter(alignment, false, supportedRoles), m_trueName(trueName), m_falseName(falseName) {} + CDefaultFormatter(alignment, false, supportedRoles), + m_trueNameVariant(BlackMisc::CVariant::from(trueName)), + m_falseNameVariant(BlackMisc::CVariant::from(falseName)) {} //! \copydoc CDefaultFormatter::displayRole virtual BlackMisc::CVariant displayRole(const BlackMisc::CVariant &dataCVariant) const override; @@ -173,8 +178,8 @@ namespace BlackGui virtual Qt::ItemFlags flags(Qt::ItemFlags flags, bool editable) const override; protected: - const QString m_trueName = "true"; //!< displayed when true - const QString m_falseName = "false"; //!< displayed when false + const BlackMisc::CVariant m_trueNameVariant = "true"; //!< displayed when true + const BlackMisc::CVariant m_falseNameVariant = "false"; //!< displayed when false }; //! Format as bool LED value @@ -200,15 +205,16 @@ namespace BlackGui return CBoolTextFormatter::displayRole(dataCVariant); } - //! Default LED - static BlackGui::CLedWidget *ledDefault() - { - return new BlackGui::CLedWidget(false, BlackGui::CLedWidget::Yellow, BlackGui::CLedWidget::Black, BlackGui::CLedWidget::Rounded); - } - protected: - QPixmap m_pixmapOnLed; //!< Pixmap used when on - QPixmap m_pixmapOffLed; //!< Pixmap used when off + BlackMisc::CVariant m_pixmapOnLedVariant; //!< Pixmap used when on + BlackMisc::CVariant m_pixmapOffLedVariant; //!< Pixmap used when off + + private: + //! Default LED + static CLedWidget *createLedDefault() + { + return new CLedWidget(false, CLedWidget::Yellow, CLedWidget::Black, CLedWidget::Rounded); + } }; //! Format as bool pixmap @@ -238,8 +244,8 @@ namespace BlackGui virtual BlackMisc::CVariant tooltipRole(const BlackMisc::CVariant &dataCVariant) const override; protected: - BlackMisc::CIcon m_iconOn; //!< Used when on - BlackMisc::CIcon m_iconOff; //!< Used when off + const BlackMisc::CVariant m_iconOnVariant; //!< Used when on + const BlackMisc::CVariant m_iconOffVariant; //!< Used when off }; //! Default formatter when column contains CValueObject @@ -282,7 +288,7 @@ namespace BlackGui static const QString &formatHmsz() { static const QString f = "HH:mm:ss.zzz"; return f; } private: - QString m_formatString = "yyyy-MM-dd HH:mm"; //!< how the value is displayed + const QString m_formatString = "yyyy-MM-dd HH:mm"; //!< how the value is displayed }; //! Formatter when column contains an integer @@ -307,7 +313,7 @@ namespace BlackGui virtual BlackMisc::CVariant displayRole(const BlackMisc::CVariant &altitude) const override; private: - bool m_flightLevel = false; + const bool m_flightLevel = false; }; //! Formatter when column contains a color @@ -320,11 +326,11 @@ namespace BlackGui //! \copydoc CDefaultFormatter::displayRole virtual BlackMisc::CVariant displayRole(const BlackMisc::CVariant &dataCVariant) const override; - //! Display the icon - virtual BlackMisc::CVariant decorationRole(const BlackMisc::CVariant &dataCVariant) const override; - //! \copydoc CDefaultFormatter::tooltipRole virtual BlackMisc::CVariant tooltipRole(const BlackMisc::CVariant &dataCVariant) const override; + + //! Display the icon + virtual BlackMisc::CVariant decorationRole(const BlackMisc::CVariant &dataCVariant) const override; }; //! Formatter for physical quantities @@ -345,7 +351,7 @@ namespace BlackGui else { Q_ASSERT_X(false, "CPhysiqalQuantiyFormatter::displayRole", "No CPhysicalQuantity class"); - return ""; + return emptyStringVariant(); } }