Use type safe display IDs.
Use a type safe wrapper around uint64_t for display ids.
We also modify the format of the EDID generated physical
display IDs by setting bit 62 to 1, which will indicate
that the display id is stable across reboot.
Bug: 160679868
Test: m && flash device
Test: take screnshot on device
Test: atest surfaceflinger_unittest
Change-Id: Ie2c7c2b737e0882fa4208c059caa85b7ded922b2
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 9add6a5..7d9ac19 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -81,7 +81,7 @@
constexpr hal::HWLayerId HWC_LAYER = 5000;
constexpr Transform DEFAULT_TRANSFORM = static_cast<Transform>(0);
-constexpr DisplayId DEFAULT_DISPLAY_ID = DisplayId{42};
+constexpr PhysicalDisplayId DEFAULT_DISPLAY_ID(42);
constexpr int DEFAULT_DISPLAY_WIDTH = 1920;
constexpr int DEFAULT_DISPLAY_HEIGHT = 1024;
diff --git a/services/surfaceflinger/tests/unittests/DisplayIdentificationTest.cpp b/services/surfaceflinger/tests/unittests/DisplayIdentificationTest.cpp
index cc6a60c..02ce079 100644
--- a/services/surfaceflinger/tests/unittests/DisplayIdentificationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayIdentificationTest.cpp
@@ -396,10 +396,10 @@
}
}
-TEST(DisplayIdentificationTest, getFallbackDisplayId) {
+TEST(DisplayIdentificationTest, fromPort) {
// Manufacturer ID should be invalid.
- ASSERT_FALSE(getPnpId(getFallbackDisplayId(0)));
- ASSERT_FALSE(getPnpId(getFallbackDisplayId(0xffu)));
+ ASSERT_FALSE(getPnpId(PhysicalDisplayId::fromPort(0)));
+ ASSERT_FALSE(getPnpId(PhysicalDisplayId::fromPort(0xffu)));
}
TEST(DisplayIdentificationTest, getVirtualDisplayId) {
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index 9130b04..175f6a8 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -253,7 +253,7 @@
sp<DisplayDevice> DisplayTransactionTest::injectDefaultInternalDisplay(
std::function<void(FakeDisplayDeviceInjector&)> injectExtra) {
- constexpr DisplayId DEFAULT_DISPLAY_ID = DisplayId{777};
+ constexpr PhysicalDisplayId DEFAULT_DISPLAY_ID(777);
constexpr int DEFAULT_DISPLAY_WIDTH = 1080;
constexpr int DEFAULT_DISPLAY_HEIGHT = 1920;
constexpr HWDisplayId DEFAULT_DISPLAY_HWC_DISPLAY_ID = 0;
@@ -332,10 +332,10 @@
*/
template <typename PhysicalDisplay>
-struct PhysicalDisplayId {};
+struct PhysicalDisplayIdType {};
-template <DisplayId::Type displayId>
-using VirtualDisplayId = std::integral_constant<DisplayId::Type, displayId>;
+template <uint64_t displayId>
+using VirtualDisplayIdType = std::integral_constant<uint64_t, displayId>;
struct NoDisplayId {};
@@ -343,18 +343,18 @@
struct IsPhysicalDisplayId : std::bool_constant<false> {};
template <typename PhysicalDisplay>
-struct IsPhysicalDisplayId<PhysicalDisplayId<PhysicalDisplay>> : std::bool_constant<true> {};
+struct IsPhysicalDisplayId<PhysicalDisplayIdType<PhysicalDisplay>> : std::bool_constant<true> {};
template <typename>
struct DisplayIdGetter;
template <typename PhysicalDisplay>
-struct DisplayIdGetter<PhysicalDisplayId<PhysicalDisplay>> {
- static std::optional<DisplayId> get() {
+struct DisplayIdGetter<PhysicalDisplayIdType<PhysicalDisplay>> {
+ static std::optional<PhysicalDisplayId> get() {
if (!PhysicalDisplay::HAS_IDENTIFICATION_DATA) {
- return getFallbackDisplayId(static_cast<bool>(PhysicalDisplay::PRIMARY)
- ? LEGACY_DISPLAY_TYPE_PRIMARY
- : LEGACY_DISPLAY_TYPE_EXTERNAL);
+ return PhysicalDisplayId::fromPort(static_cast<bool>(PhysicalDisplay::PRIMARY)
+ ? LEGACY_DISPLAY_TYPE_PRIMARY
+ : LEGACY_DISPLAY_TYPE_EXTERNAL);
}
const auto info =
@@ -364,9 +364,10 @@
}
};
-template <DisplayId::Type displayId>
-struct DisplayIdGetter<VirtualDisplayId<displayId>> {
- static std::optional<DisplayId> get() { return DisplayId{displayId}; }
+template <uint64_t displayId>
+struct DisplayIdGetter<VirtualDisplayIdType<displayId>> {
+ // TODO(b/160679868) Use VirtualDisplayId
+ static std::optional<PhysicalDisplayId> get() { return PhysicalDisplayId{displayId}; }
};
template <>
@@ -380,7 +381,7 @@
};
template <typename PhysicalDisplay>
-struct DisplayConnectionTypeGetter<PhysicalDisplayId<PhysicalDisplay>> {
+struct DisplayConnectionTypeGetter<PhysicalDisplayIdType<PhysicalDisplay>> {
static constexpr std::optional<DisplayConnectionType> value = PhysicalDisplay::CONNECTION_TYPE;
};
@@ -391,19 +392,19 @@
constexpr HWDisplayId HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID = 1010;
-template <DisplayId::Type displayId>
-struct HwcDisplayIdGetter<VirtualDisplayId<displayId>> {
+template <uint64_t displayId>
+struct HwcDisplayIdGetter<VirtualDisplayIdType<displayId>> {
static constexpr std::optional<HWDisplayId> value = HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID;
};
template <typename PhysicalDisplay>
-struct HwcDisplayIdGetter<PhysicalDisplayId<PhysicalDisplay>> {
+struct HwcDisplayIdGetter<PhysicalDisplayIdType<PhysicalDisplay>> {
static constexpr std::optional<HWDisplayId> value = PhysicalDisplay::HWC_DISPLAY_ID;
};
// DisplayIdType can be:
-// 1) PhysicalDisplayId<...> for generated ID of physical display backed by HWC.
-// 2) VirtualDisplayId<...> for hard-coded ID of virtual display backed by HWC.
+// 1) PhysicalDisplayIdType<...> for generated ID of physical display backed by HWC.
+// 2) VirtualDisplayIdType<...> for hard-coded ID of virtual display backed by HWC.
// 3) NoDisplayId for virtual display without HWC backing.
template <typename DisplayIdType, int width, int height, Critical critical, Async async,
Secure secure, Primary primary, int grallocUsage>
@@ -631,10 +632,11 @@
template <typename PhysicalDisplay, int width, int height, Critical critical>
struct PhysicalDisplayVariant
- : DisplayVariant<PhysicalDisplayId<PhysicalDisplay>, width, height, critical, Async::FALSE,
- Secure::TRUE, PhysicalDisplay::PRIMARY, GRALLOC_USAGE_PHYSICAL_DISPLAY>,
+ : DisplayVariant<PhysicalDisplayIdType<PhysicalDisplay>, width, height, critical,
+ Async::FALSE, Secure::TRUE, PhysicalDisplay::PRIMARY,
+ GRALLOC_USAGE_PHYSICAL_DISPLAY>,
HwcDisplayVariant<PhysicalDisplay::HWC_DISPLAY_ID, DisplayType::PHYSICAL,
- DisplayVariant<PhysicalDisplayId<PhysicalDisplay>, width, height,
+ DisplayVariant<PhysicalDisplayIdType<PhysicalDisplay>, width, height,
critical, Async::FALSE, Secure::TRUE,
PhysicalDisplay::PRIMARY, GRALLOC_USAGE_PHYSICAL_DISPLAY>,
PhysicalDisplay> {};
@@ -720,14 +722,14 @@
template <int width, int height, Secure secure>
struct HwcVirtualDisplayVariant
- : DisplayVariant<VirtualDisplayId<42>, width, height, Critical::FALSE, Async::TRUE, secure,
- Primary::FALSE, GRALLOC_USAGE_HWC_VIRTUAL_DISPLAY>,
- HwcDisplayVariant<
- HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID, DisplayType::VIRTUAL,
- DisplayVariant<VirtualDisplayId<42>, width, height, Critical::FALSE, Async::TRUE,
- secure, Primary::FALSE, GRALLOC_USAGE_HWC_VIRTUAL_DISPLAY>> {
- using Base = DisplayVariant<VirtualDisplayId<42>, width, height, Critical::FALSE, Async::TRUE,
- secure, Primary::FALSE, GRALLOC_USAGE_HW_COMPOSER>;
+ : DisplayVariant<VirtualDisplayIdType<42>, width, height, Critical::FALSE, Async::TRUE,
+ secure, Primary::FALSE, GRALLOC_USAGE_HWC_VIRTUAL_DISPLAY>,
+ HwcDisplayVariant<HWC_VIRTUAL_DISPLAY_HWC_DISPLAY_ID, DisplayType::VIRTUAL,
+ DisplayVariant<VirtualDisplayIdType<42>, width, height, Critical::FALSE,
+ Async::TRUE, secure, Primary::FALSE,
+ GRALLOC_USAGE_HWC_VIRTUAL_DISPLAY>> {
+ using Base = DisplayVariant<VirtualDisplayIdType<42>, width, height, Critical::FALSE,
+ Async::TRUE, secure, Primary::FALSE, GRALLOC_USAGE_HW_COMPOSER>;
using Self = HwcVirtualDisplayVariant<width, height, secure>;
static std::shared_ptr<compositionengine::Display> injectCompositionDisplay(
@@ -1821,7 +1823,9 @@
ASSERT_TRUE(displayId);
const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value;
ASSERT_TRUE(hwcDisplayId);
- state.physical = {.id = *displayId, .type = *connectionType, .hwcDisplayId = *hwcDisplayId};
+ state.physical = {.id = static_cast<PhysicalDisplayId>(*displayId),
+ .type = *connectionType,
+ .hwcDisplayId = *hwcDisplayId};
}
state.isSecure = static_cast<bool>(Case::Display::SECURE);
@@ -1997,7 +2001,7 @@
ASSERT_TRUE(displayId);
const auto hwcDisplayId = Case::Display::HWC_DISPLAY_ID_OPT::value;
ASSERT_TRUE(hwcDisplayId);
- expectedPhysical = {.id = *displayId,
+ expectedPhysical = {.id = static_cast<PhysicalDisplayId>(*displayId),
.type = *connectionType,
.hwcDisplayId = *hwcDisplayId};
}
diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
index b90b566..aab6d01 100644
--- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
+++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp
@@ -36,9 +36,9 @@
namespace {
-constexpr PhysicalDisplayId INTERNAL_DISPLAY_ID = 111;
-constexpr PhysicalDisplayId EXTERNAL_DISPLAY_ID = 222;
-constexpr PhysicalDisplayId DISPLAY_ID_64BIT = 0xabcd12349876fedcULL;
+constexpr PhysicalDisplayId INTERNAL_DISPLAY_ID(111);
+constexpr PhysicalDisplayId EXTERNAL_DISPLAY_ID(222);
+constexpr PhysicalDisplayId DISPLAY_ID_64BIT(0xabcd12349876fedcULL);
class MockVSyncSource : public VSyncSource {
public:
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index 382199d..f756b9e 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -41,7 +41,7 @@
namespace android {
-constexpr PhysicalDisplayId PHYSICAL_DISPLAY_ID = 999;
+constexpr PhysicalDisplayId PHYSICAL_DISPLAY_ID(999);
class SchedulerTest : public testing::Test {
protected:
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 256e048..215056c 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -643,7 +643,9 @@
if (const auto type = mCreationArgs.connectionType) {
LOG_ALWAYS_FATAL_IF(!displayId);
LOG_ALWAYS_FATAL_IF(!mHwcDisplayId);
- state.physical = {.id = *displayId, .type = *type, .hwcDisplayId = *mHwcDisplayId};
+ state.physical = {.id = static_cast<PhysicalDisplayId>(*displayId),
+ .type = *type,
+ .hwcDisplayId = *mHwcDisplayId};
}
state.isSecure = mCreationArgs.isSecure;