First pass fixes from discussions. Mostly linter fixes

.clang-tidy

@@ -10,4 +10,6 @@ Checks:
   - '-misc-include-cleaner'
   - '-misc-non-private-member-variables-in-classes'
   - '-modernize-use-trailing-return-type'
+  - '-bugprone-easily-swappable-parameters'
+  - '-modernize-return-braced-init-list'
 FormatStyle: 'file'

scwx-qt/source/scwx/qt/manager/marker_manager.cpp

@@ -203,9 +203,7 @@ void MarkerManager::Impl::ReadMarkerSettings()
             }
          }
 
-         util::TextureAtlas& textureAtlas = util::TextureAtlas::Instance();
+         ResourceManager::BuildAtlas();
-         textureAtlas.BuildAtlas(
-            2048, 2048); // Should this code be moved to ResourceManager?
 
          logger_->debug("{} location marker entries", markerRecords_.size());
       }
@@ -239,7 +237,7 @@ MarkerManager::Impl::GetMarkerByName(const std::string& name)
 
 MarkerManager::MarkerManager() : p(std::make_unique<Impl>(this))
 {
-   const std::vector<types::MarkerIconInfo> defaultMarkerIcons_ {
+   static const std::vector<types::MarkerIconInfo> defaultMarkerIcons_ {
       types::MarkerIconInfo(types::ImageTexture::LocationMarker, -1, -1),
       types::MarkerIconInfo(types::ImageTexture::LocationPin, 6, 16),
       types::MarkerIconInfo(types::ImageTexture::LocationCrosshair, -1, -1),
@@ -256,7 +254,7 @@ MarkerManager::MarkerManager() : p(std::make_unique<Impl>(this))
    p->InitializeMarkerSettings();
 
    boost::asio::post(p->threadPool_,
-                     [this, defaultMarkerIcons_]()
+                     [this]()
                      {
                         try
                         {
@@ -454,9 +452,7 @@ void MarkerManager::add_icon(const std::string& name, bool startup)
 
    if (!startup)
    {
-      util::TextureAtlas& textureAtlas = util::TextureAtlas::Instance();
+      ResourceManager::BuildAtlas();
-      textureAtlas.BuildAtlas(
-         2048, 2048); // Should this code be moved to ResourceManager?
       Q_EMIT IconAdded(name);
    }
 }
@@ -465,9 +461,10 @@ std::optional<types::MarkerIconInfo>
 MarkerManager::get_icon(const std::string& name)
 {
    const std::shared_lock lock(p->markerIconsLock_);
-   if (p->markerIcons_.contains(name))
+   auto it = p->markerIcons_.find(name);
+   if (it != p->markerIcons_.end())
    {
-      return p->markerIcons_.at(name);
+      return it->second;
    }
 
    return {};

scwx-qt/source/scwx/qt/manager/resource_manager.cpp

@@ -22,6 +22,9 @@ namespace ResourceManager
 static const std::string logPrefix_ = "scwx::qt::manager::resource_manager";
 static const auto        logger_    = scwx::util::Logger::Create(logPrefix_);
 
+static const size_t atlasWidth  = 2048;
+static const size_t atlasHeight = 2048;
+
 static void LoadFonts();
 static void LoadTextures();
 
@@ -68,8 +71,7 @@ LoadImageResources(const std::vector<std::string>& urlStrings)
 
    if (!images.empty())
    {
-      util::TextureAtlas& textureAtlas = util::TextureAtlas::Instance();
+      BuildAtlas();
-      textureAtlas.BuildAtlas(2048, 2048);
    }
 
    return images;
@@ -103,7 +105,13 @@ static void LoadTextures()
                                    GetTexturePath(lineTexture));
    }
 
-   textureAtlas.BuildAtlas(2048, 2048);
+   BuildAtlas();
+}
+
+void BuildAtlas()
+{
+      util::TextureAtlas& textureAtlas = util::TextureAtlas::Instance();
+      textureAtlas.BuildAtlas(atlasWidth, atlasHeight);
 }
 
 } // namespace ResourceManager

scwx-qt/source/scwx/qt/manager/resource_manager.hpp

@@ -22,6 +22,7 @@ std::shared_ptr<boost::gil::rgba8_image_t>
 LoadImageResource(const std::string& urlString);
 std::vector<std::shared_ptr<boost::gil::rgba8_image_t>>
 LoadImageResources(const std::vector<std::string>& urlStrings);
+void BuildAtlas();
 
 } // namespace ResourceManager
 } // namespace manager

