From 3250fe01596f956a88ae1de8305e0dd17c63663b Mon Sep 17 00:00:00 2001 From: Harlen Batagelo Date: Tue, 8 Oct 2024 11:45:28 -0300 Subject: [PATCH 1/3] Synthesize pointer remove event --- .../window/pointer_data_packet_converter.cc | 43 ++++++++++++++++++- lib/ui/window/pointer_data_packet_converter.h | 1 + 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/lib/ui/window/pointer_data_packet_converter.cc b/lib/ui/window/pointer_data_packet_converter.cc index 45b05a00dd852..0290d92573af5 100644 --- a/lib/ui/window/pointer_data_packet_converter.cc +++ b/lib/ui/window/pointer_data_packet_converter.cc @@ -74,7 +74,42 @@ void PointerDataPacketConverter::ConvertPointerData( break; } case PointerData::Change::kAdd: { - FML_DCHECK(states_.find(pointer_data.device) == states_.end()); + auto iter = states_.find(pointer_data.device); + if (iter != states_.end()) { + // Synthesizes a remove event if the pointer was not previously + // removed. + PointerState state = iter->second; + PointerData synthesized_data = pointer_data; + synthesized_data.physical_x = state.physical_x; + synthesized_data.physical_y = state.physical_y; + synthesized_data.pan_x = state.pan_x; + synthesized_data.pan_y = state.pan_y; + synthesized_data.scale = state.scale; + synthesized_data.rotation = state.rotation; + synthesized_data.buttons = state.buttons; + + if (delegate_.ViewExists(state.view_id)) { + synthesized_data.view_id = state.view_id; + + if (state.is_down) { + // Synthesizes cancel event if the pointer is down. + PointerData synthesized_cancel_event = synthesized_data; + synthesized_cancel_event.change = PointerData::Change::kCancel; + synthesized_cancel_event.synthesized = 1; + UpdatePointerIdentifier(synthesized_cancel_event, state, false); + + state.is_down = false; + converted_pointers.push_back(synthesized_cancel_event); + } + } + + PointerData synthesized_remove_event = synthesized_data; + synthesized_remove_event.change = PointerData::Change::kRemove; + synthesized_remove_event.synthesized = 1; + + converted_pointers.push_back(synthesized_remove_event); + } + EnsurePointerState(pointer_data); converted_pointers.push_back(pointer_data); break; @@ -84,6 +119,11 @@ void PointerDataPacketConverter::ConvertPointerData( auto iter = states_.find(pointer_data.device); FML_DCHECK(iter != states_.end()); PointerState state = iter->second; + if (state.view_id != pointer_data.view_id) { + // Ignores remove event if the pointer was previously added to a + // different view. + break; + } if (state.is_down) { // Synthesizes cancel event if the pointer is down. @@ -362,6 +402,7 @@ PointerState PointerDataPacketConverter::EnsurePointerState( state.physical_y = pointer_data.physical_y; state.pan_x = 0; state.pan_y = 0; + state.view_id = pointer_data.view_id; states_[pointer_data.device] = state; return state; } diff --git a/lib/ui/window/pointer_data_packet_converter.h b/lib/ui/window/pointer_data_packet_converter.h index 146ade258a4a4..7b1b823f53dc9 100644 --- a/lib/ui/window/pointer_data_packet_converter.h +++ b/lib/ui/window/pointer_data_packet_converter.h @@ -38,6 +38,7 @@ struct PointerState { double scale; double rotation; int64_t buttons; + int64_t view_id; }; //------------------------------------------------------------------------------ From 31d6c003ce0cd4b6c2f74c104b59181397765903 Mon Sep 17 00:00:00 2001 From: Harlen Batagelo Date: Tue, 8 Oct 2024 11:46:34 -0300 Subject: [PATCH 2/3] Add test --- ...pointer_data_packet_converter_unittests.cc | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/lib/ui/window/pointer_data_packet_converter_unittests.cc b/lib/ui/window/pointer_data_packet_converter_unittests.cc index 4f6a37f4536d8..0ccc3f5378f65 100644 --- a/lib/ui/window/pointer_data_packet_converter_unittests.cc +++ b/lib/ui/window/pointer_data_packet_converter_unittests.cc @@ -510,6 +510,62 @@ TEST(PointerDataPacketConverterTest, CanSynthesizeAdd) { ASSERT_EQ(result[3].buttons, 0); } +TEST(PointerDataPacketConverterTest, CanSynthesizeRemove) { + TestDelegate delegate; + delegate.AddView(100); + delegate.AddView(200); + PointerDataPacketConverter converter(delegate); + auto packet = std::make_unique(3); + + PointerData data; + CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0); + data.view_id = 100; + packet->SetPointerData(0, data); + CreateSimulatedPointerData(data, PointerData::Change::kDown, 0, 3.0, 4.0, 1); + data.view_id = 100; + packet->SetPointerData(1, data); + CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0); + data.view_id = 200; + packet->SetPointerData(2, data); + auto converted_packet = converter.Convert(*packet); + + std::vector result; + UnpackPointerPacket(result, std::move(converted_packet)); + + ASSERT_EQ(result.size(), (size_t)6); + ASSERT_EQ(result[0].synthesized, 0); + ASSERT_EQ(result[0].view_id, 100); + + // A hover should be synthesized. + ASSERT_EQ(result[1].change, PointerData::Change::kHover); + ASSERT_EQ(result[1].synthesized, 1); + ASSERT_EQ(result[1].physical_delta_x, 3.0); + ASSERT_EQ(result[1].physical_delta_y, 4.0); + ASSERT_EQ(result[1].buttons, 0); + + ASSERT_EQ(result[2].change, PointerData::Change::kDown); + ASSERT_EQ(result[2].pointer_identifier, 1); + ASSERT_EQ(result[2].synthesized, 0); + ASSERT_EQ(result[2].buttons, 1); + + // A cancel should be synthesized. + ASSERT_EQ(result[3].change, PointerData::Change::kCancel); + ASSERT_EQ(result[3].pointer_identifier, 1); + ASSERT_EQ(result[3].synthesized, 1); + ASSERT_EQ(result[3].physical_x, 3.0); + ASSERT_EQ(result[3].physical_y, 4.0); + ASSERT_EQ(result[3].buttons, 1); + + // A remove should be synthesized. + ASSERT_EQ(result[4].physical_x, 3.0); + ASSERT_EQ(result[4].physical_y, 4.0); + ASSERT_EQ(result[4].synthesized, 1); + ASSERT_EQ(result[4].view_id, 100); + + ASSERT_EQ(result[5].synthesized, 0); + ASSERT_EQ(result[5].view_id, 200); +} + TEST(PointerDataPacketConverterTest, CanHandleThreeFingerGesture) { // Regression test https://github.com/flutter/flutter/issues/20517. TestDelegate delegate; From 350611114218ed30616312e9376733ddae0fff65 Mon Sep 17 00:00:00 2001 From: Harlen Batagelo Date: Tue, 15 Oct 2024 10:39:38 -0300 Subject: [PATCH 3/3] Add test for double remove events, simplify synthesized event creation, and add comments --- .../window/pointer_data_packet_converter.cc | 7 ++- ...pointer_data_packet_converter_unittests.cc | 46 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/lib/ui/window/pointer_data_packet_converter.cc b/lib/ui/window/pointer_data_packet_converter.cc index 0290d92573af5..b7640288fcc7e 100644 --- a/lib/ui/window/pointer_data_packet_converter.cc +++ b/lib/ui/window/pointer_data_packet_converter.cc @@ -87,7 +87,12 @@ void PointerDataPacketConverter::ConvertPointerData( synthesized_data.scale = state.scale; synthesized_data.rotation = state.rotation; synthesized_data.buttons = state.buttons; + synthesized_data.synthesized = 1; + // The framework expects an add event to always follow a remove event, + // and remove events with invalid views are ignored. + // To meet the framework's expectations, the view ID of the add event + // is used for the remove event if the old view has been destroyed. if (delegate_.ViewExists(state.view_id)) { synthesized_data.view_id = state.view_id; @@ -95,7 +100,6 @@ void PointerDataPacketConverter::ConvertPointerData( // Synthesizes cancel event if the pointer is down. PointerData synthesized_cancel_event = synthesized_data; synthesized_cancel_event.change = PointerData::Change::kCancel; - synthesized_cancel_event.synthesized = 1; UpdatePointerIdentifier(synthesized_cancel_event, state, false); state.is_down = false; @@ -105,7 +109,6 @@ void PointerDataPacketConverter::ConvertPointerData( PointerData synthesized_remove_event = synthesized_data; synthesized_remove_event.change = PointerData::Change::kRemove; - synthesized_remove_event.synthesized = 1; converted_pointers.push_back(synthesized_remove_event); } diff --git a/lib/ui/window/pointer_data_packet_converter_unittests.cc b/lib/ui/window/pointer_data_packet_converter_unittests.cc index 0ccc3f5378f65..0eb9667e2c707 100644 --- a/lib/ui/window/pointer_data_packet_converter_unittests.cc +++ b/lib/ui/window/pointer_data_packet_converter_unittests.cc @@ -566,6 +566,52 @@ TEST(PointerDataPacketConverterTest, CanSynthesizeRemove) { ASSERT_EQ(result[5].view_id, 200); } +TEST(PointerDataPacketConverterTest, + CanAvoidDoubleRemoveAfterSynthesizedRemove) { + TestDelegate delegate; + delegate.AddView(100); + delegate.AddView(200); + PointerDataPacketConverter converter(delegate); + auto packet = std::make_unique(2); + + PointerData data; + CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0); + data.view_id = 100; + packet->SetPointerData(0, data); + CreateSimulatedPointerData(data, PointerData::Change::kAdd, 0, 0.0, 0.0, 0); + data.view_id = 200; + packet->SetPointerData(1, data); + auto converted_packet = converter.Convert(*packet); + + std::vector result; + UnpackPointerPacket(result, std::move(converted_packet)); + + ASSERT_EQ(result.size(), (size_t)3); + ASSERT_EQ(result[0].synthesized, 0); + ASSERT_EQ(result[0].view_id, 100); + + // A remove should be synthesized. + ASSERT_EQ(result[1].synthesized, 1); + ASSERT_EQ(result[1].view_id, 100); + + ASSERT_EQ(result[2].synthesized, 0); + ASSERT_EQ(result[2].view_id, 200); + + // Simulate a double remove. + packet = std::make_unique(1); + CreateSimulatedPointerData(data, PointerData::Change::kRemove, 0, 0.0, 0.0, + 0); + data.view_id = 100; + packet->SetPointerData(0, data); + converted_packet = converter.Convert(*packet); + + result.clear(); + UnpackPointerPacket(result, std::move(converted_packet)); + + // The double remove should be ignored. + ASSERT_EQ(result.size(), (size_t)0); +} + TEST(PointerDataPacketConverterTest, CanHandleThreeFingerGesture) { // Regression test https://github.com/flutter/flutter/issues/20517. TestDelegate delegate;