Skip to content

Conversation

@atsju
Copy link
Collaborator

@atsju atsju commented Nov 8, 2025

Low priority but nice small change.
test it out and we can have it in next release

list of issues that still need to be fixed:

  • ensure square ratio in basic situations of normal use (load igram, load wft, switch between wfts, rescale window, ...)
  • fix If you zoom in but don't zoom out of right side wavefront window then load new wavefront you can't zoom out #56 noticed by gr5 : if you zoom in on the contour and then load another wavefront, it doesn't zoom out (you are still zoomed in on the same area but on the new wavefront) and if you try to zoom out it won't because it thinks it already is zoomed out.
  • change scale of window then zoom and unzoom : results in non-square. New
  • fullscreen. Zoom then unzoom will result in deformed picture.
  • acceptable ? zoom doesn't ensure square ratio. zooming rectangle will expand more one direction than other
  • acceptable ? when not large enough in X direction, you cannot zoom anymore
  • colorscale is correct. Currently uses size as scale
  • show all selected wavefronts must work properly
  • report generation must work properly

@atsju atsju requested review from githubdoe and gr5 November 8, 2025 17:22
@githubdoe
Copy link
Owner

I would like to review it by using it. However I don't know how to get a build with that in it but not released. I don't really want to upgrade to Qt6 yet so I probably can't build that code. If I could then perhaps just switching to master and pulling the latest will work. I don't know. I'm busy as always. So don't have much time to commit to it.

@atsju
Copy link
Collaborator Author

atsju commented Nov 8, 2025

Dale here you go : https://github.com/githubdoe/DFTFringe/actions/runs/19196205264/artifacts/4508565928

Each time a PR is opened the installer is automatically created. I can make a tutorial for you to access it for you to store and remember or you can ask and I paste link. No problem.

@githubdoe
Copy link
Owner

First attempt gives this display
image

@atsju
Copy link
Collaborator Author

atsju commented Nov 9, 2025

I tested only Linux as windows build is very long... I have been too confident .
I will fix that. Thanks.

@githubdoe
Copy link
Owner

By the way I actually use the fact the contour displays dimensions values in pixels. That is the fastest way to check the size that someone used for their wave front analysis. I would be disappointed if that changed.

@atsju
Copy link
Collaborator Author

atsju commented Nov 9, 2025

OK it fails only if you make "files -> read wavefront" first thing.
It works with igrams and it works if you resize the window.

Probably a small fix.
image

@github-actions
Copy link

github-actions bot commented Nov 9, 2025

🚀 New build available for commit f87779e
Download installer here

@atsju
Copy link
Collaborator Author

atsju commented Nov 9, 2025

This one works. You can test with link from comment above. Low priority

linux builds are failing for server reason. Should be fine in couple of hours.

@github-actions
Copy link

🚀 New build available for commit 52bb6bb
Download installer here

@atsju atsju force-pushed the JST/squareContourview branch from 8356209 to f87779e Compare November 10, 2025 07:46
@github-actions
Copy link

🚀 New build available for commit 8356209
Download installer here

@github-actions
Copy link

🚀 New build available for commit f87779e
Download installer here

@atsju atsju force-pushed the JST/squareContourview branch from 51ae0d3 to 4aad6aa Compare November 10, 2025 08:00
@github-actions
Copy link

🚀 New build available for commit 51ae0d3
Download installer here

@atsju atsju force-pushed the JST/squareContourview branch from 4aad6aa to f87779e Compare November 10, 2025 08:07
@github-actions
Copy link

🚀 New build available for commit 4aad6aa
Download installer here

@github-actions
Copy link

🚀 New build available for commit f87779e
Download installer here

@atsju
Copy link
Collaborator Author

atsju commented Nov 10, 2025

I found an other problem. Zoom is not working when X axis is shorted then Y axis.
To be investigated.

@atsju atsju marked this pull request as draft November 10, 2025 08:30
@atsju
Copy link
Collaborator Author

atsju commented Nov 10, 2025

right click to come back from zoom also does not enforce square ratio

@gr5
Copy link
Collaborator

gr5 commented Nov 10, 2025

While you are fixing this code, please fix the other one which is issue #56

if you zoom in on the contour and then load another wavefront, it doesn't zoom out (you are still zoomed in on the same area but on the new wavefront) and if you try to zoom out it won't because it thinks it already is zoomed out.

