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

Commit 22257170 authored by Gil Dekel's avatar Gil Dekel
Browse files

SF: Remove *DisplayId::tryCast usage from VirtualDisplaySurface 

Work towards DisplayId opaqueness by eliminating call-sites to APIs that
parse the display ID values directly. One such site is
VirtualDisplaySurface.

Replace all calls to *DisplayId::tryCast with a VirtualDisplayIdVariant
guard.

Flag: com.android.graphics.surfaceflinger.flags.stable_edid_ids
Bug: 390690584
Test: libsurfaceflinger_unittest
Change-Id: I7ae9e838547c31ce09349e15f7d23e99f313646b
parent a8da2ca2

libs/ui/include/ui/DisplayId.h

+1 −0
Original line number Diff line number Diff line
@@ -189,6 +189,7 @@ private: 
};



using DisplayIdVariant = std::variant<PhysicalDisplayId, GpuVirtualDisplayId, HalVirtualDisplayId>;

using VirtualDisplayIdVariant = std::variant<GpuVirtualDisplayId, HalVirtualDisplayId>;



template <typename DisplayIdType>

inline auto asDisplayIdOfType(DisplayIdVariant variant) -> ftl::Optional<DisplayIdType> {

services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp

+29 −24
Original line number Diff line number Diff line
@@ -47,7 +47,8 @@ 


namespace android {



VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, VirtualDisplayId displayId,

VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc,

                                             VirtualDisplayIdVariant virtualIdVariant,

                                             const sp<IGraphicBufferProducer>& sink,

                                             const sp<IGraphicBufferProducer>& bqProducer,

                                             const sp<IGraphicBufferConsumer>& bqConsumer,
@@ -58,7 +59,7 @@ VirtualDisplaySurface::VirtualDisplaySurface(HWComposer& hwc, VirtualDisplayId d 
      : ConsumerBase(bqConsumer),

#endif // COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(WB_CONSUMER_BASE_OWNS_BQ)

        mHwc(hwc),

        mDisplayId(displayId),

        mVirtualIdVariant(virtualIdVariant),

        mDisplayName(name),

        mSource{},

        mDefaultOutputFormat(HAL_PIXEL_FORMAT_IMPLEMENTATION_DEFINED),
@@ -119,7 +120,7 @@ VirtualDisplaySurface::~VirtualDisplaySurface() { 
}



status_t VirtualDisplaySurface::beginFrame(bool mustRecompose) {

    if (GpuVirtualDisplayId::tryCast(mDisplayId)) {

    if (isBackedByGpu()) {

        return NO_ERROR;

    }
@@ -133,7 +134,7 @@ status_t VirtualDisplaySurface::beginFrame(bool mustRecompose) { 
}



status_t VirtualDisplaySurface::prepareFrame(CompositionType compositionType) {

    if (GpuVirtualDisplayId::tryCast(mDisplayId)) {

    if (isBackedByGpu()) {

        return NO_ERROR;

    }
@@ -181,7 +182,10 @@ status_t VirtualDisplaySurface::prepareFrame(CompositionType compositionType) { 
}



status_t VirtualDisplaySurface::advanceFrame(float hdrSdrRatio) {

    if (GpuVirtualDisplayId::tryCast(mDisplayId)) {

    const auto halVirtualDisplayId = ftl::match(

            mVirtualIdVariant, [](HalVirtualDisplayId id) { return ftl::Optional(id); },

            [](auto) { return ftl::Optional<HalVirtualDisplayId>(); });

    if (!halVirtualDisplayId) {

        return NO_ERROR;

    }
@@ -212,11 +216,8 @@ status_t VirtualDisplaySurface::advanceFrame(float hdrSdrRatio) { 
    VDS_LOGV("%s: fb=%d(%p) out=%d(%p)", __func__, mFbProducerSlot, fbBuffer.get(),

             mOutputProducerSlot, outBuffer.get());



    const auto halDisplayId = HalVirtualDisplayId::tryCast(mDisplayId);

    LOG_FATAL_IF(!halDisplayId);

    // At this point we know the output buffer acquire fence,

    // so update HWC state with it.

    mHwc.setOutputBuffer(*halDisplayId, mOutputFence, outBuffer);

    // At this point we know the output buffer acquire fence, so update HWC state with it.

    mHwc.setOutputBuffer(*halVirtualDisplayId, mOutputFence, outBuffer);



    status_t result = NO_ERROR;

    if (fbBuffer != nullptr) {
@@ -227,7 +228,7 @@ status_t VirtualDisplaySurface::advanceFrame(float hdrSdrRatio) { 
            hwcBuffer = fbBuffer; // HWC hasn't previously seen this buffer in this slot

        }

        // TODO: Correctly propagate the dataspace from GL composition

        result = mHwc.setClientTarget(*halDisplayId, mFbProducerSlot, mFbFence, hwcBuffer,

        result = mHwc.setClientTarget(*halVirtualDisplayId, mFbProducerSlot, mFbFence, hwcBuffer,

                                      ui::Dataspace::UNKNOWN, hdrSdrRatio);

    }
@@ -235,8 +236,8 @@ status_t VirtualDisplaySurface::advanceFrame(float hdrSdrRatio) { 
}



void VirtualDisplaySurface::onFrameCommitted() {

    const auto halDisplayId = HalVirtualDisplayId::tryCast(mDisplayId);

    if (!halDisplayId) {

    const auto halDisplayId = asHalDisplayId(mVirtualIdVariant);

    if (!halDisplayId.has_value()) {

        return;

    }
@@ -250,8 +251,7 @@ void VirtualDisplaySurface::onFrameCommitted() { 
        Mutex::Autolock lock(mMutex);

        int sslot = mapProducer2SourceSlot(SOURCE_SCRATCH, mFbProducerSlot);

        VDS_LOGV("%s: release scratch sslot=%d", __func__, sslot);

        addReleaseFenceLocked(sslot, mProducerBuffers[mFbProducerSlot],

                retireFence);

        addReleaseFenceLocked(sslot, mProducerBuffers[mFbProducerSlot], retireFence);

        releaseBufferLocked(sslot, mProducerBuffers[mFbProducerSlot]);

    }
@@ -299,7 +299,7 @@ const sp<Fence>& VirtualDisplaySurface::getClientTargetAcquireFence() const { 


status_t VirtualDisplaySurface::requestBuffer(int pslot,

        sp<GraphicBuffer>* outBuf) {

    if (GpuVirtualDisplayId::tryCast(mDisplayId)) {

    if (isBackedByGpu()) {

        return mSource[SOURCE_SINK]->requestBuffer(pslot, outBuf);

    }
@@ -321,7 +321,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(GpuVirtualDisplayId::tryCast(mDisplayId).has_value());

    LOG_ALWAYS_FATAL_IF(isBackedByGpu());



    status_t result =

            mSource[source]->dequeueBuffer(sslot, fence, mSinkBufferWidth, mSinkBufferHeight,
@@ -373,7 +373,7 @@ status_t VirtualDisplaySurface::dequeueBuffer(int* pslot, sp<Fence>* fence, uint 
                                              PixelFormat format, uint64_t usage,

                                              uint64_t* outBufferAge,

                                              FrameEventHistoryDelta* outTimestamps) {

    if (GpuVirtualDisplayId::tryCast(mDisplayId)) {

    if (isBackedByGpu()) {

        return mSource[SOURCE_SINK]->dequeueBuffer(pslot, fence, w, h, format, usage, outBufferAge,

                                                   outTimestamps);

    }
@@ -459,7 +459,7 @@ status_t VirtualDisplaySurface::attachBuffer(int*, const sp<GraphicBuffer>&) { 


status_t VirtualDisplaySurface::queueBuffer(int pslot,

        const QueueBufferInput& input, QueueBufferOutput* output) {

    if (GpuVirtualDisplayId::tryCast(mDisplayId)) {

    if (isBackedByGpu()) {

        return mSource[SOURCE_SINK]->queueBuffer(pslot, input, output);

    }
@@ -517,7 +517,7 @@ status_t VirtualDisplaySurface::queueBuffer(int pslot, 


status_t VirtualDisplaySurface::cancelBuffer(int pslot,

        const sp<Fence>& fence) {

    if (GpuVirtualDisplayId::tryCast(mDisplayId)) {

    if (isBackedByGpu()) {

        return mSource[SOURCE_SINK]->cancelBuffer(mapProducer2SourceSlot(SOURCE_SINK, pslot), fence);

    }
@@ -621,7 +621,10 @@ void VirtualDisplaySurface::resetPerFrameState() { 
}



status_t VirtualDisplaySurface::refreshOutputBuffer() {

    LOG_ALWAYS_FATAL_IF(GpuVirtualDisplayId::tryCast(mDisplayId).has_value());

    const auto halVirtualDisplayId = ftl::match(

            mVirtualIdVariant, [](HalVirtualDisplayId id) { return ftl::Optional(id); },

            [](auto) { return ftl::Optional<HalVirtualDisplayId>(); });

    LOG_ALWAYS_FATAL_IF(!halVirtualDisplayId);



    if (mOutputProducerSlot >= 0) {

        mSource[SOURCE_SINK]->cancelBuffer(
@@ -640,14 +643,16 @@ status_t VirtualDisplaySurface::refreshOutputBuffer() { 
    // until after GPU calls queueBuffer(). So here we just set the buffer

    // (for use in HWC prepare) but not the fence; we'll call this again with

    // the proper fence once we have it.

    const auto halDisplayId = HalVirtualDisplayId::tryCast(mDisplayId);

    LOG_FATAL_IF(!halDisplayId);

    result = mHwc.setOutputBuffer(*halDisplayId, Fence::NO_FENCE,

    result = mHwc.setOutputBuffer(*halVirtualDisplayId, Fence::NO_FENCE,

                                  mProducerBuffers[mOutputProducerSlot]);



    return result;

}



bool VirtualDisplaySurface::isBackedByGpu() const {

    return std::holds_alternative<GpuVirtualDisplayId>(mVirtualIdVariant);

}



// This slot mapping function is its own inverse, so two copies are unnecessary.

// Both are kept to make the intent clear where the function is called, and for

// the (unlikely) chance that we switch to a different mapping function.

services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h

+4 −2
Original line number Diff line number Diff line
@@ -75,7 +75,8 @@ class VirtualDisplaySurface : public compositionengine::DisplaySurface, 
                              public BnGraphicBufferProducer,

                              private ConsumerBase {

public:

    VirtualDisplaySurface(HWComposer&, VirtualDisplayId, const sp<IGraphicBufferProducer>& sink,

    VirtualDisplaySurface(HWComposer&, VirtualDisplayIdVariant,

                          const sp<IGraphicBufferProducer>& sink,

                          const sp<IGraphicBufferProducer>& bqProducer,

                          const sp<IGraphicBufferConsumer>& bqConsumer, const std::string& name);
@@ -145,6 +146,7 @@ private: 
    void updateQueueBufferOutput(QueueBufferOutput&&);

    void resetPerFrameState();

    status_t refreshOutputBuffer();

    bool isBackedByGpu() const;



    // Both the sink and scratch buffer pools have their own set of slots

    // ("source slots", or "sslot"). We have to merge these into the single
@@ -159,7 +161,7 @@ private: 
    // Immutable after construction

    //

    HWComposer& mHwc;

    const VirtualDisplayId mDisplayId;

    const VirtualDisplayIdVariant mVirtualIdVariant;

    const std::string mDisplayName;

    sp<IGraphicBufferProducer> mSource[2]; // indexed by SOURCE_*

    uint32_t mDefaultOutputFormat;

services/surfaceflinger/SurfaceFlinger.cpp

+12 −9
Original line number Diff line number Diff line
@@ -659,15 +659,15 @@ void SurfaceFlinger::enableHalVirtualDisplays(bool enable) { 
    }

}



void SurfaceFlinger::acquireVirtualDisplay(ui::Size resolution, ui::PixelFormat format,

                                           const std::string& uniqueId,

std::optional<VirtualDisplayIdVariant> SurfaceFlinger::acquireVirtualDisplay(

        ui::Size resolution, ui::PixelFormat format, const std::string& uniqueId,

        compositionengine::DisplayCreationArgsBuilder& builder) {

    if (auto& generator = mVirtualDisplayIdGenerators.hal) {

        if (const auto id = generator->generateId()) {

            if (getHwComposer().allocateVirtualDisplay(*id, resolution, &format)) {

                acquireVirtualDisplaySnapshot(*id, uniqueId);

                builder.setId(*id);

                return;

                return *id;

            }



            generator->releaseId(*id);
@@ -682,6 +682,7 @@ void SurfaceFlinger::acquireVirtualDisplay(ui::Size resolution, ui::PixelFormat 
    LOG_ALWAYS_FATAL_IF(!id, "Failed to generate ID for GPU virtual display");

    acquireVirtualDisplaySnapshot(*id, uniqueId);

    builder.setId(*id);

    return *id;

}



void SurfaceFlinger::releaseVirtualDisplay(VirtualDisplayId displayId) {
@@ -4007,9 +4008,11 @@ void SurfaceFlinger::processDisplayAdded(const wp<IBinder>& displayToken, 
    }



    compositionengine::DisplayCreationArgsBuilder builder;

    std::optional<VirtualDisplayIdVariant> virtualDisplayIdVariantOpt;

    if (const auto& physical = state.physical) {

        builder.setId(physical->id);

    } else {

        virtualDisplayIdVariantOpt =

                acquireVirtualDisplay(resolution, pixelFormat, state.uniqueId, builder);

    }
@@ -4030,10 +4033,10 @@ void SurfaceFlinger::processDisplayAdded(const wp<IBinder>& displayToken, 
    getFactory().createBufferQueue(&bqProducer, &bqConsumer, /*consumerIsSurfaceFlinger =*/false);



    if (state.isVirtual()) {

        const auto displayId = VirtualDisplayId::tryCast(compositionDisplay->getId());

        LOG_FATAL_IF(!displayId);

        auto surface = sp<VirtualDisplaySurface>::make(getHwComposer(), *displayId, state.surface,

                                                       bqProducer, bqConsumer, state.displayName);

        LOG_FATAL_IF(!virtualDisplayIdVariantOpt);

        auto surface = sp<VirtualDisplaySurface>::make(getHwComposer(), *virtualDisplayIdVariantOpt,

                                                       state.surface, bqProducer, bqConsumer,

                                                       state.displayName);

        displaySurface = surface;

        producer = std::move(surface);

    } else {

services/surfaceflinger/SurfaceFlinger.h

+4 −2
Original line number Diff line number Diff line
@@ -1126,8 +1126,10 @@ private: 
    void enableHalVirtualDisplays(bool);



    // Virtual display lifecycle for ID generation and HAL allocation.

    void acquireVirtualDisplay(ui::Size, ui::PixelFormat, const std::string& uniqueId,

    std::optional<VirtualDisplayIdVariant> acquireVirtualDisplay(

            ui::Size, ui::PixelFormat, const std::string& uniqueId,

            compositionengine::DisplayCreationArgsBuilder&) REQUIRES(mStateLock);



    template <typename ID>

    void acquireVirtualDisplaySnapshot(ID displayId, const std::string& uniqueId) {

        std::lock_guard lock(mVirtualDisplaysMutex);