Positioning shells correctly for multi zoom level

[amartya4256]

This contribution enforces displays to store x,y coordinates in point as the absolute pixels in the display coordinate system. while scaling the width and height to the points, following this the map methods are reimplemented to do the right conversion between the new display coordinate system and the coordinates within a control.

The top left coordinates of monitors (x, y) are stored in the absolurte pixels coordinate of the display to avoid any overlap of any monitor bounds on scaling down as per their zoom level, i.e., to locate the monitor for a point, we can easily skim and find which monitor it belongs to, hence confirming the zoom needed for scaling.

e.g. Monitor A: 100 x 100 (zoom: 100%, x-coordinates: 0 - 100) - On scaling down: 0 - 100
Monitor B: 100 x 100 (zoom: 100%, x-coordinates: 100 - 200) - On scaling down: 50 - 100
Here, the x-coordinates of A and B overlap, hence for a point at a location between (50, 100), there is ambiguity.

With the new system the scaledDown coordinates will be:
Monitor A: 0 - 100
Monitor B: 100 - 150
and scaling up: 0 - 100, 100 - 200, respectively

contributes to #62 and #127

[github-actions]

Test Results

   486 files  ±0     486 suites  ±0   9m 13s ⏱️ +9s
 4 159 tests ±0   4 151 ✅ ±0   8 💤 ±0  0 ❌ ±0 
16 390 runs  ±0  16 298 ✅ ±0  92 💤 ±0  0 ❌ ±0 

Results for commit b88965d. ± Comparison against base commit 8c39dcd.

♻️ This comment has been updated with latest results.

[HeikoKlare]

