Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5be0865b authored by Alan Ding's avatar Alan Ding Committed by Android (Google) Code Review
Browse files

Revert "ui: Refactor stable ID generation for GPU virtual displays" 

This reverts commit 78c90af2.

Reason for revert: Causes assert for certain GPU virtual display Ids in VirtualDisplaySurface.cpp (https://paste.googleplex.com/6474098414977024)

Change-Id: I743312bbab4fd9903e069c3ed4710f9d7901be8d
parent 78c90af2

libs/ui/include/ui/DisplayId.h

+15 −27
Original line number Diff line number Diff line
@@ -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:

libs/ui/tests/DisplayId_test.cpp

+4 −19
Original line number Diff line number Diff line
@@ -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));

services/surfaceflinger/CompositionEngine/src/Display.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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 {

services/surfaceflinger/DisplayDevice.h

+1 −1
Original line number Diff line number Diff line
@@ -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

services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp

+2 −2
Original line number Diff line number Diff line
@@ -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(