Skip to content

Some code improvements #211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
atsju merged 12 commits into from
Jul 24, 2025
Merged

Some code improvements #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
12f21ad
-Wclazy-qstring-arg
atsju Jul 20, 2025
57b012e
-Wclazy-unused-non-trivial-variable, fix memleak in movie maker, no U…
atsju Jul 20, 2025
6b0be51
-Wclazy-overloaded-signal
atsju Jul 20, 2025
f5e6f35
re-enable clazy problem matcher as there are few enough warnings
atsju Jul 20, 2025
987553e
more safety and memory leaks
atsju Jul 20, 2025
8b2c890
clazy-range-loop-reference
atsju Jul 20, 2025
04b0b77
clazy-unused-non-trivial-variable
atsju Jul 20, 2025
5ca7c49
clazy-container-anti-pattern
atsju Jul 20, 2025
cc79f50
clazy-detaching-temporary
atsju Jul 20, 2025
b418b42
fix QT5 build
atsju Jul 20, 2025
38a805c
some less warnings
atsju Jul 20, 2025
1b6b16a
Wclazy-incorrect-emit
atsju Jul 21, 2025
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
2 changes: 1 addition & 1 deletion .github/workflows/build-linux-clazy.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
- run: sudo apt update
- run: sudo apt install -y clazy apt-utils build-essential wget qt5-qmake qt5-qmake-bin qt5-assistant qtbase5-dev qtmultimedia5-dev libqt5charts5 libqt5charts5-dev libqt5multimedia* libqt5datavisualization5-dev libqt5datavisualization5 libopencv-core-dev libopencv-dev libqwt-qt5-6 libqwt-qt5-dev libarmadillo-dev
- run: qmake -spec linux-clang QMAKE_CXX="clazy"
#- uses: ammaraskar/gcc-problem-matcher@master #TODO 2023/08/05 re-enable problem matcher when all warnings are fixed
- uses: ammaraskar/gcc-problem-matcher@master
# ignore noisy dirs from QT files itself
# all level 1 checks but ignore clazy-no-connect-by-name
# no parallel make with clazy to not mess log
Expand Down
3 changes: 1 addition & 2 deletions astigpolargraph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
chart->addAxis(radialAxis, QPolarChart::PolarOrientationRadial);
double maxAstig = 1.;