Can you elaborate in more detail what this change is supposed to achieve and how to test it? I tried around a bit with multiple shells on different monitors with different scale values and the behavior is still quite broken at the border between the monitors. In particular, a shell spanning multiple monitors does not work as a proper drop target (but maybe that's not even a goal of this PR). And the region preview for a detached shell does not work properly when the new shell spans multiple monitors.

In the following video, there are two monitors next to each other (each using half of the horizontal space):

[akoch-yatta]

Can you elaborate in more detail what this change is supposed to achieve and how to test it? I tried around a bit with multiple shells on different monitors with different scale values and the behavior is still quite broken at the border between the monitors. In particular, a shell spanning multiple monitors does not work as a proper drop target (but maybe that's not even a goal of this PR). And the region preview for a detached shell does not work properly when the new shell spans multiple monitors.

Regarding the second issue with the region preview for a detached shell. That is because we are identifying the monitor change only based on x and y, but we should do it by having more than 51% on the monitor, as it aligns with the scaling behavior of windows.

About the PR in general. We want to fix the issues with Shell positioning -> having them positioned on the wrong monitor or even outside of the visible area. But as we are changing the monitor bounds for that, we have a risk of breaking things.

[amartya4256]

We have one limitation with the current implementation. If the top right corner of the shell is in another window, it considers the monitor to be the one where this point is and doesn't lay the bounds properly. An idea would be to use the middle of the rectangle to find the monitor. Or to find which monitor has most area of the rectangle (since the center point can also go out of all the monitors, for example when we drag the shell to the bottom, where some part of the shell can go off the screen). @akoch-yatta @HeikoKlare

[fedejeanne]

I tested the changes and noticed that it is difficult to re-attach a tab into the editor area if the editor area is too small.

In the video you can see that I need to make the editor area wider in order to be able to place the tab in it.

Also, the naming of some of the new methods is confusing since the word Points is in the name and it also (sometimes) takes Points as parameter. They refer to a different kind of points though, am I right? If that's the case, I'd refrain from using the word Point(s) in the name of the methods.

[amartya4256]

@akoch-yatta @fedejeanne @HeikoKlare All the issue that was mentioned earlier after the first implementation has been fixed. After a deep investigation, I found out that the obtained monitor is sometimes faulty and the scaling is done with respect to the zoom level of the primary monitor and this can occur in case of a rectangle if its top left is out of the client area of any monitor and thats why the wrong scaling is used to determine the bounds and its placement. I have fixed this for rectangles by not using its top left corner to find the monitor but by calculating the biggest intersection between the shell and the monitor and in case the shell is completely out of all the monitors (which I can't seem to reproduce - but possible), then it will use primary monitor for scaling.

[HeikoKlare]

The change definitely works better than before. In my opinion, we can accept that in case a shell is placed across two monitors with different scale values, drag operations do not work on the monitor that does not fit the shell's scale value.
However, currently the behavior on that monitor is rather inconsistent and unpredicatable, as the used coordinate system does not (seem to) fit to the shell:

• Attach operations (in tab folder) work with some horizontal offset, according to the difference in scaling between the shell and the monitor
• Split operations (separating a part stack into two) sometimes (?) work as expected
• Detach operations (creating a new shell) work for the area outside the current shell, imaginary scaled to the coordinate system of the monitor

All in all, the behavior is hard to describe and even harder to comprehend as a user. In particular, the behavior for regions (splitting up part stacks) and points (attaching to tab folders) seems to be different w.r.t. how the coordinates are scaled. Even though I do not expect every attach/split/detach operation to work properly in this edge case of having a shell across two differently scaled monitors, I would at least expect some consistent behavior. Would it be possible to simply fall back to a detach operation if dragging to another monitor than the one of the current shell? That should feel more consistent. In general, it would be good to have a precise specification (also documented in the code) of how the coordinate system should behave.

Here are some examples. All of them use two monitors with the border somewhere at the middle of the screen. The left monitor is the primary one scaled at 100%, the right monitor is a secondary one scaled at 150%.

Splitting a part that spans two monitor

The shell is assigned to the left monitor. When dragging Class1.java editor/view to a part in the shell of which almost half the horizontal space is on the left and half is on the right monitor, the preview for the split-up parts is not correct.

Splitting a part on "false" monitor vs. attaching on tab folder on "false" monitor

The shell is assigned to the left monitor. When dragging the Class1.java editor/view to a part of the shell on the right monitor, then splitting up a part stack works as expected:

But attaching to the tab folder of that part stack does not work:

Dragging into area outside the "virtually scaled" shell

The shell is assigned to the left monitor. When dragging to the right monitor, even though the cursor is on top of the shell, it performs a detach operation because it is outside of the "virtually scaled" shell, i.e., outside the shell if converting its coordinates into the coordinate system of that other monitor.

My finding has been addressed

[akoch-yatta]

@HeikoKlare The behaviour should be more consistent now, although I'm not sure, if I was able to reproduce everything as you had. Would be great, if you could have another look on it.

[HeikoKlare]

I've tested again, but unfortunately the behavior does still not appear to be comprehensible to me. I used the same setup as before:

• Left monitor (primary) at 100%
• Right monitor (secondary) at 200%
• Workbench window spans both monitors with almost equal distribution of its area to both monitors. Area on left (primary, 100%) monitor is slightly higher, so the shell is scaled according to that monitor (100%)

This is how it looks like when I drag around an editor tab:

[fedejeanne]

@akoch-yatta I tested the current state and I found a funny behavior: If I detach and drag the view near the border between the 2 monitors, sometimes the frame of the view is off.

shell_repositioning_bug.zip

How I tested

• vm arg: -Dswt.autoScale.updateOnRuntime=true
• 100% monitor on the left (primary) and 200% monitor on the right

[fedejeanne]

@akoch-yatta I tested again (this time only the known bug I mentioned before) and the bug is still there. This time you can see it if the window that is on the 100% monitor (on the left) does not occupy the whole monitor.

When dragging the view on the border of the 100% monitor, the frame is sometimes off. You can see it clearly when dragging it above the Eclipse window.

FTR this time I didn't check the new code and I did not test other use cases.

shell_repositioning_bug_2.zip

[akoch-yatta]

@akoch-yatta I tested again (this time only the known bug I mentioned before) and the bug is still there. This time you can see it if the window that is on the 100% monitor (on the left) does not occupy the whole monitor.

When dragging the view on the border of the 100% monitor, the frame is sometimes off. You can see it clearly when dragging it above the Eclipse window.

FTR this time I didn't check the new code and I did not test other use cases.

shell_repositioning_bug_2.zip

@fedejeanne I don't know, how to say it, but it seems I pushed onto the wrong remote and you just tested an incomplete version. I'm sorry, but I just pushed the changes onto the branch of this PR.

[fedejeanne]

Code looks OK and I didn't find any regressions introduced by this PR.