Bug 1648489. Send pinch starts events with some amount of scale instead of none with direct manipulation. r=kats
authorTimothy Nikkel <tnikkel@gmail.com>
Wed, 01 Jul 2020 09:31:03 +0000
changeset 538178 b2142123046300857dc78bb694d5c7ce2a443604
parent 538177 e52e5ee6f9d6217f3be04a71ff089148116f8fb0
child 538179 bb01f404ab4728eee5b3bb11e1e8620025730462
push id120455
push usertnikkel@mozilla.com
push dateWed, 01 Jul 2020 09:31:58 +0000
treeherderautoland@b21421230463 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskats
bugs1648489
milestone80.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1648489. Send pinch starts events with some amount of scale instead of none with direct manipulation. r=kats Differential Revision: https://phabricator.services.mozilla.com/D81477
widget/InputData.cpp
widget/windows/DirectManipulationOwner.cpp
--- a/widget/InputData.cpp
+++ b/widget/InputData.cpp
@@ -623,16 +623,18 @@ WidgetWheelEvent PinchGestureInput::ToWi
 
   // XXX When we write the code for other platforms to do the same we'll need to
   // make sure this calculation is reasonable.
 
   wheelEvent.mDeltaY = (mPreviousSpan - mCurrentSpan) *
                        (aWidget ? aWidget->GetDefaultScaleInternal() : 1.f);
 #endif
 
+  MOZ_ASSERT(mType == PINCHGESTURE_END || wheelEvent.mDeltaY != 0.0);
+
   return wheelEvent;
 }
 
 TapGestureInput::TapGestureInput()
     : InputData(TAPGESTURE_INPUT), mType(TAPGESTURE_LONG) {}
 
 TapGestureInput::TapGestureInput(TapGestureType aType, uint32_t aTime,
                                  TimeStamp aTimeStamp,
--- a/widget/windows/DirectManipulationOwner.cpp
+++ b/widget/windows/DirectManipulationOwner.cpp
@@ -93,29 +93,31 @@ class DManipEventHandler : public IDirec
   nsWindow* mWindow;
   DirectManipulationOwner* mOwner;
   RefPtr<VObserver> mObserver;
   float mLastScale;
   float mLastXOffset;
   float mLastYOffset;
   LayoutDeviceIntRect mBounds;
   bool mShouldSendPanStart;
+  bool mShouldSendPinchStart;
   State mState = State::eNone;
 };
 
 DManipEventHandler::DManipEventHandler(nsWindow* aWindow,
                                        DirectManipulationOwner* aOwner,
                                        const LayoutDeviceIntRect& aBounds)
     : mWindow(aWindow),
       mOwner(aOwner),
       mLastScale(1.f),
       mLastXOffset(0.f),
       mLastYOffset(0.f),
       mBounds(aBounds),
-      mShouldSendPanStart(false) {}
+      mShouldSendPanStart(false),
+      mShouldSendPinchStart(false) {}
 
 STDMETHODIMP
 DManipEventHandler::QueryInterface(REFIID iid, void** ppv) {
   const IID IID_IDirectManipulationViewportEventHandler =
       __uuidof(IDirectManipulationViewportEventHandler);
   const IID IID_IDirectManipulationInteractionEventHandler =
       __uuidof(IDirectManipulationInteractionEventHandler);
 
@@ -237,17 +239,21 @@ void DManipEventHandler::TransitionToSta
       // Only ePanning can transition to eInertia.
       MOZ_ASSERT(prevState == State::ePanning);
       SendPan(Phase::eStart, 0.f, 0.f, true);
       break;
     }
     case State::ePinching: {
       // * -> ePinching: PinchStart.
       // Pinch gesture may begin with some scroll events.
-      SendPinch(Phase::eStart, 0.f);
+      // We're being called from OnContentUpdated, it has the scale we need to
+      // pass to SendPinch(Phase::eStart), so set mShouldSendPinchStart and when
+      // we return OnContentUpdated will check it and call
+      // SendPinch(Phase::eStart).
+      mShouldSendPinchStart = true;
       break;
     }
     case State::eNone: {
       // * -> eNone: only cleanup is needed.
       break;
     }
     default:
       MOZ_ASSERT(false);
@@ -294,17 +300,22 @@ DManipEventHandler::OnContentUpdated(IDi
     } else {
       SendPan(Phase::eMiddle, mLastXOffset - xoffset, mLastYOffset - yoffset,
               false);
     }
   } else if (mState == State::eInertia) {
     SendPan(Phase::eMiddle, mLastXOffset - xoffset, mLastYOffset - yoffset,
             true);
   } else if (mState == State::ePinching) {
-    SendPinch(Phase::eMiddle, scale);
+    if (mShouldSendPinchStart) {
+      SendPinch(Phase::eStart, scale);
+      mShouldSendPinchStart = false;
+    } else {
+      SendPinch(Phase::eMiddle, scale);
+    }
   }
 
   mLastScale = scale;
   mLastXOffset = xoffset;
   mLastYOffset = yoffset;
 
   return S_OK;
 }
@@ -386,26 +397,25 @@ void DManipEventHandler::SendPinch(Phase
       PixelCastJustification::LayoutDeviceIsScreenForUntransformedEvent);
 
   POINT cursor_pos;
   ::GetCursorPos(&cursor_pos);
   HWND wnd = static_cast<HWND>(mWindow->GetNativeData(NS_NATIVE_WINDOW));
   ::ScreenToClient(wnd, &cursor_pos);
   ScreenPoint position = {(float)cursor_pos.x, (float)cursor_pos.y};
 
-  PinchGestureInput event{
-      pinchGestureType,
-      PinchGestureInput::TRACKPAD,
-      eventIntervalTime,
-      eventTimeStamp,
-      screenOffset,
-      position,
-      100.0 * ((aPhase != Phase::eMiddle) ? 1.f : aScale),
-      100.0 * ((aPhase != Phase::eMiddle) ? 1.f : mLastScale),
-      mods};
+  PinchGestureInput event{pinchGestureType,
+                          PinchGestureInput::TRACKPAD,
+                          eventIntervalTime,
+                          eventTimeStamp,
+                          screenOffset,
+                          position,
+                          100.0 * ((aPhase == Phase::eEnd) ? 1.f : aScale),
+                          100.0 * ((aPhase == Phase::eEnd) ? 1.f : mLastScale),
+                          mods};
 
   mWindow->SendAnAPZEvent(event);
 }
 
 void DManipEventHandler::SendPan(Phase aPhase, float x, float y,
                                  bool aIsInertia) {
   if (!mWindow) {
     return;