@atsju atsju force-pushed the JST/squareContourview branch from f87779e to 5a7d867 Compare November 16, 2025 07:25
@github-actions
Copy link

🚀 New build available for commit 5a7d867
Download installer here

@github-actions
Copy link

🚀 New build available for commit 87bb80e
Download installer here

@atsju atsju force-pushed the JST/squareContourview branch from f650992 to c7a70b5 Compare November 16, 2025 09:50
@github-actions
Copy link

🚀 New build available for commit f650992
Download installer here

@github-actions
Copy link

🚀 New build available for commit c7a70b5
Download installer here

@gr5
Copy link
Collaborator

gr5 commented Nov 18, 2025

Are you still working on this @atsju? It's still in draft mode. Just checking in case you forgot.

I think I'll fix something easy today from the issues list.

@atsju
Copy link
Collaborator Author

atsju commented Dec 6, 2025

Looks good in main window but still a problem with this feature here as shown:

Nice catch thanks

@githubdoe
Copy link
Owner

Also check the view in the report if you have not already.

@github-actions
Copy link

github-actions bot commented Dec 7, 2025

🚀 New build available for commit 2cbc169
Download installer here

@atsju
Copy link
Collaborator Author

atsju commented Dec 7, 2025

Ready for testing

@github-actions
Copy link

github-actions bot commented Dec 7, 2025

🚀 New build available for commit 4be683a
Download installer here

@gr5
Copy link
Collaborator

gr5 commented Dec 8, 2025

Didn't get to this today. Hope to look at it tomorrow.

@gr5
Copy link
Collaborator

gr5 commented Dec 10, 2025

Also check the view in the report if you have not already.

It's pretty bad but at least it's round. This is how it is in v8.2 also but I don't know how to fix it. I think it's been too large for a few versions. Anyway it's no worse than v8.2.

image

@gr5
Copy link
Collaborator

gr5 commented Dec 10, 2025

@atsju - I found a bug. If I lower the resolution to 500 you can't zoom in anymore. You did fix my old zoom in bug. And I like your solution. But there is a new bug (maybe an old bug?).

So load an igram and when you get to the DFT page, lower the resolution to 500 (I initially found the problem when I lowered it from 600 to 300 but 500 seems to be low enough). Then click "recompute DFT" button. Then "compute surface" as you normally would.

Now you can't zoom in on the contourplot.

I think you can get it in a bad state where other contours at 600 stop being able to zoom in but I couldn't duplicate that.

This bug was hard for me to isolate so it can be duplicated every time but here you go - just do what I said above.

I even tried deleting everything in my registry under DFTFringe and restarting dftfringe to be sure this happens every time.

Version 8.2.0 is fine with zooming in when the resolution is 500 pixels.

@gr5
Copy link
Collaborator

gr5 commented Dec 10, 2025

If you have any trouble duplicating, try lowering to 300 pixels resolution.

@gr5
Copy link
Collaborator

gr5 commented Dec 11, 2025

By the way, I really love this feature. I played with it quite a bit and when I went back to v8.2.0 I was surprised to notice how annoying it is to have an elliptical contourview.

@atsju
Copy link
Collaborator Author

atsju commented Dec 11, 2025

I will look at the bug. Thank you for extensive testing.

Also check the view in the report if you have not already.

It's pretty bad but at least it's round. This is how it is in v8.2 also but I don't know how to fix it. I think it's been too large for a few versions. Anyway it's no worse than v8.2.

This I will not fix now. As you said, it's brocken for a while. I already openned #271 so we keep track.

@atsju
Copy link
Collaborator Author

atsju commented Dec 14, 2025

@gr5 I think this is now fixed. It was a race condition in display updates.
Solving it also solver a minor think I had identified "acceptable ? when not large enough in X direction, you cannot zoom anymore"

@github-actions
Copy link

🚀 New build available for commit 8ad3499
Download installer here

@gr5
Copy link
Collaborator

gr5 commented Dec 15, 2025

I found two bugs/issues which may or may not be related to your changes.

The first one: did we get rid of elliptical mirrors recently? I know we talked about it as being a special case and how they work differently from the normal process but I didn't think we took it completely away. Did we?

Anyway either I just forget how to do it or we can no longer trace "elliptical flats".

@gr5
Copy link
Collaborator

gr5 commented Dec 15, 2025

The other issue is that you seemed to have merged in master so you have my newer code that lets the right tool bar be smaller. But it's now ALWAYS right at the minimum. I can't make it bigger. And when I did that PR, I put the minimum very small so things overlap a bit and you can't quite read some of the buttons and such. It's not ideal.

