Average profile #309

githubdoe · 2025-12-23T19:44:38Z

Added different way to create average profile and display it with right click menu.
Added other things to right click menu of profile plot.
Added right click menu of Ronchi image to create image of multiple ronchi images.
Added if you move the avg or single profile it will also move the 16 diameters profiles once they are displayed.
Profile offset will happen whenever you select different wave fronts.

…idth in preferences

…Corrected coloring of profile lines.

…lyzed igram did not display the profile after analysis.

…for now. Also added the worker thread class but not using it yet. This can be used as a model for that task.

…view ronchi and foucalut generater

gr5 · 2025-12-23T20:07:43Z

Oh. There are merge errors. This is related to my recent PR where I added the "origin" enum to a few functions. I can do the merge if you want Dale. I also marked this PR as draft. Search this web page for "draft" to see the link to switch it back.

githubdoe · 2025-12-23T20:35:26Z

Go ahead.

mainwindow.cpp

I had added a new parameter to that function. Fixed now. Also Dale added a file to one project file but forgot to add to the other 2.

gr5 · 2025-12-24T16:52:05Z

I did the merge. It was much easier than I had expected. It builds in QT5, but there is a QT6-only build error. I don't have QT6 installed on my laptop and I don't understand the error, so if @atsju could look at this error and fix it? Or I can fix it Saturday.

profileplotpicker.cpp: In member function ‘void profilePlotPicker::move(QPoint)’:
Error: profileplotpicker.cpp:152:39: error: invalid use of incomplete type ‘const class QwtText’
  152 |     emit offset(d_selectedCurve->title().text(), delta);
      |                 ~~~~~~~~~~~~~~~~~~~~~~^~

By the way, I love the new GUI for profile plot. I played with it for 15 minutes solid, lol. I don't like Dale's change to menu item "tools "stop auto invert".

Dale you should do a "git checkout averageProfile" then "git pull" either now or after this PR is complete although at that point you can just pull master.

githubdoe · 2025-12-24T18:20:16Z

Those were my temporary changes to keep the dialog from popping up until you had your permanent fix.

githubdoe · 2025-12-24T18:25:12Z

The stop auto invert is leftover from much earlier time when there was no invert dialog. It may no longer be needed.

githubdoe · 2025-12-24T18:27:11Z

I also forgot to mention the new profile line width items and fix to the color settings of the profile lines in the preferences.

githubdoe · 2025-12-24T18:29:22Z

Profile offset reset will happen whenever you select different wave fronts.

Co-authored-by: Julien Staub <atsju2@yahoo.fr>

gr5 · 2025-12-24T21:18:10Z

The stop auto invert is leftover from much earlier time when there was no invert dialog. It may no longer be needed.

I'm not sure if it's needed for most users but for me it's very helpful - well maybe only for testing. But I use it a lot. For testing. And it's kind of hidden in the menu system so I think it's fine.

githubdoe · 2025-12-25T00:20:12Z

I have pulled the code and it compiles. I see the square contour plot but do not see the vertical blue bar for the right side. It will still drag that control however.

I just realized an addition I want to add. That is overlay one ronchi with another to see how they are different. I bet AI can help me with that as well.

gr5 · 2025-12-25T00:44:31Z

not see the vertical blue bar for the right side. It will still drag that control however.

That's not merged yet. It's in a PR that still needs approval.

Also the compiler error is only for QT6. I don't understand what the cause is and hope that Julien fixes it but if not I'll look at it Saturday. Julien may be celebrating Christmas right now.

I just realized an addition I want to add. That is overlay one ronchi with another to see how they are different. I bet AI can help me with that as well.

I prefer that you do those edits in any branch you want but if you do them in the averageProfile branch then they will get added to this PR and I prefer any changes related to a ronchi overlay feature to go into a seprate PR. So please don't push new changes to averageProfile until this PR is merged. But if you do we'll just enlarge the scope of this PR.

gr5 · 2025-12-25T00:46:21Z

Also tomorrow I'll be quite busy with visiting family so may not notice anything in my emails nor anything github related. 😄

atsju · 2025-12-25T08:49:24Z

I think I fixed the build. I'm more busy than expected so I don't know how much time I have available.

Dale, you will need to pull the branch to avoid conflicts. As George explained: kindly avoid pushing new features to this branch until it's merged. It should probably already have been split into 2 or more PR but I don't want to refrain you developping so we will just take care of it as it is.
I will make a more formal code review as soon as I have more time.

