Skip to content

Commit 147626a

Browse files
stagadishstagadish
committed
SF: Remove *DisplayId::tryCast and DisplayId::isVirtual()
Work towards DisplayId opaqueness by eliminating call-sites to APIs that parse the display ID values directly. This CL removes *DisplayId::tryCast from the DislayId interface entirely and replaces it with SF APIs that check for the existence of the displays before casting. This removes direct dependency on ID value bits. It also removes DisplayId::isVirtual(). Flag: com.android.graphics.surfaceflinger.flags.stable_edid_ids Bug: 390690584 Bug: 390689313 Test: libsurfaceflinger_unittest Change-Id: I918a6b361784e41165837234b82eed027dc46673
1 parent de4ce29 commit 147626a

16 files changed

Lines changed: 158 additions & 274 deletions

‎libs/ui/include/ui/DisplayId.h‎

Lines changed: 12 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ struct DisplayId {
3737
DisplayId& operator=(const DisplayId&) = default;
3838

3939
static constexpr DisplayId fromValue(uint64_t value) { return DisplayId(value); }
40-
constexpr bool isVirtual() const { return value & FLAG_VIRTUAL; }
4140

4241
uint64_t value;
4342

@@ -67,13 +66,6 @@ struct PhysicalDisplayId : DisplayId {
6766
// TODO: b/162612135 - Remove default constructor.
6867
PhysicalDisplayId() = default;
6968

70-
static constexpr ftl::Optional<PhysicalDisplayId> tryCast(DisplayId id) {
71-
if (id.isVirtual()) {
72-
return std::nullopt;
73-
}
74-
return PhysicalDisplayId(id);
75-
}
76-
7769
// Returns a stable ID based on EDID and port information.
7870
static constexpr PhysicalDisplayId fromEdid(uint8_t port, uint16_t manufacturerId,
7971
uint32_t modelHash) {
@@ -113,13 +105,6 @@ struct VirtualDisplayId : DisplayId {
113105
// Flag indicating that this virtual display is backed by the GPU.
114106
static constexpr uint64_t FLAG_GPU = 1ULL << 61;
115107

116-
static constexpr std::optional<VirtualDisplayId> tryCast(DisplayId id) {
117-
if (id.isVirtual()) {
118-
return VirtualDisplayId(id);
119-
}
120-
return std::nullopt;
121-
}
122-
123108
static constexpr VirtualDisplayId fromValue(uint64_t value) {
124109
return VirtualDisplayId(SkipVirtualFlag{}, value);
125110
}
@@ -135,13 +120,6 @@ struct VirtualDisplayId : DisplayId {
135120
struct HalVirtualDisplayId : VirtualDisplayId {
136121
explicit constexpr HalVirtualDisplayId(BaseId baseId) : VirtualDisplayId(baseId) {}
137122

138-
static constexpr std::optional<HalVirtualDisplayId> tryCast(DisplayId id) {
139-
if (id.isVirtual() && !(id.value & FLAG_GPU)) {
140-
return HalVirtualDisplayId(id);
141-
}
142-
return std::nullopt;
143-
}
144-
145123
static constexpr HalVirtualDisplayId fromValue(uint64_t value) {
146124
return HalVirtualDisplayId(SkipVirtualFlag{}, value);
147125
}
@@ -153,13 +131,6 @@ struct HalVirtualDisplayId : VirtualDisplayId {
153131
struct GpuVirtualDisplayId : VirtualDisplayId {
154132
explicit constexpr GpuVirtualDisplayId(BaseId baseId) : VirtualDisplayId(FLAG_GPU | baseId) {}
155133

156-
static constexpr std::optional<GpuVirtualDisplayId> tryCast(DisplayId id) {
157-
if (id.isVirtual() && (id.value & FLAG_GPU)) {
158-
return GpuVirtualDisplayId(id);
159-
}
160-
return std::nullopt;
161-
}
162-
163134
static constexpr GpuVirtualDisplayId fromValue(uint64_t value) {
164135
return GpuVirtualDisplayId(SkipVirtualFlag{}, value);
165136
}
@@ -173,14 +144,6 @@ struct GpuVirtualDisplayId : VirtualDisplayId {
173144
struct HalDisplayId : DisplayId {
174145
constexpr HalDisplayId(HalVirtualDisplayId other) : DisplayId(other) {}
175146
constexpr HalDisplayId(PhysicalDisplayId other) : DisplayId(other) {}
176-
177-
static constexpr std::optional<HalDisplayId> tryCast(DisplayId id) {
178-
if (GpuVirtualDisplayId::tryCast(id)) {
179-
return std::nullopt;
180-
}
181-
return HalDisplayId(id);
182-
}
183-
184147
static constexpr HalDisplayId fromValue(uint64_t value) { return HalDisplayId(value); }
185148

186149
private:
@@ -213,6 +176,18 @@ inline auto asPhysicalDisplayId(DisplayIdVariant variant) -> ftl::Optional<Physi
213176
return asDisplayIdOfType<PhysicalDisplayId>(variant);
214177
}
215178

179+
inline auto asVirtualDisplayId(DisplayIdVariant variant) -> ftl::Optional<VirtualDisplayId> {
180+
return ftl::match(
181+
variant,
182+
[](GpuVirtualDisplayId id) -> ftl::Optional<VirtualDisplayId> {
183+
return ftl::Optional(static_cast<VirtualDisplayId>(id));
184+
},
185+
[](HalVirtualDisplayId id) -> ftl::Optional<VirtualDisplayId> {
186+
return ftl::Optional(static_cast<VirtualDisplayId>(id));
187+
},
188+
[](auto) -> ftl::Optional<VirtualDisplayId> { return std::nullopt; });
189+
}
190+
216191
inline auto asDisplayId(DisplayIdVariant variant) -> DisplayId {
217192
return ftl::match(variant, [](auto id) -> DisplayId { return static_cast<DisplayId>(id); });
218193
}

‎libs/ui/tests/Android.bp‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,6 @@ cc_test {
4444
],
4545
}
4646

47-
cc_test {
48-
name: "DisplayId_test",
49-
shared_libs: ["libui"],
50-
srcs: ["DisplayId_test.cpp"],
51-
cflags: [
52-
"-Wall",
53-
"-Werror",
54-
],
55-
}
56-
5747
cc_test {
5848
name: "DisplayIdentification_test",
5949
shared_libs: ["libui"],

‎libs/ui/tests/DisplayId_test.cpp‎

Lines changed: 0 additions & 101 deletions
This file was deleted.

‎services/surfaceflinger/DisplayDevice.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
#include <common/trace.h>
2828
#include <compositionengine/CompositionEngine.h>
29-
#include <compositionengine/Display.h>
3029
#include <compositionengine/DisplayColorProfile.h>
3130
#include <compositionengine/DisplayColorProfileCreationArgs.h>
3231
#include <compositionengine/DisplayCreationArgs.h>
@@ -308,6 +307,10 @@ DisplayId DisplayDevice::getId() const {
308307
return mCompositionDisplay->getId();
309308
}
310309

310+
bool DisplayDevice::isVirtual() const {
311+
return mCompositionDisplay->isVirtual();
312+
}
313+
311314
bool DisplayDevice::isSecure() const {
312315
return mCompositionDisplay->isSecure();
313316
}

‎services/surfaceflinger/DisplayDevice.h‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class DisplayDevice : public RefBase {
8282
return mCompositionDisplay;
8383
}
8484

85-
bool isVirtual() const { return getId().isVirtual(); }
85+
bool isVirtual() const;
8686
bool isPrimary() const { return mIsPrimary; }
8787

8888
// isSecure indicates whether this display can be trusted to display
@@ -126,17 +126,24 @@ class DisplayDevice : public RefBase {
126126
return *idVariant;
127127
}
128128

129+
std::optional<VirtualDisplayIdVariant> getVirtualDisplayIdVariant() const {
130+
return ftl::match(
131+
getDisplayIdVariant(),
132+
[](PhysicalDisplayId) { return std::optional<VirtualDisplayIdVariant>(); },
133+
[](auto id) { return std::optional<VirtualDisplayIdVariant>(id); });
134+
}
135+
129136
// Shorthand to upcast the ID of a display whose type is known as a precondition.
130137
PhysicalDisplayId getPhysicalId() const {
131-
const auto id = PhysicalDisplayId::tryCast(getId());
132-
LOG_FATAL_IF(!id);
133-
return *id;
138+
const auto physicalDisplayId = asPhysicalDisplayId(getDisplayIdVariant());
139+
LOG_FATAL_IF(!physicalDisplayId);
140+
return *physicalDisplayId;
134141
}
135142

136143
VirtualDisplayId getVirtualId() const {
137-
const auto id = VirtualDisplayId::tryCast(getId());
138-
LOG_FATAL_IF(!id);
139-
return *id;
144+
const auto virtualDisplayId = asVirtualDisplayId(getDisplayIdVariant());
145+
LOG_FATAL_IF(!virtualDisplayId);
146+
return *virtualDisplayId;
140147
}
141148

142149
const wp<IBinder>& getDisplayToken() const { return mDisplayToken; }

‎services/surfaceflinger/Layer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1433,8 +1433,8 @@ void Layer::onCompositionPresented(const DisplayDevice* display,
14331433
presentFence,
14341434
FrameTracer::FrameEvent::PRESENT_FENCE);
14351435
mDeprecatedFrameTracker.setActualPresentFence(std::shared_ptr<FenceTime>(presentFence));
1436-
} else if (const auto displayId = PhysicalDisplayId::tryCast(display->getId());
1437-
displayId && mFlinger->getHwComposer().isConnected(*displayId)) {
1436+
} else if (const auto displayId = asPhysicalDisplayId(display->getDisplayIdVariant());
1437+
displayId.has_value() && mFlinger->getHwComposer().isConnected(*displayId)) {
14381438
// The HWC doesn't support present fences, so use the present timestamp instead.
14391439
const nsecs_t presentTimestamp =
14401440
mFlinger->getHwComposer().getPresentTimestamp(*displayId);

‎services/surfaceflinger/Scheduler/include/scheduler/interface/CompositionCoverage.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ enum class CompositionCoverage : std::uint8_t {
3636

3737
using CompositionCoverageFlags = ftl::Flags<CompositionCoverage>;
3838

39-
using CompositionCoveragePerDisplay = ui::DisplayMap<DisplayId, CompositionCoverageFlags>;
39+
using CompositionCoveragePerDisplay = ui::DisplayMap<DisplayIdVariant, CompositionCoverageFlags>;
4040

4141
inline CompositionCoverageFlags multiDisplayUnion(const CompositionCoveragePerDisplay& displays) {
4242
CompositionCoverageFlags coverage;

0 commit comments

Comments
 (0)