Dalework by githubdoe · Pull Request #261 · githubdoe/DFTFringe

githubdoe · 2025-11-06T21:25:05Z

I think Dalework is ready to be pulled into master.

EDIT by atsju:
This concerns #256

Igramarea.cpp
Added e hot key command to enable outline edge zoom.

MainWindow.cpp

Added igram downloader using the “u” hot key will downoad image from skysolve camera at it’s URL of 192.168.50.5:5000. (specialized and not configurable). Could be set into preferences but still only appropriate for getting images from Skysolve.
Added ‘i’ hot key igram import igram from latest file in dir x where x is set in preferences.
Added “s” hot key to save current wave front.

main.cpp

Added DPI scaling command but it did not seem to help.

Profileplot.cpp

Enabled context menu and added change x axis to percent, inches, or mm(default)
Moved percent correction command into this menu.

Simulationview.cpp

Corrected Gaussian Blur calculation.
Changed top pushbuttons for make scan film and make to checkboxes. Because then their text is not tiny like it was for when they were buttons on high DPI screens.
Reduced file name path to last dir on the top line.

Added startest movie function.

Added e hot key command to enable outline edge zoom. MainWindow.cpp Added igram downloader using the “u” hot key will downoad image from skysolve camera at it’s URL of 192.168.50.5:5000. (specialized and not configurable). Could be set into preferences but still only appropriate for getting images from Skysolve. Added ‘i’ hot key igram import igram from latest file in dir x where x is set in preferences. Added “s” hot key to save current wave front. main.cpp Added DPI scaling command but it did not seem to help. Profileplot.cpp Enabled context menu and added change x axis to percent, inches, or mm(default) Moved percent correction command into this menu. Simulationview.cpp Corrected Gaussian Blur calculation. Changed top pushbuttons for make scan film and make to checkboxes. Because then their text is not tiny like it was for when they were buttons on high DPI screens. Reduced file name path to last dir on the top line. Added startest movie function.

atsju · 2025-11-07T09:03:12Z

I will compare this to the way I merged to point out things that need to be double checked.
I can also look at fixing the build but maybe it will take time (I have not much time this Weekend).

@githubdoe can you shortly describe the changes/features brought by this PR ? So we can keep track in title and description

gr5 · 2025-11-07T12:08:03Z

you shortly describe

He has like a 20 or 30 line commit message. I'll go find it...

gr5 · 2025-11-07T12:09:38Z

Igramarea.cpp
	Added e hot key command to enable outline edge zoom.

MainWindow.cpp
	Added igram downloader using the “u” hot key will downoad image from skysolve camera at it’s URL of 192.168.50.5:5000.  (specialized and not configurable).  Could be set into preferences  but still only appropriate for getting images from Skysolve.
	Added ‘i’ hot key igram import igram from latest file in dir x where x is set in preferences.
	Added “s” hot key to save current wave front.

main.cpp
	Added DPI scaling command but it did not seem to help.

Profileplot.cpp
	Enabled context menu and added change x axis to percent, inches, or mm(default)
	Moved percent correction command into this menu.

Simulationview.cpp
	Corrected Gaussian Blur calculation.
	Changed top pushbuttons for make scan film and make to checkboxes.  Because then their 	text is not tiny like it was for when they were buttons on high DPI screens.
	Reduced file name path to last dir on the top line.

Added startest movie function.

atsju · 2025-11-07T15:37:57Z

I missed that. Thanks a lot

atsju

Here are some things I noticed that are or could be wrong merge conflict solving.
I did not check how ui files have been merged and do not plan to do. Test will tell.

I have not reviewed actuel code yet. This is a seconds step.

How do you want to proceed ? Should I commit into Dalework to fix build and what I noticed here ? Or I do a PR targeting Dalework ?

atsju · 2025-11-07T16:11:05Z

profileplot.cpp

    QString txt = (zoomed)? "Restore to MainWindow" : "FullScreen";
-    myMenu.addAction(txt,  this, &ProfilePlot::zoom);
+
+    myMenu.addAction(txt,  this, SLOT(zoom()));


Suggested change

myMenu.addAction(txt, this, SLOT(zoom()));

myMenu.addAction(txt, this, &ProfilePlot::zoom);

profileplot.cpp

atsju · 2025-11-07T16:17:30Z

profileplot.cpp

+
+    connect(slopeLimitSB, SIGNAL(valueChanged(double)), this, SLOT(slopeLimit(double)));
+    connect(showSlopeError,SIGNAL(clicked(bool)), this, SLOT(showSlope(bool)));
+


Suggested change

connect(slopeLimitSB, SIGNAL(valueChanged(double)), this, SLOT(slopeLimit(double)));

connect(showSlopeError,SIGNAL(clicked(bool)), this, SLOT(showSlope(bool)));

Bad merge conflict solve. New style connect is already just after.
This suggestion needs to be committed for sure

atsju · 2025-11-07T16:22:27Z

simulationsview.cpp

+    connect(&m_guiTimer, SIGNAL(timeout()), this, SLOT(on_MakePB_clicked()));
+    connect(this, SIGNAL(customContextMenuRequested(QPoint)), this,
+            SLOT(showContextMenu(QPoint)));


Suggested change

connect(&m_guiTimer, SIGNAL(timeout()), this, SLOT(on_MakePB_clicked()));

connect(this, SIGNAL(customContextMenuRequested(QPoint)), this,

SLOT(showContextMenu(QPoint)));

connect(&m_guiTimer, &QTimer::timeout, this, &SimulationsView::on_MakePB_clicked);

connect(this, &QWidget::customContextMenuRequested, this,

&SimulationsView::showContextMenu);

this is regression from main. Unless there was an issue, we want new style as it was

atsju · 2025-11-07T16:24:20Z

simulationsview.cpp

-#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
-    // keep compatibility with newer version of QWT used in QT6
-    customLegend->setAlignmentInCanvas(Qt::AlignLeft | Qt::AlignBottom);
-#else
    customLegend->setAlignment(Qt::AlignLeft | Qt::AlignBottom);
-#endif


I cannot put suggestion on deleted code but bad merge conflict. The initial code comes from main and is working. This is the reason build fails in CI.

Fix however. That was not deleted by me on purpose.

atsju · 2025-11-07T16:25:54Z

simulationsview.cpp

