New issue
Advanced search Search tips

Issue 771212 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Ozone X11 should implement ConfineCursorToBounds

Project Member Reported by osh...@chromium.org, Oct 3 2017

Issue description

This is used when emulating the cursor behavior on desktop.

Kyle, I'm assigning to you for now, but please bounds it back to me if you don't have time.
 
I should be able to find some time to handle this soon. For reference, here is the deleted AshWindowTreeHostX11 code that had this behaviour:

https://chromium.googlesource.com/chromium/src/+/e929b08cf279a77f17525d88ed85b038f3c5ef7e/ash/host/ash_window_tree_host_x11.cc
oshima: For the X11 build, after the cursor was confined to the XWindow was there a way to get outside the XWindow? I can't remember how that worked. I remember that if there were multiple XWindows the cursor warp would handle warping between them but I'm unsure if you could still interact with other things outside the confined bounds.
No official way, but original barrier implementation was buggy and the cursor could escape from the edge of the region. (We have a patch in chromeos)

I don't know if it's upstreamed or not.

It was a bit annoying, but I could just exit the browser and that was enough for me. We can have a dev only path to escape if necessary.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2d58bc10e2729b564c407eae67d718b457e2e65

commit b2d58bc10e2729b564c407eae67d718b457e2e65
Author: kylechar <kylechar@chromium.org>
Date: Wed Oct 18 19:23:01 2017

Add confine bounds code for Ozone X11.

This CL implements ConfineCursorToBounds() for Ozone X11. The mouse
cursor is confined inside the XWindow to emulate a real Chrome OS
experience. Since Ozone X11 is only used for developers and moving the
cursor outside the window is sometimes necessary, the cursor barriers
intentionally leave a small gap in each corner.

Bug:  771212 
Change-Id: I763c3d96041cf95a06c63ad58012a21006dd9c6d
Reviewed-on: https://chromium-review.googlesource.com/705115
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509832}
[modify] https://crrev.com/b2d58bc10e2729b564c407eae67d718b457e2e65/ash/host/ash_window_tree_host_platform.cc
[modify] https://crrev.com/b2d58bc10e2729b564c407eae67d718b457e2e65/ash/host/ash_window_tree_host_platform.h
[modify] https://crrev.com/b2d58bc10e2729b564c407eae67d718b457e2e65/services/ui/ws/platform_display_default.cc
[modify] https://crrev.com/b2d58bc10e2729b564c407eae67d718b457e2e65/services/ui/ws/platform_display_default.h
[modify] https://crrev.com/b2d58bc10e2729b564c407eae67d718b457e2e65/ui/platform_window/x11/x11_window_base.cc
[modify] https://crrev.com/b2d58bc10e2729b564c407eae67d718b457e2e65/ui/platform_window/x11/x11_window_base.h

Comment 5 by msw@chromium.org, Oct 18 2017

Hmm, there's at least one really strange defect with this feature: The cursor is locked even if other windows are placed above the Ozone X11 window (eg. if my terminal is placed over the window and my cursor can't move freely around the terminal window).

It was also not super intuitive how to escape cursor locking; after finally finding that the bottom right corner seemed to allow escape, I thought that was the only place (only realized all corners allow escape from reading the CL desc above), and I thought that was a defect in the locking (I tried hitting Esc and Alt+Tab keys to escape locking).

Comment 6 by osh...@chromium.org, Oct 19 2017

