Bug 1445662 - Make the DPI non-static and bound to the controller thread. r?rhunt draft
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 15 Mar 2018 15:10:38 -0400
changeset 768168 94adbe204fd9d908bfede0aa1fa8657fd03c5a8e
parent 768167 db883c2615760445bfe1f28f547b0a7bf7c556bb
child 768169 833749e473baac3c09edb924073ed55bf739deed
push id102813
push userkgupta@mozilla.com
push dateThu, 15 Mar 2018 19:11:06 +0000
reviewersrhunt
bugs1445662
milestone61.0a1
Bug 1445662 - Make the DPI non-static and bound to the controller thread. r?rhunt Since we can have multiple browser windows on multiple different displays with different DPIs, it doesn't make sense to have a single static DPI value shared across all APZCTreeManagers. Instead, each APZCTM should store its own DPI value for the display the window is on. Since the DPI is only ever read from the controller thread, we can make it bound to that thread, and update the setter code to also set it on that thread. As with the previous patch, the change in APZCTreeManagerParent is a no-op but allows making some other thread in the GPU process the controller thread. And the change in nsBaseWidget is a no-op everywhere except Android. MozReview-Commit-ID: 4el6HI7IGuC
gfx/layers/apz/src/APZCTreeManager.cpp
gfx/layers/apz/src/APZCTreeManager.h
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/layers/apz/src/AsyncPanZoomController.h
gfx/layers/apz/src/Axis.cpp
gfx/layers/apz/src/GestureEventListener.cpp
gfx/layers/apz/src/InputBlockState.cpp
gfx/layers/ipc/APZCTreeManagerParent.cpp
widget/nsBaseWidget.cpp
--- a/gfx/layers/apz/src/APZCTreeManager.cpp
+++ b/gfx/layers/apz/src/APZCTreeManager.cpp
@@ -60,18 +60,16 @@ namespace layers {
 using mozilla::gfx::CompositorHitTestInfo;
 
 typedef mozilla::gfx::Point Point;
 typedef mozilla::gfx::Point4D Point4D;
 typedef mozilla::gfx::Matrix4x4 Matrix4x4;
 
 typedef CompositorBridgeParent::LayerTreeState LayerTreeState;
 
-float APZCTreeManager::sDPI = 160.0;
-
 struct APZCTreeManager::TreeBuildingState {
   TreeBuildingState(uint64_t aRootLayersId,
                     bool aIsFirstPaint, uint64_t aOriginatingLayersId,
                     APZTestData* aTestData, uint32_t aPaintSequence)
     : mIsFirstPaint(aIsFirstPaint)
     , mOriginatingLayersId(aOriginatingLayersId)
     , mPaintLogger(aTestData, aPaintSequence)
   {
@@ -225,17 +223,18 @@ private:
 APZCTreeManager::APZCTreeManager(uint64_t aRootLayersId)
     : mInputQueue(new InputQueue()),
       mRootLayersId(aRootLayersId),
       mTreeLock("APZCTreeLock"),
       mHitResultForInputBlock(CompositorHitTestInfo::eInvisibleToHitTest),
       mRetainedTouchIdentifier(-1),
       mInScrollbarTouchDrag(false),
       mApzcTreeLog("apzctree"),
-      mTestDataLock("APZTestDataLock")
+      mTestDataLock("APZTestDataLock"),
+      mDPI(160.0)
 {
   RefPtr<APZCTreeManager> self(this);
   NS_DispatchToMainThread(
     NS_NewRunnableFunction("layers::APZCTreeManager::APZCTreeManager", [self] {
       self->mFlushObserver = new CheckerboardFlushObserver(self);
     }));
   AsyncPanZoomController::InitializeGlobalState();
   mApzcTreeLog.ConditionOnPrefFunction(gfxPrefs::APZPrintTree);
@@ -3148,16 +3147,30 @@ APZCTreeManager::ComputeTransformForScro
       *aOutClipTransform = asyncCompensation;
     }
   }
   transform = transform * compensation;
 
   return transform;
 }
 
+void
+APZCTreeManager::SetDPI(float aDpiValue)
+{
+  APZThreadUtils::AssertOnControllerThread();
+  mDPI = aDpiValue;
+}
+
+float
+APZCTreeManager::GetDPI() const
+{
+  APZThreadUtils::AssertOnControllerThread();
+  return mDPI;
+}
+
 #if defined(MOZ_WIDGET_ANDROID)
 AndroidDynamicToolbarAnimator*
 APZCTreeManager::GetAndroidDynamicToolbarAnimator()
 {
   return mToolbarAnimator;
 }
 #endif // defined(MOZ_WIDGET_ANDROID)
 
--- a/gfx/layers/apz/src/APZCTreeManager.h
+++ b/gfx/layers/apz/src/APZCTreeManager.h
@@ -335,25 +335,26 @@ public:
   void ClearTree();
 
   /**
    * Tests if a screen point intersect an apz in the tree.
    */
   bool HitTestAPZC(const ScreenIntPoint& aPoint);
 
   /**
-   * Sets the dpi value used by all AsyncPanZoomControllers.
-   * DPI defaults to 72 if not set using SetDPI() at any point.
+   * Sets the dpi value used by all AsyncPanZoomControllers attached to this
+   * tree manager.
+   * DPI defaults to 160 if not set using SetDPI() at any point.
    */
-  void SetDPI(float aDpiValue) override { sDPI = aDpiValue; }
+  void SetDPI(float aDpiValue) override;
 
   /**
    * Returns the current dpi value in use.
    */
-  static float GetDPI() { return sDPI; }
+  float GetDPI() const;
 
   /**
    * Find the hit testing node for the scrollbar thumb that matches these
    * drag metrics.
    */
   RefPtr<HitTestingTreeNode> FindScrollThumbNode(const AsyncDragMetrics& aDragMetrics);
 
   /**
@@ -742,17 +743,18 @@ private:
   friend class CheckerboardFlushObserver;
   RefPtr<CheckerboardFlushObserver> mFlushObserver;
 
   // Map from layers id to APZTestData. Accesses and mutations must be
   // protected by the mTestDataLock.
   std::unordered_map<uint64_t, UniquePtr<APZTestData>> mTestData;
   mutable mozilla::Mutex mTestDataLock;
 
-  static float sDPI;
+  // This must only be touched on the controller thread.
+  float mDPI;
 
 #if defined(MOZ_WIDGET_ANDROID)
 public:
   AndroidDynamicToolbarAnimator* GetAndroidDynamicToolbarAnimator();
 
 private:
   RefPtr<AndroidDynamicToolbarAnimator> mToolbarAnimator;
 #endif // defined(MOZ_WIDGET_ANDROID)
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -863,26 +863,44 @@ AsyncPanZoomController::Destroy()
 }
 
 bool
 AsyncPanZoomController::IsDestroyed() const
 {
   return mTreeManager == nullptr;
 }
 
-/* static */ScreenCoord
-AsyncPanZoomController::GetTouchStartTolerance()
+float
+AsyncPanZoomController::GetDPI() const
 {
-  return (gfxPrefs::APZTouchStartTolerance() * APZCTreeManager::GetDPI());
+  if (APZCTreeManager* localPtr = mTreeManager) {
+    return localPtr->GetDPI();
+  }
+  // If this APZC has been destroyed then this value is not going to be
+  // used for anything that the user will end up seeing, so we can just
+  // return 0.
+  return 0.0;
 }
 
-/* static */ScreenCoord
-AsyncPanZoomController::GetSecondTapTolerance()
+ScreenCoord
+AsyncPanZoomController::GetTouchStartTolerance() const
 {
-  return (gfxPrefs::APZSecondTapTolerance() * APZCTreeManager::GetDPI());
+  return (gfxPrefs::APZTouchStartTolerance() * GetDPI());
+}
+
+ScreenCoord
+AsyncPanZoomController::GetTouchMoveTolerance() const
+{
+  return (gfxPrefs::APZTouchMoveTolerance() * GetDPI());
+}
+
+ScreenCoord
+AsyncPanZoomController::GetSecondTapTolerance() const
+{
+  return (gfxPrefs::APZSecondTapTolerance() * GetDPI());
 }
 
 /* static */AsyncPanZoomController::AxisLockMode AsyncPanZoomController::GetAxisLockMode()
 {
   return static_cast<AxisLockMode>(gfxPrefs::APZAxisLockMode());
 }
 
 /* static */AsyncPanZoomController::PinchLockMode AsyncPanZoomController::GetPinchLockMode()
@@ -2641,17 +2659,17 @@ void AsyncPanZoomController::HandlePanni
 
 void AsyncPanZoomController::HandlePanningUpdate(const ScreenPoint& aPanDistance) {
   // If we're axis-locked, check if the user is trying to break the lock
   if (GetAxisLockMode() == STICKY && !mPanDirRestricted) {
 
     double angle = atan2(aPanDistance.y, aPanDistance.x); // range [-pi, pi]
     angle = fabs(angle); // range [0, pi]
 
-    float breakThreshold = gfxPrefs::APZAxisBreakoutThreshold() * APZCTreeManager::GetDPI();
+    float breakThreshold = gfxPrefs::APZAxisBreakoutThreshold() * GetDPI();
 
     if (fabs(aPanDistance.x) > breakThreshold || fabs(aPanDistance.y) > breakThreshold) {
       if (mState == PANNING_LOCKED_X) {
         if (!IsCloseToHorizontal(angle, gfxPrefs::APZAxisBreakoutAngle())) {
           mY.SetAxisLocked(false);
           SetState(PANNING);
         }
       } else if (mState == PANNING_LOCKED_Y) {
@@ -2662,23 +2680,23 @@ void AsyncPanZoomController::HandlePanni
       }
     }
   }
 }
 
 void AsyncPanZoomController::HandlePinchLocking(ScreenCoord spanDistance, ScreenPoint focusChange) {
   if (mPinchLocked) {
     if (GetPinchLockMode() == PINCH_STICKY) {
-      ScreenCoord spanBreakoutThreshold = gfxPrefs::APZPinchLockSpanBreakoutThreshold() * APZCTreeManager::GetDPI();
+      ScreenCoord spanBreakoutThreshold = gfxPrefs::APZPinchLockSpanBreakoutThreshold() * GetDPI();
       mPinchLocked = !(spanDistance > spanBreakoutThreshold);
     }
   } else {
     if (GetPinchLockMode() != PINCH_FREE) {
-      ScreenCoord spanLockThreshold = gfxPrefs::APZPinchLockSpanLockThreshold() * APZCTreeManager::GetDPI();
-      ScreenCoord scrollLockThreshold = gfxPrefs::APZPinchLockScrollLockThreshold() * APZCTreeManager::GetDPI();
+      ScreenCoord spanLockThreshold = gfxPrefs::APZPinchLockSpanLockThreshold() * GetDPI();
+      ScreenCoord scrollLockThreshold = gfxPrefs::APZPinchLockScrollLockThreshold() * GetDPI();
 
       if (spanDistance < spanLockThreshold && focusChange.Length() > scrollLockThreshold) {
         mPinchLocked = true;
       }
     }
   }
 }
 
--- a/gfx/layers/apz/src/AsyncPanZoomController.h
+++ b/gfx/layers/apz/src/AsyncPanZoomController.h
@@ -148,30 +148,41 @@ public:
     // will not attempt to generate gesture events from MultiTouchInputs.
     DEFAULT_GESTURES,
     // An instance of GestureEventListener is used to detect gestures. This is
     // handled completely internally within this class.
     USE_GESTURE_DETECTOR
   };
 
   /**
+   * Gets the DPI from the tree manager.
+   */
+  float GetDPI() const;
+
+  /**
    * Constant describing the tolerance in distance we use, multiplied by the
    * device DPI, before we start panning the screen. This is to prevent us from
    * accidentally processing taps as touch moves, and from very short/accidental
    * touches moving the screen.
    * Note: It's an abuse of the 'Coord' class to use it to represent a 2D
    *       distance, but it's the closest thing we currently have.
    */
