Average profile by githubdoe · Pull Request #313 · githubdoe/DFTFringe

githubdoe · 2026-01-05T06:48:39Z

Fixed the highlight slope error bug.
close #312

… into averageProfile updateing from remote

gr5 · 2026-01-05T13:23:39Z

The errors above are pretty easy to fix. the qt5 pro file was never edited (that causes the qt5 errors). The main pro file seems to have an error in the pro file. I might just do the fixes myself but try building myself later today.

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

github-actions · 2026-01-05T16:09:08Z

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

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

profileplot.cpp:431:5: warning: [clang-analyzer-security.FloatLoopCounter]

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

  431 |     for (double rad = -1.0; rad < 1.0; rad += steps) {
      |     ^                       ~~~        ~~~
profileplot.cpp:431:5: note: Variable 'rad' with floating point type 'double' should not be used as a loop counter
  431 |     for (double rad = -1.0; rad < 1.0; rad += steps) {
      |     ^                       ~~~        ~~~

Have any feedback or feature suggestions? Share it here.

github-actions

Cpp-linter Review

Used clang-tidy v18.1.3

Have any feedback or feature suggestions? Share it here.

github-actions

Cpp-linter Review

Used clang-tidy v18.1.3

Have any feedback or feature suggestions? Share it here.

github-actions

Cpp-linter Review

Used clang-tidy v18.1.3

Have any feedback or feature suggestions? Share it here.

github-actions · 2026-01-05T16:13:54Z

🚀 New build available for commit b8fdb79
Download installer here

github-actions · 2026-01-05T16:14:07Z

🚀 New build available for commit fc97675
Download installer here

github-actions · 2026-01-05T16:14:26Z

🚀 New build available for commit 2c01f57
Download installer here

atsju · 2026-01-05T17:13:03Z

Even if this is old code, I suggest we get rid of the for loops using double I had to ask AI to explain the logic behind the loop while it's just going from minus wf->m_outside.m_radius to plus wf->m_outside.m_radius pixel by pixel.

Would be easier to read, less bugprone and cleaner.

Do you agree or do I leave it alone ?

github-actions

Cpp-linter Review

Used clang-tidy v18.1.3

Have any feedback or feature suggestions? Share it here.

github-actions · 2026-01-05T17:14:07Z

-    for (double rad = -1.; rad < 1.; rad += steps){
-        int dx, dy;
+    // Main Sampling Loop
+    for (double rad = -1.0; rad < 1.0; rad += steps) {


clang-tidy diagnostic

profileplot.cpp:431:5: warning: [clang-analyzer-security.FloatLoopCounter]

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

431 | for (double rad = -1.0; rad < 1.0; rad += steps) { | ^ ~~~ ~~~ profileplot.cpp:431:5: note: Variable 'rad' with floating point type 'double' should not be used as a loop counter 431 | for (double rad = -1.0; rad < 1.0; rad += steps) { | ^ ~~~ ~~~

I think it's fine as is. I don't think the outer radius (wf->m_outside.m_radius) is specified in an integral number of pixels anyway - that's a float that can be for example 300.5 (it can be integral or on the halves).

Yes you could have a fp summation rounding error and get one too few in the profile plot. At the end of the plot. At this time I don't think it's a problem (I don't think you can get one too many as there is also code to see if we go beyond the outer radius).

What I don't love is that we aren't grabbing the nearest wavefront point under the line but rounded down to the nearest integer: wf->workData(dy, dx). Dy could be 100.9 and we get the point from dy position 100.

Ideally do a weighted average of the nearest 4 points, weighted by the distance to each point. And handle edge cases where some of the 4 points are outside the mask. We do all that when we rotate wavefronts if I remember right.

But really it's all fine as is.

yes 0.5 pixels. This makes it more difficult to "fix" than first reading.
OK for me.

github-actions · 2026-01-05T17:17:12Z

🚀 New build available for commit 91c4d4b
Download installer here

gr5 · 2026-01-05T18:25:37Z

I just played with this PR and I love the new way of doing surface error. I think we should go ahead and merge.

I'm hoping we can do a release today as well. I promise to test this version a lot more than usual. I have a QA list now that I go through. It's not 100% comprehensive but will hopefully catch everything important.

I'll go add this feature to the revisionHistory PR...

githubdoe · 2026-01-05T18:31:06Z

Thanks all guys. I notice that sometimes not all when more than one wave is selected and you select show all 3D surface it displals them twice. I don't know the trigger. But it happens most when a new wave front has been added to the list. I putting that here just to see if you see it and to test that.

gr5 · 2026-01-05T19:11:28Z

and you select show all 3D surface

You mean this button below?

I think it just displayed the entire window with the expected 3d plots but it created 2 windows. I think. Maybe. Is that what you mean? I think I closed the window and there was another one behind it. I can't duplicate.

githubdoe · 2026-01-05T19:48:11Z

Yes two windows sometime. Usually the first time and sometime then again.

gr5 · 2026-01-05T20:10:34Z

Well it's not a serious problem for the user. I guess somehow I double clicked on the button? I mean I tried to double click on purpose and it didn't do it.

I'll keep my eye out to simplify duplicating the issue.

githubdoe · 2026-01-05T20:48:32Z

As far as I can tell I don't double click the button yet it happens. Sometimes for many times in a row. But, yes it is not too much of a problem. Eventually when we can figure out a cause then write up an issue.

githubdoe · 2026-01-05T20:50:47Z

The double in the for loop is not going to cause any problem. It gives a much better idea as to what the loop is doing than if you tried something else. It could be an issue if you were comparing to equal but not with a less than or greater than.

githubdoe added 2 commits January 4, 2026 21:26

reworked profile slope highlight and ploting obstruction

f82eadb

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

aea8375

… into averageProfile updateing from remote

atsju reviewed Jan 5, 2026

View reviewed changes

Comment thread DFTFringe.pro Outdated

Comment thread profilecurve.cpp

Comment thread profileplot.cpp Outdated

Comment thread profileplot.cpp Outdated

gr5 and others added 3 commits January 5, 2026 11:00

Update DFTFringe.pro

fc97675

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

Update profilecurve.cpp

2c01f57

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

Update profileplot.cpp

b8fdb79

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

github-actions Bot reviewed Jan 5, 2026

View reviewed changes

Comment thread profilecurve.cpp

Comment thread profileplot.cpp

github-actions Bot reviewed Jan 5, 2026

View reviewed changes

Comment thread profileplot.cpp

github-actions Bot reviewed Jan 5, 2026

View reviewed changes

Comment thread profileplot.cpp

fix QT5 build and add const

91c4d4b

github-actions Bot reviewed Jan 5, 2026

View reviewed changes

atsju self-requested a review January 5, 2026 17:46

atsju approved these changes Jan 5, 2026

View reviewed changes

gr5 merged commit 3e1a9b7 into master Jan 5, 2026
14 checks passed

Conversation

githubdoe commented Jan 5, 2026 • edited by atsju Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gr5 commented Jan 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cpp-Linter Report ⚠️

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

Used clang-tidy v18.1.3

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

Used clang-tidy v18.1.3

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

Used clang-tidy v18.1.3

Uh oh!

Uh oh!

github-actions Bot commented Jan 5, 2026

Uh oh!

github-actions Bot commented Jan 5, 2026

Uh oh!

github-actions Bot commented Jan 5, 2026

Uh oh!

atsju commented Jan 5, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Cpp-linter Review

Used clang-tidy v18.1.3

Uh oh!

github-actions Bot Jan 5, 2026

Choose a reason for hiding this comment

clang-tidy diagnostic

Uh oh!

gr5 Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

atsju Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jan 5, 2026

Uh oh!

gr5 commented Jan 5, 2026

Uh oh!

githubdoe commented Jan 5, 2026

Uh oh!

gr5 commented Jan 5, 2026

Uh oh!

Uh oh!

githubdoe commented Jan 5, 2026

Uh oh!

gr5 commented Jan 5, 2026

Uh oh!

githubdoe commented Jan 5, 2026

Uh oh!

githubdoe commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

githubdoe commented Jan 5, 2026 •

edited by atsju

Loading

github-actions Bot commented Jan 5, 2026 •

edited

Loading

githubdoe commented Jan 5, 2026 •

edited

Loading