From f04415557d5e42a7c2a3f3f0ce714fa299593697 Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Sun, 30 Oct 2022 02:33:14 -0400 Subject: [PATCH 1/3] Windows trackpad improvements --- shell/platform/windows/direct_manipulation.cc | 59 ++++++++---- shell/platform/windows/direct_manipulation.h | 14 +++ .../windows/direct_manipulation_unittests.cc | 93 +++++++++++++++++++ shell/platform/windows/window.cc | 2 +- shell/platform/windows/window.h | 2 +- 5 files changed, 149 insertions(+), 21 deletions(-) diff --git a/shell/platform/windows/direct_manipulation.cc b/shell/platform/windows/direct_manipulation.cc index 41ddf07b5acec..1d80d74654ae3 100644 --- a/shell/platform/windows/direct_manipulation.cc +++ b/shell/platform/windows/direct_manipulation.cc @@ -45,22 +45,49 @@ STDMETHODIMP DirectManipulationEventHandler::QueryInterface(REFIID iid, return E_NOINTERFACE; } +DirectManipulationEventHandler::GestureData +DirectManipulationEventHandler::convertToGestureData(float transform[6]) { + // DirectManipulation provides updates with very high precision. If the user + // holds their fingers steady on a trackpad, DirectManipulation sends + // jittery updates. This calculation will reduce the precision of the scale + // value of the event to avoid jitter. + const int mantissa_bits_chop = 2; + const float factor = (1 << mantissa_bits_chop) + 1; + float c = factor * transform[0]; + return GestureData{ + c - (c - transform[0]), // scale + transform[4], // pan_x + transform[5], // pan_y + }; +} + HRESULT DirectManipulationEventHandler::OnViewportStatusChanged( IDirectManipulationViewport* viewport, DIRECTMANIPULATION_STATUS current, DIRECTMANIPULATION_STATUS previous) { + if (during_synthesized_reset_) { + during_synthesized_reset_ = current != DIRECTMANIPULATION_READY; + return S_OK; + } during_inertia_ = current == DIRECTMANIPULATION_INERTIA; - if (during_synthesized_reset_ && previous == DIRECTMANIPULATION_RUNNING) { - during_synthesized_reset_ = false; - } else if (current == DIRECTMANIPULATION_RUNNING) { - if (!during_synthesized_reset_) { - // Not a false event. - if (owner_->binding_handler_delegate) { - owner_->binding_handler_delegate->OnPointerPanZoomStart(GetDeviceId()); + if (current == DIRECTMANIPULATION_RUNNING) { + IDirectManipulationContent* content; + HRESULT hr = viewport->GetPrimaryContent(IID_PPV_ARGS(&content)); + if (SUCCEEDED(hr)) { + float transform[6]; + hr = content->GetContentTransform(transform, ARRAYSIZE(transform)); + if (SUCCEEDED(hr)) { + initial_gesture_data_ = convertToGestureData(transform); + } else { + FML_LOG(ERROR) << "GetContentTransform failed"; } + } else { + FML_LOG(ERROR) << "GetPrimaryContent failed"; } - } - if (previous == DIRECTMANIPULATION_RUNNING) { + if (owner_->binding_handler_delegate) { + owner_->binding_handler_delegate->OnPointerPanZoomStart(GetDeviceId()); + } + } else if (previous == DIRECTMANIPULATION_RUNNING) { // Reset deltas to ensure only inertia values will be compared later. last_pan_delta_x_ = 0.0; last_pan_delta_y_ = 0.0; @@ -113,16 +140,10 @@ HRESULT DirectManipulationEventHandler::OnContentUpdated( return S_OK; } if (!during_synthesized_reset_) { - // DirectManipulation provides updates with very high precision. If the user - // holds their fingers steady on a trackpad, DirectManipulation sends - // jittery updates. This calculation will reduce the precision of the scale - // value of the event to avoid jitter. - const int mantissa_bits_chop = 2; - const float factor = (1 << mantissa_bits_chop) + 1; - float c = factor * transform[0]; - float scale = c - (c - transform[0]); - float pan_x = transform[4]; - float pan_y = transform[5]; + GestureData data = convertToGestureData(transform); + float scale = data.scale / initial_gesture_data_.scale; + float pan_x = data.pan_x - initial_gesture_data_.pan_x; + float pan_y = data.pan_y - initial_gesture_data_.pan_y; last_pan_delta_x_ = pan_x - last_pan_x_; last_pan_delta_y_ = pan_y - last_pan_y_; last_pan_x_ = pan_x; diff --git a/shell/platform/windows/direct_manipulation.h b/shell/platform/windows/direct_manipulation.h index e9aef840fdafa..5b6019db6d79c 100644 --- a/shell/platform/windows/direct_manipulation.h +++ b/shell/platform/windows/direct_manipulation.h @@ -106,6 +106,13 @@ class DirectManipulationEventHandler DIRECTMANIPULATION_INTERACTION_TYPE interaction) override; private: + struct GestureData { + float scale; + float pan_x; + float pan_y; + }; + // Convert transform array to Flutter-usable values. + GestureData convertToGestureData(float transform[6]); // Unique identifier to associate with all gesture event updates. int32_t GetDeviceId(); // Parent object, used to store the target for gesture event updates. @@ -117,6 +124,13 @@ class DirectManipulationEventHandler // Store whether current events are from synthetic inertia rather than user // input. bool during_inertia_ = false; + // The transform might not be able to be reset before the next gesture, so + // the initial state needs to be stored for reference. + GestureData initial_gesture_data_ = { + 1, // scale + 0, // pan_x + 0, // pan_y + }; // Store the difference between the last pan offsets to determine if inertia // has been cancelled in the middle of an animation. float last_pan_x_ = 0.0; diff --git a/shell/platform/windows/direct_manipulation_unittests.cc b/shell/platform/windows/direct_manipulation_unittests.cc index 1d03a0396ddde..5b1e687ba5225 100644 --- a/shell/platform/windows/direct_manipulation_unittests.cc +++ b/shell/platform/windows/direct_manipulation_unittests.cc @@ -155,6 +155,20 @@ TEST(DirectManipulationTest, TestGesture) { auto handler = fml::MakeRefCounted(owner.get()); int32_t device_id = (int32_t) reinterpret_cast(handler.get()); + EXPECT_CALL(viewport, GetPrimaryContent(_, _)) + .WillOnce(::testing::Invoke([&content](REFIID in, void** out) { + *out = &content; + return S_OK; + })) + .RetiresOnSaturation(); + EXPECT_CALL(content, GetContentTransform(_, 6)) + .WillOnce(::testing::Invoke([scale](float* transform, DWORD size) { + transform[0] = 1.0f; + transform[4] = 0.0; + transform[5] = 0.0; + return S_OK; + })) + .RetiresOnSaturation(); EXPECT_CALL(delegate, OnPointerPanZoomStart(device_id)); handler->OnViewportStatusChanged((IDirectManipulationViewport*)&viewport, DIRECTMANIPULATION_RUNNING, @@ -203,6 +217,20 @@ TEST(DirectManipulationTest, TestRounding) { auto handler = fml::MakeRefCounted(owner.get()); int32_t device_id = (int32_t) reinterpret_cast(handler.get()); + EXPECT_CALL(viewport, GetPrimaryContent(_, _)) + .WillOnce(::testing::Invoke([&content](REFIID in, void** out) { + *out = &content; + return S_OK; + })) + .RetiresOnSaturation(); + EXPECT_CALL(content, GetContentTransform(_, 6)) + .WillOnce(::testing::Invoke([scale](float* transform, DWORD size) { + transform[0] = 1.0f; + transform[4] = 0.0; + transform[5] = 0.0; + return S_OK; + })) + .RetiresOnSaturation(); EXPECT_CALL(delegate, OnPointerPanZoomStart(device_id)); handler->OnViewportStatusChanged((IDirectManipulationViewport*)&viewport, DIRECTMANIPULATION_RUNNING, @@ -364,5 +392,70 @@ TEST(DirectManipulationTest, TestInertiaCamcelNotSentAtInertiaEnd) { DIRECTMANIPULATION_INERTIA); } +// Have some initial values in the matrix, only the differences should be +// reported. +TEST(DirectManipulationTest, TestGestureWithInitialData) { + MockIDirectManipulationContent content; + MockWindowBindingHandlerDelegate delegate; + MockIDirectManipulationViewport viewport; + const float scale = 1.5; + const float pan_x = 32.0; + const float pan_y = 16.0; + const int DISPLAY_WIDTH = 800; + const int DISPLAY_HEIGHT = 600; + auto owner = std::make_unique(nullptr); + owner->SetBindingHandlerDelegate(&delegate); + auto handler = + fml::MakeRefCounted(owner.get()); + int32_t device_id = (int32_t) reinterpret_cast(handler.get()); + EXPECT_CALL(viewport, GetPrimaryContent(_, _)) + .WillOnce(::testing::Invoke([&content](REFIID in, void** out) { + *out = &content; + return S_OK; + })) + .RetiresOnSaturation(); + EXPECT_CALL(content, GetContentTransform(_, 6)) + .WillOnce(::testing::Invoke([scale](float* transform, DWORD size) { + transform[0] = 2.0f; + transform[4] = 234.0; + transform[5] = 345.0; + return S_OK; + })) + .RetiresOnSaturation(); + EXPECT_CALL(delegate, OnPointerPanZoomStart(device_id)); + handler->OnViewportStatusChanged((IDirectManipulationViewport*)&viewport, + DIRECTMANIPULATION_RUNNING, + DIRECTMANIPULATION_READY); + EXPECT_CALL(content, GetContentTransform(_, 6)) + .WillOnce(::testing::Invoke( + [scale, pan_x, pan_y](float* transform, DWORD size) { + transform[0] = 2.0f * scale; + transform[4] = 234.0 + pan_x; + transform[5] = 345.0 + pan_y; + return S_OK; + })); + EXPECT_CALL(delegate, + OnPointerPanZoomUpdate(device_id, pan_x, pan_y, scale, 0)); + handler->OnContentUpdated((IDirectManipulationViewport*)&viewport, + (IDirectManipulationContent*)&content); + EXPECT_CALL(delegate, OnPointerPanZoomEnd(device_id)); + EXPECT_CALL(viewport, GetViewportRect(_)) + .WillOnce(::testing::Invoke([DISPLAY_WIDTH, DISPLAY_HEIGHT](RECT* rect) { + rect->left = 0; + rect->top = 0; + rect->right = DISPLAY_WIDTH; + rect->bottom = DISPLAY_HEIGHT; + return S_OK; + })); + EXPECT_CALL(viewport, ZoomToRect(0, 0, DISPLAY_WIDTH, DISPLAY_HEIGHT, false)) + .WillOnce(::testing::Return(S_OK)); + handler->OnViewportStatusChanged((IDirectManipulationViewport*)&viewport, + DIRECTMANIPULATION_INERTIA, + DIRECTMANIPULATION_RUNNING); + handler->OnViewportStatusChanged((IDirectManipulationViewport*)&viewport, + DIRECTMANIPULATION_READY, + DIRECTMANIPULATION_INERTIA); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/window.cc b/shell/platform/windows/window.cc index 9581d05ab9d51..d3cfb3db95cf4 100644 --- a/shell/platform/windows/window.cc +++ b/shell/platform/windows/window.cc @@ -111,7 +111,7 @@ void Window::InitializeChild(const char* title, ZeroMemory(&dmi, sizeof(dmi)); dmi.dmSize = sizeof(dmi); if (EnumDisplaySettings(NULL, ENUM_CURRENT_SETTINGS, &dmi)) { - directManipulationPollingRate_ = dmi.dmDisplayFrequency; + directManipulationPollingRate_ = 4 * dmi.dmDisplayFrequency; } else { OutputDebugString( L"Failed to get framerate, will use default of 60 Hz for gesture " diff --git a/shell/platform/windows/window.h b/shell/platform/windows/window.h index 95784d0eff986..d0a3df426680a 100644 --- a/shell/platform/windows/window.h +++ b/shell/platform/windows/window.h @@ -301,7 +301,7 @@ class Window : public KeyboardManager::WindowDelegate { const static int kDirectManipulationTimer = 1; // Frequency (Hz) to poll for DirectManipulation updates. - int directManipulationPollingRate_ = 60; + int directManipulationPollingRate_ = 240; }; } // namespace flutter From 63584b353ba255a3758771d66333905f363f165b Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Thu, 3 Nov 2022 01:39:03 -0400 Subject: [PATCH 2/3] Address feedback, remove dynamic polling rate --- shell/platform/windows/direct_manipulation.cc | 6 +++--- shell/platform/windows/direct_manipulation.h | 2 +- shell/platform/windows/window.cc | 18 ++++-------------- shell/platform/windows/window.h | 3 --- 4 files changed, 8 insertions(+), 21 deletions(-) diff --git a/shell/platform/windows/direct_manipulation.cc b/shell/platform/windows/direct_manipulation.cc index 1d80d74654ae3..72e676a1b3283 100644 --- a/shell/platform/windows/direct_manipulation.cc +++ b/shell/platform/windows/direct_manipulation.cc @@ -46,7 +46,7 @@ STDMETHODIMP DirectManipulationEventHandler::QueryInterface(REFIID iid, } DirectManipulationEventHandler::GestureData -DirectManipulationEventHandler::convertToGestureData(float transform[6]) { +DirectManipulationEventHandler::ConvertToGestureData(float transform[6]) { // DirectManipulation provides updates with very high precision. If the user // holds their fingers steady on a trackpad, DirectManipulation sends // jittery updates. This calculation will reduce the precision of the scale @@ -77,7 +77,7 @@ HRESULT DirectManipulationEventHandler::OnViewportStatusChanged( float transform[6]; hr = content->GetContentTransform(transform, ARRAYSIZE(transform)); if (SUCCEEDED(hr)) { - initial_gesture_data_ = convertToGestureData(transform); + initial_gesture_data_ = ConvertToGestureData(transform); } else { FML_LOG(ERROR) << "GetContentTransform failed"; } @@ -140,7 +140,7 @@ HRESULT DirectManipulationEventHandler::OnContentUpdated( return S_OK; } if (!during_synthesized_reset_) { - GestureData data = convertToGestureData(transform); + GestureData data = ConvertToGestureData(transform); float scale = data.scale / initial_gesture_data_.scale; float pan_x = data.pan_x - initial_gesture_data_.pan_x; float pan_y = data.pan_y - initial_gesture_data_.pan_y; diff --git a/shell/platform/windows/direct_manipulation.h b/shell/platform/windows/direct_manipulation.h index 5b6019db6d79c..6270015fa6f4c 100644 --- a/shell/platform/windows/direct_manipulation.h +++ b/shell/platform/windows/direct_manipulation.h @@ -112,7 +112,7 @@ class DirectManipulationEventHandler float pan_y; }; // Convert transform array to Flutter-usable values. - GestureData convertToGestureData(float transform[6]); + GestureData ConvertToGestureData(float transform[6]); // Unique identifier to associate with all gesture event updates. int32_t GetDeviceId(); // Parent object, used to store the target for gesture event updates. diff --git a/shell/platform/windows/window.cc b/shell/platform/windows/window.cc index d3cfb3db95cf4..70dfdd99ca9f8 100644 --- a/shell/platform/windows/window.cc +++ b/shell/platform/windows/window.cc @@ -107,20 +107,12 @@ void Window::InitializeChild(const char* title, OutputDebugString(message); LocalFree(message); } - DEVMODE dmi; - ZeroMemory(&dmi, sizeof(dmi)); - dmi.dmSize = sizeof(dmi); - if (EnumDisplaySettings(NULL, ENUM_CURRENT_SETTINGS, &dmi)) { - directManipulationPollingRate_ = 4 * dmi.dmDisplayFrequency; - } else { - OutputDebugString( - L"Failed to get framerate, will use default of 60 Hz for gesture " - L"polling."); - } SetUserObjectInformationA(GetCurrentProcess(), UOI_TIMERPROC_EXCEPTION_SUPPRESSION, FALSE, 1); - SetTimer(result, kDirectManipulationTimer, - 1000 / directManipulationPollingRate_, nullptr); + // SetTimer is not precise, if a 16 ms interval is requested, it will instead + // often fire after 32 ms. Providing a value of 14 will ensure it runs every + // 16 ms, which will allow for 60 Hz trackpad gesture events. + SetTimer(result, kDirectManipulationTimer, 14, nullptr); direct_manipulation_owner_ = std::make_unique(this); direct_manipulation_owner_->Init(width, height); } @@ -487,8 +479,6 @@ Window::HandleMessage(UINT const message, case WM_TIMER: if (wparam == kDirectManipulationTimer) { direct_manipulation_owner_->Update(); - SetTimer(window_handle_, kDirectManipulationTimer, - 1000 / directManipulationPollingRate_, nullptr); return 0; } break; diff --git a/shell/platform/windows/window.h b/shell/platform/windows/window.h index d0a3df426680a..ee9af55950b59 100644 --- a/shell/platform/windows/window.h +++ b/shell/platform/windows/window.h @@ -299,9 +299,6 @@ class Window : public KeyboardManager::WindowDelegate { // Timer identifier for DirectManipulation gesture polling. const static int kDirectManipulationTimer = 1; - - // Frequency (Hz) to poll for DirectManipulation updates. - int directManipulationPollingRate_ = 240; }; } // namespace flutter From 16ae4cb546800e3890e1203ceab1ef819d90c872 Mon Sep 17 00:00:00 2001 From: Callum Moffat Date: Thu, 3 Nov 2022 20:11:59 -0400 Subject: [PATCH 3/3] Adjust comment --- shell/platform/windows/window.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shell/platform/windows/window.cc b/shell/platform/windows/window.cc index 70dfdd99ca9f8..a6fe97b0d5383 100644 --- a/shell/platform/windows/window.cc +++ b/shell/platform/windows/window.cc @@ -110,8 +110,9 @@ void Window::InitializeChild(const char* title, SetUserObjectInformationA(GetCurrentProcess(), UOI_TIMERPROC_EXCEPTION_SUPPRESSION, FALSE, 1); // SetTimer is not precise, if a 16 ms interval is requested, it will instead - // often fire after 32 ms. Providing a value of 14 will ensure it runs every - // 16 ms, which will allow for 60 Hz trackpad gesture events. + // often fire in an interval of 32 ms. Providing a value of 14 will ensure it + // runs every 16 ms, which will allow for 60 Hz trackpad gesture events, which + // is the maximal frequency supported by SetTimer. SetTimer(result, kDirectManipulationTimer, 14, nullptr); direct_manipulation_owner_ = std::make_unique(this); direct_manipulation_owner_->Init(width, height);