Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix the issue 1512 #1588

Closed
wants to merge 1 commit into from
Closed

Conversation

kunmonster
Copy link
Contributor

I have found the true reason of the issue #1512. It is caused by multiple monitors.

When you have two monitors and set your primary screen to the right. You open the IGV in the main screen and drag the window of IGV to the other screen, then turn off IGV , the setApplicationFrameBounds(Rectangle bounds) will store a IGV.Bounds with an x-coordinate less than the negative width of the screen. At this point, if you unplug the secondary monitor and open IGV again, you will reproduce this problem.

The original code only considers another case. When the main display is on the left,.In that scenario,if you open IGV on the main display and then drag the IGV window to the secondary display before closing IGV. setApplicationFrameBounds(Rectangle bounds) will save an x-coordinate ​​greater than width of the screen . The original code can handle this case, but it does not account for the situation described above.

To address this, I added additional conditions to handle the aforementioned scenario, as well as to check the y-coordinate when creating the window and closing IGV, ensuring minimum width and height (300, 300).

… -width or the value of y is less than -height,the windows will be hidden.
@jrobinso
Copy link
Contributor

Thanks for the investigation and PR! Perhaps we should make the minimums slightly larger, 800 wide X 600 height? I don't think there are any displays that can't handle that. What do you think?

@jrobinso
Copy link
Contributor

jrobinso commented Sep 27, 2024

Actually, elsewhere (in IGV line 250) we make the minimum 1150 x 800. We should probably be consistent. What is the smallest screen resolution we can imagine is still being used?

 int width = Math.min(1150, (int) screenBounds.getWidth());
int height = Math.min(800, (int) screenBounds.getHeight());

@kunmonster
Copy link
Contributor Author

Thanks, actually when I reivewd your code,I noticed the (1150,800) too ,but at the line 243 in IGV.java you set a minimum threshold (300,300) mainFrame.setMinimumSize(new Dimension(300, 300));

In my opinion, what we need to do is to provide users with an operable interface where they can adjust the size themselves, rather than the interface being unable to open.So, I think it is unnecessary to change the minimum threshold.

I am consistent with your values(300X300), ensuring that the smallest 300x300 frame can be displayed on the desktop. In fact, after testing, as long as the width and height are not adjusted to numbers less than 300 manually, it is almost impossible for these two values ​​to be 300.

In addition, when the situation of issue #1512 occurs, the window will inevitably be reset to (0,0,1150,800)

@kunmonster
Copy link
Contributor Author

Have you successfully reproduced the problem using my method? I have tested it many times on my machine and it can be reproduced.

@jrobinso
Copy link
Contributor

jrobinso commented Sep 28, 2024 via email

@kunmonster
Copy link
Contributor Author

Sure,you can just let IGV.Bounds=x,y,width,height like (x+width)<0 or (y+height) <0