> (eg. if my terminal is placed over the window and my cursor can't move freely around the terminal window).

That's how X barrier works, and expected behavior. We can remove barriers if none of windows is active. I use(d) this only when I needed to troubleshoot the issue that requires this behavior (such as warping, snapping, dragging), so it's probably not high priority.
I tend to agree that this maybe isn't the ideal behaviour always. Maybe we could have the Ozone X11 window confinement only active if command line flag is passed in? Developers could use it when testing out things that it's useful for but it wouldn't be active by default?

I'm not sure how implementing it only if the window is active would work. Focus isn't the correct attribute to track, because focus doesn't change until you click on a new window. After mouse warping the old window would have focus still and confinement wouldn't be on for the new window.
Another idea is a --ash-dev-shortcut enabled hotkey to turn confinement on/off would probably be even better but it would be more work to implement.

Comment 9 by msw@chromium.org, Oct 19 2017

Cc: msw@chromium.org
Perhaps another issue: The cursor is locked to the first two windows when I have three displays (it's not teleported to the third window):
./chrome --ash-host-window-bounds=0+20-500x500,510+20-500x500,1020+20-500x500 --ash-dev-shortcuts --ash-debug-shortcuts --use-first-display-as-internal --ash-enable-unified-desktop

Comment 10 by msw@chromium.org, Oct 19 2017

(The third window has a separate boundary area setup from the first two windows)
It's also a little weird how teleporting only works when the cursor is directly over the window, not when another window is in front of the cros window, but that doesn't match the boundary behavior.

Also, on exiting unified mode in that same config, the left edge of the middle display teleports back to its own right edge, rather than teleporting to the right edge of the leftmost display, and I got trapped in a state where warping was broken after moving the window around, and I couldn't use the mouse at all, I luckily escaped with Ctrl+Shift+Q+Q (new to me).

Sorry if these seem like complaints, I'm actually just aiming to give defect reports and don't really have a problem with this feature in principle.
This is super-annoying. I frequently have editor windows open on top of the Ozone X11 window and having my cursor constrained is highly unexpected.

Can you make it do this only when a command line flag is passed?

Comment 12 by katydek@google.com, Oct 19 2017

Hi there, I've just synced up and also would appreciate not having this as default behavior. It's making it a lot harder for me to develop. Thanks!
The unified mode cursor warp code only works with two displays at the moment. That is  an issue that afakhry@ is working on making unified work with 3+ displays now.

I'm reverting this now and will reland it with a flag tomorrow.
Oh, it's enabled by default? That should be a bug and should be enabled only with --ash-constrain-pointer-to-root flag.
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a7e11db759a5d16fdd641f79489ae060687572b3

commit a7e11db759a5d16fdd641f79489ae060687572b3
Author: kylechar <kylechar@chromium.org>
Date: Fri Oct 20 01:12:30 2017

Revert "Add confine bounds code for Ozone X11."

This reverts commit b2d58bc10e2729b564c407eae67d718b457e2e65.

Reason for revert: This change makes CrOS development too difficult.

Original change's description:
> Add confine bounds code for Ozone X11.
> 
> This CL implements ConfineCursorToBounds() for Ozone X11. The mouse
> cursor is confined inside the XWindow to emulate a real Chrome OS
> experience. Since Ozone X11 is only used for developers and moving the
> cursor outside the window is sometimes necessary, the cursor barriers
> intentionally leave a small gap in each corner.
> 
> Bug:  771212 
> Change-Id: I763c3d96041cf95a06c63ad58012a21006dd9c6d
> Reviewed-on: https://chromium-review.googlesource.com/705115
> Commit-Queue: kylechar <kylechar@chromium.org>
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#509832}

TBR=sadrul@chromium.org,oshima@chromium.org,kylechar@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  771212 
Change-Id: I2ecf7fb98e23c0337d15ce55da840dcf24a6448a
Reviewed-on: https://chromium-review.googlesource.com/729179
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510284}
[modify] https://crrev.com/a7e11db759a5d16fdd641f79489ae060687572b3/ash/host/ash_window_tree_host_platform.cc
[modify] https://crrev.com/a7e11db759a5d16fdd641f79489ae060687572b3/ash/host/ash_window_tree_host_platform.h
[modify] https://crrev.com/a7e11db759a5d16fdd641f79489ae060687572b3/services/ui/ws/platform_display_default.cc
[modify] https://crrev.com/a7e11db759a5d16fdd641f79489ae060687572b3/services/ui/ws/platform_display_default.h
[modify] https://crrev.com/a7e11db759a5d16fdd641f79489ae060687572b3/ui/platform_window/x11/x11_window_base.cc
[modify] https://crrev.com/a7e11db759a5d16fdd641f79489ae060687572b3/ui/platform_window/x11/x11_window_base.h

I should have clarified more when asking how it was supposed to work, sorry about that! I'll reland only the X11WindowBase changes which add the ConfineCursorToBounds() code.
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9e43f8812c7a4bcd02167f8f8cb162fd0df6f007

commit 9e43f8812c7a4bcd02167f8f8cb162fd0df6f007
Author: kylechar <kylechar@chromium.org>
Date: Mon Oct 23 23:24:43 2017

Add confine bounds code for Ozone X11.

This CL implements ConfineCursorToBounds() for Ozone X11. This is only
active if --ash-constrain-pointer-to-root is passed in.

Bug:  771212 
Change-Id: I605c704846a34d61a77a87518f1872355e821773
Reviewed-on: https://chromium-review.googlesource.com/731028
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510957}
[modify] https://crrev.com/9e43f8812c7a4bcd02167f8f8cb162fd0df6f007/ui/platform_window/x11/x11_window_base.cc
[modify] https://crrev.com/9e43f8812c7a4bcd02167f8f8cb162fd0df6f007/ui/platform_window/x11/x11_window_base.h