QVector<wavefront *> wavefronts =SurfaceManager::get_instance()->m_wavefronts;
QScreen *screen = QGuiApplication::primaryScreen();
qreal screenDPI = screen->physicalDotsPerInchX();
int pensize = 5 * screenDPI/256.; // adjust pen size to screen resolution. 256 is DPI of my 4K 17 inch laptop
Expand Down Expand Up @@ -127,7 +126,7 @@
qreal distance(INT_MAX);
int closeNdx = -1;
int ndx = 0;
for (auto sample : m_list) {
for (const auto& sample : m_list) {

Check warning on line 129 in astigpolargraph.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy 

c++11 range-loop might detach Qt container (QList) [-Wclazy-range-loop-detach]
Copy link
Collaborator

@gr5 gr5 Jul 23, 2025

Choose a reason for hiding this comment

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

Did you fix one warning only to create a different one? I don't really understand the warning.

Copy link
Collaborator Author

@atsju atsju Jul 23, 2025

Choose a reason for hiding this comment

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

correct. I fixed an actual warning where the container was detaching.
And now we have a warning about C++11 not perfectly managing it. But this will be fixed with future compiler if I remember well. So I consider this warning less harmful and more future ready than previous one.

QPointF currentPoint(sample.m_xastig, sample.m_yastig);
qreal currentDistance = qSqrt((currentPoint.x() - clickedPoint.x())
* (currentPoint.x() - clickedPoint.x())
Expand Down
2 changes: 1 addition & 1 deletion astigscatterplot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ astigScatterPlot::astigScatterPlot(QWidget *parent):QwtPlot( parent ),m_max(.3)
{

Picker = new myPlotPicker( this );
connect(Picker, SIGNAL(selected(QString)),this,SLOT(selectedwave(QString)));
connect(Picker, SIGNAL(mySelected(QString)),this,SLOT(selectedwave(QString)));

QwtPlotGrid *grid = new QwtPlotGrid();
grid->setPen( Qt::gray, 0.0, Qt::DotLine );
Expand Down
1 change: 0 additions & 1 deletion batchigramwizard.cpp
View file
Open in desktop
Copy link
Owner

@githubdoe githubdoe Jul 21, 2025

Choose a reason for hiding this comment

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

What I remember is that this emit is important to make the GUI work correctly. During Batch processing the signals need to be process differently. This enables that. So it should not be changed. Why is it removed?

Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

The compiler warning was correct; this emit has no effect.
Indeed., the connect was done after emit. But you you call correct function after so it's working as intended. That makes me think the connect and signal can be removed completely as unused. Thank you for heads up.

    batchWiz = new batchIgramWizard(m_igramsToProcess, this,Qt::Window);
    connect(batchWiz,SIGNAL(swapBathConnections(bool)),this, SLOT(batchConnections(bool)));
    batchConnections(true);

Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ batchIgramWizard::batchIgramWizard(QStringList files, QWidget *parent, Qt::Windo
layout << QWizard::Stretch << QWizard::CancelButton;
setButtonLayout(layout);
resize(800,600);
emit swapBathConnections(true);
}

batchIgramWizard::~batchIgramWizard()
Expand Down
6 changes: 2 additions & 4 deletions camwizardpage1.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ void CamWizardPage1::on_compute_clicked()
std::vector<cv::KeyPoint> keypoints;
detector.detect( dial, keypoints);

int horz = 1;
/*int horz = 1;
int vert = 1;
for (std::size_t ndx = 1; ndx < keypoints.size(); ++ndx){
float del = keypoints[ndx-1].pt.x - keypoints[ndx].pt.x;
Expand All @@ -258,7 +258,7 @@ void CamWizardPage1::on_compute_clicked()
horz = 1;
++vert;
}
}
}*/

}
}
Expand Down Expand Up @@ -462,7 +462,6 @@ void CamWizardPage1::on_pushButton_2_clicked()

void CamWizardPage1::showResuslts()
{
QStringList parms;
ui->xFv->setText(QString().number(cameraMatrix.at<double>(0,0)));
ui->yFv->setText(QString().number(cameraMatrix.at<double>(1,1)));
ui->XFc->setText(QString().number(cameraMatrix.at<double>(0,2)));
Expand All @@ -473,7 +472,6 @@ void CamWizardPage1::showResuslts()
ui->P1->setText(QString().number(distCoeffs.at<double>(0,2)));
ui->P2->setText(QString().number(distCoeffs.at<double>(0,3)));
ui->K3->setText(QString().number(distCoeffs.at<double>(0,4)));

}

void CamWizardPage1::on_currentLens_clicked()
Expand Down
11 changes: 4 additions & 7 deletions contourplot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,20 +181,17 @@ void ContourPlot::showContoursChanged(double val){
}

void ContourPlot::applyZeroOffset(bool useMiddle){
double min,max;
max = m_wf->max;
min = m_wf->min;
//double max = m_wf->max;
double min = m_wf->min;
if (useMiddle){
zOffset = 0.;
m_useMiddleOffset = true;

}
else {
zOffset = min;
m_useMiddleOffset = false;
max -= zOffset;
min -= zOffset;

//max -= zOffset;
//min -= zOffset;
}
setZRange();

Expand Down
2 changes: 0 additions & 2 deletions dftarea.cpp
View file
Open in desktop
Copy link
Owner

@githubdoe githubdoe Jul 21, 2025

Choose a reason for hiding this comment

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

Put some debug in the signal processor for that emit and see if it does get processed to confirm that assertion.

Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

I did not change the code here so I did not introduce new problems. Only warnings that were not processed before.

You are right it needs to be checked and I will do it some day.

Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@
QSettings set;
m_center_filter = set.value("DFT Center Filter", 10).toDouble();
qDebug() << "init center" << m_center_filter;
emit updateFilterSize(m_center_filter);

Check warning on line 143 in dftarea.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy 

Emitting inside constructor probably has no effect [-Wclazy-incorrect-emit]
installEventFilter(this);

/*
Expand Down Expand Up @@ -720,8 +720,6 @@
dft(imageMat,fdomMat);
imageMat.release();

double dc = fdomMat.at<double>(0,0,0);
dc/=size;
int count = 0;
bool *bp = m_mask.ptr<bool>(0);
for (int i = 0; i < size; ++i){
Expand Down
13 changes: 6 additions & 7 deletions edgeplot.cpp
View file
Open in desktop
Copy link
Owner

@githubdoe githubdoe Jul 21, 2025

Choose a reason for hiding this comment

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

Just another way of coding what was already done. This is not necessary. It does not improve readability in my opinion.

Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

It avoids unnecessary object copies because of detaching temporary.
This whole pull request is about fixing warning you never saw before (from clazy tool). I did not do it for readability.

Copy link
Collaborator

@gr5 gr5 Jul 24, 2025

Choose a reason for hiding this comment

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

My understanding could be wrong but he creates axesH and axesV and then they are destroyed (deleted) at the end of the function. The older code also calls axes() which returns an object but it never gets deleted. maybe?

Copy link
Collaborator Author

@atsju atsju Jul 25, 2025

Choose a reason for hiding this comment

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

Makes sense. I checked the internet and it does not cause memory leak. Temporary get freed correctly. It's just you copy the temporary thus double the object.
Inefficient but not dangerous.

Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,12 @@ void EdgePlot::Update(){

m_chart->createDefaultAxes();

m_chart->axes(Qt::Horizontal).first()->setRange(-1,105);

m_chart->axes(Qt::Vertical).first()->setRange(-10,10);


m_chart->axes(Qt::Horizontal).first()->setTitleText("start edge at percent of radius of mirror");
m_chart->axes(Qt::Vertical).first()->setTitleText("waves");
auto axesH = m_chart->axes(Qt::Horizontal);
axesH.first()->setRange(-1,105);
axesH.first()->setTitleText("start edge at percent of radius of mirror");
auto axesV = m_chart->axes(Qt::Vertical);
axesV.first()->setRange(-10,10);
axesV.first()->setTitleText("waves");
}
void EdgePlot::on_height_valueChanged(int value)
{
Expand Down
5 changes: 2 additions & 3 deletions intensityplot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class iSurfaceData: public QwtSyntheticPointData
int m_rad;
iSurfaceData(cv::Mat plane):
QwtSyntheticPointData( plane.cols ),
m_plane(plane), m_rad(plane.cols/2),m_angle(90)
m_plane(plane), m_rad(plane.cols/2)
{

}
Expand All @@ -73,9 +73,8 @@ class iSurfaceData: public QwtSyntheticPointData

}


private:
double m_angle;

};

bool intensityPlot::eventFilter( QObject *object, QEvent *event )
Expand Down
48 changes: 30 additions & 18 deletions mainwindow.cpp
View file
Open in desktop
Copy link
Owner

@githubdoe githubdoe Jul 21, 2025

Choose a reason for hiding this comment

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

Explain why you think an additional variable (selectedFiles) was needed.

Explain what the modifications to the code that watches the FFMPEG output does. The previous code does occasionally hang and so I know it's not exactly correct. Forgive me if you have explained the changes somewhere that I have not seen.

Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

dialog.selectedFiles().first() will detach temporary
(https://github.com/KDE/clazy/blob/master/docs/checks/README-detaching-temporary.md)

Now that I read it again and again, I think the new code does exactly that but explicitly. So there is no warning but it's not really performance improved.

I should probably have changed to loadFile(dialog.selectedFiles().constFirst()); if it works it's better. I will change in a different PR if it works

Let me explain FFMPEG changes in a different comment

Original file line number Diff line number Diff line change
Expand Up @@ -475,8 +475,8 @@ void MainWindow::on_actionLoad_Interferogram_triggered()
ui->useAnnulust->hide();
if (dialog.exec()){
if (dialog.selectedFiles().size() == 1){
loadFile(dialog.selectedFiles().first());

QStringList selectedFiles = dialog.selectedFiles();
loadFile(selectedFiles.first());
}
else{
m_igramsToProcess = dialog.selectedFiles();
Expand Down Expand Up @@ -1764,8 +1764,9 @@ void MainWindow::on_actionCreate_Movie_of_wavefronts_triggered()
if (QMessageBox::Yes == QMessageBox::question(0,"---------- 3D wave front Video maker -------------","Do you have FFMpeg and want it to make a video from these images?"))

{
QString cmd = QString("ffmpeg -framerate 1 -i %1%03d.jpg -c:v libx264 -vf format=yuv420p -y -r 25 %2").arg(dowavefront? waveprefix:astigprefix). \
arg(dowavefront? "waveFronts.mp4": "astig.mp4");
QString cmd = QString("ffmpeg -framerate 1 -i %1%03d.jpg -c:v libx264 -vf format=yuv420p -y -r 25 %2")
.arg(dowavefront ? waveprefix : astigprefix,
dowavefront ? "waveFronts.mp4" : "astig.mp4");

bool ok = false;
QString text = QInputDialog::getText(this,
Expand All @@ -1777,11 +1778,11 @@ void MainWindow::on_actionCreate_Movie_of_wavefronts_triggered()
QDialog *dialog = new QDialog;
dialog->setWindowTitle("ffmpeg output");
dialog->resize(1000,1000);
QTextEdit *textEdit = new QTextEdit();
QTextEdit *textEdit = new QTextEdit(dialog);

QPushButton *closeButton = new QPushButton("Close");
QPushButton *closeButton = new QPushButton("Close", dialog);

QVBoxLayout *layout = new QVBoxLayout;
QVBoxLayout *layout = new QVBoxLayout(dialog);
layout->addWidget(textEdit);
layout->addWidget(closeButton);

Expand All @@ -1795,32 +1796,43 @@ void MainWindow::on_actionCreate_Movie_of_wavefronts_triggered()
qDebug() << "plain text"<< text;
QApplication::setOverrideCursor(Qt::WaitCursor);
QProcess *proc = new QProcess;
QObject::connect(proc, SIGNAL(finished(int, QProcess::ExitStatus)), proc, SLOT(deleteLater()));
connect(proc, QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
[=](int exitCode, QProcess::ExitStatus exitStatus){ qDebug() << "what" << exitStatus << "code" << exitCode; });

// ensure we kill ffmpeg if the dialog is closed
connect(dialog, &QDialog::finished, dialog, [=](int) {
if (proc->state() == QProcess::Running) {
proc->kill();
proc->waitForFinished();
qDebug() << "ffmpeg Process killed by user";
}
});

Comment on lines +1803 to +1811
Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

this connects a dialog close (X) to QProcess proc. In some cases I have hang whole application by closing dialog at wrong time. Closing was not handled

proc->setProcessChannelMode(QProcess::MergedChannels);
proc->setWorkingDirectory(dir);
QStringList args = text.split(" ");
QStringList args = QProcess::splitCommand(text);
qDebug() << "args" << args.mid(1);
proc->start("ffmpeg",args.mid(1));


while (!proc->waitForFinished(200)){
QString q = proc->readAll();
if (q != "")
textEdit->append(q);
QApplication::processEvents();
}
connect(proc, &QProcess::readyRead, textEdit, [proc, textEdit]() {
QString output = proc->readAll();
if (!output.isEmpty())
textEdit->append(output);
});

QEventLoop loop;
QObject::connect(proc, SIGNAL(finished(int, QProcess::ExitStatus)), &loop, SLOT(quit()));
loop.exec();
Comment on lines -1808 to +1827
Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

other changes are more self explanatory so I will focus on this one. Let me know if you have more questions on the other changes and I will answer happily.

Original code was doing proc->readAll(); in a busy loop. Instead I connected QProcess::readyRead to what the loop was doing.

Then I created a QEventLoop loop who does the waiting instead of original waitForFinished. All the close and finish signals are connected correctly to kill everything properly.


qDebug() << "done" ;


proc->waitForFinished();
QString out = proc->readAll() ;
QApplication::restoreOverrideCursor();

QString fn = dir + "/" + args[args.length()-1];
QString fn = dir + "/" + args.last();
QDesktopServices::openUrl(fn);

}
}

Expand Down
2 changes: 1 addition & 1 deletion myplotpicker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ void myPlotPicker::select(const QPointF &p){
QString s = find(p);
if (s == "")
return;
emit selected(s);
emit mySelected(s);
}
QString myPlotPicker::find(const QPointF &p)const{

Expand Down
2 changes: 1 addition & 1 deletion myplotpicker.h
View file
Open in desktop
Copy link
Owner

@githubdoe githubdoe Jul 21, 2025

Choose a reason for hiding this comment

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

Explain why changing the name was necessary. I don't think it was.

Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

because selected was clashing with a QWT function we inherited in this class. Nasty warning that could mask important issue. It probably did work fine but now we are 100% sure.

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class myPlotPicker : public QwtPlotPicker
QString find(const QPointF &p)const;
QwtPlot *m_plot;
signals:
void selected(const QString);
void mySelected(const QString);
public slots:
};

Expand Down
4 changes: 2 additions & 2 deletions nullvariationdlg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,10 @@ QMap<double,int > histo(const QList<double> data, int bins){
{
intervals[ *minma.first + histInterval * (i+1)] = 0;
}
QVector<double> keys = intervals.keys().toVector();
const QList<double>& keys = intervals.keys();
for (int i = 0; i < data.size(); ++i){
for (int j = 0; j < bins; ++j){
if (data[i]<= keys[j]) {
if (data[i] <= keys[j]) {
++intervals[keys[j]];
break;
}
Expand Down
2 changes: 0 additions & 2 deletions percentcorrectiondlg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@

myJsonObject["exclusion radius"] = m_exclusionRadius/m_radius;
QJsonArray jzones;
for (const auto &item : m_zoneCenters) {

Check warning on line 76 in percentcorrectiondlg.cpp

View workflow job for this annotation

GitHub Actions / build-linux-clazy 

c++11 range-loop might detach Qt container (QList) [-Wclazy-range-loop-detach]

jzones.append((double)item/m_radius); // zone centers are saved as a percentage.
}
Expand Down Expand Up @@ -345,8 +345,6 @@

QVector<double> IdealzoneKnife;
QVector<double> ActualZoneKnife;
QVector<double> idealDelta;
QVector<double> actualDelta;
IdealzoneKnife << 0.;
ActualZoneKnife << 0.0;

Expand Down
2 changes: 1 addition & 1 deletion pixelstats.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ void CanvasPicker::move( const QPoint &pos )
plot()->replot();
plotCanvas->setPaintAttribute( QwtPlotCanvas::ImmediatePaint, false );

emit(markerMoved());
emit markerMoved();
}


Expand Down
5 changes: 2 additions & 3 deletions psi_dlg.cpp
View file
Open in desktop
Copy link
Owner

@githubdoe githubdoe Jul 21, 2025

Choose a reason for hiding this comment

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

Explain why an additional variable (files) was needed. I don't think one is.

Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

same as above. https://github.com/githubdoe/DFTFringe/pull/211/files#r2229198458

Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

in fact here it's somewhat different. you see files was already created at line 70. I just reused it

Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,11 @@ void PSI_dlg::on_browse_clicked()
});
dialog.setNameFilters(filters);
if (dialog.exec()){
QFileInfo a(dialog.selectedFiles().first());
QStringList files = dialog.selectedFiles();
QFileInfo a(files.first());
settings.setValue("lastPath",a.absoluteFilePath());
//m_surfaceManager->process_psi(dialog.selectedFiles());
QStringList files = dialog.selectedFiles();
ui->igramList->addItems(files);
QStringList phases;
double deltaAngle = ui->PSIPhaseValue->value();
int index = 0;
double startAngle = 0;
Expand Down
6 changes: 3 additions & 3 deletions settingsigramimportconfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ settingsIgramImportConfig::~settingsIgramImportConfig()

void settingsIgramImportConfig::on_browse_clicked()
{
QSettings set;
QString defaultPath = set.value("lastPath",".").toString();
QString path = set.value("importIgramPath",defaultPath).toString();
//QSettings set;
//QString defaultPath = set.value("lastPath",".").toString();
//QString path = set.value("importIgramPath",defaultPath).toString(); //unused
QString directory = QFileDialog::getExistingDirectory(
0, "Choose directory", "",
QFileDialog::ShowDirsOnly | QFileDialog::DontResolveSymlinks);
Expand Down
4 changes: 2 additions & 2 deletions unwraperrorsview.cpp
View file
Open in desktop
Copy link
Owner

@githubdoe githubdoe Jul 21, 2025

Choose a reason for hiding this comment

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

While technically correct a char and an uchar have the same size I think. Thus each is equivalent in this context.

Copy link
Collaborator Author

@atsju atsju Jul 24, 2025

Choose a reason for hiding this comment

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

Yes they are 100% equivalent. But one is not overflowing and avoids warning. Thus I used CV's expected signness

Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ void unwrapErrorsView::createUnwrapErrors(){
double v2 = m_wf.data.at<double>(y,x+1);
double v3 = m_wf.data.at<double>(y+1,x);
if ((fabs(v1 - v2) > .5) || (fabs(v1 - v3) > .5)){
errorView.at<char>(y,x) = 255;
errorView.at<uchar>(y,x) = 255;
++cnt;
}
}
else {
errorView.at<char>(y,x) = 100;
errorView.at<uchar>(y,x) = 100;
}

}
Expand Down
4 changes: 2 additions & 2 deletions wavefrontloader.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void waveFrontLoader::loadx( SurfaceManager *sm){
int size = m_list.size();

emit status(0);
bool mirrorConfigChanged = false;
//bool mirrorConfigChanged = false;
done = false;

emit progressRange(0, size);
Expand Down Expand Up @@ -84,7 +84,7 @@ void waveFrontLoader::loadx( SurfaceManager *sm){
emit currentWavefront(file);

emit status(++prog);
mirrorConfigChanged |= sm->loadWavefront(file);
/*mirrorConfigChanged |= */sm->loadWavefront(file);
}
}
catch (const std::bad_alloc &ex)
Expand Down
Loading