Skip to content

Commit

Permalink
Eliminate duplicated code when dealing with pointer data (flutter#36822)
Browse files Browse the repository at this point in the history
* add methods

* add empty test

* add unit test

* Update licenses_flutter

* add const

* size -> get length

* add boundary checks

* add tests

* remove death test since it says "Death tests use fork(), which is unsafe particularly in a threaded context."
  • Loading branch information
fzyzcjy authored Nov 10, 2022
1 parent 80e31ed commit 2f5b7ac
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 34 deletions.
1 change: 1 addition & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -1781,6 +1781,7 @@ FILE: ../../../flutter/lib/ui/window/pointer_data_packet.h
FILE: ../../../flutter/lib/ui/window/pointer_data_packet_converter.cc
FILE: ../../../flutter/lib/ui/window/pointer_data_packet_converter.h
FILE: ../../../flutter/lib/ui/window/pointer_data_packet_converter_unittests.cc
FILE: ../../../flutter/lib/ui/window/pointer_data_packet_unittests.cc
FILE: ../../../flutter/lib/ui/window/viewport_metrics.cc
FILE: ../../../flutter/lib/ui/window/viewport_metrics.h
FILE: ../../../flutter/lib/ui/window/window.cc
Expand Down
1 change: 1 addition & 0 deletions lib/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ if (enable_unittests) {
"window/platform_message_response_dart_port_unittests.cc",
"window/platform_message_response_dart_unittests.cc",
"window/pointer_data_packet_converter_unittests.cc",
"window/pointer_data_packet_unittests.cc",
]

deps = [
Expand Down
13 changes: 13 additions & 0 deletions lib/ui/window/pointer_data_packet.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

#include "flutter/lib/ui/window/pointer_data_packet.h"
#include "flutter/fml/logging.h"

#include <cstring>

Expand All @@ -17,7 +18,19 @@ PointerDataPacket::PointerDataPacket(uint8_t* data, size_t num_bytes)
PointerDataPacket::~PointerDataPacket() = default;

void PointerDataPacket::SetPointerData(size_t i, const PointerData& data) {
FML_DCHECK(i < GetLength());
memcpy(&data_[i * sizeof(PointerData)], &data, sizeof(PointerData));
}

PointerData PointerDataPacket::GetPointerData(size_t i) const {
FML_DCHECK(i < GetLength());
PointerData result;
memcpy(&result, &data_[i * sizeof(PointerData)], sizeof(PointerData));
return result;
}

size_t PointerDataPacket::GetLength() const {
return data_.size() / sizeof(PointerData);
}

} // namespace flutter
2 changes: 2 additions & 0 deletions lib/ui/window/pointer_data_packet.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ class PointerDataPacket {
~PointerDataPacket();

void SetPointerData(size_t i, const PointerData& data);
PointerData GetPointerData(size_t i) const;
size_t GetLength() const;
const std::vector<uint8_t>& data() const { return data_; }

private:
Expand Down
10 changes: 2 additions & 8 deletions lib/ui/window/pointer_data_packet_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,11 @@ PointerDataPacketConverter::~PointerDataPacketConverter() = default;

std::unique_ptr<PointerDataPacket> PointerDataPacketConverter::Convert(
std::unique_ptr<PointerDataPacket> packet) {
size_t kBytesPerPointerData = kPointerDataFieldCount * kBytesPerField;
auto buffer = packet->data();
size_t buffer_length = buffer.size();

std::vector<PointerData> converted_pointers;
// Converts each pointer data in the buffer and stores it in the
// converted_pointers.
for (size_t i = 0; i < buffer_length / kBytesPerPointerData; i++) {
PointerData pointer_data;
memcpy(&pointer_data, &buffer[i * kBytesPerPointerData],
sizeof(PointerData));
for (size_t i = 0; i < packet->GetLength(); i++) {
PointerData pointer_data = packet->GetPointerData(i);
ConvertPointerData(pointer_data, converted_pointers);
}

Expand Down
10 changes: 2 additions & 8 deletions lib/ui/window/pointer_data_packet_converter_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,8 @@ void CreateSimulatedTrackpadGestureData(PointerData& data, // NOLINT

void UnpackPointerPacket(std::vector<PointerData>& output, // NOLINT
std::unique_ptr<PointerDataPacket> packet) {
size_t kBytesPerPointerData = kPointerDataFieldCount * kBytesPerField;
auto buffer = packet->data();
size_t buffer_length = buffer.size();

for (size_t i = 0; i < buffer_length / kBytesPerPointerData; i++) {
PointerData pointer_data;
memcpy(&pointer_data, &buffer[i * kBytesPerPointerData],
sizeof(PointerData));
for (size_t i = 0; i < packet->GetLength(); i++) {
PointerData pointer_data = packet->GetPointerData(i);
output.push_back(pointer_data);
}
packet.reset();
Expand Down
69 changes: 69 additions & 0 deletions lib/ui/window/pointer_data_packet_unittests.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/lib/ui/window/pointer_data.h"

#include <cstring>

#include "gtest/gtest.h"
#include "pointer_data_packet.h"

namespace flutter {
namespace testing {

void CreateSimpleSimulatedPointerData(PointerData& data, // NOLINT
PointerData::Change change,
int64_t device,
double dx,
double dy,
int64_t buttons) {
data.time_stamp = 0;
data.change = change;
data.kind = PointerData::DeviceKind::kTouch;
data.signal_kind = PointerData::SignalKind::kNone;
data.device = device;
data.pointer_identifier = 0;
data.physical_x = dx;
data.physical_y = dy;
data.physical_delta_x = 0.0;
data.physical_delta_y = 0.0;
data.buttons = buttons;
data.obscured = 0;
data.synthesized = 0;
data.pressure = 0.0;
data.pressure_min = 0.0;
data.pressure_max = 0.0;
data.distance = 0.0;
data.distance_max = 0.0;
data.size = 0.0;
data.radius_major = 0.0;
data.radius_minor = 0.0;
data.radius_min = 0.0;
data.radius_max = 0.0;
data.orientation = 0.0;
data.tilt = 0.0;
data.platformData = 0;
data.scroll_delta_x = 0.0;
data.scroll_delta_y = 0.0;
}

TEST(PointerDataPacketTest, CanGetPointerData) {
auto packet = std::make_unique<PointerDataPacket>(1);
PointerData data;
CreateSimpleSimulatedPointerData(data, PointerData::Change::kAdd, 1, 2.0, 3.0,
4);
packet->SetPointerData(0, data);

PointerData data_recovered = packet->GetPointerData(0);
ASSERT_EQ(data_recovered.physical_x, 2.0);
ASSERT_EQ(data_recovered.physical_y, 3.0);
}

TEST(PointerDataPacketTest, CanGetLength) {
auto packet = std::make_unique<PointerDataPacket>(6);
ASSERT_EQ(packet->GetLength(), (size_t)6);
}

} // namespace testing
} // namespace flutter
11 changes: 2 additions & 9 deletions shell/platform/fuchsia/flutter/platform_view_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -379,15 +379,8 @@ std::string ToString(const fml::Mapping& mapping) {
// Stolen from pointer_data_packet_converter_unittests.cc.
void UnpackPointerPacket(std::vector<flutter::PointerData>& output, // NOLINT
std::unique_ptr<flutter::PointerDataPacket> packet) {
size_t kBytesPerPointerData =
flutter::kPointerDataFieldCount * flutter::kBytesPerField;
auto buffer = packet->data();
size_t buffer_length = buffer.size();

for (size_t i = 0; i < buffer_length / kBytesPerPointerData; i++) {
flutter::PointerData pointer_data;
memcpy(&pointer_data, &buffer[i * kBytesPerPointerData],
sizeof(flutter::PointerData));
for (size_t i = 0; i < packet->GetLength(); i++) {
flutter::PointerData pointer_data = packet->GetPointerData(i);
output.push_back(pointer_data);
}
packet.reset();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,15 +484,8 @@ std::string ToString(const fml::Mapping& mapping) {
// Stolen from pointer_data_packet_converter_unittests.cc.
void UnpackPointerPacket(std::vector<flutter::PointerData>& output, // NOLINT
std::unique_ptr<flutter::PointerDataPacket> packet) {
size_t kBytesPerPointerData =
flutter::kPointerDataFieldCount * flutter::kBytesPerField;
auto buffer = packet->data();
size_t buffer_length = buffer.size();

for (size_t i = 0; i < buffer_length / kBytesPerPointerData; i++) {
flutter::PointerData pointer_data;
memcpy(&pointer_data, &buffer[i * kBytesPerPointerData],
sizeof(flutter::PointerData));
for (size_t i = 0; i < packet->GetLength(); i++) {
flutter::PointerData pointer_data = packet->GetPointerData(i);
output.push_back(pointer_data);
}
packet.reset();
Expand Down

0 comments on commit 2f5b7ac

Please sign in to comment.