Do not scale ScrollEvent by device scale factor

ash/display/event_transformation_handler.cc

 // Copyright (c) 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 "ash/display/event_transformation_handler.h"
 
 #include <cmath>
 
 #include "ash/screen_ash.h"
 #include "ash/shell.h"
@@ skipping 22 matching lines @@
     : transformation_mode_(TRANSFORM_AUTO) {
 }
 
 EventTransformationHandler::~EventTransformationHandler() {
 }
 
 void EventTransformationHandler::OnScrollEvent(ui::ScrollEvent* event) {
   if (transformation_mode_ == TRANSFORM_NONE)
     return;
 
-  // Get the device scale factor and stack it on the final scale factor.
+  // The original purpose of this function is to scale scroll events
+  // w.r.t. the screen resolution and physical size as what we do for
+  // pointer motions in the CrOS X server patch (i.e. linear scaling
+  // w.r.t. the device scale factor and 20% more for non-integrated
+  // display). However, Aura scroll events differs from X's pointer
+  // motions in an important way where the consumers of scroll events
+  // assume the use of DIP instead of real pixels. Scaling them by the
+  // device scale factor would result in over-scaling and make high
+  // resolution device (e.g. Pixel) to have faster scrolling speed than
+  // normal ones.
+  //
+  // Our solution here is to NOT scale events by the device scale factor
+  // and instead let the scroll event to enjoy the "implicit" scaling
+  // due to the use of DIP (e.g. treating the scroll values as DIPs
+  // implies an implicit scaling factor of 2 in terms of physical
+  // distance on the Pixel screen). All the Views that consume these
+  // events would also behave identically both on Pixel and non-Pixel
+  // devices as they take the values as DIPs (e.g. 2f back/forward
+  // gesture and 3f tab switching would have equal sensitivity).

[sadrul, 2013/09/18 15:36:41]

Does 'It is not necessary to scale the event for the device scale factor since
the event location etc. are already in DIP.' effectively summarize this long
comment?

[oshima, 2013/09/18 16:26:10]

Agreed.

[Shecky Lin, 2013/09/20 04:44:10]

Done.

   gfx::Point point_in_screen(event->location());
   aura::Window* target = static_cast<aura::Window*>(event->target());
-  const float scale_at_target = ui::GetDeviceScaleFactor(target->layer());
-  float scale = scale_at_target;
+  float scale = 1.0;
 
   // Apply some additional scaling if the display is non-integrated.
   wm::ConvertPointToScreen(target, &point_in_screen);
   const gfx::Display& display =
       Shell::GetScreen()->GetDisplayNearestPoint(point_in_screen);
   if (!display.IsInternal())
     scale *= kBoostForNonIntegrated;

[oshima, 2013/09/18 16:26:10]

if (display.Internal())
   event->Scale(kBoostForNonIntegrated);

and nuke scale variable.

[Shecky Lin, 2013/09/20 04:44:10]

Done.

 
   event->Scale(scale);
 }
 
 #if defined(OS_CHROMEOS)
 // This is to scale the TouchEvent's radius when the touch display is in
 // mirror mode. TouchEvent's radius is often reported in the touchscreen's
 // native resolution. In mirror mode, the touch display could be configured
 // at a lower resolution. We scale down the radius using the ratio defined as
 // the sqrt of
@@ skipping 24 matching lines @@
   }
 
   float area_ratio_sqrt = std::sqrt(area_ratio_map.begin()->second);
   event->set_radius_x(event->radius_x() * area_ratio_sqrt);
   event->set_radius_y(event->radius_y() * area_ratio_sqrt);
 }
 #endif  // defined(OS_CHROMEOS)
 
 }  // namespace internal
 }  // namespace ash