Improve existing auto invert feature that uses conic constant to decide to invert wavefront

[gr5]

I have never been super happy about the way DFTF asks if you want to invert the wavefront. Also Yordan asked for the ability to automatically invert related to defocus term as an option instead of the SA term. For spherical mirrors.

I have an idea how to make this easier for users and also add the defocus method.

The popup would give you more options: 4 radio buttons with options to invert manually, invert assuming mirror is partly parabolized, assuming inside focus, or assuming outside focus (4 options). Then it won't ask again until you restart the program or change mirror config settings (if null value changes). Program will have a 5th mode internally which is to ask the user to choose one of 4 options again but if you choose manual it will stop bugging you.

In addition this setting will be visible in the mirror config if you need to change it and it might have the 5th option to "ask me later".

Also save in .OLN files and when loading OLN files if it doesn't match the mirror configuration then either prompt user or (better) just reset things to "ask user" mode again. If config was in "ask user" mode and OLN file has a mode then use the OLN mode for this one igram only.

If I don't get any comments or if only positive comments I'll draw this up visually to make it easier to see, understand, and talk about.

[gr5]

Here is a mockup of my proposed mirror-inverted-dialog. There are many special cases so please read above.
choosing any option makes the dialog go away
dialog comes back if you restart the program or chose to open the dialog manually
dialog comes back if you change mirror config
choice is saved in OLN files but only applied to that wavefront
choice is displayed in mirror config dialog and dialog can be relaunched from there

[githubdoe]

Ok, but I like having a specific way to turn off the auto feature like I just did. I don't like having that be a side affect of using the invert button.

[gr5]

That's what the above "manual" button does. It disables the feature. I mean I also expect the above dialog to be launchable from the mirror configuration dialog. And next to that launch button it would show which of the 4 or so modes it is in.

[githubdoe]

I think that manual button will not show up unless you are exercising a make wave front operation. So it is not universally available. It also does not show up if you have already said use auto invert.

[gr5]

exercising a make wave front operation
Not sure what you mean.

My proposed dialog would appear the same times your current dialog appears. I propose replacing your current dialog with this newer one. Also this newer one would appear when you process the first igram whether the wavefront appears to need the inversion or not.

You could launch the above dialog manually - I propose it's launched from the mirror config dialog. It would show the current state with a single word ("manual", "parabolic", "inside focus", "outside focus" (okay that's 2 words)) and next to that a button to change it. Potentially you could launch it some other way.

ALSO, if you click the invert button for a wavefront I think it should probably also reset the mode/state to manual.

There's a 5th state of course that is "null" or something. That 5th state indicates to ask the user the next time they process a wavefront. This is the start up state.

[gr5]

I'm also worried about the "parabolized" state/mode. Your current code looks at the null sign and want's S.A. to match. Is the null ever positive? Should I worry about that? Should the "parabolized" mode have a stronger explanation regarding if the null is positive? What if people are doing elliptical mirrors - should I use a different word than "parabolized"?

[githubdoe]

Yes they can do any conic. Say does not match the sign of the conic. It could have looked a the sign of the desired conic. Don't remember why it looked at the sign of the null but it is derived from the conic.

[gr5]

Oh yeah - the conic sign. Not the null. But it's basically the same thing.

Well do you like the idea in general? Why don't you do a pull request so I can merge your changes into master. No need to edit revisionHistory as I don't see a need for a release just yet. I already looked at your code and your change is quite simple and good in it's own right.

Then if you like my idea so far, I can code it up and make a video and see what you think and see what people on groups.io think.

I think the code will be pretty easy to implement. So it's not a big deal if I code it up and then throw it away.

[githubdoe]

I'm not apposed to it. I think it could be more complex than you realize. There are a lot of places where it gets used but maybe they all just call a routine.

[gr5]

Dale wrote:

Nobody has addressed my comments.

I liked most of your invert modifications. I have recently been comparing wave fronts using the subtract. I not sure the code processes that correctly if it thinks the sign of the SA is not correct. I think in this case just like in the do not use the null case it should not invert. But I think it is silently inverting the result when it things all wave front signs should match the desired conic sign.

I just looked at the code and as near as I can tell it does nothing special after the subtract. Meaning that it is going to invert the wave front quietly if it has been told to for regular processing. Please someone see what they think about this.

I was looking forward to seeing your invert dialog pop up where I could then know if it was going to try and invert. Mine of course will not pop up if already told to invert which it has been in my current session.

You made the above comments in a PR that fixes compiler warnings and fixes some things that I think will be needed for the next version of QT. But I think maybe your comments belong here? The discussion was mostly on groups.io as I wanted to bring in non-programmers.

Regarding my auto-invert proposal: I never created a pull request. It was meant to be just a discussion (although I did do all the code behind the scenes so I could make a video). Those code changes are on a branch so it can be easily ignored if I'm not doing it. I didn't think you liked any of it. It seemed like the only thing you were okay with was a small change in the text of the existing popup but no changes to code functionality and no

I also wasn't aware of an auto-invert bug in the subtraction feature but now that you mention it, I could think about that if you want.

And not only did it seem like you (and others) didn't like any of my proposals, it seemed like no one else was excited about the changes.

[gr5]

Okay, Dale and I had a few emails back and forth and we settled on a GUI for this feature. It's done but I need to merge in master. But should I merge in 7.4 or 8.0?

I guess I'll do both and commit both merges so I can go back to 7.4 if needed but I don't see why it would be needed.

I'll do the merges and then create a PR today.

[atsju]

You mean you need to merge master into feature branch. Yes merging 7.4 and then 8.0 into feature is a good idea. Then open PR to merge feature into master.

For the PR merge, can it wait my review October 10th or so ? No joke I'm currently at grand canyon.