Happy Xmas to you 2 :)

github-actions · 2025-12-25T08:54:18Z

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-tidy (v18.1.3) reports: 1 concern(s)

autoinvertdlg.cpp:57:59: warning: [clang-diagnostic-unused-parameter]

unused parameter 'checked'

   57 | void autoInvertDlg::on_InvertWithoutAskingRb_clicked(bool checked)
      |                                                           ^

Have any feedback or feature suggestions? Share it here.

github-actions · 2025-12-25T08:57:08Z

🚀 New build available for commit 90db809
Download installer here

githubdoe · 2025-12-25T09:31:51Z

I would remove the ifdef notnowold. But then you would get my new feature as well.

atsju · 2025-12-25T11:40:22Z

I would remove the ifdef notnowold. But then you would get my new feature as well.

I'm not sure to understand.
If you are talking about the warning, the ifdef is not the cause. The problem is you have 2 macro after ifdef instead one. Compiler might not complain but it's not language compliant.

If you want to remove it and talk about "no new feature", go ahead and remove, your new feature is aleady in the code and compiled by default. Removing the old code is part of this PR. What we want to avoid is getting more unrelated features.

Just remember to pull the branch now to avoid conflicts. As George and I pushed changes

gr5 · 2025-12-25T13:10:13Z

I would remove the ifdef notnowold. But then you would get my new feature as well.

Noted! I only played with the profile stuff. I didn't look at the Ronchi stuff yet. I didn't look at the code changes yet either. I'll try removing that ifdef and rebuild. But not today.

atsju

I did not test. Only code review.

atsju · 2025-12-25T16:38:19Z

autoinvertdlg.ui

+      <layout class="QHBoxLayout" name="horizontalLayout_5">
+       <item>
+        <widget class="QRadioButton" name="InvertWithoutAskingRb">
+         <property name="text">
+          <string>Don't Ask but Invert</string>
+         </property>
+        </widget>
+       </item>
+       <item>
+        <widget class="QRadioButton" name="AskDoNotInvertRb">
+         <property name="text">
+          <string>Ask but Don't Invert</string>
+         </property>
+         <property name="checked">
+          <bool>true</bool>
+         </property>
+        </widget>
+       </item>
+       <item>
+        <widget class="QRadioButton" name="DoNotAskorInvertRb">
+         <property name="text">
+          <string>Don't ask or invert</string>
+         </property>
+        </widget>


I think this code and maybe more comes from when you modified the UI to show us a printscreen in discussion about the autoInvertDialog. But it seems unused. Should be removed.

Yes that code is my temp fix. It should not be merged.

foucaultview.cpp

foucaultview.h

atsju · 2025-12-25T16:55:21Z

foucaultview.cpp

+    // Note: outputLambda should be defined/accessible in your scope
+    settings.outputLambda   = 550.0;


magic number ? Needs to be settable in UI ?

Same as previous comment. Except filled in from the preference setting.

sorry I don't get this one. What one is "previous" ?
Does code need change so that 550 can be set in preference settings ?

That value and all values in the new settings structure generated by AI are values that will not be used by current code. The real values come from foucaultview UI and the 550 is the output wavelength specified in the preferences under the configuration menu. 550 is the wavelength of green light in nanometers. I'm impressed that AI figured out a value for [t there even though it did not have access to the code where that value is set.

AI generated those default values which would be used only if we ever write code outside the foucaultview.cpp file to generate ronchi images where we don't have access to the foucaultview UI.

profileplot.cpp

atsju · 2025-12-25T17:40:39Z

profileplot.cpp

