Revert "ui: Refactor stable ID generation for GPU virtual displays"
This reverts commit 78c90af2224b43467d854e356492527db0f8eb70.
Reason for revert: Causes assert for certain GPU virtual display Ids in VirtualDisplaySurface.cpp (https://paste.googleplex.com/6474098414977024)
Change-Id: I743312bbab4fd9903e069c3ed4710f9d7901be8d
diff --git a/libs/ui/include/ui/DisplayId.h b/libs/ui/include/ui/DisplayId.h
index 8a14db8..3a31fa0 100644
--- a/libs/ui/include/ui/DisplayId.h
+++ b/libs/ui/include/ui/DisplayId.h
@@ -20,7 +20,6 @@
#include <ostream>
#include <string>
-#include <ftl/hash.h>
#include <ftl/optional.h>
namespace android {
@@ -39,9 +38,6 @@
constexpr DisplayId(const DisplayId&) = default;
DisplayId& operator=(const DisplayId&) = default;
- constexpr bool isVirtual() const { return value & FLAG_VIRTUAL; }
- constexpr bool isStable() const { return value & FLAG_STABLE; }
-
uint64_t value;
// For deserialization.
@@ -80,10 +76,10 @@
// DisplayId of a physical display, such as the internal display or externally connected display.
struct PhysicalDisplayId : DisplayId {
static constexpr ftl::Optional<PhysicalDisplayId> tryCast(DisplayId id) {
- if (id.isVirtual()) {
+ if (id.value & FLAG_VIRTUAL) {
return std::nullopt;
}
- return PhysicalDisplayId(id);
+ return {PhysicalDisplayId(id)};
}
// Returns a stable ID based on EDID information.
@@ -121,23 +117,25 @@
static constexpr uint64_t FLAG_GPU = 1ULL << 61;
static constexpr std::optional<VirtualDisplayId> tryCast(DisplayId id) {
- if (id.isVirtual()) {
- return VirtualDisplayId(id);
+ if (id.value & FLAG_VIRTUAL) {
+ return {VirtualDisplayId(id)};
}
return std::nullopt;
}
protected:
- explicit constexpr VirtualDisplayId(uint64_t value) : DisplayId(FLAG_VIRTUAL | value) {}
+ constexpr VirtualDisplayId(uint64_t flags, BaseId baseId)
+ : DisplayId(DisplayId::FLAG_VIRTUAL | flags | baseId) {}
+
explicit constexpr VirtualDisplayId(DisplayId other) : DisplayId(other) {}
};
struct HalVirtualDisplayId : VirtualDisplayId {
- explicit constexpr HalVirtualDisplayId(BaseId baseId) : VirtualDisplayId(baseId) {}
+ explicit constexpr HalVirtualDisplayId(BaseId baseId) : VirtualDisplayId(0, baseId) {}
static constexpr std::optional<HalVirtualDisplayId> tryCast(DisplayId id) {
- if (id.isVirtual() && !(id.value & FLAG_GPU)) {
- return HalVirtualDisplayId(id);
+ if ((id.value & FLAG_VIRTUAL) && !(id.value & VirtualDisplayId::FLAG_GPU)) {
+ return {HalVirtualDisplayId(id)};
}
return std::nullopt;
}
@@ -147,27 +145,17 @@
};
struct GpuVirtualDisplayId : VirtualDisplayId {
- explicit constexpr GpuVirtualDisplayId(BaseId baseId) : VirtualDisplayId(FLAG_GPU | baseId) {}
-
- static constexpr std::optional<GpuVirtualDisplayId> fromUniqueId(const std::string& uniqueId) {
- if (const auto hashOpt = ftl::stable_hash(uniqueId)) {
- return GpuVirtualDisplayId(HashTag{}, *hashOpt);
- }
- return {};
- }
+ explicit constexpr GpuVirtualDisplayId(BaseId baseId)
+ : VirtualDisplayId(VirtualDisplayId::FLAG_GPU, baseId) {}
static constexpr std::optional<GpuVirtualDisplayId> tryCast(DisplayId id) {
- if (id.isVirtual() && (id.value & FLAG_GPU)) {
- return GpuVirtualDisplayId(id);
+ if ((id.value & FLAG_VIRTUAL) && (id.value & VirtualDisplayId::FLAG_GPU)) {
+ return {GpuVirtualDisplayId(id)};
}
return std::nullopt;
}
private:
- struct HashTag {}; // Disambiguate with BaseId constructor.
- constexpr GpuVirtualDisplayId(HashTag, uint64_t hash)
- : VirtualDisplayId(FLAG_STABLE | FLAG_GPU | hash) {}
-
explicit constexpr GpuVirtualDisplayId(DisplayId other) : VirtualDisplayId(other) {}
};
@@ -181,7 +169,7 @@
if (GpuVirtualDisplayId::tryCast(id)) {
return std::nullopt;
}
- return HalDisplayId(id);
+ return {HalDisplayId(id)};
}
private:
diff --git a/libs/ui/tests/DisplayId_test.cpp b/libs/ui/tests/DisplayId_test.cpp
index 1115804..8ddee7e 100644
--- a/libs/ui/tests/DisplayId_test.cpp
+++ b/libs/ui/tests/DisplayId_test.cpp
@@ -24,7 +24,7 @@
constexpr uint8_t port = 1;
constexpr uint16_t manufacturerId = 13;
constexpr uint32_t modelHash = 42;
- const PhysicalDisplayId id = PhysicalDisplayId::fromEdid(port, manufacturerId, modelHash);
+ PhysicalDisplayId id = PhysicalDisplayId::fromEdid(port, manufacturerId, modelHash);
EXPECT_EQ(port, id.getPort());
EXPECT_EQ(manufacturerId, id.getManufacturerId());
EXPECT_FALSE(VirtualDisplayId::tryCast(id));
@@ -39,7 +39,7 @@
TEST(DisplayIdTest, createPhysicalIdFromPort) {
constexpr uint8_t port = 3;
- const PhysicalDisplayId id = PhysicalDisplayId::fromPort(port);
+ PhysicalDisplayId id = PhysicalDisplayId::fromPort(port);
EXPECT_EQ(port, id.getPort());
EXPECT_FALSE(VirtualDisplayId::tryCast(id));
EXPECT_FALSE(HalVirtualDisplayId::tryCast(id));
@@ -52,22 +52,7 @@
}
TEST(DisplayIdTest, createGpuVirtualId) {
- const GpuVirtualDisplayId id(42);
- EXPECT_TRUE(VirtualDisplayId::tryCast(id));
- EXPECT_TRUE(GpuVirtualDisplayId::tryCast(id));
- EXPECT_FALSE(HalVirtualDisplayId::tryCast(id));
- EXPECT_FALSE(PhysicalDisplayId::tryCast(id));
- EXPECT_FALSE(HalDisplayId::tryCast(id));
-
- EXPECT_EQ(id, DisplayId::fromValue(id.value));
- EXPECT_EQ(id, DisplayId::fromValue<GpuVirtualDisplayId>(id.value));
-}
-
-TEST(DisplayIdTest, createGpuVirtualIdFromUniqueId) {
- static const std::string kUniqueId("virtual:ui:DisplayId_test");
- const auto idOpt = GpuVirtualDisplayId::fromUniqueId(kUniqueId);
- ASSERT_TRUE(idOpt.has_value());
- const GpuVirtualDisplayId id = idOpt.value();
+ GpuVirtualDisplayId id(42);
EXPECT_TRUE(VirtualDisplayId::tryCast(id));
EXPECT_TRUE(GpuVirtualDisplayId::tryCast(id));
EXPECT_FALSE(HalVirtualDisplayId::tryCast(id));
@@ -79,7 +64,7 @@
}
TEST(DisplayIdTest, createHalVirtualId) {
- const HalVirtualDisplayId id(42);
+ HalVirtualDisplayId id(42);
EXPECT_TRUE(VirtualDisplayId::tryCast(id));
EXPECT_TRUE(HalVirtualDisplayId::tryCast(id));
EXPECT_FALSE(GpuVirtualDisplayId::tryCast(id));
diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp
index 83b1b68..c18be7a 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -79,7 +79,7 @@
}
bool Display::isVirtual() const {
- return mId.isVirtual();
+ return VirtualDisplayId::tryCast(mId).has_value();
}
std::optional<DisplayId> Display::getDisplayId() const {
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index ef3e77d..c2d09c9 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -89,7 +89,7 @@
return mCompositionDisplay;
}
- bool isVirtual() const { return getId().isVirtual(); }
+ bool isVirtual() const { return VirtualDisplayId::tryCast(getId()).has_value(); }
bool isPrimary() const { return mIsPrimary; }
// isSecure indicates whether this display can be trusted to display
diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
index d69bfaf..4b5a68c 100644
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
@@ -316,7 +316,7 @@
status_t VirtualDisplaySurface::dequeueBuffer(Source source,
PixelFormat format, uint64_t usage, int* sslot, sp<Fence>* fence) {
- LOG_ALWAYS_FATAL_IF(mDisplayId.isVirtual());
+ LOG_ALWAYS_FATAL_IF(GpuVirtualDisplayId::tryCast(mDisplayId).has_value());
status_t result =
mSource[source]->dequeueBuffer(sslot, fence, mSinkBufferWidth, mSinkBufferHeight,
@@ -616,7 +616,7 @@
}
status_t VirtualDisplaySurface::refreshOutputBuffer() {
- LOG_ALWAYS_FATAL_IF(mDisplayId.isVirtual());
+ LOG_ALWAYS_FATAL_IF(GpuVirtualDisplayId::tryCast(mDisplayId).has_value());
if (mOutputProducerSlot >= 0) {
mSource[SOURCE_SINK]->cancelBuffer(