From 9cf987cd4e7d6663a64ef22300d5e2ee36d473ec Mon Sep 17 00:00:00 2001 From: Filip Leonarski Date: Mon, 12 Oct 2026 19:46:48 +0200 Subject: [PATCH] jfjoch_viewer: live updates no longer rebuild open combo boxes During a collection every dataset tick ran JFJochViewerDatasetInfo::UpdateLabels(), which clear()ed the plot-type combo and re-added every item, so an open dropdown lost its hovered row and scroll position on each tick and picking a plot type was a race against the next update. The same clear-and-refill ran per tick or per image in the calibration window's calibration combo, the ROI list combo and the powder calibrant combo; the last two also threw away the user's choice (the calibrant was reset to the first entry on every image). New LiveComboBox::SetItems() takes the full item list and leaves the combo untouched when the list is unchanged. When it does change while the popup is open, the change is held back and applied (queued) once the popup window hides, so a click that closes the popup lands first. A replacement keeps the selection by item data, blocks signals, and a held-back one emits itemsReplaced() so the owner can re-plot. The calibration window now re-shows the selected calibration on a dataset tick instead of always reloading the pixel mask over it. Tested offscreen: a timer fires datasetLoaded() every 25 ms while the dropdown is open; the popup and hovered row survive, newly available indexing metrics are held back until the popup closes, and the picked type sticks and is what the chart plots. The rc175 code fails the hover checks of the same harness. Co-Authored-By: Claude Opus 5.5 (1M context) --- viewer/CMakeLists.txt | 2 + viewer/JFJochViewerDatasetInfo.cpp | 81 ++++++++++++---------- viewer/JFJochViewerDatasetInfo.h | 3 +- viewer/widgets/JFJochViewerROIList.cpp | 18 +++-- viewer/widgets/JFJochViewerROIList.h | 3 +- viewer/widgets/LiveComboBox.cpp | 71 +++++++++++++++++++ viewer/widgets/LiveComboBox.h | 43 ++++++++++++ viewer/widgets/PowderCalibrationWidget.cpp | 28 ++++---- viewer/widgets/PowderCalibrationWidget.h | 3 +- viewer/windows/JFJochCalibrationWindow.cpp | 21 +++--- viewer/windows/JFJochCalibrationWindow.h | 3 +- 11 files changed, 204 insertions(+), 72 deletions(-) create mode 100644 viewer/widgets/LiveComboBox.cpp create mode 100644 viewer/widgets/LiveComboBox.h diff --git a/viewer/CMakeLists.txt b/viewer/CMakeLists.txt index cc5ef09b8..e11938c98 100644 --- a/viewer/CMakeLists.txt +++ b/viewer/CMakeLists.txt @@ -56,6 +56,8 @@ ADD_EXECUTABLE(jfjoch_viewer jfjoch_viewer.cpp JFJochViewerWindow.cpp JFJochView windows/JFJochViewerImageListWindow.h widgets/NumericComboBox.cpp widgets/NumericComboBox.h + widgets/LiveComboBox.cpp + widgets/LiveComboBox.h windows/JFJochViewerMetadataWindow.cpp windows/JFJochViewerMetadataWindow.h windows/JFJochMergeStatsWindow.cpp diff --git a/viewer/JFJochViewerDatasetInfo.cpp b/viewer/JFJochViewerDatasetInfo.cpp index dc5757683..4ffed3d36 100644 --- a/viewer/JFJochViewerDatasetInfo.cpp +++ b/viewer/JFJochViewerDatasetInfo.cpp @@ -40,7 +40,7 @@ namespace { JFJochViewerDatasetInfo::JFJochViewerDatasetInfo(QWidget *parent) : QWidget(parent) { auto layout = new QGridLayout(this); - combo_box = new QComboBox(this); + combo_box = new LiveComboBox(this); layout->addWidget(combo_box, 0, 0); @@ -117,6 +117,16 @@ JFJochViewerDatasetInfo::JFJochViewerDatasetInfo(QWidget *parent) : QWidget(pare SaveFavourite(); }); + // Items held back while the dropdown was open went in once it closed: re-apply the selection + // rules and re-plot, as after any other rebuild. + connect(combo_box, &LiveComboBox::itemsReplaced, this, [this] { + RestoreSelection(); + if (combo_is_image_mode_) + comboBoxSelected(combo_box->currentIndex()); + else + UpdatePlot(); + }); + connect(grid_scan_image, &JFJochGridScanImage::imageSelected, this, &JFJochViewerDatasetInfo::imageSelectedInChart); @@ -249,32 +259,32 @@ void JFJochViewerDatasetInfo::changeEvent(QEvent *event) { } void JFJochViewerDatasetInfo::UpdateLabels() { - QSignalBlocker bl(combo_box); - if (combo_box->count() > 0) (combo_is_image_mode_ ? last_image_selection : last_selection) = combo_box->currentData().toInt(); - combo_box->clear(); combo_is_image_mode_ = image_button && image_button->isChecked(); + // Built in full and handed over as one list: the combo only changes when the list does, so the + // live updates that call this on every tick leave an open dropdown and the selection alone. + QVector items; + auto add = [&items](const QString &text, int code) { items.push_back({text, code}); }; + auto separator = [&items] { items.push_back({.separator = true}); }; + // In per-image mode the one combo selects which per-image profile to show (no second combo). if (combo_is_image_mode_) { for (const auto &pt : JFJochViewerSidePanelChart::PlotTypes()) - combo_box->addItem(pt.first, pt.second); - return; - } - - if (this->dataset) { - combo_box->addItem("Background estimate", 0); + add(pt.first, pt.second); + } else if (this->dataset) { + add("Background estimate", 0); if (!dataset->experiment.GetGridScan().has_value()) - combo_box->addItem("Spots + background", SPOTS_PLUS_BACKGROUND); - combo_box->addItem("Ice ring score", 14); - combo_box->addItem("Spindle blind fraction", 15); - combo_box->addItem("Resolution estimate", 7); - combo_box->addItem("Spot count", 1); - combo_box->addItem("Spot count (indexed)", 2); - combo_box->addItem("Spot count (ice rings)", 3); - combo_box->addItem("Spot count (low res.)", 4); + add("Spots + background", SPOTS_PLUS_BACKGROUND); + add("Ice ring score", 14); + add("Spindle blind fraction", 15); + add("Resolution estimate", 7); + add("Spot count", 1); + add("Spot count (indexed)", 2); + add("Spot count (ice rings)", 3); + add("Spot count (low res.)", 4); // Offer indexing/scaling metrics if the active run, any overlay run, or the live run has them. bool has_indexing = !dataset->indexing_result.empty(); @@ -290,31 +300,30 @@ void JFJochViewerDatasetInfo::UpdateLabels() { } if (has_indexing) { - combo_box->insertSeparator(1000); - combo_box->addItem("Indexing result", 5); - combo_box->addItem("Profile radius", 6); - combo_box->addItem("B-factor", 8); - combo_box->addItem("Mosaicity", 9); - combo_box->addItem("Integrated reflections", 12); - combo_box->addItem("Indexing lattice count", 13); + separator(); + add("Indexing result", 5); + add("Profile radius", 6); + add("B-factor", 8); + add("Mosaicity", 9); + add("Integrated reflections", 12); + add("Indexing lattice count", 13); } if (has_scale) { - combo_box->insertSeparator(1000); - combo_box->addItem("Scale factor", 10); - combo_box->addItem("CC", 11); + separator(); + add("Scale factor", 10); + add("CC", 11); } for (int i = 0; i < this->dataset->roi.size(); i++) { - std::string name = std::string("ROI ") + this->dataset->roi[i]; - combo_box->insertSeparator(1000); - combo_box->addItem(QString::fromStdString(name + " mean"), 100 + i * 4); - combo_box->addItem(QString::fromStdString(name + " sum"), 100 + i * 4 + 1); - combo_box->addItem(QString::fromStdString(name + " weighted x"), 100 + i * 4 + 2); - combo_box->addItem(QString::fromStdString(name + " weighted y"), 100 + i * 4 + 3); + const QString name = QString::fromStdString("ROI " + this->dataset->roi[i]); + separator(); + add(name + " mean", 100 + i * 4); + add(name + " sum", 100 + i * 4 + 1); + add(name + " weighted x", 100 + i * 4 + 2); + add(name + " weighted y", 100 + i * 4 + 3); } - } else { - combo_box->clear(); } + combo_box->SetItems(items); } diff --git a/viewer/JFJochViewerDatasetInfo.h b/viewer/JFJochViewerDatasetInfo.h index 0397aeeaf..3cedce864 100644 --- a/viewer/JFJochViewerDatasetInfo.h +++ b/viewer/JFJochViewerDatasetInfo.h @@ -14,10 +14,11 @@ #include "image_viewer/JFJochGridScanImage.h" #include "widgets/JFJochViewerSidePanelChart.h" #include "RunData.h" +#include "widgets/LiveComboBox.h" class JFJochViewerDatasetInfo : public QWidget { Q_OBJECT - QComboBox *combo_box; + LiveComboBox *combo_box; JFJochDatasetInfoChartView *chart_view; // Second half-height plot under the first, shown only for "Spots + background": the spot // count and the background each get their own compact plot instead of sharing twin axes. diff --git a/viewer/widgets/JFJochViewerROIList.cpp b/viewer/widgets/JFJochViewerROIList.cpp index 78800a731..ab4a701b3 100644 --- a/viewer/widgets/JFJochViewerROIList.cpp +++ b/viewer/widgets/JFJochViewerROIList.cpp @@ -14,15 +14,14 @@ #include #include #include -#include -#include JFJochViewerROIList::JFJochViewerROIList(QWidget *parent) : QWidget(parent) { auto *layout = new QVBoxLayout(this); - combo_ = new QComboBox(this); + combo_ = new LiveComboBox(this); layout->addWidget(combo_); connect(combo_, &QComboBox::currentIndexChanged, this, &JFJochViewerROIList::OnSelectionChanged); + connect(combo_, &LiveComboBox::itemsReplaced, this, &JFJochViewerROIList::OnSelectionChanged); auto *buttons = new QHBoxLayout(); type_combo_ = new QComboBox(this); @@ -154,22 +153,21 @@ void JFJochViewerROIList::loadImage(std::shared_ptr ima void JFJochViewerROIList::RebuildCombo(const QString &keep_selected) { rebuilding_ = true; - combo_->clear(); // Running index across boxes/circles/azimuthal - matches the enumeration order used by the // image overlay, so each item's swatch colour equals its annotation colour on the image. - int color_index = 0; + // Called on every image of a live acquisition; the combo only changes when the list does. + QVector items; auto add_item = [&](const std::string &name, const char *type) { // display "name · type", with the bare name kept as item data for lookups - QPixmap swatch(14, 14); - swatch.fill(ROIAnnotationColor(color_index++)); - combo_->addItem(QIcon(swatch), - QString("%1 · %2").arg(QString::fromStdString(name), type), - QString::fromStdString(name)); + items.push_back({QString("%1 · %2").arg(QString::fromStdString(name), type), + QString::fromStdString(name), QString(), + ROIAnnotationColor(static_cast(items.size()))}); }; for (const auto &b : rois_.boxes) add_item(b.GetName(), "box"); for (const auto &c : rois_.circles) add_item(c.GetName(), "circle"); for (const auto &a : rois_.azimuthal) add_item(a.GetName(), "azim"); + combo_->SetItems(items); if (!keep_selected.isEmpty()) { const int idx = combo_->findData(keep_selected); diff --git a/viewer/widgets/JFJochViewerROIList.h b/viewer/widgets/JFJochViewerROIList.h index c1138b13f..ecbbc4bbb 100644 --- a/viewer/widgets/JFJochViewerROIList.h +++ b/viewer/widgets/JFJochViewerROIList.h @@ -13,6 +13,7 @@ #include "JFJochViewerROIResult.h" #include "../../common/ROIDefinition.h" #include "../../reader/JFJochReaderImage.h" +#include "LiveComboBox.h" // Manages the dataset's ROI list: select one, add (box/circle/azimuthal), rename or // delete, and show the selected ROI's statistics for the current image. Edits are @@ -21,7 +22,7 @@ class JFJochViewerROIList : public QWidget { Q_OBJECT - QComboBox *combo_; + LiveComboBox *combo_; QComboBox *type_combo_; QLabel *flabel_[4]; QLineEdit *fedit_[4]; diff --git a/viewer/widgets/LiveComboBox.cpp b/viewer/widgets/LiveComboBox.cpp new file mode 100644 index 000000000..9884bb539 --- /dev/null +++ b/viewer/widgets/LiveComboBox.cpp @@ -0,0 +1,71 @@ +// SPDX-FileCopyrightText: 2026 Filip Leonarski, Paul Scherrer Institute +// SPDX-License-Identifier: GPL-3.0-only + +#include "LiveComboBox.h" + +#include +#include +#include +#include +#include +#include + +LiveComboBox::LiveComboBox(QWidget *parent) : QComboBox(parent) { + // The popup is a separate window holding the view. Not every way of closing it goes through + // hidePopup() (a click outside just hides the window), so watch the window itself. + view()->window()->installEventFilter(this); +} + +bool LiveComboBox::SetItems(const QVector &items) { + if (view()->isVisible()) { + pending_ = items; + return false; + } + pending_.reset(); + if (items == items_) + return false; + Replace(items); + return true; +} + +void LiveComboBox::Replace(const QVector &items) { + QSignalBlocker blocker(this); + const QVariant selected = currentData(); + clear(); + for (const auto &item : items) { + if (item.separator) { + insertSeparator(count()); + continue; + } + if (item.swatch.isValid()) { + QPixmap pixmap(14, 14); + pixmap.fill(item.swatch); + addItem(QIcon(pixmap), item.text, item.data); + } else + addItem(item.text, item.data); + if (!item.tooltip.isEmpty()) + setItemData(count() - 1, item.tooltip, Qt::ToolTipRole); + } + items_ = items; + const int index = selected.isValid() ? findData(selected) : -1; + setCurrentIndex(index >= 0 ? index : (count() > 0 ? 0 : -1)); +} + +void LiveComboBox::ApplyPending() { + if (!pending_ || view()->isVisible()) + return; + const QVector items = std::move(*pending_); + pending_.reset(); + if (items == items_) + return; + Replace(items); + emit itemsReplaced(); +} + +bool LiveComboBox::eventFilter(QObject *watched, QEvent *event) { + // Queued: a click on an item hides the popup first and selects the item afterwards, so the + // held-back items go in once the pick has landed, and keep it. + if (event->type() == QEvent::Hide && pending_) + QTimer::singleShot(0, this, &LiveComboBox::ApplyPending); + return QComboBox::eventFilter(watched, event); +} diff --git a/viewer/widgets/LiveComboBox.h b/viewer/widgets/LiveComboBox.h new file mode 100644 index 000000000..df203e300 --- /dev/null +++ b/viewer/widgets/LiveComboBox.h @@ -0,0 +1,43 @@ +// SPDX-FileCopyrightText: 2026 Filip Leonarski, Paul Scherrer Institute +// SPDX-License-Identifier: GPL-3.0-only + +#pragma once + +#include +#include +#include + +#include + +// A combo box whose items are refreshed by live updates. SetItems() leaves the items alone when +// they are unchanged, and while the popup is open it holds the change back until the popup has +// closed, so a live update never closes the dropdown or moves the selection under the user. +// Replacing the items keeps the selection by item data (the first item when it is gone) and emits +// no currentIndexChanged; a replacement held back over an open popup emits itemsReplaced() instead. +class LiveComboBox : public QComboBox { + Q_OBJECT +public: + struct Item { + QString text; + QVariant data; + QString tooltip; + QColor swatch; // shown as a small square icon when valid + bool separator = false; + bool operator==(const Item &other) const = default; + }; + + explicit LiveComboBox(QWidget *parent = nullptr); + // True if the items were replaced now; false if unchanged or held back for an open popup. + bool SetItems(const QVector &items); + +signals: + void itemsReplaced(); + +private: + QVector items_; + std::optional> pending_; + + void Replace(const QVector &items); + void ApplyPending(); + bool eventFilter(QObject *watched, QEvent *event) override; +}; diff --git a/viewer/widgets/PowderCalibrationWidget.cpp b/viewer/widgets/PowderCalibrationWidget.cpp index d40f4aeec..3acbe83d0 100644 --- a/viewer/widgets/PowderCalibrationWidget.cpp +++ b/viewer/widgets/PowderCalibrationWidget.cpp @@ -13,7 +13,7 @@ PowderCalibrationWidget::PowderCalibrationWidget(QWidget *parent) : QWidget(parent) { auto layout = new QVBoxLayout(this); - calibrantCombo = new QComboBox(this); + calibrantCombo = new LiveComboBox(this); // Without this a combo asks to be as wide as its longest entry - and it is never allowed to // shrink below that, which is what used to pin the whole settings dock wide as soon as the // powder section was expanded. The current text elides instead; the tooltip carries the rest. @@ -107,24 +107,24 @@ CalibrationSelection PowderCalibrationWidget::Selection() const { } void PowderCalibrationWidget::updateCalibrantList() { - calibrantCombo->clear(); - + // Called for every image, live ones included: the combo only changes when the list does, and + // keeps the user's calibrant by name. + QVector items; for (const auto &c : Calibrants()) - calibrantCombo->addItem(QString::fromStdString(c.name)); + items.push_back({QString::fromStdString(c.name), QString::fromStdString(c.name)}); if (sample_cell) { // The cell goes in the tooltip, not in the entry: a 40-character item would set the combo's // minimum width and, through it, the minimum width of the dock this widget sits in. - calibrantCombo->addItem("Current sample cell"); - calibrantCombo->setItemData(calibrantCombo->count() - 1, - QString("%1 %2 %3 %4 %5 %6") - .arg(QString::number(sample_cell->a, 'f', 1)) - .arg(QString::number(sample_cell->b, 'f', 1)) - .arg(QString::number(sample_cell->c, 'f', 1)) - .arg(QString::number(sample_cell->alpha, 'f', 1)) - .arg(QString::number(sample_cell->beta, 'f', 1)) - .arg(QString::number(sample_cell->gamma, 'f', 1)), Qt::ToolTipRole); + items.push_back({"Current sample cell", "Current sample cell", + QString("%1 %2 %3 %4 %5 %6") + .arg(QString::number(sample_cell->a, 'f', 1)) + .arg(QString::number(sample_cell->b, 'f', 1)) + .arg(QString::number(sample_cell->c, 'f', 1)) + .arg(QString::number(sample_cell->alpha, 'f', 1)) + .arg(QString::number(sample_cell->beta, 'f', 1)) + .arg(QString::number(sample_cell->gamma, 'f', 1))}); } - calibrantCombo->setCurrentIndex(0); + calibrantCombo->SetItems(items); } void PowderCalibrationWidget::loadImage(std::shared_ptr image) { diff --git a/viewer/widgets/PowderCalibrationWidget.h b/viewer/widgets/PowderCalibrationWidget.h index a4be9cc23..2ec7014b4 100644 --- a/viewer/widgets/PowderCalibrationWidget.h +++ b/viewer/widgets/PowderCalibrationWidget.h @@ -11,6 +11,7 @@ #include "../../common/UnitCell.h" #include "../../reader/JFJochReaderImage.h" #include "../../image_analysis/geom_refinement/PowderCalibration.h" // CalibrationMethod +#include "LiveComboBox.h" // What a whole-dataset calibration run is set to. The analysis library takes the calibrant as a ring // list rather than a name (ice has no cell to enumerate rings from), so the name is resolved here. @@ -31,7 +32,7 @@ class PowderCalibrationWidget : public QWidget { // the group's absences, or the fit pairs the innermost OBSERVED ring with a forbidden one. std::optional sample_sg; - QComboBox* calibrantCombo{nullptr}; // stores current calibrant selection + LiveComboBox* calibrantCombo{nullptr}; // stores current calibrant selection QComboBox* methodCombo{nullptr}; // rings / spots, for the whole-dataset run QCheckBox* refineTiltCheck{nullptr};// fit the detector tilt, or hold it where it is diff --git a/viewer/windows/JFJochCalibrationWindow.cpp b/viewer/windows/JFJochCalibrationWindow.cpp index 50ded0fcb..d53fccfce 100644 --- a/viewer/windows/JFJochCalibrationWindow.cpp +++ b/viewer/windows/JFJochCalibrationWindow.cpp @@ -17,7 +17,7 @@ JFJochCalibrationWindow::JFJochCalibrationWindow(QWidget *parent) : JFJochHelper viewer = new JFJochSimpleImage(this); - calibration_option = new QComboBox(this); + calibration_option = new LiveComboBox(this); color_map_select = new QComboBox(this); color_map_select->addItem("Viridis", static_cast(ColorScaleEnum::Viridis)); @@ -61,6 +61,9 @@ JFJochCalibrationWindow::JFJochCalibrationWindow(QWidget *parent) : JFJochHelper }); connect(calibration_option, &QComboBox::currentIndexChanged, this, &JFJochCalibrationWindow::CalibrationSelected); + connect(calibration_option, &LiveComboBox::itemsReplaced, this, [this] { + CalibrationSelected(calibration_option->currentIndex()); + }); centralWidget->setLayout(grid_layout); statusBar = new QStatusBar(this); @@ -73,13 +76,15 @@ JFJochCalibrationWindow::JFJochCalibrationWindow(QWidget *parent) : JFJochHelper void JFJochCalibrationWindow::datasetLoaded(std::shared_ptr in_dataset) { dataset = std::move(in_dataset); - QSignalBlocker b(calibration_option); - calibration_option->clear(); + // Arrives on every dataset tick of a live acquisition; the combo only changes when the list + // does, and keeps the selection by name. + QVector items; if (dataset) { - calibration_option->addItem("Pixel mask"); + items.push_back({"Pixel mask", "Pixel mask"}); for (auto &calibration : dataset->calibration_data) - calibration_option->addItem(calibration.c_str()); + items.push_back({calibration.c_str(), calibration.c_str()}); } + calibration_option->SetItems(items); // Rebuilding the mask is a full-detector copy plus a full-resolution recolour on the GUI // thread, and this arrives on every dataset tick when following a live acquisition. Do not pay @@ -88,14 +93,14 @@ void JFJochCalibrationWindow::datasetLoaded(std::shared_ptrcurrentIndex()); // the user's pick, not always the mask } void JFJochCalibrationWindow::showEvent(QShowEvent *event) { JFJochHelperWindow::showEvent(event); if (pending_mask_reload) { pending_mask_reload = false; - LoadMask(); + CalibrationSelected(calibration_option->currentIndex()); } } @@ -116,7 +121,7 @@ void JFJochCalibrationWindow::LoadMask() { } void JFJochCalibrationWindow::CalibrationSelected(int val) { - if (val == 0) + if (val <= 0) LoadMask(); else emit loadCalibration(calibration_option->itemText(val)); diff --git a/viewer/windows/JFJochCalibrationWindow.h b/viewer/windows/JFJochCalibrationWindow.h index 577f86b74..334a598ac 100644 --- a/viewer/windows/JFJochCalibrationWindow.h +++ b/viewer/windows/JFJochCalibrationWindow.h @@ -5,6 +5,7 @@ #include #include +#include "../widgets/LiveComboBox.h" #include #include @@ -17,7 +18,7 @@ class JFJochCalibrationWindow : public JFJochHelperWindow { Q_OBJECT - QComboBox *calibration_option; + LiveComboBox *calibration_option; QComboBox *color_map_select; SliderPlusBox *foreground_slider; SliderPlusBox *background_slider;