Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
223 changes: 201 additions & 22 deletions foucaultview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@
connect(this, &QWidget::customContextMenuRequested, this,
&foucaultView::showContextMenu);
setContextMenuPolicy(Qt::CustomContextMenu);

// Load Grid Settings with the "ronchiGrid" key
m_gridMode = static_cast<GridMode>(set.value("ronchiGrid/mode", (int)GridMode::None).toInt());
m_gridSpacing = set.value("ronchiGrid/spacing", 10.0).toDouble();
m_gridLineWidth = set.value("ronchiGrid/lineWidth", 1).toInt();
m_showUnitLabels = set.value("ronchiGrid/showLabels", true).toBool();

// Using name() and string check for robust color persistence
m_gridColor = QColor(set.value("ronchiGrid/color", "#00FFFF").toString()); // Default Cyan
m_textColor = QColor(set.value("ronchiGrid/textColor", "#FFFFFF").toString()); // Default White
}


Expand Down Expand Up @@ -78,10 +88,176 @@
connect (showAllRonchi, &QAction::triggered,this, &foucaultView::showSelectedRonchiImages);
myMenu.addAction(showAllRonchi);

QAction *showGrid = new QAction("Show Circular Grid");
connect(showGrid, &QAction::triggered, this, &foucaultView::showGrid);
myMenu.addSeparator();
myMenu.addAction(showGrid);

// Show context menu at handling position
myMenu.exec(globalPos);
}

void foucaultView::showGrid() {
QDialog dlg(this);
dlg.setWindowTitle("Ronchi Grid Settings");
QFormLayout form(&dlg);

// Create UI Elements
QComboBox *unitCombo = new QComboBox(&dlg);
unitCombo->addItems({"None", "Inches", "Millimeters", "Percentage"});
unitCombo->setCurrentIndex((int)m_gridMode);

QDoubleSpinBox *spacingSpin = new QDoubleSpinBox(&dlg);
spacingSpin->setRange(0.01, 1000.0);
spacingSpin->setValue(m_gridSpacing);

QSpinBox *widthSpin = new QSpinBox(&dlg);
widthSpin->setRange(1, 10);
widthSpin->setValue(m_gridLineWidth);

QCheckBox *textToggle = new QCheckBox("Show unit labels", &dlg);
textToggle->setChecked(m_showUnitLabels);

// Color storage (using local temps for the dialog session)
struct State { QColor grid; QColor text; } colors = { m_gridColor, m_textColor };

QPushButton *btnGridCol = new QPushButton("Grid Color");
QPushButton *btnTextCol = new QPushButton("Text Color");

connect(btnGridCol, &QPushButton::clicked, [&]() {

Check warning on line 127 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

Pass a context object as 3rd connect parameter [-Wclazy-connect-3arg-lambda]

Check warning on line 127 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

Pass a context object as 3rd connect parameter [-Wclazy-connect-3arg-lambda]
QColor c = QColorDialog::getColor(colors.grid, this);

Check warning on line 128 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]

Check warning on line 128 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]
if (c.isValid()) colors.grid = c;
});
connect(btnTextCol, &QPushButton::clicked, [&]() {

Check warning on line 131 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

Pass a context object as 3rd connect parameter [-Wclazy-connect-3arg-lambda]

Check warning on line 131 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

Pass a context object as 3rd connect parameter [-Wclazy-connect-3arg-lambda]
QColor c = QColorDialog::getColor(colors.text, this);

Check warning on line 132 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]

Check warning on line 132 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]
if (c.isValid()) colors.text = c;
});

form.addRow("Grid Units:", unitCombo);
form.addRow("Spacing Value:", spacingSpin);
form.addRow("Line Width (px):", widthSpin);
form.addRow("Labels:", textToggle);
form.addRow("Grid Color:", btnGridCol);
form.addRow("Text Color:", btnTextCol);

// Button Box with Reset
QDialogButtonBox buttonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, Qt::Horizontal, &dlg);
QPushButton *resetBtn = buttonBox.addButton("Reset to Defaults", QDialogButtonBox::ResetRole);
form.addRow(&buttonBox);

// Reset Logic
connect(resetBtn, &QPushButton::clicked, [&]() {

Check warning on line 149 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

Pass a context object as 3rd connect parameter [-Wclazy-connect-3arg-lambda]

Check warning on line 149 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

Pass a context object as 3rd connect parameter [-Wclazy-connect-3arg-lambda]
unitCombo->setCurrentIndex(0); // None

Check warning on line 150 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]

Check warning on line 150 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]
spacingSpin->setValue(10.0);

Check warning on line 151 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]

Check warning on line 151 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]
widthSpin->setValue(1);

Check warning on line 152 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]

