From b682e6b37dac6269acd934ba75d0caa68eedecea Mon Sep 17 00:00:00 2001 From: Alexander Vieth Date: Mon, 27 Oct 2025 14:57:30 +0100 Subject: [PATCH 1/7] Formatting: Move variable to where it's used --- src/ScatterplotPlugin.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ScatterplotPlugin.cpp b/src/ScatterplotPlugin.cpp index 0429892..1555562 100644 --- a/src/ScatterplotPlugin.cpp +++ b/src/ScatterplotPlugin.cpp @@ -797,9 +797,6 @@ void ScatterplotPlugin::loadColors(const Dataset& clusters) if (!clusters.isValid() || !_positionDataset.isValid()) return; - // Mapping from local to global indices - std::vector globalIndices; - // Get global indices from the position dataset int totalNumPoints = 0; if (_positionDataset->isDerivedData()) @@ -807,6 +804,8 @@ void ScatterplotPlugin::loadColors(const Dataset& clusters) else totalNumPoints = _positionDataset->getFullDataset()->getNumPoints(); + // Mapping from local to global indices + std::vector globalIndices; _positionDataset->getGlobalIndices(globalIndices); // Generate color buffer for global and local colors From 99c7c03b4a2c35870cd10ddd6247afe7ec90e161 Mon Sep 17 00:00:00 2001 From: Alexander Vieth Date: Mon, 27 Oct 2025 14:58:05 +0100 Subject: [PATCH 2/7] Formatting: Use member variable instead of manual check --- src/ScatterplotPlugin.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ScatterplotPlugin.cpp b/src/ScatterplotPlugin.cpp index 1555562..cc95657 100644 --- a/src/ScatterplotPlugin.cpp +++ b/src/ScatterplotPlugin.cpp @@ -810,11 +810,11 @@ void ScatterplotPlugin::loadColors(const Dataset& clusters) // Generate color buffer for global and local colors std::vector globalColors(totalNumPoints); - std::vector localColors(_positions.size()); + std::vector localColors(_numPoints); const auto& clusterVec = clusters->getClusters(); - if (totalNumPoints == _positions.size() && clusterVec.size() == totalNumPoints) + if (totalNumPoints == _numPoints && clusterVec.size() == totalNumPoints) { for (size_t i = 0; i < static_cast(clusterVec.size()); i++) { From de2ad83b1c1d1cb8e0a3791256f4724b77f9835b Mon Sep 17 00:00:00 2001 From: Alexander Vieth Date: Mon, 27 Oct 2025 14:58:30 +0100 Subject: [PATCH 3/7] Clean-up: Do not repeat construction of same color --- src/ScatterplotPlugin.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ScatterplotPlugin.cpp b/src/ScatterplotPlugin.cpp index cc95657..ce9e67d 100644 --- a/src/ScatterplotPlugin.cpp +++ b/src/ScatterplotPlugin.cpp @@ -830,9 +830,10 @@ void ScatterplotPlugin::loadColors(const Dataset& clusters) // Loop over all clusters and populate global colors for (const auto& cluster : clusterVec) { - const auto color = cluster.getColor(); + const auto color = cluster.getColor(); + const auto colVec = Vector3f(color.redF(), color.greenF(), color.blueF()); for (const auto& index : cluster.getIndices()) - globalColors[index] = Vector3f(color.redF(), color.greenF(), color.blueF()); + globalColors[index] = colVec; } From f59ab58464dd8fc96ee27c0b4f9bc24c3b1a7a70 Mon Sep 17 00:00:00 2001 From: Alexander Vieth Date: Mon, 27 Oct 2025 14:58:48 +0100 Subject: [PATCH 4/7] Fix crash on cluster and point mismatch --- src/ScatterplotPlugin.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ScatterplotPlugin.cpp b/src/ScatterplotPlugin.cpp index ce9e67d..05ddec0 100644 --- a/src/ScatterplotPlugin.cpp +++ b/src/ScatterplotPlugin.cpp @@ -825,7 +825,7 @@ void ScatterplotPlugin::loadColors(const Dataset& clusters) } } - else + else if(globalIndices.size() == _numPoints) { // Loop over all clusters and populate global colors for (const auto& cluster : clusterVec) From e75d7c60ab0efcfb70f7a3bf2e1d4bf1df16e2f5 Mon Sep 17 00:00:00 2001 From: Alexander Vieth Date: Mon, 27 Oct 2025 14:59:12 +0100 Subject: [PATCH 5/7] Respect cluster re-coloring on startup --- src/ScatterplotWidget.cpp | 6 ++++-- src/ScatterplotWidget.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/ScatterplotWidget.cpp b/src/ScatterplotWidget.cpp index 535c985..ff6f6f8 100644 --- a/src/ScatterplotWidget.cpp +++ b/src/ScatterplotWidget.cpp @@ -41,6 +41,7 @@ ScatterplotWidget::ScatterplotWidget(mv::plugin::ViewPlugin* parentPlugin) : _pointRenderer(this), _isInitialized(false), _renderMode(SCATTERPLOT), + _scalarEffect(PointEffect::Color), _backgroundColor(255, 255, 255, 255), _coloringMode(ColoringMode::Constant), _dataRectangleAction(this, "Data rectangle"), @@ -334,7 +335,7 @@ void ScatterplotWidget::setScalars(const std::vector& scalars) void ScatterplotWidget::setColors(const std::vector& colors) { _pointRenderer.setColors(colors); - _pointRenderer.setScalarEffect(None); + setScalarEffect(PointEffect::None); update(); } @@ -367,6 +368,7 @@ void ScatterplotWidget::setPointScaling(mv::gui::PointScaling scalingMode) void ScatterplotWidget::setScalarEffect(PointEffect effect) { _pointRenderer.setScalarEffect(effect); + _scalarEffect = effect; update(); } @@ -621,7 +623,7 @@ void ScatterplotWidget::initializeGL() _densityRenderer.init(); // Set a default color map for both renderers - _pointRenderer.setScalarEffect(PointEffect::Color); + _pointRenderer.setScalarEffect(_scalarEffect); _pointRenderer.setPointScaling(Absolute); _pointRenderer.setSelectionOutlineColor(Vector3f(1, 0, 0)); diff --git a/src/ScatterplotWidget.h b/src/ScatterplotWidget.h index cf0618a..9e73b49 100644 --- a/src/ScatterplotWidget.h +++ b/src/ScatterplotWidget.h @@ -296,6 +296,7 @@ private slots: private: bool _isInitialized; /** Boolean determining whether the widget it properly initialized or not */ RenderMode _renderMode; /** Current render mode */ + PointEffect _scalarEffect; /** Current scalar effect */ QColor _backgroundColor; /** Background color */ ColoringMode _coloringMode; /** Type of point/density coloring */ DecimalRectangleAction _dataRectangleAction; /** Rectangle action for the bounds of the loaded data */ From 9e35f148e718155f2eb3e5b8e60da9e98d5b847e Mon Sep 17 00:00:00 2001 From: Alexander Vieth Date: Mon, 27 Oct 2025 14:59:24 +0100 Subject: [PATCH 6/7] Clean-up: less duplication --- src/ColoringAction.cpp | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/ColoringAction.cpp b/src/ColoringAction.cpp index 18e04e8..7c52fed 100644 --- a/src/ColoringAction.cpp +++ b/src/ColoringAction.cpp @@ -127,24 +127,29 @@ ColoringAction::ColoringAction(QObject* parent, const QString& title) : connect(&_scatterplotPlugin->getPositionDataset(), &Dataset::childAdded, this, &ColoringAction::updateColorByActionOptions); connect(&_scatterplotPlugin->getPositionDataset(), &Dataset::childRemoved, this, &ColoringAction::updateColorByActionOptions); - connect(&_scatterplotPlugin->getScatterplotWidget(), &ScatterplotWidget::renderModeChanged, this, &ColoringAction::updateScatterPlotWidgetColors); - connect(&_scatterplotPlugin->getScatterplotWidget(), &ScatterplotWidget::coloringModeChanged, this, &ColoringAction::updateScatterPlotWidgetColors); + connect(&_scatterplotPlugin->getScatterplotWidget(), &ScatterplotWidget::coloringModeChanged, this, [this](const ScatterplotWidget::ColoringMode& coloringMode) { + updateScatterPlotWidgetColors(); + updateColorMapActionsReadOnly(); + }); + + connect(&_scatterplotPlugin->getScatterplotWidget(), &ScatterplotWidget::renderModeChanged, this, [this](const ScatterplotWidget::RenderMode& renderMode) { + updateScatterPlotWidgetColors(); + updateColorMapActionsReadOnly(); + }); - connect(&_dimensionAction, &DimensionPickerAction::currentDimensionIndexChanged, this, &ColoringAction::updateScatterPlotWidgetColors); - connect(&_dimensionAction, &DimensionPickerAction::currentDimensionIndexChanged, this, &ColoringAction::updateColorMapActionScalarRange); + connect(&_dimensionAction, &DimensionPickerAction::currentDimensionIndexChanged, this, [this](const int32_t& currentDimensionIndex) { + updateScatterPlotWidgetColors(); + updateColorMapActionsReadOnly(); + updateColorMapActionScalarRange(); + }); connect(&_constantColorAction, &ColorAction::colorChanged, this, &ColoringAction::updateScatterplotWidgetColorMap); connect(&_colorMap1DAction, &ColorMapAction::imageChanged, this, &ColoringAction::updateScatterplotWidgetColorMap); connect(&_colorMap2DAction, &ColorMapAction::imageChanged, this, &ColoringAction::updateScatterplotWidgetColorMap); - connect(&_scatterplotPlugin->getScatterplotWidget(), &ScatterplotWidget::coloringModeChanged, this, &ColoringAction::updateScatterplotWidgetColorMap); - connect(&_scatterplotPlugin->getScatterplotWidget(), &ScatterplotWidget::renderModeChanged, this, &ColoringAction::updateScatterplotWidgetColorMap); connect(&_colorMap1DAction.getRangeAction(ColorMapAction::Axis::X), &DecimalRangeAction::rangeChanged, this, &ColoringAction::updateScatterPlotWidgetColorMapRange); connect(&_colorMap2DAction.getRangeAction(ColorMapAction::Axis::X), &DecimalRangeAction::rangeChanged, this, &ColoringAction::updateScatterPlotWidgetColorMapRange); - connect(&_scatterplotPlugin->getScatterplotWidget(), &ScatterplotWidget::coloringModeChanged, this, &ColoringAction::updateColorMapActionsReadOnly); - connect(&_scatterplotPlugin->getScatterplotWidget(), &ScatterplotWidget::renderModeChanged, this, &ColoringAction::updateColorMapActionsReadOnly); - const auto updateReadOnly = [this]() { setEnabled(_scatterplotPlugin->getPositionDataset().isValid() && _scatterplotPlugin->getScatterplotWidget().getRenderMode() == ScatterplotWidget::SCATTERPLOT); }; From b402206320ec49c11140922dbf4e5326e0a91609 Mon Sep 17 00:00:00 2001 From: Alexander Vieth Date: Mon, 27 Oct 2025 14:59:44 +0100 Subject: [PATCH 7/7] Clean-up: use local variable instead of repeated function call --- src/ColoringAction.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ColoringAction.cpp b/src/ColoringAction.cpp index 7c52fed..c3fca1f 100644 --- a/src/ColoringAction.cpp +++ b/src/ColoringAction.cpp @@ -281,7 +281,8 @@ void ColoringAction::updateScatterplotWidgetColorMap() { case ScatterplotWidget::SCATTERPLOT: { - if (_colorByAction.getCurrentIndex() == 0) { + const int32_t currentIndex = _colorByAction.getCurrentIndex(); + if (currentIndex == 0) { QPixmap colorPixmap(1, 1); colorPixmap.fill(_constantColorAction.getColor()); @@ -290,7 +291,7 @@ void ColoringAction::updateScatterplotWidgetColorMap() scatterplotWidget.setScalarEffect(PointEffect::Color); scatterplotWidget.setColoringMode(ScatterplotWidget::ColoringMode::Constant); } - else if (_colorByAction.getCurrentIndex() == 1) { + else if (currentIndex == 1) { scatterplotWidget.setColorMap(_colorMap2DAction.getColorMapImage()); scatterplotWidget.setScalarEffect(PointEffect::Color2D); scatterplotWidget.setColoringMode(ScatterplotWidget::ColoringMode::Scatter);