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

Commit 3c363247 authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

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
parent 87d026ac

services/surfaceflinger/SurfaceFlinger.cpp

+15 −13
Original line number Diff line number Diff line
@@ -582,14 +582,8 @@ void SurfaceFlinger::releaseVirtualDisplay(VirtualDisplayId displayId) { 
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 @@ std::vector<PhysicalDisplayId> SurfaceFlinger::getPhysicalDisplayIdsLocked() con 


status_t SurfaceFlinger::getPrimaryPhysicalDisplayId(PhysicalDisplayId* id) const {

    Mutex::Autolock lock(mStateLock);

    *id = getInternalDisplayIdLocked();

    *id = getPrimaryDisplayIdLocked();

    return NO_ERROR;

}
@@ -1319,12 +1313,20 @@ status_t SurfaceFlinger::getDisplayNativePrimaries(const sp<IBinder>& displayTok 
        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 @@ sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal( 
    }



    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);

services/surfaceflinger/SurfaceFlinger.h

+17 −32
Original line number Diff line number Diff line
@@ -525,19 +525,6 @@ private: 
    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 @@ private: 
        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 @@ private: 
        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 @@ private: 
        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 @@ private: 
        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();

    }