New issue
Advanced search Search tips

Issue 723902 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Rounding error when calculating MOUSE_PRESSED vs. MOUSE_RELEASED

Project Member Reported by ricardoq@chromium.org, May 17 2017

Issue description

Chrome Version: 60.0.3101.0
OS: ChromiumOS

What steps will reproduce the problem?
(1) Launch an Android application that uses SYSTEM_ALERT type window, like Messenger in Chat Head mode
(2) Once in Chat Head mode try to click the Edit Box
(3) Focus won't appear.

What is the expected result?
Focus should appear.

What happens instead?
Focus doesn't appear.

Please use labels and text to provide additional information.

The easiest way to reproduce it, is with an Android application, but it is not related to Android.

But happens because MOUSE_PRESSED location_f() is different than MOUSE_RELEASED location_f(). And when they are not exactly the same, a MOUSE_MOVE event is generated.
And this MOUSE_MOVE event triggers a bug in Android, but could trigger similar bugs on other Chrome windows, specially if using a big screen.



Trace that shows the bug:


window_event_dispatcher.cc(466)] WindowEventDispatcher::GetRootForEvent()172.407196,591.895264    <--- Where the PRESS-Event was generated
window.cc(1096)] Window::ConvertEventToTarget()RootWindow-0 (0x21001f324b00) layer bounds(0, 0, 1366, 768) (172.407196,591.895264) --> ScreenRotationContainer (0x21001f324448) layer bounds(0, 0, 1366, 768) (172.407196,591.895264)
window.cc(1096)] Window::ConvertEventToTarget()ScreenRotationContainer (0x21001f324420) layer bounds(0, 0, 1366, 768) (172.407196,591.895264) --> PowerButtonAnimationContainer (0x21001f402868) layer bounds(0, 0, 1366, 768) (172.407196,591.895264)
...
window.cc(1096)] Window::ConvertEventToTarget()SystemModalContainer (0x21001f3bf840) layer bounds(0, 0, 1366, 768) (172.407196,591.895264) --> ExoShellSurface (0x210022bb1708) layer bounds(9937, 117, 63, 651) (-9764.592773,474.895264)
window.cc(1096)] Window::ConvertEventToTarget()ExoShellSurface (0x210022bb16e0) layer bounds(9937, 117, 63, 651) (-9764.592773,474.895264) --> ExoSurface (0x210022cdd708) layer bounds(-9937, -117, 1366, 768) (172.407227,591.895264)
window.cc(1096)] Window::ConvertEventToTarget()ExoSurface (0x210022cdd6e0) layer bounds(-9937, -117, 1366, 768) (172.407227,591.895264) --> ExoSurface (0x210022cdd708) layer bounds(-9937, -117, 1366, 768) (172.407227,591.895264)
window_targeter.cc(40)] WindowTargeter::FindTargetForLocatedEvent() "ExoSurface" = (172.407227,591.895264)

event_dispatcher.cc(50)] EventDispatcherDelegate::DispatchEvent()172.407227,591.895264   <-- DISPATCHES the event to the newly converted coordinate. Notice the rounding error! 172.407196 vs. 172.407227

window_event_dispatcher.cc(943)] WindowEventDispatcher::PreDispatchMouseEvent() ET_MOUSE_PRESSED
pointer.cc(314)] Pointer::GetEffectiveTargetForEvent()
pointer.cc(143)] >>> OnMouseEvent() ev: 172.407227,591.895264 lo_: 172.407227,591.895264
pointer.cc(180)] IsMouseEvent(): ev: 172.407227,591.895264 lo_: 172.407227,591.895264
pointer.cc(196)] PRESSED/RELEASED: ev: 172.407227,591.895264 lo_: 172.407227,591.895264
pointer.cc(324)] Pointer::UpdateCursorScale()
pointer.cc(261)] <<< OnMouseEvent() ev: 172.407227,591.895264 lo_: 172.407227,591.895264

window_event_dispatcher.cc(466)] WindowEventDispatcher::GetRootForEvent()172.407196,591.895264    <-- RELEASE EVENT is generated using the original coordinates (no rounding error)
window.cc(1096)] Window::ConvertEventToTarget()RootWindow-0 (0x21001f324b00) layer bounds(0, 0, 1366, 768) (172.407196,591.895264) --> ExoSurface (0x210022cdd708) layer bounds(-9937, -117, 1366, 768) (172.407196,591.895264)

      ^
      |
It didn't do the whole conversion here... it went directly from "RootWindow-0" to the correct "ExoSurface", so there is no rounding error

window_targeter.cc(29)] WindowTargeter::FindTargetForLocatedEvent() "ExoSurface" = (172.407196,591.895264)
event_dispatcher.cc(50)] EventDispatcherDelegate::DispatchEvent()172.407196,591.895264
window_event_dispatcher.cc(528)] WindowEventDispatcher::PreDispatchEvent()172.407196,591.895264
window_event_dispatcher.cc(858)] WindowEventDispatcher::PreDispatchMouseEvent() 172.407196,591.895264
window_event_dispatcher.cc(952)] WindowEventDispatcher::PreDispatchMouseEvent() ET_MOUSE_RELEASED
pointer.cc(314)] Pointer::GetEffectiveTargetForEvent()
pointer.cc(143)] >>> OnMouseEvent() ev: 172.407196,591.895264 lo_: 172.407227,591.895264
pointer.cc(180)] IsMouseEvent(): ev: 172.407196,591.895264 lo_: 172.407227,591.895264
pointer.cc(186)] MOVE: ev: 172.407196,591.895264 lo_: 172.407227,591.895264                     <-- Compares new event (original) vs. the cached one. They are differnet. Generates MOVE event
pointer.cc(196)] PRESSED/RELEASED: ev: 172.407196,591.895264 lo_: 172.407196,591.895264
pointer.cc(324)] Pointer::UpdateCursorScale()
pointer.cc(261)] <<< OnMouseEvent() ev: 172.407196,591.895264 lo_: 172.407196,591.895264
window_event_dispatcher.cc(569)] WindowEventDispatcher::PostDispatchEvent()172.407196,591.895264
window_event_dispatcher.cc(511)] WindowEventDispatcher::OnEventProcessingFinished()172.407196,591.895264


original value:  x=172.407196, transformed value with rounding error: x=172.407227, and error of ~0.00003... might vary a bit depending where the location is.

 
Project Member

Comment 1 by bugdroid1@chromium.org, May 26 2017

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

commit 0092647e4e34a209afc7f8715a9512c26a787457
Author: ricardoq <ricardoq@chromium.org>
Date: Fri May 26 00:51:50 2017

Fixes rounding error when calculating MOVE event

In general, it is good practice to compare floats using an Epsilon.
In particular, a mouse location_f() could differ between the
MOUSE_PRESSED and MOUSE_RELEASED events. At MOUSE_RELEASED, it will have
a targeter() already cached, while at MOUSE_PRESSED, it will have to
calculate it passing through all  windows, and that could generate
rounding error.

This patch fixes, that situation. It could be triggered when one of the
window->layer()->bounds()'s origin is at a value bigger than 9000

BUG= 723902 
R=reveman@chromium.org

Review-Url: https://codereview.chromium.org/2894503002
Cr-Commit-Position: refs/heads/master@{#474854}

[modify] https://crrev.com/0092647e4e34a209afc7f8715a9512c26a787457/components/exo/pointer.cc

Status: Fixed (was: Untriaged)
fixed with https://codereview.chromium.org/2894503002

Sign in to add a comment