igvteam / igv

Integrative Genomics Viewer. Fast, efficient, scalable visualization tool for genomics data and annotations
https://igv.org
MIT License
647 stars 386 forks source link

fix the issue 1512 #1588

Closed kunmonster closed 1 month ago

kunmonster commented 1 month ago

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).

jrobinso commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

jrobinso commented 1 month ago

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 https://github.com/igvteam/igv/pull/1588#issuecomment-2380334789, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHD2HANWH36OHTENGIG2JLZYX7JFAVCNFSM6AAAAABO7KLPHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBQGMZTINZYHE . You are receiving this because you commented.Message ID: @.***>

kunmonster commented 1 month ago

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 commented 1 month ago
  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 $<=0$ then there is a problem.

jrobinso commented 1 month ago

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

jrobinso commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

jrobinso commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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

kunmonster commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Thank you, take a rest now.

kunmonster commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

Ok,I will realize it with your thoughts.