Check warning on line 152 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]
textToggle->setChecked(true);

Check warning on line 153 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]

Check warning on line 153 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]
colors.grid = Qt::cyan;

Check warning on line 154 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]

Check warning on line 154 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy (ubuntu-24.04)

captured local variable by reference might go out of scope before lambda is called [-Wclazy-lambda-in-connect]
colors.text = Qt::white;
});

connect(&buttonBox, &QDialogButtonBox::accepted, &dlg, &QDialog::accept);
connect(&buttonBox, &QDialogButtonBox::rejected, &dlg, &QDialog::reject);

if (dlg.exec() == QDialog::Accepted) {
m_gridMode = (GridMode)unitCombo->currentIndex();
m_gridSpacing = spacingSpin->value();
m_gridLineWidth = widthSpin->value();
m_showUnitLabels = textToggle->isChecked();
m_gridColor = colors.grid;
m_textColor = colors.text;

// Save to QSettings with "ronchiGrid" prefix
QSettings set;
set.setValue("ronchiGrid/mode", (int)m_gridMode);
set.setValue("ronchiGrid/spacing", m_gridSpacing);
set.setValue("ronchiGrid/lineWidth", m_gridLineWidth);
set.setValue("ronchiGrid/showLabels", m_showUnitLabels);
set.setValue("ronchiGrid/color", m_gridColor.name());
set.setValue("ronchiGrid/textColor", m_textColor.name());

on_makePb_clicked();
}
}

