Skip to content

Commit 5f18ceb

Browse files
author
Arpit Singh
committed
Make topology validator a static function
This CL removes the isValid method from DisplayTopologyGraph and adds it as a static method in DisplayTopologyGraph. This is a follow up from ag/32211998. Test: atest DisplayTopologyGraphTest/DisplayTopologyGraphTestFixture Bug: 401219231 Flag: com.android.input.flags.enable_display_topology_validation Change-Id: Ib853c5b39ce956737bb547917043da29492ff97b
1 parent 92e0a7e commit 5f18ceb

File tree

5 files changed

+218
-109
lines changed

5 files changed

+218
-109
lines changed

‎include/input/DisplayTopologyGraph.h‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#pragma once
1818

19+
#include <android-base/result.h>
1920
#include <ftl/enum.h>
2021
#include <ui/LogicalDisplayId.h>
2122

@@ -58,8 +59,23 @@ struct DisplayTopologyGraph {
5859
std::unordered_map<ui::LogicalDisplayId, std::vector<DisplayTopologyAdjacentDisplay>> graph;
5960
std::unordered_map<ui::LogicalDisplayId, int> displaysDensity;
6061

61-
bool isValid() const;
62+
DisplayTopologyGraph() = default;
6263
std::string dump() const;
64+
65+
// Builds the topology graph from components.
66+
// Returns error if a valid graph cannot be built from the supplied components.
67+
static base::Result<const DisplayTopologyGraph> create(
68+
ui::LogicalDisplayId primaryDisplay,
69+
std::unordered_map<ui::LogicalDisplayId, std::vector<DisplayTopologyAdjacentDisplay>>&&
70+
adjacencyGraph,
71+
std::unordered_map<ui::LogicalDisplayId, int>&& displaysDensityMap);
72+
73+
private:
74+
DisplayTopologyGraph(
75+
ui::LogicalDisplayId primaryDisplay,
76+
std::unordered_map<ui::LogicalDisplayId, std::vector<DisplayTopologyAdjacentDisplay>>&&
77+
adjacencyGraph,
78+
std::unordered_map<ui::LogicalDisplayId, int>&& displaysDensityMap);
6379
};
6480

6581
} // namespace android

‎libs/input/DisplayTopologyGraph.cpp‎

Lines changed: 70 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
#include <android-base/logging.h>
2020
#include <android-base/stringprintf.h>
21+
#include <com_android_input_flags.h>
2122
#include <ftl/enum.h>
2223
#include <input/DisplayTopologyGraph.h>
2324
#include <input/PrintTools.h>
@@ -27,6 +28,8 @@
2728

2829
#define INDENT " "
2930

