Set the coordinates of the synthetic touch event correctly in SyntheticGestureTargetAura

content/browser/renderer_host/input/synthetic_gesture_target_aura.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/input/synthetic_gesture_target_aura.h"
 
 #include <stddef.h>
 
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_aura.h"
 #include "content/browser/renderer_host/ui_events_helper.h"
 #include "third_party/WebKit/public/platform/WebScreenInfo.h"
 #include "ui/aura/window.h"
+#include "ui/aura/window_event_dispatcher.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/events/event_processor.h"
 #include "ui/events/event_utils.h"
 #include "ui/events/gesture_detection/gesture_configuration.h"
 
 using blink::WebTouchEvent;
 using blink::WebMouseWheelEvent;
 
 namespace content {
 
 SyntheticGestureTargetAura::SyntheticGestureTargetAura(
     RenderWidgetHostImpl* host)
     : SyntheticGestureTargetBase(host) {
   blink::WebScreenInfo screenInfo;
   host->GetWebScreenInfo(&screenInfo);
   device_scale_factor_ = screenInfo.deviceScaleFactor;
 }
 
 void SyntheticGestureTargetAura::DispatchWebTouchEventToPlatform(
     const WebTouchEvent& web_touch,
     const ui::LatencyInfo& latency_info) {
   TouchEventWithLatencyInfo touch_with_latency(web_touch, latency_info);
   for (size_t i = 0; i < touch_with_latency.event.touchesLength; i++) {
-    touch_with_latency.event.touches[i].position.x *= device_scale_factor_;
-    touch_with_latency.event.touches[i].position.y *= device_scale_factor_;

[bokan, 2016/08/25 12:36:51]

Why did this get removed in the latest patch?

[lanwei, 2016/08/25 15:19:57]

If we use local coordinates, then we will use event.touches[i].position as the
position in the ui::TouchEvent, when we create ui::TouchEvent from WebTouchEvent
in MakeUITouchEventsFromWebTouchEvents. Then we do not need to apply the
device_scale_factor_, because later we do not converted back as I set transform
event to false, host->dispatcher()->set_transform_events(false).

I think here the order is wrong which makes the coordinates is not the same as
we pass at beginning.
Here when we get a pair of x, y from gpu_benchmarking.tap function, we apply the
device_scale_factor and then ConvertLocationToTarget, but in
EventProcessor::OnEventFromSource, it tries to transform the event position back
from device scale factor. Later when it is finding the target for the event, it
tries to convert point to target, then it gets a wrong coordinate. I feel it is
easy we do not device_scale_factor at all, and we can make sure we get the right
coordinate. What do you think?

 

[bokan, 2016/08/25 18:53:07]

Hmm, in that case, why did we need this in the first place? In other words, if
this works, why wouldn't leaving the |*= device_scale_factor| here and removing
the |set_transform_events(false)| work? Shouldn't it be equivalent? There's
something I'm not understanding here...
The comment at
https://cs.chromium.org/chromium/src/ui/aura/window_event_dispatcher.h?dr=CSs...
seems to say that the dispatcher gets events in physical pixel coordinates (i.e.
with device_scale_factor applied. [DeviceIndependentPixels *
device_scale_factor_ = physical pixels]).

The web_touch we get as a parameter in this method comes as input from tests so
it's in DIPs (device independent pixels). So to me it seems we *should* be
multiplying by device_scale_factor_ to get it into physical pixels and then
handing it off to the EventDispatcher which then *should* divide by
device_scale_factor (in  WindowEventDispatcher::OnEventProcessingStarted the
TransformEventForDeviceScaleFactor the function divides by DSF). Any idea where
this chain of events is breaking?

     touch_with_latency.event.touches[i].radiusX *= device_scale_factor_;
     touch_with_latency.event.touches[i].radiusY *= device_scale_factor_;
   }
   ScopedVector<ui::TouchEvent> events;
   bool conversion_success = MakeUITouchEventsFromWebTouchEvents(
       touch_with_latency, &events, LOCAL_COORDINATES);
   DCHECK(conversion_success);
 
   aura::Window* window = GetWindow();
   aura::WindowTreeHost* host = window->GetHost();
+
+  // Synthetic touch events will be transformed here before being sent to the
+  // EventProcessor, so they should not be transformed again later in
+  // WindowEventDispatcher.
+  host->dispatcher()->set_transform_events(false);
   for (ScopedVector<ui::TouchEvent>::iterator iter = events.begin(),
       end = events.end(); iter != end; ++iter) {
     (*iter)->ConvertLocationToTarget(window, host->window());
     ui::EventDispatchDetails details =
         host->event_processor()->OnEventFromSource(*iter);
     if (details.dispatcher_destroyed)
       break;
   }
 }
 
@@ skipping 106 matching lines @@
       ->min_distance_for_pinch_scroll_in_pixels();
 }
 
 aura::Window* SyntheticGestureTargetAura::GetWindow() const {
   aura::Window* window = render_widget_host()->GetView()->GetNativeView();
   DCHECK(window);
   return window;
 }
 
 }  // namespace content