New issue
Advanced search Search tips

Issue 848970 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

SetBoundsInScreen origin calculation is off

Project Member Reported by timzheng@chromium.org, Jun 2 2018

Issue description

https://cs.chromium.org/chromium/src/ash/wm/window_positioning_utils.cc?rcl=17982700ad2b8ed7b1c139ab80afdad17d15dd40&l=153
https://cs.chromium.org/chromium/src/ash/wm/window_positioning_utils.cc?rcl=17982700ad2b8ed7b1c139ab80afdad17d15dd40&l=182

As linked above, in source file ash/wm/window_positioning_utils.cc method SetBoundsInScreen, there are 2 statements like this 'origin.Offset(-display_origin.x(), -display_origin.y());'. The negative signs look to be in error.

For example, if the origin of display is (-2560, -400) and bounds_in_screen origin is (0, 0), the result bounds origin should be (-2560, -400) while the code sets it at (2560, 400).

I traced it here when trying to debug a problem when calling aura::Window::SetWindowInScreen. When I tried to fix this by removing the negation of the origin coordinates, however, no shelf or launcher is showing on the second monitor.


 

Comment 1 by msw@chromium.org, Jun 4 2018

Cc: msw@chromium.org jamescook@chromium.org varkha@chromium.org
In your example, are there two connected displays or one display? If two, is the one at -2560,-400 the primary or secondary?

Cc: -varkha@chromium.org
To answer comment #2, there are 2 displays. The primary is the PixelBook built in display with origin of (0, 0) and dimension of 1200x800. The secondary has origin of (-2560, -400) and dimension of 2560x1600.

That is my test setup. The secondary display is physically on the left side of the PixelBook so I changed the settings to make it on the left of the PixelBook as well.
Tim, when you were calling SetBoundsInScreen() with bounds_in_screen (0,0), what display ID were you passing?

I'm asking because |bounds_in_screen| is in "virtual screen coordinates", a coordinate system across all known displays. I would expect bounds_in_screen (0,0) to be the top-left corner of the primary monitor, regardless of display configuration. So a window in the top-left of the primary display should be at (0,0) in the coordinates of the primary display. (2560,400) sounds like the same position, but in the coordinates of the secondary (on the left) display. It would also be odd for the |display| to be the secondary display and the bounds_in_screen to be (0,0), since that would imply the window is offscreen.

Is it possible you were expecting |bounds_in_screen| to mean the position in the coordinate system of the |display|?

I get pretty reasonable results for |new_bounds| when I drag a window from display 1 to display 2 -- it's in the coordinates of display 2.

(I have no idea why we pass both parameters to that function, since you should be able to use display::Screen::GetDisplayNearestPoint to figure out the display.)

James, you're right that I was expecting |bounds_in_screen| to mean the position in the coordinate system of the |display|. So It might be my misunderstanding of SetBoundsInScreen().

So what does SetBoundsInScreen() actually do? I am using it to re-parent a window from a display to another display. The window is in the primary display at the top left corner, so the origin is (0,0). I want to move it to the second display at the same top left corner, which has the coordinate of (-2560, -400). So I call SetBoundsInScreen((0,0) ..., second_display_id). After the call the window is re-parent to the second display but the origin becomes (2560, 400), out of screen.

Please not that the second screen has x axis range from -2560 to 0 and the y axis range from -400 to 1200.
https://cs.chromium.org/chromium/src/ui/aura/window.h?rcl=f8e6447dcc8d0d35455592c4b259de42d1009030&l=223

The name of the first parameter of SetBoundsInScreen() is 'new_bounds_in_screen_coords'. I understand the screen as the same as display. So do you think the name is misleading here?
Never mind. I should read more carefully. So screen is the "virtual screen". Okay, I think it's my misunderstanding. Thanks for the help.
Status: WontFix (was: Untriaged)

Comment 10 by msw@chromium.org, Jun 5 2018

Right, Screen is composed of some number of Displays:
  https://cs.chromium.org/chromium/src/ui/display/screen.h?l=29
Yeah, the names aren't great. We don't have a good name for "bounds in current display" vs. "bounds across all displays". (We also have problems with dips vs pixels.) For better or worse, |in_screen| in variables and method names means virtual screen coordinates.

Sign in to add a comment