Skip to content

Fix potential dip in profile plot when using Gaussian blur + minor display issues - #359

Merged
gr5 merged 4 commits into
masterfrom
JST/333
Aug 10, 2026
Merged

Fix potential dip in profile plot when using Gaussian blur + minor display issues#359
gr5 merged 4 commits into
masterfrom
JST/333

Conversation

@atsju

@atsju atsju commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

fix #333

Problem was due to Gaussian blur not using same inclusive/exclusive boundaries as cicle mask code (second commit)
I also changed some float to int truncature into rounding as small epsilon could cause issues with sin/cos (first commit).

Fix #319
Fix #317

@atsju
atsju requested review from githubdoe and gr5 August 5, 2026 08:48
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

🚀 New build available for commit 99c48c9
Download installer here

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

🚀 New build available for commit 4e983c2
Download installer here

@atsju atsju changed the title Fix potential dip in profile plot when using Gaussian blur Fix potential dip in profile plot when using Gaussian blur + minor dispaly issues Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

🚀 New build available for commit a7ebba5
Download installer here

@atsju atsju changed the title Fix potential dip in profile plot when using Gaussian blur + minor dispaly issues Fix potential dip in profile plot when using Gaussian blur + minor display issues Aug 5, 2026
@gr5

gr5 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Something wrong with github - these "download installer here" links don't work and if I go to actions to get them from there - also not working. Instead I have to build these myself. No big deal but I've heard that github has lots of new bugs related to AI somehow?

@atsju

atsju commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

Very strange. I can download the installers (I did not even build once on my own computer this week).
I didn't hear of new bugs but sure GitHub business model is changing for enterprise users.

@atsju

atsju commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

It's probably easier you just wait for it to work again.
Eventually you can check if you can download artifacts form a pull request openned by yourself. And I will try your artifact. This would cross check if it's an issue with rights/privilege management.

Comment thread utils.cpp
Comment on lines 205 to 208
// Right now we shrink outer outline by 2 pixels and enlarge inner outline by 1 pixel. As far as I can tell, this is an arbitrary
// hack. These values should at a minimum be in some constant somewhere or better, part of the outline class or maybe we should
// get rid of these alltogether? For now, know that if you mess with it one one place in DFTFringe, you have to mess with it
// in other places as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

double radm =wf->m_outside.m_radius + outsideOffset-2;

rado += (insideOffset + 1);

From my understanding the arbitrary shrink comes from there and this is the "other place" to mess with.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes. Probably. It's tough to know if one caught all the spots.

@gr5 gr5 mentioned this pull request Aug 7, 2026
4 tasks

@gr5 gr5 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please don't merge if we are doing a release where I don't have a chance to test every feature in DFTF.

I tested the hell out of this feature (odd and even diameter wavefronts, averaging wavefronts, subtracting, ronchi, foucault, saving and loading wavefronts) and it looks fantastic.

But it's a little crazy how changing one feature can affect a feature that is seemingly unrelated so I'd like to test everything else e.g. "test stand astig removal" on the next release with this code (again - it shouldn't be affected but I've gotten burned before).

@gr5

gr5 commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

To be clear, I recommend we merge unless we are doing a release in the next 8 days.

@gr5
gr5 merged commit a2f5703 into master Aug 10, 2026
14 checks passed
@atsju
atsju deleted the JST/333 branch August 10, 2026 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants