Deprecate ui::Event::root_location and replace it with ui::Event::screen_location |
||||||||||||
Issue descriptionFrom an email exchange with sadrul@: In mus/mustash: 1. An app's window can be arbitrarily transformed by CSS (embedded iframe), and 2. We no longer have the ash concept of one root window per display In this world, there are really only two coordinate systems that make sense for events (at least as they enter an app): location inside window (ui::Event::location) and location in screen. ui::Event::root_location is confusing, because it means different things in classic ash vs. mustash vs. desktop aura. It's been a source of confusion for me for some time. It seems to mean these things: 1. In ash, coordinates relative to the display (root window = display) 2. In desktop aura, coordinates relative to the root window 3. In mustash, screen coordinates (this seems like abuse of the field to me) Also, PointerWatcher under mus exists to track events outside an app's window. From the app's perspective, those events have no target, and hence no root window. rjkroege@, oshima@ and I think we should deprecate ui::Event::root_location and replace it with ui::Event::screen_location. This will take a bunch of cleanup, but is probably worth it. And some comments from sadrul@: One thing I would explicitly point out that we need to be careful about: screen-coordinates/system-coordinates, whatever we call it, need to be really well-defined, e.g. is it in physical pixels, or DIP? If the display is rotated, does that affect the coordinate or not? and so on.
,
May 3 2016
It doesn't look like there's a lot of usage of root_location() in ash, perhaps we can just fix them instead to use screen_location()?
,
May 3 2016
Do you guys know what the current interpretations are for physical vs. DIP in the current usage of "screen" coordinates?
,
May 3 2016
Coordinates are always in DIP.
,
May 5 2016
Wouldn't it be preferable if we maintained (at least inside of mus) screen coordinates in device-dependent pixels to simplify buffer allocation and display management?
,
May 5 2016
Yep. Window size etc. on the server-side should always be 'pixel space', as well as the events. Any rescaling etc. should only happen on the client side (preferably in the client-lib, so each client doesn't need to re-implement it).
,
May 18 2016
,
Aug 9 2016
,
Aug 16 2016
,
Aug 16 2016
I'm not going to get to this anytime soon. Other folks feel free to pick it up.
,
Oct 4 2016
,
Feb 26 2018
,
Feb 26 2018
,
May 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f41362bacf2f71e419784a6fbd88a2556170698b commit f41362bacf2f71e419784a6fbd88a2556170698b Author: James Cook <jamescook@chromium.org> Date: Mon May 14 17:14:05 2018 Fix pointer watcher screen coordinates under mus Previously we were using the "root location" as the screen coordinates for events sent to pointer watchers. This was sometimes incorrect on secondary displays. Instead, plumb around the display id for the event and compute the correct screen coordinates in the //ui/views/mus layer. This is needed to get the touch hud app working with the new Window Service on Chrome OS. TBR=tsepez@chromium.org Bug: 608547, 840380 Test: added to views_mus_unittests and aura_unittests Change-Id: I46e90b76ec1562a5277a5679d8a4a9d64fd4333c Reviewed-on: https://chromium-review.googlesource.com/1056205 Reviewed-by: James Cook <jamescook@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: James Cook <jamescook@chromium.org> Cr-Commit-Position: refs/heads/master@{#558347} [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ash/window_manager.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ash/window_manager.h [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ash/window_manager_unittest.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/mash/simple_wm/simple_wm.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/mash/simple_wm/simple_wm.h [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/services/ui/demo/mus_demo.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/services/ui/demo/mus_demo.h [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/services/ui/test_wm/test_wm.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/services/ui/ws/window_server_test_base.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/services/ui/ws/window_server_test_base.h [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/aura/mus/window_tree_client.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/aura/mus/window_tree_client_delegate.h [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/aura/mus/window_tree_client_unittest.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/aura/test/aura_test_base.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/aura/test/aura_test_base.h [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/aura/test/mus/test_window_tree_client_delegate.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/aura/test/mus/test_window_tree_client_delegate.h [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/events/mojo/event_struct_traits.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/views/mus/mus_client.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/views/mus/mus_client.h [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/views/mus/pointer_watcher_event_router.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/views/mus/pointer_watcher_event_router.h [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/views/mus/pointer_watcher_event_router_unittest.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/wm/test/wm_test_helper.cc [modify] https://crrev.com/f41362bacf2f71e419784a6fbd88a2556170698b/ui/wm/test/wm_test_helper.h
,
Aug 13
While annoying, this isn't necessary for mash. I'm removing the mash tags from this.
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d859d8917a8f25c2b1df516bc67590e37bc2750f commit d859d8917a8f25c2b1df516bc67590e37bc2750f Author: Mike Wasserman <msw@chromium.org> Date: Thu Sep 27 05:20:28 2018 ui: Add EventTarget::GetScreenLocation[F] helper functions. Implement screen coordinate helpers for aura::Window and views::View. Simplify various places that gets screen locations from events. TODO: Simplify more View::ConvertPointToScreen() callers. (this CL focused on aura::client::ScreenPositionClient callers) Bug: 608547 Test: No regressions in event coordinate handling Change-Id: I23e4fef3264e5e7d94211d7520aef7d3392b080e Reviewed-on: https://chromium-review.googlesource.com/1246826 Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#594597} [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ash/pointer_watcher_adapter.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ash/public/cpp/immersive/immersive_fullscreen_controller.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ash/public/cpp/immersive/immersive_fullscreen_controller.h [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ash/shelf/overflow_bubble.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ash/utility/screenshot_controller.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ash/wm/immersive_gesture_handler_classic.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/chrome/browser/chromeos/accessibility/select_to_speak_event_handler_delegate.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/content/browser/renderer_host/render_widget_host_view_event_handler.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/aura/mus/mus_mouse_location_updater.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/aura/window.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/aura/window.h [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/aura/window_targeter.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/events/blink/web_input_event.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/events/blink/web_input_event.h [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/events/blink/web_input_event_unittest.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/events/event_target.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/events/event_target.h [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/views/corewm/tooltip_controller.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/views/view.cc [modify] https://crrev.com/d859d8917a8f25c2b1df516bc67590e37bc2750f/ui/views/view.h
,
Nov 27
***UI Mass Triage *** jamescook@ If there is no pending work/issue fixed, please feel free to close the issue.
,
Nov 27
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by osh...@chromium.org
, May 3 2016