SF: Clean up display lookup
Remove redundant functions. Phase out the "internal display" misnomer,
which is confusing for foldables.
Validate the DisplayPrimaries API against the connection type, rather
than the active display.
Bug: 182939859
Bug: 74619554
Test: Boot and fold/unfold
Change-Id: Ibe69e3337806a3f269f355522bcf65eab974d7bb
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 3bfc2cc..f33ea5b 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -582,14 +582,8 @@
std::vector<PhysicalDisplayId> SurfaceFlinger::getPhysicalDisplayIdsLocked() const {
std::vector<PhysicalDisplayId> displayIds;
displayIds.reserve(mPhysicalDisplayTokens.size());
- const auto defaultDisplayId = [this]() REQUIRES(mStateLock) {
- if (const auto display = getDefaultDisplayDeviceLocked()) {
- return display->getPhysicalId();
- }
- // fallback to the internal display id if the active display is unknown
- return getInternalDisplayIdLocked();
- }();
+ const auto defaultDisplayId = getDefaultDisplayDeviceLocked()->getPhysicalId();
displayIds.push_back(defaultDisplayId);
for (const auto& [id, token] : mPhysicalDisplayTokens) {
@@ -603,7 +597,7 @@
status_t SurfaceFlinger::getPrimaryPhysicalDisplayId(PhysicalDisplayId* id) const {
Mutex::Autolock lock(mStateLock);
- *id = getInternalDisplayIdLocked();
+ *id = getPrimaryDisplayIdLocked();
return NO_ERROR;
}
@@ -1314,17 +1308,25 @@
}
status_t SurfaceFlinger::getDisplayNativePrimaries(const sp<IBinder>& displayToken,
- ui::DisplayPrimaries &primaries) {
+ ui::DisplayPrimaries& primaries) {
if (!displayToken) {
return BAD_VALUE;
}
- // Currently we only support this API for a single internal display.
- if (getInternalDisplayToken() != displayToken) {
+ Mutex::Autolock lock(mStateLock);
+
+ const auto display = getDisplayDeviceLocked(displayToken);
+ if (!display) {
return NAME_NOT_FOUND;
}
- memcpy(&primaries, &mInternalDisplayPrimaries, sizeof(ui::DisplayPrimaries));
+ const auto connectionType = display->getConnectionType();
+ if (connectionType != ui::DisplayConnectionType::Internal) {
+ return INVALID_OPERATION;
+ }
+
+ // TODO(b/229846990): For now, assume that all internal displays have the same primaries.
+ primaries = mInternalDisplayPrimaries;
return NO_ERROR;
}
@@ -2820,7 +2822,7 @@
}
if (const auto id = PhysicalDisplayId::tryCast(compositionDisplay->getId())) {
- creationArgs.isPrimary = id == getInternalDisplayIdLocked();
+ creationArgs.isPrimary = id == getPrimaryDisplayIdLocked();
if (useColorManagement) {
std::vector<ColorMode> modes = getHwComposer().getColorModes(*id);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 011aaef..6aa54ff 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -525,19 +525,6 @@
bool callingThreadHasUnscopedSurfaceFlingerAccess(bool usePermissionCache = true)
EXCLUDES(mStateLock);
- // the following two methods are moved from ISurfaceComposer.h
- // TODO(b/74619554): Remove this stopgap once the framework is display-agnostic.
- std::optional<PhysicalDisplayId> getInternalDisplayId() const {
- const auto displayIds = getPhysicalDisplayIds();
- return displayIds.empty() ? std::nullopt : std::make_optional(displayIds.front());
- }
-
- // TODO(b/74619554): Remove this stopgap once the framework is display-agnostic.
- sp<IBinder> getInternalDisplayToken() const {
- const auto displayId = getInternalDisplayId();
- return displayId ? getPhysicalDisplayToken(*displayId) : nullptr;
- }
-
// Implements ISurfaceComposer
sp<ISurfaceComposerClient> createConnection() override;
sp<IBinder> createDisplay(const String8& displayName, bool secure);
@@ -922,6 +909,13 @@
return nullptr;
}
+ sp<const DisplayDevice> getDisplayDeviceLocked(DisplayId id) const REQUIRES(mStateLock) {
+ // TODO(b/182939859): Replace tokens with IDs for display lookup.
+ return findDisplay([id](const auto& display) { return display.getId() == id; });
+ }
+
+ // Returns the primary display or (for foldables) the active display, assuming that the inner
+ // and outer displays have mutually exclusive power states.
sp<const DisplayDevice> getDefaultDisplayDeviceLocked() const REQUIRES(mStateLock) {
return const_cast<SurfaceFlinger*>(this)->getDefaultDisplayDeviceLocked();
}
@@ -930,12 +924,9 @@
if (const auto display = getDisplayDeviceLocked(mActiveDisplayToken)) {
return display;
}
- // The active display is outdated, fall back to the internal display
+ // The active display is outdated, so fall back to the primary display.
mActiveDisplayToken.clear();
- if (const auto token = getInternalDisplayTokenLocked()) {
- return getDisplayDeviceLocked(token);
- }
- return nullptr;
+ return getDisplayDeviceLocked(getPrimaryDisplayTokenLocked());
}
sp<const DisplayDevice> getDefaultDisplayDevice() const EXCLUDES(mStateLock) {
@@ -952,11 +943,6 @@
return it == mDisplays.end() ? nullptr : it->second;
}
- sp<const DisplayDevice> getDisplayDeviceLocked(DisplayId id) const REQUIRES(mStateLock) {
- // TODO(b/182939859): Replace tokens with IDs for display lookup.
- return findDisplay([id](const auto& display) { return display.getId() == id; });
- }
-
std::vector<PhysicalDisplayId> getPhysicalDisplayIdsLocked() const REQUIRES(mStateLock);
// mark a region of a layer stack dirty. this updates the dirty
@@ -1066,18 +1052,17 @@
return {};
}
- // TODO(b/182939859): SF conflates the primary (a.k.a. default) display with the first display
- // connected at boot, which is typically internal. (Theoretically, it must be internal because
- // SF does not support disconnecting it, though in practice HWC may circumvent this limitation.)
+ // Returns the first display connected at boot.
//
- // SF inherits getInternalDisplayToken and getInternalDisplayId from ISurfaceComposer, so these
- // locked counterparts are named consistently. Once SF supports headless mode and can designate
- // any display as primary, the "internal" misnomer will be phased out.
- sp<IBinder> getInternalDisplayTokenLocked() const REQUIRES(mStateLock) {
- return getPhysicalDisplayTokenLocked(getInternalDisplayIdLocked());
+ // TODO(b/229851933): SF conflates the primary display with the first display connected at boot,
+ // which typically has DisplayConnectionType::Internal. (Theoretically, it must be an internal
+ // display because SF does not support disconnecting it, though in practice HWC may circumvent
+ // this limitation.)
+ sp<IBinder> getPrimaryDisplayTokenLocked() const REQUIRES(mStateLock) {
+ return getPhysicalDisplayTokenLocked(getPrimaryDisplayIdLocked());
}
- PhysicalDisplayId getInternalDisplayIdLocked() const REQUIRES(mStateLock) {
+ PhysicalDisplayId getPrimaryDisplayIdLocked() const REQUIRES(mStateLock) {
return getHwComposer().getPrimaryDisplayId();
}