Status: Fixed (was: Assigned)
Status: Started (was: Fixed)
This is still broken. Reverting again.
Project Member

Comment 20 by bugdroid1@chromium.org, Oct 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2701b19a9077c26a81f69af5768687951225daf4

commit 2701b19a9077c26a81f69af5768687951225daf4
Author: kylechar <kylechar@chromium.org>
Date: Tue Oct 24 14:37:27 2017

Revert "Add confine bounds code for Ozone X11."

This reverts commit 9e43f8812c7a4bcd02167f8f8cb162fd0df6f007.

Reason for revert: This is still broken.

Original change's description:
> Add confine bounds code for Ozone X11.
> 
> This CL implements ConfineCursorToBounds() for Ozone X11. This is only
> active if --ash-constrain-pointer-to-root is passed in.
> 
> Bug:  771212 
> Change-Id: I605c704846a34d61a77a87518f1872355e821773
> Reviewed-on: https://chromium-review.googlesource.com/731028
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Commit-Queue: kylechar <kylechar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510957}

TBR=sadrul@chromium.org,kylechar@chromium.org

Change-Id: I084728aefee51ba92c0df41413ea20afd66ac4d2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  771212 
Reviewed-on: https://chromium-review.googlesource.com/734954
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511143}
[modify] https://crrev.com/2701b19a9077c26a81f69af5768687951225daf4/ui/platform_window/x11/x11_window_base.cc
[modify] https://crrev.com/2701b19a9077c26a81f69af5768687951225daf4/ui/platform_window/x11/x11_window_base.h

Project Member

Comment 21 by bugdroid1@chromium.org, Feb 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ee840b8fc7316efadf8a825dec6f928cadde6a7

commit 6ee840b8fc7316efadf8a825dec6f928cadde6a7
Author: kylechar <kylechar@chromium.org>
Date: Wed Feb 28 18:13:28 2018

Add confine bounds code for Ozone X11.

This CL implements ConfineCursorToBounds() for Ozone X11. By default we
don't want to confine bounds on Ozone X11, because it makes development
painful, so only if --ash-constrain-pointer-to-root is passed in.

This requires some changes to AshWindowTreeHost to move the check for if
we want to confine cursor into a place that an AshWindowTreeHost can
check it every time ConfineCursorToRootWindow() is called.

Bug:  771212 
Change-Id: I3092c808e44d6f6c56c995ecd3e8547ba3b4a1cd
Reviewed-on: https://chromium-review.googlesource.com/940276
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539884}
[modify] https://crrev.com/6ee840b8fc7316efadf8a825dec6f928cadde6a7/ash/display/window_tree_host_manager.cc
[modify] https://crrev.com/6ee840b8fc7316efadf8a825dec6f928cadde6a7/ash/host/ash_window_tree_host.cc
[modify] https://crrev.com/6ee840b8fc7316efadf8a825dec6f928cadde6a7/ash/host/ash_window_tree_host.h
[modify] https://crrev.com/6ee840b8fc7316efadf8a825dec6f928cadde6a7/ash/host/ash_window_tree_host_mus.cc
[modify] https://crrev.com/6ee840b8fc7316efadf8a825dec6f928cadde6a7/ash/host/ash_window_tree_host_mus.h
[modify] https://crrev.com/6ee840b8fc7316efadf8a825dec6f928cadde6a7/ash/host/ash_window_tree_host_platform.cc
[modify] https://crrev.com/6ee840b8fc7316efadf8a825dec6f928cadde6a7/ash/host/ash_window_tree_host_platform.h
[modify] https://crrev.com/6ee840b8fc7316efadf8a825dec6f928cadde6a7/ui/platform_window/x11/x11_window_base.cc
[modify] https://crrev.com/6ee840b8fc7316efadf8a825dec6f928cadde6a7/ui/platform_window/x11/x11_window_base.h

Status: Fixed (was: Started)

Sign in to add a comment