Like IGV.Bounds=30,-926,1150,800(from #1512) or -1200,0,1150,800.

I haven't been able to reproduce the issue, but that really doesn't matter. IGV should open regardless of the values of IGV.Bounds. Could you just post an example "Bounds" value that produces the issue. That should suffice.

On Fri, Sep 27, 2024 at 6:14 PM kunmonster @.> wrote: Have you successfully reproduced the problem using my method? I have tested it many times on my machine and it can be reproduced. — Reply to this email directly, view it on GitHub <#1588 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHD2HANWH36OHTENGIG2JLZYX7JFAVCNFSM6AAAAABO7KLPHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBQGMZTINZYHE . You are receiving this because you commented.Message ID: @.>

@kunmonster
Copy link
Contributor Author

  if (applicationBounds == null || applicationBounds.getMaxX() > screenBounds.getWidth() ||
              applicationBounds.getMaxX() < 300 ||  //new
                 applicationBounds.getMaxY() > screenBounds.getHeight() ||
                applicationBounds.getMaxY() < 300 || //new
                applicationBounds.width == 0 || applicationBounds.height == 0) {
            int width = Math.min(1150, (int) screenBounds.getWidth());
            int height = Math.min(800, (int) screenBounds.getHeight());

The code in your version you just considers the upper limit but the nether of getMaxX() and getMaxY(). If one of the above values is $&lt;=0$ then there is a problem.

@jrobinso
Copy link
Contributor

Actually I still can't reproduce it, even with IGV.Bounds=-100000000,75,1150,800

@jrobinso
Copy link
Contributor

This could be due to a difference in platforms or even java implementations. My setup on MacOS / Java 17 is perfectly happy to accept absurd values for X and Y here.

        mainFrame.setBounds(applicationBounds);

@kunmonster
Copy link
Contributor Author

kunmonster commented Sep 28, 2024

Yes,the problem only occurs in windows. #1512 is based on windows operating system.If you have a pc with windows,you can have a try.And whatever version of java , the problem always exists.

@jrobinso
Copy link
Contributor

However we shouldn't feed it absurd bounds, but I don't think your fix will not work in general, it would not work on my Mac. At least not in the way expected. I have 2 monitors, my laptop screen itself is to the left of my main screen. If I drag IGV to that screen and shut it down the bounds are

IGV.Bounds=-1734,103,1150,800

Note the maxX here is negative. If I start IGV it opens just where I left it. If I set X to 0, it opens on the main screen. Not where I left it.

@jrobinso
Copy link
Contributor

I don't unfortunately (have a Windows machine). So is the assertion that Windows can't handle negative values for the bounds? Perhaps it interprets the coordinates for multiple screens differently. We might need a platform test, which is unfortunate, or we just never allow negative coordinates and users on other machines will have to perhaps continually drag IGV to where they want it, which could get annoying.

@kunmonster
Copy link
Contributor Author

Actually,if the MaxX or MaxY is negative, the mainframe will doesn't show in windows. Actually I have tested in windows , if you plug the secondary monitor at the point of above,the igv will show in the left monitor, but if you don't plug it ,it will down .

I know your consideration,you think with my code,the igv will doesn't show in the same place as last position .But we don't do that ,it doesn't work in windows.May I should take into platform into account?

@kunmonster
Copy link
Contributor Author

I mean I can consider different platforms, and directly test it under Linux as well, taking all three platforms into account.

@jrobinso
Copy link
Contributor

On Windows, can the auxillary screen be the "main" screen? If it is, and and other screen (presumably the laptop) is to the left, what are IGV bounds if you drag it over there? Is the X and Y negative?

@kunmonster
Copy link
Contributor Author

kunmonster commented Sep 28, 2024

Sure, the auxiliary screen can be main monitor in windows.But I haven't tried the case you described ,latter I will try and report it to you.If you give me a email ,I could give you a video or picture when the problem occurs .

@jrobinso
Copy link
Contributor

I think the right solution for this is going to involve gathering information on all monitors, perhaps with GraphicsDevice getScreenDevices(), then figuring out the permissible application bounds. Again this would be on startup, not when saving the preferences. I can take a crack at this for a Mac, since that's what I have. Ideally we won't need a platform test if we consider the actual environment.

I'm shutting down for today.

@jrobinso
Copy link
Contributor

Well I couldn't resist trying. Here's what I get for my 2 screens in their current configuration

0.0,0.0 2560.0x1440.0
-1792.0,0.0 1792.0x1120.0

Code below. Could you try this on Windows with your screens in both configurations (left / right)? I think we could possibly come up with a general rule that won't involve platform testing.

        GraphicsEnvironment ge = GraphicsEnvironment.getLocalGraphicsEnvironment();
        GraphicsDevice[] gs = ge.getScreenDevices();
        for(GraphicsDevice curGs : gs)
        {
            GraphicsConfiguration[] gc = curGs.getConfigurations();
            for(GraphicsConfiguration curGc : gc)
            {
                Rectangle bounds = curGc.getBounds();
                System.out.println(bounds.getX() + "," + bounds.getY() + " " + bounds.getWidth() + "x" + bounds.getHeight());
            }
        }

@kunmonster
Copy link
Contributor Author

Sure ,but may be 20 min later,because in my region it's lunchtime.

@kunmonster
Copy link
Contributor Author

I have got the value .
Auxiliary screen as main screen on the left of lap screen as secondary screen : 0.0,0.0 1739.0X978.0 1920.0,0.0 1739x978.0
Auxiliary screen as secondary screen on the left of lap screen as main screen: -1920.0,0.0 1739x978.0 0.0,0.0 1739.0X978.0
Lap screen as main screen on the left of auxiliary screen as secondary screen: 1920.0,0.0 1739x978.0 0.0,0.0 1739.0X978.0
Lap screen as secondary screen on the left of auxiliary screen as main screen: 0.0,0.0 1739.0X978.0 -1920.0,0.0 1739x978.0

@jrobinso
Copy link
Contributor

OK, so here is a possible strategy that should work on all platforms.

(1) Gather all monitor bounds
(2) Use the application bounds x,y value to assign IGV to one of the monitors. If its out-of-bounds of all monitors assign x,y to 0,0 (main screen)
(3) Insure that IGV's bounds are within the bounds of the selected monitor.

That's my contribution for this evening, I really am signing off now.

@kunmonster
Copy link
Contributor Author

Thank you, take a rest now.

@kunmonster
Copy link
Contributor Author

Actually there is another question , when the bottom right corner of the window exceeds the bottom right corner of the screen,the condition applicationBounds.getMaxX() > screenBounds.getWidth() and applicationBounds.getMaxY() > screenBounds.getHeight() will be triggered, then the (x,y) will be set to (0,0) ,so the position will be changed when the next opening.

@jrobinso
Copy link
Contributor

jrobinso commented Sep 29, 2024

I think my suggestion above will cover all cases. If screens are in the same configuration the conditions of application bounds max X and Y will never be > screenbounds width and height. If screen configurations have changed reverting to default position is fine. However I think it can be simplified, this should cover all cases:

(1) Gather all monitor bounds (code snippet above)
(2) Search for a monitor whose screen bounds contains application bounds. If a monitor is found assign the application bounds as recorded. If no monitor is found assign default bounds

This would replace the unwieldy tests currently in place.

This assumes that there is never a case when the IGV window spans multiple screens. This is not possible on a Mac, I'm not sure about other platforms. o cover that case the test (2) would be modified to test that the union of all screen bounds contains the application bounds. But I think I would wait for someone to raise an issue before covering that case as it may not even be possible.

@jrobinso
Copy link
Contributor

If you want to implement and test this on your machine, since you can reproduce the error, please do so and update the PR. I can implement it but can't really test since the Mac never complains about the bounds.

@kunmonster
Copy link
Contributor Author

Ok,I will realize it with your thoughts.

@kunmonster kunmonster closed this Sep 29, 2024
@kunmonster kunmonster mentioned this pull request Oct 1, 2024
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.

2 participants