scwx-qt/source/scwx/qt/map/map_widget.cpp

@@ -219,7 +219,7 @@ public:
    std::shared_ptr<model::LayerModel> layerModel_ {
       model::LayerModel::Instance()};
 
-   std::shared_ptr<ui::EditMarkerDialog> editMarkerDialog_;
+   ui::EditMarkerDialog* editMarkerDialog_;
 
    std::shared_ptr<manager::HotkeyManager> hotkeyManager_ {
       manager::HotkeyManager::Instance()};
@@ -286,7 +286,10 @@ MapWidget::MapWidget(std::size_t id, const QMapLibre::Settings& settings) :
 
    ImGui_ImplQt_RegisterWidget(this);
 
-   p->editMarkerDialog_ = std::make_shared<ui::EditMarkerDialog>(this);
+   // Qt parent deals with memory management
+   // NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
+   p->editMarkerDialog_ = new ui::EditMarkerDialog(this);
+
    p->ConnectSignals();
 }
 

scwx-qt/source/scwx/qt/map/marker_layer.cpp

@@ -77,9 +77,21 @@ void MarkerLayer::Impl::ReloadMarkers()
          const std::shared_ptr<gl::draw::GeoIconDrawItem> icon =
             geoIcons_->AddIcon();
 
+         const std::string latitudeString =
+            common::GetLatitudeString(marker.latitude);
+         const std::string longitudeString =
+            common::GetLongitudeString(marker.longitude);
+
+         const std::string hoverText =
+            marker.name != "" ?
+               fmt::format(
+                  "{}\n{}, {}", marker.name, latitudeString, longitudeString) :
+               fmt::format("{}, {}", latitudeString, longitudeString);
+
+
          geoIcons_->SetIconTexture(icon, marker.iconName, 0);
          geoIcons_->SetIconLocation(icon, marker.latitude, marker.longitude);
