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

5 files changed, 23 insertions, 50 deletions

diff --git a/libs/ui/include/ui/DisplayId.h b/libs/ui/include/ui/DisplayId.h
index 8a14db8ffa..3a31fa0848 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 @@ struct DisplayId {
     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 @@ inline std::ostream& operator<<(std::ostream& stream, DisplayId displayId) {
 // 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 @@ struct VirtualDisplayId : DisplayId {
     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 @@ private:
 };
 
 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 @@ struct HalDisplayId : DisplayId {
         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 1115804954..8ddee7e740 100644
--- a/libs/ui/tests/DisplayId_test.cpp
+++ b/libs/ui/tests/DisplayId_test.cpp
@@ -24,7 +24,7 @@ TEST(DisplayIdTest, createPhysicalIdFromEdid) {
     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, createPhysicalIdFromEdid) {
 
 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, createPhysicalIdFromPort) {
 }
 
 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, createGpuVirtualIdFromUniqueId) {
 }
 
 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 83b1b68a17..c18be7a8e8 100644
--- a/services/surfaceflinger/CompositionEngine/src/Display.cpp
+++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp
@@ -79,7 +79,7 @@ void Display::setSecure(bool secure) {
 }
 
 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 ef3e77d4da..c2d09c910d 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -89,7 +89,7 @@ public:
         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 d69bfafd2a..4b5a68cefa 100644
--- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp
@@ -316,7 +316,7 @@ status_t VirtualDisplaySurface::setAsyncMode(bool async) {
 
 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 @@ void VirtualDisplaySurface::resetPerFrameState() {
 }
 
 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(