When I try to make the right tool bar bigger, the mouse flickers to the other mouse pointer shape and when I try to click and drag, it just does buggy things. But it never allows me to make the right toolbar bigger. Sometimes it does nothing, sometimes it dissappears (and recovers if I just click over there anywhere).

I'm thinking @atsju that you changed something: maybe the code that deals with the "teal" separators or maybe the code that deals with the mouse dragging to zoom in on the contour plot. Anyway, for some reason I can't change the size of the right tool bar anymore.

@atsju
Copy link
Collaborator Author

atsju commented Dec 15, 2025

I found two bugs/issues which may or may not be related to your changes.

The first one: did we get rid of elliptical mirrors recently? I know we talked about it as being a special case and how they work differently from the normal process but I didn't think we took it completely away. Did we?

Anyway either I just forget how to do it or we can no longer trace "elliptical flats".

Last time we discussed Elliptical whas there #272
I don't think we removed the feature but I never used it.
Good idea to test it. I didn't for the moment. mirror config=> "Elliptical flat or nulled optic" is probably what you need

@atsju
Copy link
Collaborator Author

atsju commented Dec 15, 2025

The other issue is that you seemed to have merged in master so you have my newer code that lets the right tool bar be smaller. But it's now ALWAYS right at the minimum. I can't make it bigger. And when I did that PR, I put the minimum very small so things overlap a bit and you can't quite read some of the buttons and such. It's not ideal.

When I try to make the right tool bar bigger, the mouse flickers to the other mouse pointer shape and when I try to click and drag, it just does buggy things. But it never allows me to make the right toolbar bigger. Sometimes it does nothing, sometimes it dissappears (and recovers if I just click over there anywhere).

I'm thinking @atsju that you changed something: maybe the code that deals with the "teal" separators or maybe the code that deals with the mouse dragging to zoom in on the contour plot. Anyway, for some reason I can't change the size of the right tool bar anymore.

I will have a look. I did not change the code in the way you think. However to keep things square during resize, there are many QT events and I now recompute canva and picture ratio when you drag any menu.
I do always drag the middle separator I did not try the right bar menu. I will try to reproduce and fix.

@gr5
Copy link
Collaborator

gr5 commented Dec 15, 2025

Okay so "elliptical secondary mirrors" that are wider than tall work fine with your new code. I mean it works really well!

Ones that are taller than wide chop off the top and bottom. But this happened with the old code as well. If there is a fix it might be to enforce that the images are horizontally wider than tall.

I don't think you should worry about this.

@atsju
Copy link
Collaborator Author

atsju commented Dec 16, 2025

Hi @gr5

I tried the lastest installer and I reproduce same behavior at 125% I already signaled when reviewing the right tab size PR
#295 (comment)
But I did not reproduce the mouse flickering or anything you described here. I'm able to resize right tab without problem.

My process is:

  • full screen 100%
  • load 1 igram
  • compute
  • move righ side bar
  • everything adjusts fine

Could you share a video capture of your test/behavior ? It's not urgent at all though. No pressure!
Also it might be worth it installing master CI build just to confirm the behavior is linked to this PR.

I feel a bit sorry you do all the testing and some things I could have seen. So I really want to thank you for this work and the effort you put here !

@gr5
Copy link
Collaborator

gr5 commented Dec 17, 2025

I used the version of DFTF above that github created where it says "download installer here". It says "3 days ago" on the post. about 7 posts above. I'll create a video of the issue and post on youtube and put a link to it here.

https://www.youtube.com/watch?v=wvbl5Mw1j3g

Okay I can't duplicate the problem. And I tried very hard. So I'll approve it. But there is still an issue - please watch the video. Actually I'll create an issue that links to this video.

@atsju
Copy link
Collaborator Author

atsju commented Dec 17, 2025

Thank you again for extensive testing and the video.
I don't think the problem you observed is linked specifically to this PR. Good think you have opened an issue for tracking. And the video is an amazing way to explain, I really like it.

@githubdoe do you have any remarks before we merge this ?

@gr5 gr5 merged commit 470001a into master Dec 22, 2025
14 checks passed
@atsju atsju deleted the JST/squareContourview branch December 23, 2025 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

If you zoom in but don't zoom out of right side wavefront window then load new wavefront you can't zoom out

4 participants