-         geoIcons_->SetIconHoverText(icon, marker.name);
+         geoIcons_->SetIconHoverText(icon, hoverText);
          geoIcons_->SetIconModulate(icon, marker.iconColor);
          geoIcons_->RegisterEventHandler(
             icon,

scwx-qt/source/scwx/qt/model/marker_model.cpp

@@ -129,7 +129,20 @@ QVariant MarkerModel::data(const QModelIndex& index, int role) const
       break;
       break;
    case static_cast<int>(Column::Icon):
-      if (role == Qt::ItemDataRole::DecorationRole)
+      if (role == Qt::ItemDataRole::DisplayRole)
+      {
+         std::optional<types::MarkerIconInfo> icon =
+            p->markerManager_->get_icon(markerInfo->iconName);
+         if (icon)
+         {
+            return QString::fromStdString(icon->shortName);
+         }
+         else
+         {
+            return {};
+         }
+      }
+      else if (role == Qt::ItemDataRole::DecorationRole)
       {
          std::optional<types::MarkerIconInfo> icon =
             p->markerManager_->get_icon(markerInfo->iconName);

scwx-qt/source/scwx/qt/types/marker_types.hpp

@@ -2,31 +2,29 @@
 
 #include <scwx/qt/types/texture_types.hpp>
 
-#include <string>
 #include <cstdint>
+#include <string>
+#include <utility>
 
 #include <boost/gil.hpp>
+#include <QFileInfo>
 #include <QIcon>
 
-namespace scwx
+namespace scwx::qt::types
-{
-namespace qt
-{
-namespace types
 {
 using MarkerId = std::uint64_t;
 
 struct MarkerInfo
 {
-   MarkerInfo(const std::string&               name,
+   MarkerInfo(std::string                      name,
               double                           latitude,
               double                           longitude,
-              const std::string&               iconName,
+              std::string                      iconName,
               const boost::gil::rgba8_pixel_t& iconColor) :
-       name {name},
+       name {std::move(name)},
        latitude {latitude},
        longitude {longitude},
-       iconName {iconName},
+       iconName {std::move(iconName)},
        iconColor {iconColor}
    {
    }
@@ -41,6 +39,7 @@ struct MarkerInfo
 
 struct MarkerIconInfo
 {
+   // Initializer for default icons (which use a texture)
    explicit MarkerIconInfo(types::ImageTexture texture,
                            std::int32_t        hotX,
                            std::int32_t        hotY) :
@@ -51,14 +50,19 @@ struct MarkerIconInfo
        qIcon {QIcon(QString::fromStdString(path))},
        image {}
    {
+      auto qName = QString::fromStdString(name);
+      QStringList parts = qName.split("location-");
+      shortName = parts.last().toStdString();
    }
 
+   // Initializer for custom icons (which use a file path)
    explicit MarkerIconInfo(const std::string&                         path,
                            std::int32_t                               hotX,
                            std::int32_t                               hotY,
                            std::shared_ptr<boost::gil::rgba8_image_t> image) :
        name {path},
        path {path},
+       shortName {QFileInfo(path.c_str()).fileName().toStdString()},
        hotX {hotX},
        hotY {hotY},
        qIcon {QIcon(QString::fromStdString(path))},
@@ -68,12 +72,11 @@ struct MarkerIconInfo
 
    std::string                                               name;
    std::string                                               path;
+   std::string                                               shortName;
    std::int32_t                                              hotX;
    std::int32_t                                              hotY;
    QIcon                                                     qIcon;
    std::optional<std::shared_ptr<boost::gil::rgba8_image_t>> image;
 };
 
-} // namespace types
+} // namespace scwx::qt::types
-} // namespace qt
-} // namespace scwx

scwx-qt/source/scwx/qt/ui/edit_marker_dialog.cpp

@@ -17,11 +17,7 @@
 #include <QPushButton>
 #include <QFileDialog>
 
-namespace scwx
+namespace scwx::qt::ui
-{
-namespace qt
-{
-namespace ui
 {
 
 static const std::string logPrefix_ = "scwx::qt::ui::edit_marker_dialog";
@@ -71,9 +67,10 @@ EditMarkerDialog::EditMarkerDialog(QWidget* parent) :
 
    for (auto& markerIcon : p->markerManager_->get_icons())
    {
-      ui->iconComboBox->addItem(markerIcon.second.qIcon,
+      ui->iconComboBox->addItem(
-                                QString(""),
+         markerIcon.second.qIcon,
-                                QString::fromStdString(markerIcon.second.name));
+         QString::fromStdString(markerIcon.second.shortName),
+         QString::fromStdString(markerIcon.second.name));
    }
    p->deleteButton_ =
       ui->buttonBox->addButton("Delete", QDialogButtonBox::DestructiveRole);
@@ -154,7 +151,8 @@ types::MarkerInfo EditMarkerDialog::get_marker_info() const
 
 void EditMarkerDialog::Impl::show_color_dialog()
 {
-
+   // WA_DeleteOnClose manages memory
+   // NOLINTNEXTLINE(cppcoreguidelines-owning-memory)
    auto* dialog = new QColorDialog(self_);
 
    dialog->setAttribute(Qt::WA_DeleteOnClose);
@@ -184,7 +182,7 @@ void EditMarkerDialog::Impl::show_icon_file_dialog()
    auto* dialog = new QFileDialog(self_);
 
    dialog->setFileMode(QFileDialog::ExistingFile);
-   dialog->setNameFilters({"Icon (*.png *.svg)", "All (*)"});
+   dialog->setNameFilters({"Icon (*.png *.svg)", "All Files (*)"});
    dialog->setAttribute(Qt::WA_DeleteOnClose);
 
    QObject::connect(dialog,
@@ -256,6 +254,11 @@ void EditMarkerDialog::Impl::connect_signals()
                  }
               }
            });
+
+   connect(self_->ui->buttonBox->button(QDialogButtonBox::Apply),
+           &QAbstractButton::clicked,
+           self_,
+           [this]() { handle_accepted(); });
 }
 
 void EditMarkerDialog::Impl::set_icon_color(const std::string& color)
@@ -276,7 +279,9 @@ void EditMarkerDialog::Impl::set_icon_color(const std::string& color)
       if (i < 0)
       {
          iconComboBox->addItem(
-            icon, QString(""), QString::fromStdString(markerIcon.second.name));
+            icon,
+            QString::fromStdString(markerIcon.second.shortName),
+            QString::fromStdString(markerIcon.second.name));
       }
       else
       {
@@ -306,6 +311,4 @@ void EditMarkerDialog::Impl::handle_rejected()
    }
 }
 
-} // namespace ui
+} // namespace scwx::qt::ui
-} // namespace qt
-} // namespace scwx

scwx-qt/source/scwx/qt/ui/edit_marker_dialog.hpp

@@ -8,11 +8,7 @@ namespace Ui
 class EditMarkerDialog;
 }
 