-
-void SimulationsView::saveImage(){
+void SimulationsView::saveImage(QString fileName){
    QSettings settings;
    QString path = settings.value("lastPath","").toString();
-    QString fileName = QFileDialog::getSaveFileName(0,
+    if (fileName == "")
+        fileName = QFileDialog::getSaveFileName(0,
                                        "File name for image to be saved", path);
-
-    saveImageNamed(fileName);
-}
-
-void SimulationsView::saveImageNamed(QString fileName){


this needs to be reverted to main too. Regression.

Fix however as long as the image can be saved with the given name if supplied.

atsju · 2025-11-07T16:29:25Z

simulationsview.cpp


-void SimulationsView::showContextMenu(QPoint pos){
-    // Handle global position
+void SimulationsView::showContextMenu(const QPoint &pos)


Suggested change

void SimulationsView::showContextMenu(const QPoint &pos)

void SimulationsView::showContextMenu(QPoint pos)

regression from main

What ever works. It worked for me in my build.

atsju · 2025-11-07T16:29:52Z

simulationsview.cpp

    // Create menu and insert some actions
    QMenu myMenu;
-    myMenu.addAction("Save as image", this, &SimulationsView::saveImage); // connects to QAction::triggered(bool checked = false)
+    myMenu.addAction("Save as image",  this, SLOT(saveImage()));


Suggested change

myMenu.addAction("Save as image", this, SLOT(saveImage()));

myMenu.addAction("Save as image", this, &SimulationsView::saveImage); // connects to QAction::triggered(bool checked = false)

regression from main

atsju · 2025-11-07T16:30:34Z

simulationsview.cpp

 #include <opencv2/imgproc.hpp>
 using namespace cv;
-cv::Mat make_obstructionMask(const cv::Mat &mask){
+cv::Mat make_obstructionMask(cv::Mat mask){


Suggested change

cv::Mat make_obstructionMask(cv::Mat mask){

cv::Mat make_obstructionMask(const cv::Mat &mask){

regression from main

atsju · 2025-11-07T16:31:18Z

simulationsview.h

+    void showContextMenu(const QPoint &pos);
+
+    void saveImage(QString fname = "");


Suggested change

void showContextMenu(const QPoint &pos);

void saveImage(QString fname = "");

void showContextMenu(QPoint pos);

void saveImage();

regression from main

atsju

Code reviewed

atsju · 2025-11-07T16:39:00Z

autoinvertdlg.ui

    <x>0</x>
    <y>0</y>
-    <width>678</width>
+    <width>1078</width>


I believe you changed this testing with your Qt5 build.
On the other hand you said Qt6 official 8.1.0 displays correctly.
This needs to be validated with CI build which is the one we distribute to end users.

It's fine to make it "too wide". "too narrow" is more of a problem.

While trying to fix the dialog width I played with some parameters that no longer need to be changed but lost track of what changed.

atsju · 2025-11-07T16:40:47Z

DFTFringe_Dale.pro

    profileplot.cpp \
    profileplotpicker.cpp \
    settingsigramimportconfig.cpp \
+    startestmoviedlg.cpp \


Changes in this file need to be ported to DFTFringe_QT5.pro and DFTFringe.pro too

atsju · 2025-11-07T16:42:07Z

igramarea.cpp

+    QObject::connect(shortcut, SIGNAL(activated()), this, SLOT(zoomOut()));
+    shortcut = new QShortcut(QKeySequence("e"), this);
+    QObject::connect(shortcut, SIGNAL(activated()), this, SLOT(edgeMode()));


Suggested change

QObject::connect(shortcut, SIGNAL(activated()), this, SLOT(zoomOut()));

shortcut = new QShortcut(QKeySequence("e"), this);

QObject::connect(shortcut, SIGNAL(activated()), this, SLOT(edgeMode()));

QObject::connect(shortcut, &QShortcut::activated, this, &IgramArea::zoomOut);

shortcut = new QShortcut(QKeySequence("e"), this);

QObject::connect(shortcut, &QShortcut::activated, this, &IgramArea::edgeMode);

atsju · 2025-11-07T16:44:54Z

main.cpp


 int main(int argc, char *argv[])
 {   
+   // QApplication::setAttribute(Qt::AA_EnableHighDpiScaling);


Suggested change

// QApplication::setAttribute(Qt::AA_EnableHighDpiScaling);

Qt6 doesn't use this https://doc.qt.io/qt-6/highdpi.html

It seems high DPI is managed much better in Qt6 (as seen in autoinvertdlg). Dale this might be a good reason to migrate

It might be.

atsju · 2025-11-07T16:45:57Z

mainwindow.cpp

    QObject::connect(shortcut, &QShortcut::activated, this, &MainWindow::importIgram);

+    shortcut = new QShortcut(QKeySequence(Qt::Key_U), this);
+    QObject::connect(shortcut, SIGNAL(activated()), this, SLOT(load_from_url()));


Suggested change

QObject::connect(shortcut, SIGNAL(activated()), this, SLOT(load_from_url()));

QObject::connect(shortcut, &QShortcut::activated, this, &MainWindow::load_from_url);

atsju · 2025-11-07T16:48:59Z

mainwindow.cpp

+         url.setScheme("http"); // or "https" if using SSL
+         url.setHost("192.168.50.5"); // Replace with your IP address
+         url.setPort(5000); // Specify the port
+         url.setPath("/downloadImage");


Is this configurable ? If not it looks like some personal feature you don't want into official release as is.

Also it looks like this can be only called with a special shortcut. That's some easter egg at this point

Yes and I thought I talked about it in the commit comments. Yes view it as an easter egg. It is only good for any user who uses my skysolve camera as their camera for the Bath. I doubt there is such a user. It's code is specific to skysolves html web page.

Ok you did. I didn't make the link in my brain.
Why not. I recommend to add some comments in the code for futur reader to know why this function is here without digging history

atsju · 2025-11-07T16:53:42Z

profileplot.cpp

    double lower = -m_wf->diameter/2 - 10;
    double upper = m_wf->diameter/2 + 10;
+    if (m_displayInches){
+        lower = lower /= 25.4;


Suggested change

lower = lower /= 25.4;

lower /= 25.4;

or

Suggested change

lower = lower /= 25.4;

lower = lower / 25.4;

Yes. I've been doing python to long and forgot that C++ syntax. Which python does not always copy.

atsju · 2025-11-07T16:54:04Z

profileplot.cpp

    double upper = m_wf->diameter/2 + 10;
+    if (m_displayInches){
+        lower = lower /= 25.4;
+        upper = upper /= 25.4;


Suggested change

upper = upper /= 25.4;

upper = upper / 25.4;

or

Suggested change

upper = upper /= 25.4;

upper /= 25.4;

atsju · 2025-11-07T16:57:33Z

simulationsview.cpp

+#include <opencv2/core/core.hpp>
+#include <opencv2/highgui/highgui.hpp>
+#include <opencv2/imgproc/imgproc.hpp>


move includes at top of file

CppCore is a reliable website with some widely accepted coding rules with great explanations.
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rs-include-order

Sure. That was intended but during a quick fix just added near by.

atsju · 2025-11-07T16:58:12Z

simulationsview.cpp

-        return;
-    int cnt = 0;
-    for (double wave = .1; wave < 10. ; wave+= .2){
+void SimulationsView::makeFrame(double defocus, startestMovieDlg * dlg){


fix indentation of function

gr5 · 2025-11-07T17:05:22Z

Dale if you look at these online, each small change - typically one or two lines of code - are shown highlighted and clear and you can click the button that says "commit suggestion" if you are okay with the change and it will get checked into this branch which is also your dalework branch (but at some point you should pull it back to your computer).

Julien, I think all the slot/signal ones can be just accepted as is because you are the slot/signal expert among the 3 of us.

Let's see what Dale says.

githubdoe · 2025-11-07T18:21:32Z

Of all the things I hope you can merge into the master the most important are

Profile plot menu.
The hot keys.
My fix to the blurring in the Ronchi and Foucault simulation.

I will look through all the comments as I have time. Thank you for your work.

atsju · 2025-11-08T14:52:45Z

Hi,
I have some time right now. If you are OK I will try to fix the build and take into account the review comments.

If you are fine with that, please do not play/accept with my PR suggestions while I work in background. I will make this in a single commit.

gr5 · 2025-11-08T15:02:22Z

please do not play/accept with my PR suggestions

Sounds good. I don't know if you have noticed but Dale's "work hours" in his local time zone appear to be typically 22:00 to 04:00. So he is unlikely to even look at your comments until long after you go to sleep. Like around 06:00 on Sunday for France time zone.

atsju · 2025-11-08T16:26:42Z

I'm closing this. Please review #262 instead.

githubdoe · 2025-11-08T18:26:47Z

My work hours on DFTFringe are anywhere from 10:30 am to 3am CDT. I see comments and usually look at them. I may not have any action on them until late.

githubdoe added 3 commits November 1, 2025 18:22

merged with master

25a6206

Made auto invert resizable

577296f

atsju requested changes Nov 7, 2025

View reviewed changes

atsju mentioned this pull request Nov 8, 2025

Several enhancements from dalework. See details in PR #262

Merged

atsju closed this Nov 8, 2025

	myMenu.addAction(txt, this, SLOT(zoom()));
	myMenu.addAction(txt, this, &ProfilePlot::zoom);


		connect(slopeLimitSB, SIGNAL(valueChanged(double)), this, SLOT(slopeLimit(double)));
		connect(showSlopeError,SIGNAL(clicked(bool)), this, SLOT(showSlope(bool)));

	void SimulationsView::showContextMenu(const QPoint &pos)
	void SimulationsView::showContextMenu(QPoint pos)

	myMenu.addAction("Save as image", this, SLOT(saveImage()));
	myMenu.addAction("Save as image", this, &SimulationsView::saveImage); // connects to QAction::triggered(bool checked = false)

	cv::Mat make_obstructionMask(cv::Mat mask){
	cv::Mat make_obstructionMask(const cv::Mat &mask){

		void showContextMenu(const QPoint &pos);

		void saveImage(QString fname = "");

	QObject::connect(shortcut, SIGNAL(activated()), this, SLOT(load_from_url()));
	QObject::connect(shortcut, &QShortcut::activated, this, &MainWindow::load_from_url);

Comments

Conversation

githubdoe commented Nov 6, 2025 • edited by atsju Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atsju commented Nov 7, 2025

Uh oh!

gr5 commented Nov 7, 2025

Uh oh!

gr5 commented Nov 7, 2025

Uh oh!

atsju commented Nov 7, 2025

Uh oh!

atsju left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atsju left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atsju Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

githubdoe commented Nov 6, 2025 •

edited by atsju

Loading

atsju Nov 7, 2025 •

edited

Loading

atsju Nov 7, 2025 •

edited

Loading

githubdoe commented Nov 7, 2025 •

edited

Loading