From 7ab12e7b4bc9ed14dc0a38a0cf1f4ad00b121888 Mon Sep 17 00:00:00 2001 From: AdenKoperczak Date: Sat, 14 Dec 2024 11:17:30 -0500 Subject: [PATCH] location markers part2 initial clang-tidy changes --- .../source/scwx/qt/manager/marker_manager.cpp | 40 ++++++++--------- scwx-qt/source/scwx/qt/map/marker_layer.cpp | 5 ++- .../source/scwx/qt/ui/edit_marker_dialog.cpp | 44 ++++++++++--------- .../source/scwx/qt/ui/edit_marker_dialog.hpp | 2 +- .../scwx/qt/ui/marker_settings_widget.cpp | 4 +- .../source/scwx/qt/util/q_color_modulate.cpp | 23 +++++++--- 6 files changed, 66 insertions(+), 52 deletions(-) diff --git a/scwx-qt/source/scwx/qt/manager/marker_manager.cpp b/scwx-qt/source/scwx/qt/manager/marker_manager.cpp index 21cb90cd..cf25cbcb 100644 --- a/scwx-qt/source/scwx/qt/manager/marker_manager.cpp +++ b/scwx-qt/source/scwx/qt/manager/marker_manager.cpp @@ -168,7 +168,7 @@ void MarkerManager::Impl::ReadMarkerSettings() boost::json::value markerJson = nullptr; { - std::unique_lock lock(markerRecordLock_); + const std::unique_lock lock(markerRecordLock_); // Determine if marker settings exists if (std::filesystem::exists(markerSettingsPath_)) @@ -188,9 +188,9 @@ void MarkerManager::Impl::ReadMarkerSettings() { auto record = boost::json::value_to(markerEntry); - types::MarkerId id = NewId(); - size_t index = markerRecords_.size(); - record.markerInfo_.id = id; + const types::MarkerId id = NewId(); + const size_t index = markerRecords_.size(); + record.markerInfo_.id = id; markerRecords_.emplace_back( std::make_shared(record.markerInfo_)); idToIndex_.emplace(id, index); @@ -218,7 +218,7 @@ void MarkerManager::Impl::WriteMarkerSettings() { logger_->info("Saving location marker settings"); - std::shared_lock lock(markerRecordLock_); + const std::shared_lock lock(markerRecordLock_); auto markerJson = boost::json::value_from(markerRecords_); util::json::WriteJsonFile(markerSettingsPath_, markerJson); } @@ -263,7 +263,7 @@ MarkerManager::MarkerManager() : p(std::make_unique(this)) // Read Marker settings on startup main::Application::WaitForInitialization(); { - std::unique_lock lock(p->markerIconsLock_); + const std::unique_lock lock(p->markerIconsLock_); p->markerIcons_.reserve( defaultMarkerIcons_.size()); for (auto& icon : defaultMarkerIcons_) @@ -295,7 +295,7 @@ size_t MarkerManager::marker_count() std::optional MarkerManager::get_marker(types::MarkerId id) { - std::shared_lock lock(p->markerRecordLock_); + const std::shared_lock lock(p->markerRecordLock_); if (!p->idToIndex_.contains(id)) { return {}; @@ -313,7 +313,7 @@ std::optional MarkerManager::get_marker(types::MarkerId id) std::optional MarkerManager::get_index(types::MarkerId id) { - std::shared_lock lock(p->markerRecordLock_); + const std::shared_lock lock(p->markerRecordLock_); if (!p->idToIndex_.contains(id)) { return {}; @@ -325,7 +325,7 @@ void MarkerManager::set_marker(types::MarkerId id, const types::MarkerInfo& marker) { { - std::unique_lock lock(p->markerRecordLock_); + const std::unique_lock lock(p->markerRecordLock_); if (!p->idToIndex_.contains(id)) { return; @@ -336,9 +336,9 @@ void MarkerManager::set_marker(types::MarkerId id, logger_->warn("id in idToIndex_ but out of range!"); return; } - std::shared_ptr& markerRecord = + const std::shared_ptr& markerRecord = p->markerRecords_[index]; - markerRecord->markerInfo_ = marker; + markerRecord->markerInfo_ = marker; markerRecord->markerInfo_.id = id; add_icon(marker.iconName); @@ -351,7 +351,7 @@ types::MarkerId MarkerManager::add_marker(const types::MarkerInfo& marker) { types::MarkerId id; { - std::unique_lock lock(p->markerRecordLock_); + const std::unique_lock lock(p->markerRecordLock_); id = p->NewId(); size_t index = p->markerRecords_.size(); p->idToIndex_.emplace(id, index); @@ -368,7 +368,7 @@ types::MarkerId MarkerManager::add_marker(const types::MarkerInfo& marker) void MarkerManager::remove_marker(types::MarkerId id) { { - std::unique_lock lock(p->markerRecordLock_); + const std::unique_lock lock(p->markerRecordLock_); if (!p->idToIndex_.contains(id)) { return; @@ -399,7 +399,7 @@ void MarkerManager::remove_marker(types::MarkerId id) void MarkerManager::move_marker(size_t from, size_t to) { { - std::unique_lock lock(p->markerRecordLock_); + const std::unique_lock lock(p->markerRecordLock_); if (from >= p->markerRecords_.size() || to >= p->markerRecords_.size()) { return; @@ -430,7 +430,7 @@ void MarkerManager::move_marker(size_t from, size_t to) void MarkerManager::for_each(std::function func) { - std::shared_lock lock(p->markerRecordLock_); + const std::shared_lock lock(p->markerRecordLock_); for (auto marker : p->markerRecords_) { func(marker->markerInfo_); @@ -440,12 +440,12 @@ void MarkerManager::for_each(std::function func) void MarkerManager::add_icon(const std::string& name, bool startup) { { - std::unique_lock lock(p->markerIconsLock_); + const std::unique_lock lock(p->markerIconsLock_); if (p->markerIcons_.contains(name)) { return; } - std::shared_ptr image = + const std::shared_ptr image = ResourceManager::LoadImageResource(name); auto icon = types::MarkerIconInfo(name, -1, -1, image); @@ -464,7 +464,7 @@ void MarkerManager::add_icon(const std::string& name, bool startup) std::optional MarkerManager::get_icon(const std::string& name) { - std::shared_lock lock(p->markerIconsLock_); + const std::shared_lock lock(p->markerIconsLock_); if (p->markerIcons_.contains(name)) { return p->markerIcons_.at(name); @@ -476,7 +476,7 @@ MarkerManager::get_icon(const std::string& name) const std::unordered_map MarkerManager::get_icons() { - std::shared_lock lock(p->markerIconsLock_); + const std::shared_lock lock(p->markerIconsLock_); return p->markerIcons_; } @@ -492,7 +492,7 @@ std::shared_ptr MarkerManager::Instance() static std::weak_ptr markerManagerReference_ {}; static std::mutex instanceMutex_ {}; - std::unique_lock lock(instanceMutex_); + const std::unique_lock lock(instanceMutex_); std::shared_ptr markerManager = markerManagerReference_.lock(); diff --git a/scwx-qt/source/scwx/qt/map/marker_layer.cpp b/scwx-qt/source/scwx/qt/map/marker_layer.cpp index 0bad94f6..a0ac668f 100644 --- a/scwx-qt/source/scwx/qt/map/marker_layer.cpp +++ b/scwx-qt/source/scwx/qt/map/marker_layer.cpp @@ -72,9 +72,10 @@ void MarkerLayer::Impl::ReloadMarkers() { // must use local ID, instead of reference to marker in event handler // callback. - types::MarkerId id = marker.id; + const types::MarkerId id = marker.id; - std::shared_ptr icon = geoIcons_->AddIcon(); + const std::shared_ptr icon = + geoIcons_->AddIcon(); geoIcons_->SetIconTexture(icon, marker.iconName, 0); geoIcons_->SetIconLocation(icon, marker.latitude, marker.longitude); diff --git a/scwx-qt/source/scwx/qt/ui/edit_marker_dialog.cpp b/scwx-qt/source/scwx/qt/ui/edit_marker_dialog.cpp index b3c34e96..47bfb07f 100644 --- a/scwx-qt/source/scwx/qt/ui/edit_marker_dialog.cpp +++ b/scwx-qt/source/scwx/qt/ui/edit_marker_dialog.cpp @@ -30,7 +30,12 @@ static const auto logger_ = scwx::util::Logger::Create(logPrefix_); class EditMarkerDialog::Impl { public: - explicit Impl(EditMarkerDialog* self) : self_ {self} {} + explicit Impl(EditMarkerDialog* self) : + self_ {self}, + deleteButton_ {self_->ui->buttonBox->addButton( + "Delete", QDialogButtonBox::DestructiveRole)} + { + } void show_color_dialog(); void show_icon_file_dialog(); @@ -49,8 +54,8 @@ public: std::shared_ptr markerManager_ = manager::MarkerManager::Instance(); - types::MarkerId editId_; - bool adding_; + types::MarkerId editId_ {0}; + bool adding_ {false}; std::string setIconOnAdded_ {""}; }; @@ -75,8 +80,6 @@ EditMarkerDialog::EditMarkerDialog(QWidget* parent) : QString(""), QString::fromStdString(markerIcon.second.name)); } - p->deleteButton_ = - ui->buttonBox->addButton("Delete", QDialogButtonBox::DestructiveRole); p->connect_signals(); } @@ -94,8 +97,8 @@ void EditMarkerDialog::setup(double latitude, double longitude) { // By default use foreground color as marker color, mainly so the icons // are vissable in the dropdown menu. - QColor color = QWidget::palette().color(QWidget::foregroundRole()); - p->editId_ = p->markerManager_->add_marker(types::MarkerInfo( + const QColor color = QWidget::palette().color(QWidget::foregroundRole()); + p->editId_ = p->markerManager_->add_marker(types::MarkerInfo( "", latitude, longitude, @@ -120,7 +123,8 @@ void EditMarkerDialog::setup(types::MarkerId id) p->editId_ = id; p->adding_ = false; - std::string iconColorStr = util::color::ToArgbString(marker->iconColor); + const std::string iconColorStr = + util::color::ToArgbString(marker->iconColor); p->set_icon_color(iconColorStr); int iconIndex = @@ -139,8 +143,8 @@ void EditMarkerDialog::setup(types::MarkerId id) types::MarkerInfo EditMarkerDialog::get_marker_info() const { - QString colorName = ui->iconColorLineEdit->text(); - boost::gil::rgba8_pixel_t color = + const QString colorName = ui->iconColorLineEdit->text(); + const boost::gil::rgba8_pixel_t color = util::color::ToRgba8PixelT(colorName.toStdString()); return types::MarkerInfo( @@ -159,7 +163,7 @@ void EditMarkerDialog::Impl::show_color_dialog() dialog->setAttribute(Qt::WA_DeleteOnClose); dialog->setOption(QColorDialog::ColorDialogOption::ShowAlphaChannel); - QColor initialColor(self_->ui->iconColorLineEdit->text()); + const QColor initialColor(self_->ui->iconColorLineEdit->text()); if (initialColor.isValid()) { dialog->setCurrentColor(initialColor); @@ -170,7 +174,7 @@ void EditMarkerDialog::Impl::show_color_dialog() self_, [this](const QColor& qColor) { - QString colorName = + const QString colorName = qColor.name(QColor::NameFormat::HexArgb); self_->ui->iconColorLineEdit->setText(colorName); set_icon_color(colorName.toStdString()); @@ -180,7 +184,6 @@ void EditMarkerDialog::Impl::show_color_dialog() void EditMarkerDialog::Impl::show_icon_file_dialog() { - auto* dialog = new QFileDialog(self_); dialog->setFileMode(QFileDialog::ExistingFile); @@ -192,7 +195,7 @@ void EditMarkerDialog::Impl::show_icon_file_dialog() self_, [this](const QString& file) { - std::string path = + const std::string path = QDir::toNativeSeparators(file).toStdString(); setIconOnAdded_ = path; markerManager_->add_icon(path); @@ -224,13 +227,12 @@ void EditMarkerDialog::Impl::connect_signals() connect(self_->ui->iconColorLineEdit, &QLineEdit::textEdited, self_, - [=, this](const QString& text) - { set_icon_color(text.toStdString()); }); + [this](const QString& text) { set_icon_color(text.toStdString()); }); connect(self_->ui->iconColorButton, &QAbstractButton::clicked, self_, - [=, this]() { show_color_dialog(); }); + [this]() { show_color_dialog(); }); connect(self_->ui->iconFileOpenButton, &QPushButton::clicked, @@ -242,13 +244,13 @@ void EditMarkerDialog::Impl::connect_signals() self_, [this]() { - std::string color = + const std::string color = self_->ui->iconColorLineEdit->text().toStdString(); set_icon_color(color); if (setIconOnAdded_ != "") { - int i = self_->ui->iconComboBox->findData( + const int i = self_->ui->iconComboBox->findData( QString::fromStdString(setIconOnAdded_)); if (i >= 0) { @@ -269,9 +271,9 @@ void EditMarkerDialog::Impl::set_icon_color(const std::string& color) self_->ui->iconComboBox->clear(); for (auto& markerIcon : markerManager_->get_icons()) { - int i = + const int i = iconComboBox->findData(QString::fromStdString(markerIcon.second.name)); - QIcon icon = get_colored_icon(markerIcon.second, color); + const QIcon icon = get_colored_icon(markerIcon.second, color); if (i < 0) { iconComboBox->addItem( diff --git a/scwx-qt/source/scwx/qt/ui/edit_marker_dialog.hpp b/scwx-qt/source/scwx/qt/ui/edit_marker_dialog.hpp index 4008b990..6c6f6e6c 100644 --- a/scwx-qt/source/scwx/qt/ui/edit_marker_dialog.hpp +++ b/scwx-qt/source/scwx/qt/ui/edit_marker_dialog.hpp @@ -27,7 +27,7 @@ public: void setup(double latitude, double longitude); void setup(types::MarkerId id); - types::MarkerInfo get_marker_info() const; + [[nodiscard]] types::MarkerInfo get_marker_info() const; private: class Impl; diff --git a/scwx-qt/source/scwx/qt/ui/marker_settings_widget.cpp b/scwx-qt/source/scwx/qt/ui/marker_settings_widget.cpp index ba6cf88b..cc98302d 100644 --- a/scwx-qt/source/scwx/qt/ui/marker_settings_widget.cpp +++ b/scwx-qt/source/scwx/qt/ui/marker_settings_widget.cpp @@ -102,7 +102,7 @@ void MarkerSettingsWidgetImpl::ConnectSignals() return; } - bool itemSelected = selected.size() > 0; + const bool itemSelected = selected.size() > 0; self_->ui->removeButton->setEnabled(itemSelected); }); QObject::connect(self_->ui->markerView, @@ -110,7 +110,7 @@ void MarkerSettingsWidgetImpl::ConnectSignals() self_, [this](const QModelIndex& index) { - int row = index.row(); + const int row = index.row(); if (row < 0) { return; diff --git a/scwx-qt/source/scwx/qt/util/q_color_modulate.cpp b/scwx-qt/source/scwx/qt/util/q_color_modulate.cpp index 567fc62d..007ceb47 100644 --- a/scwx-qt/source/scwx/qt/util/q_color_modulate.cpp +++ b/scwx-qt/source/scwx/qt/util/q_color_modulate.cpp @@ -20,11 +20,22 @@ void modulateColors_(QImage& image, const QColor& color) QRgb* line = reinterpret_cast(image.scanLine(y)); for (int x = 0; x < image.width(); ++x) { - QRgb& rgb = line[x]; - int red = qRed(rgb) * color.redF(); - int green = qGreen(rgb) * color.greenF(); - int blue = qBlue(rgb) * color.blueF(); - int alpha = qAlpha(rgb) * color.alphaF(); + QRgb& rgb = line[x]; + /* clang-format off + * NOLINTBEGIN(cppcoreguidelines-narrowing-conversions, bugprone-narrowing-conversions) + * qRed/qGreen/qBlue/qAlpha return values 0-255, handlable by float + * redF/greenF/blueF/alphaF are all 0-1, so output is 0-255 + * Rounding is fine for this. + * clang-format on + */ + const int red = qRed(rgb) * color.redF(); + const int green = qGreen(rgb) * color.greenF(); + const int blue = qBlue(rgb) * color.blueF(); + const int alpha = qAlpha(rgb) * color.alphaF(); + /* clang-format off + * NOLINTEND(cppcoreguidelines-narrowing-conversions, bugprone-narrowing-conversions) + * clang-format on + */ rgb = qRgba(red, green, blue, alpha); } @@ -47,7 +58,7 @@ QPixmap modulateColors(const QPixmap& pixmap, const QColor& color) QIcon modulateColors(const QIcon& icon, const QSize& size, const QColor& color) { - QPixmap pixmap = modulateColors(icon.pixmap(size), color); + const QPixmap pixmap = modulateColors(icon.pixmap(size), color); return QIcon(pixmap); }