-namespace scwx
+namespace scwx::qt::ui
-{
-namespace qt
-{
-namespace ui
 {
 class EditMarkerDialog : public QDialog
 {
@@ -35,6 +31,4 @@ private:
    Ui::EditMarkerDialog* ui;
 };
 
-} // namespace ui
+} // namespace scwx::qt::ui
-} // namespace qt
-} // namespace scwx

scwx-qt/source/scwx/qt/ui/edit_marker_dialog.ui

@@ -101,7 +101,7 @@
       <enum>Qt::Orientation::Horizontal</enum>
      </property>
      <property name="standardButtons">
-      <set>QDialogButtonBox::StandardButton::Cancel|QDialogButtonBox::StandardButton::Ok</set>
+      <set>QDialogButtonBox::StandardButton::Apply|QDialogButtonBox::StandardButton::Cancel|QDialogButtonBox::StandardButton::Ok</set>
      </property>
     </widget>
    </item>

scwx-qt/source/scwx/qt/ui/marker_settings_widget.cpp

@@ -23,15 +23,21 @@ class MarkerSettingsWidgetImpl
 {
 public:
    explicit MarkerSettingsWidgetImpl(MarkerSettingsWidget* self) :
-      self_ {self},
+       self_ {self},
-      markerModel_ {new model::MarkerModel(self_)}
+       markerModel_ {new model::MarkerModel(self_)},
+       proxyModel_ {new QSortFilterProxyModel(self_)}
    {
+      proxyModel_->setSourceModel(markerModel_);
+      proxyModel_->setSortRole(Qt::DisplayRole); // TODO types::SortRole
+      proxyModel_->setFilterCaseSensitivity(Qt::CaseInsensitive);
+      proxyModel_->setFilterKeyColumn(-1);
    }
 
    void ConnectSignals();
 
-   MarkerSettingsWidget* self_;
+   MarkerSettingsWidget*                   self_;
-   model::MarkerModel* markerModel_;
+   model::MarkerModel*                     markerModel_;
+   QSortFilterProxyModel*                  proxyModel_;
    std::shared_ptr<manager::MarkerManager> markerManager_ {
       manager::MarkerManager::Instance()};
    std::shared_ptr<ui::EditMarkerDialog> editMarkerDialog_ {nullptr};
@@ -46,7 +52,7 @@ MarkerSettingsWidget::MarkerSettingsWidget(QWidget* parent) :
    ui->setupUi(this);
 
    ui->removeButton->setEnabled(false);
-   ui->markerView->setModel(p->markerModel_);
+   ui->markerView->setModel(p->proxyModel_);
 
    p->editMarkerDialog_ = std::make_shared<ui::EditMarkerDialog>(this);
 

scwx-qt/source/scwx/qt/util/q_color_modulate.cpp

@@ -6,11 +6,7 @@
 #include <QPixmap>
 #include <QSize>
 
-namespace scwx
+namespace scwx::qt::util
-{
-namespace qt
-{
-namespace util
 {
 
 void modulateColors_(QImage& image, const QColor& color)
@@ -20,6 +16,9 @@ void modulateColors_(QImage& image, const QColor& color)
       QRgb* line = reinterpret_cast<QRgb*>(image.scanLine(y));
       for (int x = 0; x < image.width(); ++x)
       {
+         // This is pulled from Qt Documentation
+         // https://doc.qt.io/qt-6/qimage.html#scanLine
+         // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
          QRgb& rgb = line[x];
          /* clang-format off
           * NOLINTBEGIN(cppcoreguidelines-narrowing-conversions, bugprone-narrowing-conversions)
@@ -62,6 +61,4 @@ QIcon modulateColors(const QIcon& icon, const QSize& size, const QColor& color)
    return QIcon(pixmap);
 }
 
-} // namespace util
+} // namespace scwx::qt::util
-} // namespace qt
-} // namespace scwx

scwx-qt/source/scwx/qt/util/q_color_modulate.hpp

@@ -6,17 +6,11 @@
 #include <QPixmap>
 #include <QSize>
 
-namespace scwx
+namespace scwx::qt::util
-{
-namespace qt
-{
-namespace util
 {
 
 QImage  modulateColors(const QImage& image, const QColor& color);
 QPixmap modulateColors(const QPixmap& pixmap, const QColor& color);
 QIcon modulateColors(const QIcon& icon, const QSize& size, const QColor& color);
 
-} // namespace util
+} // namespace scwx::qt::util
-} // namespace qt
-} // namespace scwx