From 516e48ba7e29cdd1d061b76dd628c0785faa4cdd Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 5 Apr 2024 16:23:57 -0700 Subject: [PATCH 1/3] Impl --- .../window/pointer_data_packet_converter.cc | 6 ++--- lib/ui/window/pointer_data_packet_converter.h | 6 ++--- ...pointer_data_packet_converter_unittests.cc | 24 +++++++++---------- runtime/runtime_controller.cc | 6 ++++- runtime/runtime_controller.h | 2 ++ shell/common/platform_view.cc | 3 +-- shell/common/platform_view.h | 2 -- 7 files changed, 25 insertions(+), 24 deletions(-) diff --git a/lib/ui/window/pointer_data_packet_converter.cc b/lib/ui/window/pointer_data_packet_converter.cc index a2270382e226d..5cf200530e57d 100644 --- a/lib/ui/window/pointer_data_packet_converter.cc +++ b/lib/ui/window/pointer_data_packet_converter.cc @@ -16,12 +16,12 @@ PointerDataPacketConverter::PointerDataPacketConverter() {} PointerDataPacketConverter::~PointerDataPacketConverter() = default; std::unique_ptr PointerDataPacketConverter::Convert( - std::unique_ptr packet) { + const PointerDataPacket& packet) { std::vector converted_pointers; // Converts each pointer data in the buffer and stores it in the // converted_pointers. - for (size_t i = 0; i < packet->GetLength(); i++) { - PointerData pointer_data = packet->GetPointerData(i); + for (size_t i = 0; i < packet.GetLength(); i++) { + PointerData pointer_data = packet.GetPointerData(i); ConvertPointerData(pointer_data, converted_pointers); } diff --git a/lib/ui/window/pointer_data_packet_converter.h b/lib/ui/window/pointer_data_packet_converter.h index d0e59803255aa..e903e4e1d06c7 100644 --- a/lib/ui/window/pointer_data_packet_converter.h +++ b/lib/ui/window/pointer_data_packet_converter.h @@ -89,12 +89,10 @@ class PointerDataPacketConverter { /// embedding. /// /// @return A full converted packet with all the required information - /// filled. - /// It may contain synthetic pointer data as the result of + /// filled. It may contain synthetic pointer data as the result of /// converter's attempt to correct illegal pointer transitions. /// - std::unique_ptr Convert( - std::unique_ptr packet); + std::unique_ptr Convert(const PointerDataPacket& packet); private: std::map states_; diff --git a/lib/ui/window/pointer_data_packet_converter_unittests.cc b/lib/ui/window/pointer_data_packet_converter_unittests.cc index 69e43cba54fe0..69f798dffc5d4 100644 --- a/lib/ui/window/pointer_data_packet_converter_unittests.cc +++ b/lib/ui/window/pointer_data_packet_converter_unittests.cc @@ -160,7 +160,7 @@ TEST(PointerDataPacketConverterTest, CanConvertPointerDataPacket) { CreateSimulatedPointerData(data, PointerData::Change::kRemove, 0, 3.0, 4.0, 0); packet->SetPointerData(5, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); std::vector result; UnpackPointerPacket(result, std::move(converted_packet)); @@ -205,7 +205,7 @@ TEST(PointerDataPacketConverterTest, CanSynthesizeDownAndUp) { CreateSimulatedPointerData(data, PointerData::Change::kRemove, 0, 3.0, 4.0, 0); packet->SetPointerData(3, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); std::vector result; UnpackPointerPacket(result, std::move(converted_packet)); @@ -262,7 +262,7 @@ TEST(PointerDataPacketConverterTest, CanUpdatePointerIdentifier) { CreateSimulatedPointerData(data, PointerData::Change::kRemove, 0, 3.0, 0.0, 0); packet->SetPointerData(6, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); std::vector result; UnpackPointerPacket(result, std::move(converted_packet)); @@ -312,7 +312,7 @@ TEST(PointerDataPacketConverterTest, AlwaysForwardMoveEvent) { CreateSimulatedPointerData(data, PointerData::Change::kUp, 0, 0.0, 0.0, 0); packet->SetPointerData(3, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); std::vector result; UnpackPointerPacket(result, std::move(converted_packet)); @@ -365,7 +365,7 @@ TEST(PointerDataPacketConverterTest, CanWorkWithDifferentDevices) { CreateSimulatedPointerData(data, PointerData::Change::kRemove, 1, 0.0, 4.0, 0); packet->SetPointerData(11, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); std::vector result; UnpackPointerPacket(result, std::move(converted_packet)); @@ -441,7 +441,7 @@ TEST(PointerDataPacketConverterTest, CanSynthesizeAdd) { packet->SetPointerData(0, data); CreateSimulatedPointerData(data, PointerData::Change::kUp, 0, 0.0, 0.0, 0); packet->SetPointerData(1, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); std::vector result; UnpackPointerPacket(result, std::move(converted_packet)); @@ -485,14 +485,14 @@ TEST(PointerDataPacketConverterTest, CanHandleThreeFingerGesture) { auto packet = std::make_unique(1); CreateSimulatedPointerData(data, PointerData::Change::kDown, 0, 0.0, 0.0, 1); packet->SetPointerData(0, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); UnpackPointerPacket(result, std::move(converted_packet)); // Second finger down. packet = std::make_unique(1); CreateSimulatedPointerData(data, PointerData::Change::kDown, 1, 33.0, 44.0, 1); packet->SetPointerData(0, data); - converted_packet = converter.Convert(std::move(packet)); + converted_packet = converter.Convert(*packet); UnpackPointerPacket(result, std::move(converted_packet)); // Triggers three cancels. packet = std::make_unique(3); @@ -505,7 +505,7 @@ TEST(PointerDataPacketConverterTest, CanHandleThreeFingerGesture) { CreateSimulatedPointerData(data, PointerData::Change::kCancel, 2, 40.0, 50.0, 0); packet->SetPointerData(2, data); - converted_packet = converter.Convert(std::move(packet)); + converted_packet = converter.Convert(*packet); UnpackPointerPacket(result, std::move(converted_packet)); ASSERT_EQ(result.size(), (size_t)6); @@ -582,7 +582,7 @@ TEST(PointerDataPacketConverterTest, CanConvertPointerSignals) { CreateSimulatedMousePointerData(data, PointerData::Change::kHover, kind, 2, 10.0, 20.0, 30.0, 40.0, 0); packet->SetPointerData(5, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); std::vector result; UnpackPointerPacket(result, std::move(converted_packet)); @@ -678,7 +678,7 @@ TEST(PointerDataPacketConverterTest, CanConvertTrackpadGesture) { CreateSimulatedTrackpadGestureData(data, PointerData::Change::kPanZoomEnd, 0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0); packet->SetPointerData(2, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); std::vector result; UnpackPointerPacket(result, std::move(converted_packet)); @@ -726,7 +726,7 @@ TEST(PointerDataPacketConverterTest, CanConvertViewId) { CreateSimulatedPointerData(data, PointerData::Change::kHover, 0, 1.0, 0.0, 0); data.view_id = 200; packet->SetPointerData(1, data); - auto converted_packet = converter.Convert(std::move(packet)); + auto converted_packet = converter.Convert(*packet); std::vector result; UnpackPointerPacket(result, std::move(converted_packet)); diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 062d31c88b85c..56a91c286c444 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -358,7 +358,11 @@ bool RuntimeController::DispatchPointerDataPacket( const PointerDataPacket& packet) { if (auto* platform_configuration = GetPlatformConfigurationIfAvailable()) { TRACE_EVENT0("flutter", "RuntimeController::DispatchPointerDataPacket"); - platform_configuration->DispatchPointerDataPacket(packet); + std::unique_ptr converted_packet = + pointer_data_packet_converter_.Convert(packet); + if (converted_packet->GetLength() != 0) { + platform_configuration->DispatchPointerDataPacket(*converted_packet); + } return true; } diff --git a/runtime/runtime_controller.h b/runtime/runtime_controller.h index 46077b5a847d2..89f3eaf86cf41 100644 --- a/runtime/runtime_controller.h +++ b/runtime/runtime_controller.h @@ -20,6 +20,7 @@ #include "flutter/lib/ui/volatile_path_tracker.h" #include "flutter/lib/ui/window/platform_configuration.h" #include "flutter/lib/ui/window/pointer_data_packet.h" +#include "flutter/lib/ui/window/pointer_data_packet_converter.h" #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/platform_data.h" #include "flutter/runtime/platform_isolate_manager.h" @@ -687,6 +688,7 @@ class RuntimeController : public PlatformConfigurationClient { const fml::closure isolate_shutdown_callback_; std::shared_ptr persistent_isolate_data_; UIDartState::Context context_; + PointerDataPacketConverter pointer_data_packet_converter_; std::shared_ptr platform_isolate_manager_ = std::shared_ptr(new PlatformIsolateManager()); bool has_flushed_runtime_state_ = false; diff --git a/shell/common/platform_view.cc b/shell/common/platform_view.cc index 48439c252447a..f4ba18cfc73cc 100644 --- a/shell/common/platform_view.cc +++ b/shell/common/platform_view.cc @@ -32,8 +32,7 @@ void PlatformView::DispatchPlatformMessage( void PlatformView::DispatchPointerDataPacket( std::unique_ptr packet) { - delegate_.OnPlatformViewDispatchPointerDataPacket( - pointer_data_packet_converter_.Convert(std::move(packet))); + delegate_.OnPlatformViewDispatchPointerDataPacket(std::move(packet)); } void PlatformView::DispatchSemanticsAction(int32_t node_id, diff --git a/shell/common/platform_view.h b/shell/common/platform_view.h index bfb6489b88e49..f9bb200dd0424 100644 --- a/shell/common/platform_view.h +++ b/shell/common/platform_view.h @@ -20,7 +20,6 @@ #include "flutter/lib/ui/window/key_data_packet.h" #include "flutter/lib/ui/window/platform_message.h" #include "flutter/lib/ui/window/pointer_data_packet.h" -#include "flutter/lib/ui/window/pointer_data_packet_converter.h" #include "flutter/lib/ui/window/viewport_metrics.h" #include "flutter/shell/common/platform_message_handler.h" #include "flutter/shell/common/pointer_data_dispatcher.h" @@ -872,7 +871,6 @@ class PlatformView { PlatformView::Delegate& delegate_; const TaskRunners task_runners_; - PointerDataPacketConverter pointer_data_packet_converter_; fml::WeakPtrFactory weak_factory_; // Must be the last member. private: From ec0fd5b7b9619a5d191943d421d7e663778039c5 Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Fri, 5 Apr 2024 17:36:16 -0700 Subject: [PATCH 2/3] Fix tests --- shell/common/input_events_unittests.cc | 5 ++++- shell/common/shell_test.cc | 6 +++++- shell/common/shell_test.h | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/shell/common/input_events_unittests.cc b/shell/common/input_events_unittests.cc index 66a2b64a39e02..13793b34d1ebd 100644 --- a/shell/common/input_events_unittests.cc +++ b/shell/common/input_events_unittests.cc @@ -124,7 +124,9 @@ static void TestSimulatedInputEvents( for (int i = 0, j = 0; i < num_events; j += 1) { double t = j * frame_time; while (i < num_events && delivery_time(i) <= t) { - ShellTest::DispatchFakePointerData(shell.get()); + // Use a different x every time for the pointer data converter to + // generate non-empty events. + ShellTest::DispatchFakePointerData(shell.get(), /*x=*/i); i += 1; } ShellTest::VSyncFlush(shell.get(), &will_draw_new_frame); @@ -146,6 +148,7 @@ static void TestSimulatedInputEvents( // Make sure that all events have been consumed so // https://github.com/flutter/flutter/issues/40863 won't happen again. + ASSERT_GT(events_consumed_at_frame.size(), 0u); ASSERT_EQ(events_consumed_at_frame.back(), num_events); } diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index efcf43ce82668..6fcd15d6aa9a2 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -258,8 +258,12 @@ void ShellTest::PumpOneFrame(Shell* shell, FrameContent frame_content) { latch.Wait(); } -void ShellTest::DispatchFakePointerData(Shell* shell) { +void ShellTest::DispatchFakePointerData(Shell* shell, double x) { auto packet = std::make_unique(1); + packet->SetPointerData(0, PointerData{ + .change = PointerData::Change::kHover, + .physical_x = x, + }); DispatchPointerData(shell, std::move(packet)); } diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index c11ad1174dc88..03c30bc815db0 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -110,7 +110,7 @@ class ShellTest : public FixtureTest { static void PumpOneFrame(Shell* shell); static void PumpOneFrame(Shell* shell, FrameContent frame_content); - static void DispatchFakePointerData(Shell* shell); + static void DispatchFakePointerData(Shell* shell, double x); static void DispatchPointerData(Shell* shell, std::unique_ptr packet); // Declare |UnreportedTimingsCount|, |GetNeedsReportTimings| and From 6717f8575cce90d0711b23480662aed4b43a7d7b Mon Sep 17 00:00:00 2001 From: Tong Mu Date: Mon, 8 Apr 2024 16:26:21 -0700 Subject: [PATCH 3/3] Remove the length check --- runtime/runtime_controller.cc | 4 +--- shell/common/input_events_unittests.cc | 4 +--- shell/common/shell_test.cc | 6 +----- shell/common/shell_test.h | 2 +- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/runtime/runtime_controller.cc b/runtime/runtime_controller.cc index 56a91c286c444..ad4fe0985c959 100644 --- a/runtime/runtime_controller.cc +++ b/runtime/runtime_controller.cc @@ -360,9 +360,7 @@ bool RuntimeController::DispatchPointerDataPacket( TRACE_EVENT0("flutter", "RuntimeController::DispatchPointerDataPacket"); std::unique_ptr converted_packet = pointer_data_packet_converter_.Convert(packet); - if (converted_packet->GetLength() != 0) { - platform_configuration->DispatchPointerDataPacket(*converted_packet); - } + platform_configuration->DispatchPointerDataPacket(*converted_packet); return true; } diff --git a/shell/common/input_events_unittests.cc b/shell/common/input_events_unittests.cc index 13793b34d1ebd..d8ee75fc33244 100644 --- a/shell/common/input_events_unittests.cc +++ b/shell/common/input_events_unittests.cc @@ -124,9 +124,7 @@ static void TestSimulatedInputEvents( for (int i = 0, j = 0; i < num_events; j += 1) { double t = j * frame_time; while (i < num_events && delivery_time(i) <= t) { - // Use a different x every time for the pointer data converter to - // generate non-empty events. - ShellTest::DispatchFakePointerData(shell.get(), /*x=*/i); + ShellTest::DispatchFakePointerData(shell.get()); i += 1; } ShellTest::VSyncFlush(shell.get(), &will_draw_new_frame); diff --git a/shell/common/shell_test.cc b/shell/common/shell_test.cc index 6fcd15d6aa9a2..efcf43ce82668 100644 --- a/shell/common/shell_test.cc +++ b/shell/common/shell_test.cc @@ -258,12 +258,8 @@ void ShellTest::PumpOneFrame(Shell* shell, FrameContent frame_content) { latch.Wait(); } -void ShellTest::DispatchFakePointerData(Shell* shell, double x) { +void ShellTest::DispatchFakePointerData(Shell* shell) { auto packet = std::make_unique(1); - packet->SetPointerData(0, PointerData{ - .change = PointerData::Change::kHover, - .physical_x = x, - }); DispatchPointerData(shell, std::move(packet)); } diff --git a/shell/common/shell_test.h b/shell/common/shell_test.h index 03c30bc815db0..c11ad1174dc88 100644 --- a/shell/common/shell_test.h +++ b/shell/common/shell_test.h @@ -110,7 +110,7 @@ class ShellTest : public FixtureTest { static void PumpOneFrame(Shell* shell); static void PumpOneFrame(Shell* shell, FrameContent frame_content); - static void DispatchFakePointerData(Shell* shell, double x); + static void DispatchFakePointerData(Shell* shell); static void DispatchPointerData(Shell* shell, std::unique_ptr packet); // Declare |UnreportedTimingsCount|, |GetNeedsReportTimings| and