+            }
+qDebug() << "offsets" << m_waveFrontyOffsets<< y_offset;
+            // if show one angle
+            if (m_show_oneAngle or (!m_showAvg and !m_show_16_diameters & !m_show_oneAngle)){


Suggested change

if (m_show_oneAngle or (!m_showAvg and !m_show_16_diameters & !m_show_oneAngle)){

if (m_show_oneAngle or (!m_showAvg and !m_show_16_diameters and !m_show_oneAngle)){

you are mixing logical operators and bitwise operator. Here you need && or and.

Yes the "and" is correct fix

atsju · 2025-12-25T17:41:16Z

foucaultview.cpp

+        double lpi_val = s.lpi * (s.useMM ? 25.4 : 1.0);
+        int ppl = std::max(1, (int)((0.5 / lpi_val) / pixwidth));
+        int start = (size / 2) - (ppl / 2);
+        bool even = ((start / ppl) % 2 == 0) ^ s.clearCenter;


Suggested change

bool even = ((start / ppl) % 2 == 0) ^ s.clearCenter;

bool even = ((start / ppl) % 2 == 0) != s.clearCenter;

^ is a bitwise operator.

Yes I think in this case ^ is correct. This was code generated by AI a clever trick to test the int result being 0 or 1 along with the test for a clear center setting.

It's working but it's bitwise operation. ^ is xor (mutual exclusive). But you shall use != For bool. It's same logic table but it's Boolean operation instead bitwise operation.

surfacemanager.cpp

atsju · 2025-12-25T17:45:36Z

wavefrontloaderworker.cpp

+{
+}
+
+void WavefrontLoaderWorker::process(QStringList args, SurfaceManager* manager) {


Suggested change

void WavefrontLoaderWorker::process(QStringList args, SurfaceManager* manager) {

void WavefrontLoaderWorker::process(const QStringList& args, SurfaceManager* manager) {

Wavefrontloaderworker.cpp is not used and not really needed. It is a model for how to make a worker thread that I want to keep because it may someday help to restructure the code and make the UI more responsive. Go ahead and make the change if you want.

githubdoe · 2025-12-25T19:08:20Z

I think nearly all the changes to Foucaultview.cpp and .h are AI generated. I looked it over just a small amount and then just tested to see if it did what I wanted. For me it is not worth the trouble to modify any of it's decisions. I like what it produced. Julian my have other ideas and can run them by me but I don't really want to have to spend any time on it.

atsju · 2025-12-26T07:39:28Z

I think nearly all the changes to Foucaultview.cpp and .h are AI generated. I looked it over just a small amount and then just tested to see if it did what I wanted. For me it is not worth the trouble to modify any of it's decisions. I like what it produced. Julian my have other ideas and can run them by me but I don't really want to have to spend any time on it.

Hi @githubdoe,
First of all, let me say I'm really impressed by your work, by your use of IA and everything you do and have done so far. The 21 comments I did in the review do not reflect bad code. I just want to ensure the code is as maintainable and bug free as possible. The code review is complement to tests you and George do roll out fantastically. I know I'm less good on this test side. Some comments are to fix build warnings, some really need fix before merging into master and some few I'm just unsure and can't answer alone.
Remember AI has also learned on low quality code. So "generated by IA" doesn't mean it doesn't need corrections. Yet I understand you don't want to spend time to fix that.

Julian my have other ideas and can run them by me but I don't really want to have to spend any time on it.

I'm really sorry if I'm sounding dense sometimes, English is not my first language. Can you kindly clarify if you want to merge this as is or if you want/allow me to do the changes in the branch ? I have no problem doing the changes so you can focus on the really important part. I might need some help, for example with #309 (comment)

githubdoe · 2025-12-26T09:09:36Z

I can not make any changes to that code because my current code has added functionality in it that both you and George said not to push.

gr5 · 2025-12-26T16:30:34Z

The multiple-ronchi feature is nice but whether I build it QT5 or QT6 I get the same issue with these ok/cancel buttons:

gr5 · 2025-12-26T16:51:19Z

I can not make any changes to that code because my current code has added functionality in it that both you and George said not to push.

Well there are 2 options.

option 1, grab the changes we made so far and merge them in. I don't think we changed much so far. I merged in master - okay that's a big change but there were no merge conflicts reported by git and there was one small edit (one line of code changed) to get it to compile after the merge. Also Julien made a tiny change to get it to compile in QT6 (he added a single include).

option 2, instead of pushing your current working copy to git (averageProfile branch) you can switch to a brand new branch and push that out so that it doesn't affect this PR. That way your working branch would be exactly as it is now. But eventually we want you to merge in master and the 2 changes I just mentioned in option 1.

I'm not sure what Julien and Dale prefer.

githubdoe · 2025-12-26T18:01:59Z

I wondered about those buttons and thought that it was a Qt5 thing. So did not play with them. There are just too tiny for me to read the text. Looks like the text does not even make through the black. So perhaps there are two problems.

Tiny text and buttons that I get with Qt5.
Background color and text color. Maybe just remove the black background.

githubdoe · 2025-12-26T18:26:36Z

I have asked AI about the problem and it says there must be a style sheet somewhere that is setting the global button style. To experiment add this code just before the compare dialog is opened.

It works and now I can read the buttons. So I need to look for the overriding style sheet.

Here is the code that makes the buttons readable:

QString buttonStyle = "QPushButton { background-color: palette(button); color: palette(button-text); border: 1px solid gray; padding: 4px; }";
compareBtn->setStyleSheet(buttonStyle);
saveBtn->setStyleSheet(buttonStyle);
cancelBtn->setStyleSheet(buttonStyle);

githubdoe · 2025-12-26T18:39:08Z

AI found the problem. It is code up in main. Here is an image of part of the Conversation:

githubdoe · 2025-12-26T18:43:18Z

AI gives two fixes.

githubdoe · 2025-12-26T18:49:34Z

But I should not post too soon. Removing the lines in main.cpp did not fix it. Nor did turning off dark mode on my machine.

githubdoe · 2025-12-26T19:19:31Z

Finally found the exact cause. The foucaultview.ui had a stylesheet that set background to black. Children of it inherit that. I no longer remember why it has that stylesheet. While trying to modify the background color in Qt Creator UI tool I deleted the stylesheet. There seems to be no undo. It is gone for now and does not seem to cause any problem on my machine. However others need to try once the code is updated.

AI actually helped my isolate the issue to the foucaultview and then when given the .ui found the problem.

githubdoe · 2025-12-26T19:29:21Z

The FoucaultView UI has nicer update and scan buttons now. I think I had put that old style onto them trying to make the better but I failed and forgot to remove the style.

Sorry guys I'm planning on pushing all my current code to the averageprofile branh. I really want you guys to review it all anyway. So I will merge any current master parts I don't have then push. That is unless I get a push back no from you.

Update from master that was changed since last update

githubdoe · 2025-12-27T03:40:19Z

I merged master into my branch and it all works. I can then push it to the remote if that is ok.

… into averageProfile

gr5 · 2025-12-27T17:42:07Z

No, this is great. I think the change to the .pro files got lost. I'll fix that. I think maybe I can do the "cherry pick" feature in git - I've never done that before. Regardless, I'll get this to compile. Should be easy.

…lly.

gr5 · 2025-12-27T17:51:05Z

Oh it was indeed easy. Took 7 minutes (didn't feel that long!). Dale added ronchicomparedialog to only one of 3 project files. Hopefully it will compile with no errors and then I'll look it over.

github-actions · 2025-12-27T18:02:20Z

🚀 New build available for commit cfa0e9a
Download installer here

githubdoe · 2025-12-27T19:38:56Z

Sorry I keep forgetting to add new files to other .pro files. Thanks

gr5 · 2025-12-27T20:07:05Z

I played with the ability to diff two ronchi's. It shows the changes in red and green and is very cool - it trains your eyes where to look to see subtle changes to the mirror in the ronchigram. Nice feature.

Yeah we need this to be "not set". Note that wavefront files (and much more) no longer prompt the users to change the wavefront. Co-authored-by: Julien Staub <atsju2@yahoo.fr>

Co-authored-by: Julien Staub <atsju2@yahoo.fr>

github-actions · 2025-12-27T20:32:28Z

🚀 New build available for commit cc7f73f
Download installer here

github-actions · 2025-12-27T20:33:03Z

🚀 New build available for commit 4a5293b
Download installer here

githubdoe · 2025-12-28T18:32:31Z

I see George has the vertical blue bar in the code now. I like it.

atsju · 2025-12-30T10:45:10Z

Hi, sorry for delay, I haven't had much time available.
I closed some of my initial review comments. I can do the fix for most of them.

Remains for you :

this one Average profile #309 (comment)
I'm dense but I don't understand if the magic 550 value is something for the futur that cannot be changed right now (use a #define or better yet, use a constexpr) OR if this value is supposed to comme from UI but has not be linked to UI. To explain differently: is the 550 used ? if it's used, does it need to be settable ? Is the code finished ?

I will do review of added code later.

githubdoe · 2025-12-30T18:45:15Z

Concerning the 550.

There is now a new routine that users can call with this structure that makes ronchi images. The old code that made ronchi images has been refactored into using that new make ronchi image code. That new routine uses this structure with the 550 in it.

The caller can use those default values if desired however the refactored code fills them in and in the case of the 550 it fills that in from the preferences data. Go look at the code that references that structure and you will see. Using Qt Creator it is easy to find all code that references something using a right click on the highlighted something.

One way that structure could have been written is with no default values. Then there would be no 550. However AI elected to fill it with reasonable default values. If you think you need it put a comment out beside the 550 that describes it as the output wave length in nanometers.

githubdoe added 6 commits December 12, 2025 04:39

Changed how profiles are selected and displayed. Added profile line w…

9f092e8

…idth in preferences

change autoinvert default value to useConic. Improved profile menus. …

55041ba

…Corrected coloring of profile lines.

corrected displaying percentage and inches. Fixed bug where first ana…

6ac13da

…lyzed igram did not display the profile after analysis.

Corrected bug in average display of inches and mm.

434b166

Modified code to shift the 16 diameters. Not perfect but good enough …

e9ef0a9

…for now. Also added the worker thread class but not using it yet. This can be used as a model for that task.

added make ronchi image of selected wave fronts. Restructed foucault …

d4e87ae

…view ronchi and foucalut generater

gr5 marked this pull request as draft December 23, 2025 20:04

atsju reviewed Dec 24, 2025

View reviewed changes

mainwindow.cpp Outdated Show resolved Hide resolved

gr5 added 2 commits December 24, 2025 11:04

Merge branch 'master' into averageProfile

0145d7b

Dale added a new call to CreateSurfaceFromPhaseMap() and coincidentally

de7ec73

I had added a new parameter to that function. Fixed now. Also Dale added a file to one project file but forgot to add to the other 2.

Update mainwindow.cpp

80178b0

Co-authored-by: Julien Staub <atsju2@yahoo.fr>

fix qt6 compilation issue (due to different qwt version ?)

90db809

atsju reviewed Dec 25, 2025

View reviewed changes

githubdoe added 2 commits December 26, 2025 16:01

added ronchi comparison of two files. Corrected black buttons

5194e62

Merge branch 'master' into averageProfile

1f027b3

Update from master that was changed since last update

Merge branch 'averageProfile' of https://github.com/githubdoe/DFTFringe…

3df6d01

… into averageProfile

Dale edited one project file but not the other two. Fixed now. Hopefu…

cfa0e9a

…lly.

gr5 and others added 2 commits December 27, 2025 15:18

Update surfacemanager.cpp

cc7f73f

Yeah we need this to be "not set". Note that wavefront files (and much more) no longer prompt the users to change the wavefront. Co-authored-by: Julien Staub <atsju2@yahoo.fr>

Update plotcolor.cpp

4a5293b

Co-authored-by: Julien Staub <atsju2@yahoo.fr>

		// Note: outputLambda should be defined/accessible in your scope
		settings.outputLambda = 550.0;

	if (m_show_oneAngle or (!m_showAvg and !m_show_16_diameters & !m_show_oneAngle)){
	if (m_show_oneAngle or (!m_showAvg and !m_show_16_diameters and !m_show_oneAngle)){

	bool even = ((start / ppl) % 2 == 0) ^ s.clearCenter;
	bool even = ((start / ppl) % 2 == 0) != s.clearCenter;

	void WavefrontLoaderWorker::process(QStringList args, SurfaceManager* manager) {
	void WavefrontLoaderWorker::process(const QStringList& args, SurfaceManager* manager) {

Average profile #309

Are you sure you want to change the base?

Average profile #309

Uh oh!

Conversation

githubdoe commented Dec 23, 2025

Uh oh!

gr5 commented Dec 23, 2025

Uh oh!

githubdoe commented Dec 23, 2025

Uh oh!

Uh oh!

gr5 commented Dec 24, 2025

Uh oh!

githubdoe commented Dec 24, 2025

Uh oh!

githubdoe commented Dec 24, 2025

Uh oh!

githubdoe commented Dec 24, 2025

Uh oh!

githubdoe commented Dec 24, 2025

Uh oh!

gr5 commented Dec 24, 2025

Uh oh!

githubdoe commented Dec 25, 2025

Uh oh!

gr5 commented Dec 25, 2025

Uh oh!

gr5 commented Dec 25, 2025

Uh oh!

atsju commented Dec 25, 2025

Uh oh!

github-actions bot commented Dec 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cpp-Linter Report ⚠️

Uh oh!

github-actions bot commented Dec 25, 2025

Uh oh!

githubdoe commented Dec 25, 2025

Uh oh!

atsju commented Dec 25, 2025

Uh oh!

gr5 commented Dec 25, 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

atsju Dec 25, 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 25, 2025 •

edited

Loading

atsju Dec 25, 2025 •

edited

Loading

githubdoe commented Dec 26, 2025 •

edited

Loading