-  static ScreenCoord GetTouchStartTolerance();
+  ScreenCoord GetTouchStartTolerance() const;
+  /**
+   * Same as GetTouchStartTolerance, but the tolerance for how far the touch
+   * has to move before it starts allowing touchmove events to be dispatched
+   * to content, for non-scrollable content.
+   */
+  ScreenCoord GetTouchMoveTolerance() const;
   /**
    * Same as GetTouchStartTolerance, but the tolerance for how close the second
    * tap has to be to the first tap in order to be counted as part of a multi-tap
    * gesture (double-tap or one-touch-pinch).
    */
-  static ScreenCoord GetSecondTapTolerance();
+  ScreenCoord GetSecondTapTolerance() const;
 
   AsyncPanZoomController(uint64_t aLayersId,
                          APZCTreeManager* aTreeManager,
                          const RefPtr<InputQueue>& aInputQueue,
                          GeckoContentController* aController,
                          GestureBehavior aGestures = DEFAULT_GESTURES);
 
   // --------------------------------------------------------------------------
--- a/gfx/layers/apz/src/Axis.cpp
+++ b/gfx/layers/apz/src/Axis.cpp
@@ -55,17 +55,17 @@ Axis::Axis(AsyncPanZoomController* aAsyn
     mAxisLocked(false),
     mAsyncPanZoomController(aAsyncPanZoomController),
     mOverscroll(0),
     mMSDModel(0.0, 0.0, 0.0, 400.0, 1.2)
 {
 }
 
 float Axis::ToLocalVelocity(float aVelocityInchesPerMs) const {
-  ScreenPoint velocity = MakePoint(aVelocityInchesPerMs * APZCTreeManager::GetDPI());
+  ScreenPoint velocity = MakePoint(aVelocityInchesPerMs * mAsyncPanZoomController->GetDPI());
   // Use ToScreenCoordinates() to convert a point rather than a vector by
   // treating the point as a vector, and using (0, 0) as the anchor.
   ScreenPoint panStart = mAsyncPanZoomController->ToScreenCoordinates(
       mAsyncPanZoomController->PanStart(),
       ParentLayerPoint());
   ParentLayerPoint localVelocity =
       mAsyncPanZoomController->ToParentLayerCoordinates(velocity, panStart);
   return localVelocity.Length();
--- a/gfx/layers/apz/src/GestureEventListener.cpp
+++ b/gfx/layers/apz/src/GestureEventListener.cpp
@@ -258,25 +258,25 @@ bool GestureEventListener::MoveDistanceE
   const ParentLayerPoint start = mLastTouchInput.mTouches[0].mLocalScreenPoint;
   ParentLayerPoint delta = start - mTouchStartPosition;
   ScreenPoint screenDelta = mAsyncPanZoomController->ToScreenCoordinates(delta, start);
   return (screenDelta.Length() > aThreshold);
 }
 
 bool GestureEventListener::MoveDistanceIsLarge() const
 {
-  return MoveDistanceExceeds(AsyncPanZoomController::GetTouchStartTolerance());
+  return MoveDistanceExceeds(mAsyncPanZoomController->GetTouchStartTolerance());
 }
 
 bool GestureEventListener::SecondTapIsFar() const
 {
   // Allow a little more room here, because the is actually lifting their finger
   // off the screen before replacing it, and that tends to have more error than
   // wiggling the finger while on the screen.
-  return MoveDistanceExceeds(AsyncPanZoomController::GetSecondTapTolerance());
+  return MoveDistanceExceeds(mAsyncPanZoomController->GetSecondTapTolerance());
 }
 
 nsEventStatus GestureEventListener::HandleInputTouchMove()
 {
   nsEventStatus rv = nsEventStatus_eIgnore;
 
   switch (mState) {
   case GESTURE_NONE:
--- a/gfx/layers/apz/src/InputBlockState.cpp
+++ b/gfx/layers/apz/src/InputBlockState.cpp
@@ -1,17 +1,16 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "InputBlockState.h"
 
-#include "APZCTreeManager.h"                // for APZCTreeManager::GetDPI
 #include "AsyncPanZoomController.h"         // for AsyncPanZoomController
 #include "ScrollAnimationPhysics.h"         // for kScrollSeriesTimeoutMs
 #include "gfxPrefs.h"                       // for gfxPrefs
 #include "mozilla/MouseEvents.h"
 #include "mozilla/Telemetry.h"              // for Telemetry
 #include "mozilla/layers/IAPZCTreeManager.h" // for AllowedTouchBehavior
 #include "OverscrollHandoffState.h"
 #include "QueuedInput.h"
@@ -885,19 +884,23 @@ TouchBlockState::UpdateSlopState(const M
     mInSlop = (aInput.mTouches.Length() == 1);
     if (mInSlop) {
       mSlopOrigin = aInput.mTouches[0].mScreenPoint;
       TBS_LOG("%p entering slop with origin %s\n", this, Stringify(mSlopOrigin).c_str());
     }
     return false;
   }
   if (mInSlop) {
-    ScreenCoord threshold = aApzcCanConsumeEvents
-        ? AsyncPanZoomController::GetTouchStartTolerance()
-        : ScreenCoord(gfxPrefs::APZTouchMoveTolerance() * APZCTreeManager::GetDPI());
+    ScreenCoord threshold = 0;
+    // If the target was confirmed to null then the threshold doesn't
+    // matter anyway since the events will never be processed.
+    if (const RefPtr<AsyncPanZoomController>& apzc = GetTargetApzc()) {
+      threshold = aApzcCanConsumeEvents ? apzc->GetTouchStartTolerance()
+                                        : apzc->GetTouchMoveTolerance();
+    }
     bool stayInSlop = (aInput.mType == MultiTouchInput::MULTITOUCH_MOVE) &&
         (aInput.mTouches.Length() == 1) &&
         ((aInput.mTouches[0].mScreenPoint - mSlopOrigin).Length() < threshold);
     if (!stayInSlop) {
       // we're out of the slop zone, and will stay out for the remainder of
       // this block
       TBS_LOG("%p exiting slop\n", this);
       mInSlop = false;
--- a/gfx/layers/ipc/APZCTreeManagerParent.cpp
+++ b/gfx/layers/ipc/APZCTreeManagerParent.cpp
@@ -243,17 +243,21 @@ APZCTreeManagerParent::RecvUpdateZoomCon
 
   mTreeManager->UpdateZoomConstraints(aGuid, aConstraints);
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvSetDPI(const float& aDpiValue)
 {
-  mTreeManager->SetDPI(aDpiValue);
+  APZThreadUtils::RunOnControllerThread(NewRunnableMethod<float>(
+    "layers::IAPZCTreeManager::SetDPI",
+    mTreeManager,
+    &IAPZCTreeManager::SetDPI,
+    aDpiValue));
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult
 APZCTreeManagerParent::RecvSetAllowedTouchBehavior(
     const uint64_t& aInputBlockId,
     nsTArray<TouchBehaviorFlags>&& aValues)
 {
--- a/widget/nsBaseWidget.cpp
+++ b/widget/nsBaseWidget.cpp
@@ -933,17 +933,23 @@ nsBaseWidget::CreateRootContentControlle
 
 void nsBaseWidget::ConfigureAPZCTreeManager()
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(mAPZC);
 
   ConfigureAPZControllerThread();
 
-  mAPZC->SetDPI(GetDPI());
+  float dpi = GetDPI();
+  // On Android the main thread is not the controller thread
+  APZThreadUtils::RunOnControllerThread(NewRunnableMethod<float>(
+      "layers::IAPZCTreeManager::SetDPI",
+      mAPZC,
+      &IAPZCTreeManager::SetDPI,
+      dpi));
 
   if (gfxPrefs::APZKeyboardEnabled()) {
     KeyboardMap map = nsXBLWindowKeyHandler::CollectKeyboardShortcuts();
     // On Android the main thread is not the controller thread
     APZThreadUtils::RunOnControllerThread(NewRunnableMethod<KeyboardMap>(
         "layers::IAPZCTreeManager::SetKeyboardMap",
         mAPZC,
         &IAPZCTreeManager::SetKeyboardMap,