void foucaultView::drawGridOverlay(QImage &img) {
if (m_gridMode == GridMode::None || !m_wf || m_gridSpacing <= 0) return;

QPainter painter(&img);
painter.setRenderHint(QPainter::Antialiasing);
painter.setRenderHint(QPainter::TextAntialiasing);

// 1. Grid Pen (Using your saved settings)
QPen gridPen(m_gridColor, m_gridLineWidth, Qt::DotLine);
painter.setPen(gridPen);

// 2. Dynamic Font Scaling
// Scales between 10pt and 16pt based on image height to stay proportional
int dynamicSize = qBound(10, img.height() / 33, 16);
QFont font("Arial", dynamicSize, QFont::Bold);
painter.setFont(font);

int w = img.width();
int h = img.height();
int centerX = w / 2;
int centerY = h / 2;
int maxPixelRadius = w / 2;

mirrorDlg *md = mirrorDlg::get_Instance();
double mirrorRadiusMM = md->diameter / 2.0;

// 3. Determine physics-to-pixel scale
double stepSizeMM = 0;
if (m_gridMode == GridMode::Millimeters) stepSizeMM = m_gridSpacing;
else if (m_gridMode == GridMode::Inches) stepSizeMM = m_gridSpacing * 25.4;
else if (m_gridMode == GridMode::Percentage) stepSizeMM = (m_gridSpacing / 100.0) * mirrorRadiusMM;

if (stepSizeMM <= 0) return;

// 4. Draw Rings and Labels
for (double currentMM = stepSizeMM; currentMM <= mirrorRadiusMM; currentMM += stepSizeMM) {

Check warning on line 217 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux (ubuntu-24.04)

foucaultview.cpp:217:5 [clang-analyzer-security.FloatLoopCounter]

Variable 'currentMM' with floating point type 'double' should not be used as a loop counter

Check warning on line 217 in foucaultview.cpp

View workflow job for this annotation

GitHub Actions / build-linux (ubuntu-24.04)

foucaultview.cpp:217:5 [clang-analyzer-security.FloatLoopCounter]

Variable 'currentMM' with floating point type 'double' should not be used as a loop counter

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy diagnostic

foucaultview.cpp:217:5: warning: [clang-analyzer-security.FloatLoopCounter]

Variable 'currentMM' with floating point type 'double' should not be used as a loop counter

  217 |     for (double currentMM = stepSizeMM; currentMM <= mirrorRadiusMM; currentMM += stepSizeMM) {
      |     ^                                   ~~~~~~~~~                    ~~~~~~~~~
foucaultview.cpp:217:5: note: Variable 'currentMM' with floating point type 'double' should not be used as a loop counter
  217 |     for (double currentMM = stepSizeMM; currentMM <= mirrorRadiusMM; currentMM += stepSizeMM) {
      |     ^                                   ~~~~~~~~~                    ~~~~~~~~~

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should leave this code as is. I thought I'd rewrite it as an integer loop like this:

for (int rPx = stepSizeMM / mirrorRadiusMM * maxPixelRadius; rPx <= maxPixelRadius; rPx += stepSizeMM / mirrorRadiusMM * maxPixelRadius) {
    double currentMM = rPx * mirrorRadiusMM / maxPixelRadius;

And we could do the above replacement and it would work fine. I just swapped the 2 variables that are looping through (rPx and mirrorRadiusMM) and fixed the scaling for it to work the other way around. But I think it's clearer the first way and it's fine in this case. If you do % and set the stepping to 10% it will do 10% to 100% circles. There is a tiny chance it will miss the 100% circle. But I tried the existing program and it worked fine. Plus it's no big deal if the 100% circle isn't printed. People know where 100% is (it's the outer edge of the ronchi/foucault).

And for other cases such as stepping by 10mm or stepping by 1 inch etc, it's unlikely to ever end up exactly at the outer edge anyway.

Also my suggested code now has a small bug because we have rounded (or "floored") the step size to the nearest integer so it will understep in many cases. So really it's buggier if we use an integer in the loop.

So @atsju, is there some comment we can add to this line of code to tell it not to worry about a double as a loop counter?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, I'm leaning to agree with clang on that other complaint of a double in a loop, the one where it went from -1 to 1. I'm going to look at that code probably in the next few days. But in this case I think the double is the better way to do this code.

int rPx = (int)((currentMM / mirrorRadiusMM) * maxPixelRadius);

painter.setPen(gridPen);
painter.drawEllipse(QPoint(centerX, centerY), rPx, rPx);

if (m_showUnitLabels) {
QString label;
if (m_gridMode == GridMode::Millimeters)
label = QString::number(currentMM, 'f', 1);
else if (m_gridMode == GridMode::Inches)
label = QString::number(currentMM / 25.4, 'f', 2);
else
label = QString::number((currentMM / mirrorRadiusMM) * 100.0, 'f', 0) + "%";

// Determine initial Y position at the top of the ring
int yPos = centerY - rPx;

// EDGE CLIPPING FIX:
// If the label is too close to the top edge (within 30 pixels),
// shift it down so the text box doesn't get cut off.
if (yPos < 30) {
yPos = 30;
}

// Define a wide bounding box for the text
// Centered on X, and centered vertically on our adjusted yPos
QRect textRect(centerX - 60, yPos - 15, 120, 30);

// Draw shadow for legibility (Black offset)
painter.setPen(Qt::black);
painter.drawText(textRect.translated(1, 1), Qt::AlignCenter, label);

// Draw actual colored text from your settings
painter.setPen(m_textColor);
painter.drawText(textRect, Qt::AlignCenter, label);
}
}

// 5. Center Crosshair
painter.setPen(QPen(m_gridColor, 1, Qt::SolidLine));
painter.drawLine(centerX - 12, centerY, centerX + 12, centerY);
painter.drawLine(centerX, centerY - 12, centerX, centerY + 12);
}
void foucaultView::showSelectedRonchiImages(){

surfaceAnalysisTools *saTools = surfaceAnalysisTools::get_Instance();
Expand Down Expand Up @@ -312,8 +488,12 @@
if (img.isNull()) return;

QSize s = label->size();
QPixmap pix = QPixmap::fromImage(img.scaledToWidth(s.width()));
QImage displayImg = img.scaled(s, Qt::KeepAspectRatio, Qt::SmoothTransformation);

// Overlay grid
drawGridOverlay(displayImg);

QPixmap pix = QPixmap::fromImage(displayImg);
QPainter painter(&pix);
painter.save();
painter.setPen(QPen(QColor(Qt::white)));
Expand Down Expand Up @@ -375,21 +555,19 @@

int headerHeight = 70;
int textBuffer = 40;

int cellW = imgDim;
int cellH = imgDim + textBuffer;

// Total Canvas size
QImage canvas(cellW * cols, (cellH * rows) + headerHeight, QImage::Format_RGB32);
canvas.fill(Qt::black);

QPainter painter(&canvas);
painter.setRenderHint(QPainter::Antialiasing);
QApplication::setOverrideCursor(Qt::WaitCursor);

// NEW: Container for individual Ronchi images to be used in comparison
QList<QImage> individualRonchis;
QList<QString> names;

// 5. Draw Simulation Header
painter.setPen(Qt::white);
painter.setFont(QFont("Arial", 12, QFont::Bold));
Expand All @@ -410,20 +588,30 @@
int row = i / cols;
int col = i % cols;

// Generate the raw Ronchi image
QImage ronchi = generateOpticalTestImage(OpticalTestType::Ronchi, currentWf, s, ui->autocollimation->isChecked());

if (!ronchi.isNull()) {
// Store a copy for the comparison feature
// STORE RAW: Add to list for the Compare Dialog (Prevents crash & artifacting)
individualRonchis.append(ronchi);

// STORE NAME: Essential for the Compare Dialog to avoid out-of-bounds crash
QFileInfo fileInfo(currentWf->name);
QString displayName = fileInfo.baseName();
names.append(displayName);

// GRID OVERLAY: Apply only to a copy for the batch preview canvas
QImage displayCopy = ronchi;
if (m_gridMode != GridMode::None) {
drawGridOverlay(displayCopy);
}

int xPos = col * cellW;
int yPos = headerHeight + (row * cellH);

painter.drawImage(xPos, yPos, ronchi);
painter.drawImage(xPos, yPos, displayCopy);

QFileInfo fileInfo(currentWf->name);
QString displayName = fileInfo.baseName();
names << displayName;
// Draw Label on Canvas
int textWidth = fm.horizontalAdvance(displayName);
int xText = xPos + (cellW - textWidth) / 2;
int yText = yPos + imgDim + (textBuffer / 2) + (fm.ascent() / 2);
Expand All @@ -448,14 +636,13 @@
QScrollArea *scroll = new QScrollArea(&previewDlg);
scroll->setWidgetResizable(true);
scroll->setAlignment(Qt::AlignCenter);
//scroll->setStyleSheet("background-color: #1a1a1a;");

QLabel *imgLabel = new QLabel();
imgLabel->setAlignment(Qt::AlignCenter);
scroll->setWidget(imgLabel);
layout->addWidget(scroll);

// 8. Zoom Slider Integration
// 8. Zoom Slider
QPixmap previewPixmap = QPixmap::fromImage(canvas);
QHBoxLayout *zoomLayout = new QHBoxLayout();
QSlider *slider = new QSlider(Qt::Horizontal);
Expand All @@ -479,13 +666,10 @@
connect(slider, &QSlider::valueChanged, updateZoom);
updateZoom(100);

// 9. Navigation and Comparison Buttons
// 9. Buttons
QHBoxLayout *btns = new QHBoxLayout();

// NEW: Comparison button
QPushButton *compareBtn = new QPushButton(tr("Compare Top Two Patterns"));
compareBtn->setIcon(style()->standardIcon(QStyle::SP_BrowserReload));
// Feature only enabled if 2 or more wavefronts were processed
compareBtn->setEnabled(individualRonchis.size() >= 2);

QPushButton *saveBtn = new QPushButton(tr("Save Grid Image"));
Expand All @@ -497,23 +681,18 @@
btns->addWidget(cancelBtn);
layout->addLayout(btns);

// Connect the comparison trigger
// Comparison Trigger (Uses captured lists)
connect(compareBtn, &QPushButton::clicked, &previewDlg, [=, &previewDlg]() {
// [=] copies individualRonchis and names so they stay
// valid even after generateBatchRonchiImage() returns.
if (individualRonchis.size() >= 2) {
if (individualRonchis.size() >= 2 && names.size() >= 2) {
RonchiCompareDialog compDlg(individualRonchis[0], names[0],
individualRonchis[1], names[1], &previewDlg);
compDlg.exec();
}
});


connect(saveBtn, &QPushButton::clicked, &previewDlg, &QDialog::accept);
connect(cancelBtn, &QPushButton::clicked, &previewDlg, &QDialog::reject);


// 10. Execute Dialog and Save Grid
if (previewDlg.exec() == QDialog::Accepted) {
QString path = QFileDialog::getSaveFileName(this, tr("Save Ronchi Grid"),
imageDir, tr("Images (*.png *.jpg)"));
Expand Down
11 changes: 11 additions & 0 deletions foucaultview.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ enum class OpticalTestType {
Ronchi
};

enum class GridMode { None, Inches, Millimeters, Percentage };


// Parameter container so this can be called without needing the UI pointers
struct OpticalTestSettings {
double rocOffset = 0.0;
Expand Down Expand Up @@ -81,6 +84,7 @@ private slots:
void on_SaveImageCB_clicked(bool checked);
void saveRonchiImage();
void saveFoucaultImage();
void showGrid();
void on_overlayProfile_stateChanged(int arg1);
void on_RonchiX_valueChanged(double arg1);
void on_pushButton_clicked();
Expand All @@ -99,7 +103,14 @@ private slots:
double m_temp_sag;
wavefront *m_wf;
int lateralOffset;
GridMode m_gridMode = GridMode::None;
double m_gridSpacing = 10.0; // Default spacing value
int m_gridLineWidth = 1;
bool m_showUnitLabels = true;
QColor m_gridColor = Qt::cyan;
QColor m_textColor = Qt::white;

void drawGridOverlay(QImage &img);
explicit foucaultView(QWidget *parent = 0, SurfaceManager *sm = 0);
cv::Mat compute_star_test(int pupil_size, double defocus, double pad, bool use_OPD);
double getStep();
Expand Down
Loading