31+
namespace input_flags = com::android::input::flags;
32+
3033
namespace android {
3134

3235
namespace {
@@ -44,16 +47,19 @@ DisplayTopologyPosition getOppositePosition(DisplayTopologyPosition position) {
4447
}
4548
}
4649

47-
bool validatePrimaryDisplay(const android::DisplayTopologyGraph& displayTopologyGraph) {
48-
return displayTopologyGraph.primaryDisplayId != ui::LogicalDisplayId::INVALID &&
49-
displayTopologyGraph.graph.contains(displayTopologyGraph.primaryDisplayId);
50+
bool validatePrimaryDisplay(ui::LogicalDisplayId primaryDisplayId,
51+
const std::unordered_map<ui::LogicalDisplayId, int>& displaysDensity) {
52+
return primaryDisplayId != ui::LogicalDisplayId::INVALID &&
53+
displaysDensity.contains(primaryDisplayId);
5054
}
5155

52-
bool validateTopologyGraph(const android::DisplayTopologyGraph& displayTopologyGraph) {
53-
for (const auto& [sourceDisplay, adjacentDisplays] : displayTopologyGraph.graph) {
56+
bool validateTopologyGraph(
57+
const std::unordered_map<ui::LogicalDisplayId, std::vector<DisplayTopologyAdjacentDisplay>>&
58+
graph) {
59+
for (const auto& [sourceDisplay, adjacentDisplays] : graph) {
5460
for (const DisplayTopologyAdjacentDisplay& adjacentDisplay : adjacentDisplays) {
55-
const auto adjacentGraphIt = displayTopologyGraph.graph.find(adjacentDisplay.displayId);
56-
if (adjacentGraphIt == displayTopologyGraph.graph.end()) {
61+
const auto adjacentGraphIt = graph.find(adjacentDisplay.displayId);
62+
if (adjacentGraphIt == graph.end()) {
5763
LOG(ERROR) << "Missing adjacent display in topology graph: "
5864
<< adjacentDisplay.displayId << " for source " << sourceDisplay;
5965
return false;
@@ -90,9 +96,11 @@ bool validateTopologyGraph(const android::DisplayTopologyGraph& displayTopologyG
9096
return true;
9197
}
9298

93-
bool validateDensities(const android::DisplayTopologyGraph& displayTopologyGraph) {
94-
for (const auto& [sourceDisplay, adjacentDisplays] : displayTopologyGraph.graph) {
95-
if (!displayTopologyGraph.displaysDensity.contains(sourceDisplay)) {
99+
bool validateDensities(const std::unordered_map<ui::LogicalDisplayId,
100+
std::vector<DisplayTopologyAdjacentDisplay>>& graph,
101+
const std::unordered_map<ui::LogicalDisplayId, int>& displaysDensity) {
102+
for (const auto& [sourceDisplay, adjacentDisplays] : graph) {
103+
if (!displaysDensity.contains(sourceDisplay)) {
96104
LOG(ERROR) << "Missing density value in topology graph for display: " << sourceDisplay;
97105
return false;
98106
}
@@ -113,22 +121,23 @@ std::string adjacentDisplayVectorToString(
113121
return dumpVector(adjacentDisplays, adjacentDisplayToString);
114122
}
115123

116-
} // namespace
117-
118-
std::string DisplayTopologyAdjacentDisplay::dump() const {
119-
std::string dump;
120-
dump += base::StringPrintf("DisplayTopologyAdjacentDisplay: {displayId: %d, position: %s, "
121-
"offsetDp: %f}",
122-
displayId.val(), ftl::enum_string(position).c_str(), offsetDp);
123-
return dump;
124-
}
125-
126-
bool DisplayTopologyGraph::isValid() const {
127-
return validatePrimaryDisplay(*this) && validateTopologyGraph(*this) &&
128-
validateDensities(*this);
124+
bool areTopologyGraphComponentsValid(
125+
ui::LogicalDisplayId primaryDisplayId,
126+
const std::unordered_map<ui::LogicalDisplayId, std::vector<DisplayTopologyAdjacentDisplay>>&
127+
graph,
128+
const std::unordered_map<ui::LogicalDisplayId, int>& displaysDensity) {
129+
if (!input_flags::enable_display_topology_validation()) {
130+
return true;
131+
}
132+
return validatePrimaryDisplay(primaryDisplayId, displaysDensity) &&
133+
validateTopologyGraph(graph) && validateDensities(graph, displaysDensity);
129134
}
130135

131-
std::string DisplayTopologyGraph::dump() const {
136+
std::string dumpTopologyGraphComponents(
137+
ui::LogicalDisplayId primaryDisplayId,
138+
const std::unordered_map<ui::LogicalDisplayId, std::vector<DisplayTopologyAdjacentDisplay>>&
139+
graph,
140+
const std::unordered_map<ui::LogicalDisplayId, int>& displaysDensity) {
132141
std::string dump;
133142
dump += base::StringPrintf("PrimaryDisplayId: %d\n", primaryDisplayId.val());
134143
dump += base::StringPrintf("TopologyGraph:\n");
@@ -141,4 +150,41 @@ std::string DisplayTopologyGraph::dump() const {
141150
return dump;
142151
}
143152

153+
} // namespace
154+
155+
std::string DisplayTopologyAdjacentDisplay::dump() const {
156+
std::string dump;
157+
dump += base::StringPrintf("DisplayTopologyAdjacentDisplay: {displayId: %d, position: %s, "
158+
"offsetDp: %f}",
159+
displayId.val(), ftl::enum_string(position).c_str(), offsetDp);
160+
return dump;
161+
}
162+
163+
DisplayTopologyGraph::DisplayTopologyGraph(
164+
ui::LogicalDisplayId primaryDisplay,
165+
std::unordered_map<ui::LogicalDisplayId, std::vector<DisplayTopologyAdjacentDisplay>>&&
166+
adjacencyGraph,
167+
std::unordered_map<ui::LogicalDisplayId, int>&& displaysDensityMap)
168+
: primaryDisplayId(primaryDisplay),
169+
graph(std::move(adjacencyGraph)),
170+
displaysDensity(std::move(displaysDensityMap)) {}
171+
172+
std::string DisplayTopologyGraph::dump() const {
173+
return dumpTopologyGraphComponents(primaryDisplayId, graph, displaysDensity);
174+
}
175+
176+
base::Result<const DisplayTopologyGraph> DisplayTopologyGraph::create(
177+
ui::LogicalDisplayId primaryDisplay,
178+
std::unordered_map<ui::LogicalDisplayId, std::vector<DisplayTopologyAdjacentDisplay>>&&
179+
adjacencyGraph,
180+
std::unordered_map<ui::LogicalDisplayId, int>&& displaysDensityMap) {
181+
if (areTopologyGraphComponentsValid(primaryDisplay, adjacencyGraph, displaysDensityMap)) {
182+
return DisplayTopologyGraph(primaryDisplay, std::move(adjacencyGraph),
183+
std::move(displaysDensityMap));
184+
}
185+
return base::Error() << "Invalid display topology components: "
186+
<< dumpTopologyGraphComponents(primaryDisplay, adjacencyGraph,
187+
displaysDensityMap);
188+
}
189+
144190
} // namespace android

‎services/inputflinger/tests/DisplayTopologyGraph_test.cpp‎

Lines changed: 72 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,16 @@
1414
* limitations under the License.
1515
*/
1616

17+
#include <com_android_input_flags.h>
1718
#include <gtest/gtest.h>
1819
#include <input/DisplayTopologyGraph.h>
1920

2021
#include <string>
2122
#include <string_view>
2223
#include <tuple>
2324

25+
#include "ScopedFlagOverride.h"
26+
2427
namespace android {
2528

2629
namespace {
@@ -31,87 +34,104 @@ constexpr int DENSITY_MEDIUM = 160;
3134

3235
} // namespace
3336

37+
using DisplayTopologyAdjacentDisplayMap =
38+
std::unordered_map<ui::LogicalDisplayId, std::vector<DisplayTopologyAdjacentDisplay>>;
39+
using DisplayTopologyDisplaysDensityMapVector = std::unordered_map<ui::LogicalDisplayId, int>;
3440
using DisplayTopologyGraphTestFixtureParam =
35-
std::tuple<std::string_view /*name*/, DisplayTopologyGraph, bool /*isValid*/>;
41+
std::tuple<std::string_view /*name*/, ui::LogicalDisplayId /*primaryDisplayId*/,
42+
DisplayTopologyAdjacentDisplayMap, DisplayTopologyDisplaysDensityMapVector,
43+
bool /*isValid*/>;
3644

3745
class DisplayTopologyGraphTestFixture
3846
: public testing::Test,
3947
public testing::WithParamInterface<DisplayTopologyGraphTestFixtureParam> {};
4048

4149
TEST_P(DisplayTopologyGraphTestFixture, DisplayTopologyGraphTest) {
42-
const auto& [_, displayTopology, isValid] = GetParam();
43-
EXPECT_EQ(isValid, displayTopology.isValid());
50+
SCOPED_FLAG_OVERRIDE(enable_display_topology_validation, true);
51+
auto [_, primaryDisplayId, graph, displaysDensity, isValid] = GetParam();
52+
auto result = DisplayTopologyGraph::create(primaryDisplayId, std::move(graph),
53+
std::move(displaysDensity));
54+
EXPECT_EQ(isValid, result.ok());
4455
}
4556

4657
INSTANTIATE_TEST_SUITE_P(
4758
DisplayTopologyGraphTest, DisplayTopologyGraphTestFixture,
4859
testing::Values(
49-
std::make_tuple(
50-
"InvalidPrimaryDisplay",
51-
DisplayTopologyGraph{.primaryDisplayId = ui::LogicalDisplayId::INVALID,
52-
.graph = {},
53-
.displaysDensity = {}},
54-
false),
60+
std::make_tuple("InvalidPrimaryDisplay",
61+
/*primaryDisplayId=*/ui::LogicalDisplayId::INVALID,
62+
/*graph=*/DisplayTopologyAdjacentDisplayMap{},
63+
/*displaysDensity=*/DisplayTopologyDisplaysDensityMapVector{},
64+
false),
5565
std::make_tuple("PrimaryDisplayNotInGraph",
56-
DisplayTopologyGraph{.primaryDisplayId = DISPLAY_ID_1,
57-
.graph = {},
58-
.displaysDensity = {}},
66+
/*primaryDisplayId=*/DISPLAY_ID_1,
67+
/*graph=*/DisplayTopologyAdjacentDisplayMap{},
68+
/*displaysDensity=*/DisplayTopologyDisplaysDensityMapVector{},
5969
false),
6070
std::make_tuple("DisplayDensityMissing",
61-
DisplayTopologyGraph{.primaryDisplayId = DISPLAY_ID_1,
62-
.graph = {{DISPLAY_ID_1, {}}},
63-
.displaysDensity = {}},
71+
/*primaryDisplayId=*/DISPLAY_ID_1,
72+
/*graph=*/DisplayTopologyAdjacentDisplayMap{{DISPLAY_ID_1, {}}},
73+
/*displaysDensity=*/DisplayTopologyDisplaysDensityMapVector{},
6474
false),
6575
std::make_tuple("ValidSingleDisplayTopology",
66-
DisplayTopologyGraph{.primaryDisplayId = DISPLAY_ID_1,
67-
.graph = {{DISPLAY_ID_1, {}}},
68-
.displaysDensity = {{DISPLAY_ID_1,
69-
DENSITY_MEDIUM}}},
76+
/*primaryDisplayId=*/DISPLAY_ID_1,
77+
/*graph=*/DisplayTopologyAdjacentDisplayMap{{DISPLAY_ID_1, {}}},
78+
/*displaysDensity=*/
79+
DisplayTopologyDisplaysDensityMapVector{
80+
{DISPLAY_ID_1, DENSITY_MEDIUM}},
7081
true),
7182
std::make_tuple(
7283
"MissingReverseEdge",
73-
DisplayTopologyGraph{.primaryDisplayId = DISPLAY_ID_1,
74-
.graph = {{DISPLAY_ID_1,
75-
{{DISPLAY_ID_2,
76-
DisplayTopologyPosition::TOP, 0}}}},
77-
.displaysDensity = {{DISPLAY_ID_1, DENSITY_MEDIUM},
78-
{DISPLAY_ID_2, DENSITY_MEDIUM}}},
84+
/*primaryDisplayId=*/DISPLAY_ID_1,
85+
/*graph=*/
86+
DisplayTopologyAdjacentDisplayMap{
87+
{DISPLAY_ID_1, {{DISPLAY_ID_2, DisplayTopologyPosition::TOP, 0}}}},
88+
/*displaysDensity=*/
89+
DisplayTopologyDisplaysDensityMapVector{{DISPLAY_ID_1, DENSITY_MEDIUM},
90+
{DISPLAY_ID_2, DENSITY_MEDIUM}},
7991
false),
8092
std::make_tuple(
8193
"IncorrectReverseEdgeDirection",
82-
DisplayTopologyGraph{.primaryDisplayId = DISPLAY_ID_1,
83-
.graph = {{DISPLAY_ID_1,
84-
{{DISPLAY_ID_2,
85-
DisplayTopologyPosition::TOP, 0}}},
86-
{DISPLAY_ID_2,
87-
{{DISPLAY_ID_1,
88-
DisplayTopologyPosition::TOP, 0}}}},
89-
.displaysDensity = {{DISPLAY_ID_1, DENSITY_MEDIUM},
90-
{DISPLAY_ID_2, DENSITY_MEDIUM}}},
94+
/*primaryDisplayId=*/DISPLAY_ID_1,
95+
/*graph=*/
96+
DisplayTopologyAdjacentDisplayMap{{DISPLAY_ID_1,
97+
{{DISPLAY_ID_2,
98+
DisplayTopologyPosition::TOP, 0}}},
99+
{DISPLAY_ID_2,
100+
{{DISPLAY_ID_1,
101+
DisplayTopologyPosition::TOP, 0}}}},
102+
/*displaysDensity=*/
103+
DisplayTopologyDisplaysDensityMapVector{{DISPLAY_ID_1, DENSITY_MEDIUM},
104+
{DISPLAY_ID_2, DENSITY_MEDIUM}},
91105
false),
92106
std::make_tuple(
93107
"IncorrectReverseEdgeOffset",
94-
DisplayTopologyGraph{.primaryDisplayId = DISPLAY_ID_1,
95-
.graph = {{DISPLAY_ID_1,
96-
{{DISPLAY_ID_2,
97-
DisplayTopologyPosition::TOP, 10}}},
98-
{DISPLAY_ID_2,
99-
{{DISPLAY_ID_1,
100-
DisplayTopologyPosition::BOTTOM, 20}}}},
101-
.displaysDensity = {{DISPLAY_ID_1, DENSITY_MEDIUM},
102-
{DISPLAY_ID_2, DENSITY_MEDIUM}}},
108+
/*primaryDisplayId=*/DISPLAY_ID_1,
109+
/*graph=*/
110+
DisplayTopologyAdjacentDisplayMap{{DISPLAY_ID_1,
111+
{{DISPLAY_ID_2,
112+
DisplayTopologyPosition::TOP, 10}}},
113+
{DISPLAY_ID_2,
114+
{{DISPLAY_ID_1,
115+
DisplayTopologyPosition::BOTTOM,
116+
20}}}},
117+
/*displaysDensity=*/
118+
DisplayTopologyDisplaysDensityMapVector{{DISPLAY_ID_1, DENSITY_MEDIUM},
119+
{DISPLAY_ID_2, DENSITY_MEDIUM}},
103120
false),
104121
std::make_tuple(
105122
"ValidMultiDisplayTopology",
106-
DisplayTopologyGraph{.primaryDisplayId = DISPLAY_ID_1,
107-
.graph = {{DISPLAY_ID_1,
108-
{{DISPLAY_ID_2,
109-
DisplayTopologyPosition::TOP, 10}}},
110-
{DISPLAY_ID_2,
111-
{{DISPLAY_ID_1,
112-
DisplayTopologyPosition::BOTTOM, -10}}}},
113-
.displaysDensity = {{DISPLAY_ID_1, DENSITY_MEDIUM},
114-
{DISPLAY_ID_2, DENSITY_MEDIUM}}},
123+
/*primaryDisplayId=*/DISPLAY_ID_1,
124+
/*graph=*/
125+
DisplayTopologyAdjacentDisplayMap{{DISPLAY_ID_1,
126+
{{DISPLAY_ID_2,
127+
DisplayTopologyPosition::TOP, 10}}},
128+
{DISPLAY_ID_2,
129+
{{DISPLAY_ID_1,
130+
DisplayTopologyPosition::BOTTOM,
131+
-10}}}},
132+
/*displaysDensity=*/
133+
DisplayTopologyDisplaysDensityMapVector{{DISPLAY_ID_1, DENSITY_MEDIUM},
134+
{DISPLAY_ID_2, DENSITY_MEDIUM}},
115135
true)),
116136
[](const testing::TestParamInfo<DisplayTopologyGraphTestFixtureParam>& p) {
117137
return std::string{std::get<0>(p.param)};

‎services/inputflinger/tests/InputDispatcher_test.cpp‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15493,14 +15493,18 @@ INSTANTIATE_TEST_SUITE_P(WithAndWithoutTransfer, TransferOrDontTransferFixture,
1549315493
class InputDispatcherConnectedDisplayTest : public InputDispatcherDragTests {
1549415494
constexpr static int DENSITY_MEDIUM = 160;
1549515495

15496-
const DisplayTopologyGraph
15497-
mTopology{.primaryDisplayId = DISPLAY_ID,
15498-
.graph = {{DISPLAY_ID,
15499-
{{SECOND_DISPLAY_ID, DisplayTopologyPosition::TOP, 0.0f}}},
15500-
{SECOND_DISPLAY_ID,
15501-
{{DISPLAY_ID, DisplayTopologyPosition::BOTTOM, 0.0f}}}},
15502-
.displaysDensity = {{DISPLAY_ID, DENSITY_MEDIUM},
15503-
{SECOND_DISPLAY_ID, DENSITY_MEDIUM}}};
15496+
const DisplayTopologyGraph mTopology =
15497+
DisplayTopologyGraph::create(/*primaryDisplayId=*/DISPLAY_ID,
15498+
/*adjacencyGraph=*/
15499+
{{DISPLAY_ID,
15500+
{{SECOND_DISPLAY_ID, DisplayTopologyPosition::TOP,
15501+
0.0f}}},
15502+
{SECOND_DISPLAY_ID,
15503+
{{DISPLAY_ID, DisplayTopologyPosition::BOTTOM, 0.0f}}}},
15504+
/*displaysDensity=*/
15505+
{{DISPLAY_ID, DENSITY_MEDIUM},
15506+
{SECOND_DISPLAY_ID, DENSITY_MEDIUM}})
15507+
.value();
1550415508

1550515509
protected:
1550615510
void SetUp() override {

0 commit comments

Comments
 (0)