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

Commit 0449b0fa authored by Lloyd Pique's avatar Lloyd Pique
Browse files

Revert "SurfaceFlinger: protect state members in Layer" 

State update transactions must be atomic. The fine-grained lock on each
Layer implied otherwise. Revert the fine grained lock as being
unhelpful.

Unfortunately there does not seem to be a way to use Clang thread
annotations to specify the desired locking behavior.

Note that the parent CL addresses the locking problem that led to the
bug.

This reverts commit 83729883.

Bug: 119481871
Test: SurfaceFlinger unit tests
Test: go/wm-smoke
Change-Id: I361741f8d10102aeb57f164c847c6063ff93dd14
parent f2c79393

services/surfaceflinger/BufferLayer.cpp

+7 −14
Original line number Diff line number Diff line
@@ -396,7 +396,6 @@ Region BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime 
    }



    // Capture the old state of the layer for comparisons later

    Mutex::Autolock lock(mStateMutex);

    const State& s(getDrawingState());

    const bool oldOpacity = isOpaque(s);

    sp<GraphicBuffer> oldBuffer = mActiveBuffer;
@@ -503,7 +502,7 @@ Region BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime 


    // FIXME: postedRegion should be dirty & bounds

    // transform the dirty region to window-manager space

    return getTransformLocked().transform(Region(getBufferSize(s)));

    return getTransform().transform(Region(getBufferSize(s)));

}



// transaction
@@ -551,7 +550,7 @@ bool BufferLayer::latchUnsignaledBuffers() { 


// h/w composer set-up

bool BufferLayer::allTransactionsSignaled() {

    auto headFrameNumber = getHeadFrameNumberLocked();

    auto headFrameNumber = getHeadFrameNumber();

    bool matchingFramesFound = false;

    bool allTransactionsApplied = true;

    Mutex::Autolock lock(mLocalSyncPointMutex);
@@ -604,7 +603,6 @@ bool BufferLayer::needsFiltering(const RenderArea& renderArea) const { 


void BufferLayer::drawWithOpenGL(const RenderArea& renderArea, bool useIdentityTransform) const {

    ATRACE_CALL();

    Mutex::Autolock lock(mStateMutex);

    const State& s(getDrawingState());



    computeGeometry(renderArea, getBE().mMesh, useIdentityTransform);
@@ -623,9 +621,9 @@ void BufferLayer::drawWithOpenGL(const RenderArea& renderArea, bool useIdentityT 
     * minimal value)? Or, we could make GL behave like HWC -- but this feel

     * like more of a hack.

     */

    const Rect bounds{computeBoundsLocked()}; // Rounds from FloatRect

    const Rect bounds{computeBounds()}; // Rounds from FloatRect



    ui::Transform t = getTransformLocked();

    ui::Transform t = getTransform();

    Rect win = bounds;

    const int bufferWidth = getBufferSize(s).getWidth();

    const int bufferHeight = getBufferSize(s).getHeight();
@@ -644,7 +642,7 @@ void BufferLayer::drawWithOpenGL(const RenderArea& renderArea, bool useIdentityT 
    texCoords[2] = vec2(right, 1.0f - bottom);

    texCoords[3] = vec2(right, 1.0f - top);



    const auto roundedCornerState = getRoundedCornerStateLocked();

    const auto roundedCornerState = getRoundedCornerState();

    const auto cropRect = roundedCornerState.cropRect;

    setupRoundedCornersCropCoordinates(win, cropRect);
@@ -666,12 +664,7 @@ void BufferLayer::drawWithOpenGL(const RenderArea& renderArea, bool useIdentityT 
}



uint64_t BufferLayer::getHeadFrameNumber() const {

    Mutex::Autolock lock(mStateMutex);

    return getHeadFrameNumberLocked();

}



uint64_t BufferLayer::getHeadFrameNumberLocked() const {

    if (hasFrameUpdateLocked()) {

    if (hasFrameUpdate()) {

        return getFrameNumber();

    } else {

        return mCurrentFrameNumber;
@@ -698,7 +691,7 @@ Rect BufferLayer::getBufferSize(const State& s) const { 
        std::swap(bufWidth, bufHeight);

    }



    if (getTransformToDisplayInverseLocked()) {

    if (getTransformToDisplayInverse()) {

        uint32_t invTransform = DisplayDevice::getPrimaryDisplayOrientationTransform();

        if (invTransform & ui::Transform::ROT_90) {

            std::swap(bufWidth, bufHeight);

services/surfaceflinger/BufferLayer.h

+22 −36
Original line number Diff line number Diff line
@@ -69,7 +69,7 @@ public: 
    bool isOpaque(const Layer::State& s) const override;



    // isVisible - true if this layer is visible, false otherwise

    bool isVisible() const override EXCLUDES(mStateMutex);

    bool isVisible() const override;



    // isProtected - true if the layer may contain protected content in the

    // GRALLOC_USAGE_PROTECTED sense.
@@ -91,7 +91,7 @@ public: 
    bool onPostComposition(const std::optional<DisplayId>& displayId,

                           const std::shared_ptr<FenceTime>& glDoneFence,

                           const std::shared_ptr<FenceTime>& presentFence,

                           const CompositorTiming& compositorTiming) override EXCLUDES(mStateMutex);

                           const CompositorTiming& compositorTiming) override;



    // latchBuffer - called each time the screen is redrawn and returns whether

    // the visible regions need to be recomputed (this is a fairly heavy
@@ -101,13 +101,13 @@ public: 
    // releaseFence will be populated with a native fence that fires when

    // composition has completed.

    Region latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime,

                       const sp<Fence>& releaseFence) override EXCLUDES(mStateMutex);

                       const sp<Fence>& releaseFence) override;



    bool isBufferLatched() const override { return mRefreshPending; }



    void notifyAvailableFrames() override;



    bool hasReadyFrame() const override EXCLUDES(mStateMutex);

    bool hasReadyFrame() const override;



    // Returns the current scaling mode, unless mOverrideScalingMode

    // is set, in which case, it returns mOverrideScalingMode
@@ -118,24 +118,19 @@ public: 
    // Functions that must be implemented by derived classes

    // -----------------------------------------------------------------------

private:

    virtual bool fenceHasSignaled() const EXCLUDES(mStateMutex) = 0;

    virtual bool fenceHasSignaled() const = 0;



    virtual nsecs_t getDesiredPresentTime() = 0;

    std::shared_ptr<FenceTime> getCurrentFenceTime() const EXCLUDES(mStateMutex) {

        Mutex::Autolock lock(mStateMutex);

        return getCurrentFenceTimeLocked();

    }



    virtual std::shared_ptr<FenceTime> getCurrentFenceTimeLocked() const REQUIRES(mStateMutex) = 0;

    virtual std::shared_ptr<FenceTime> getCurrentFenceTime() const = 0;



    virtual void getDrawingTransformMatrix(float *matrix) = 0;

    virtual uint32_t getDrawingTransform() const REQUIRES(mStateMutex) = 0;

    virtual ui::Dataspace getDrawingDataSpace() const REQUIRES(mStateMutex) = 0;

    virtual Rect getDrawingCrop() const REQUIRES(mStateMutex) = 0;

    virtual uint32_t getDrawingTransform() const = 0;

    virtual ui::Dataspace getDrawingDataSpace() const = 0;

    virtual Rect getDrawingCrop() const = 0;

    virtual uint32_t getDrawingScalingMode() const = 0;

    virtual Region getDrawingSurfaceDamage() const EXCLUDES(mStateMutex) = 0;

    virtual const HdrMetadata& getDrawingHdrMetadata() const EXCLUDES(mStateMutex) = 0;

    virtual int getDrawingApi() const EXCLUDES(mStateMutex) = 0;

    virtual Region getDrawingSurfaceDamage() const = 0;

    virtual const HdrMetadata& getDrawingHdrMetadata() const = 0;

    virtual int getDrawingApi() const = 0;

    virtual PixelFormat getPixelFormat() const = 0;



    virtual uint64_t getFrameNumber() const = 0;
@@ -143,21 +138,20 @@ private: 
    virtual bool getAutoRefresh() const = 0;

    virtual bool getSidebandStreamChanged() const = 0;



    virtual std::optional<Region> latchSidebandStream(bool& recomputeVisibleRegions)

            EXCLUDES(mStateMutex) = 0;

    virtual std::optional<Region> latchSidebandStream(bool& recomputeVisibleRegions) = 0;



    virtual bool hasFrameUpdateLocked() const REQUIRES(mStateMutex) = 0;

    virtual bool hasFrameUpdate() const = 0;



    virtual void setFilteringEnabled(bool enabled) = 0;



    virtual status_t bindTextureImage() EXCLUDES(mStateMutex) = 0;

    virtual status_t bindTextureImage() = 0;

    virtual status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,

                                    const sp<Fence>& flushFence) REQUIRES(mStateMutex) = 0;

                                    const sp<Fence>& flushFence) = 0;



    virtual status_t updateActiveBuffer() REQUIRES(mStateMutex) = 0;

    virtual status_t updateActiveBuffer() = 0;

    virtual status_t updateFrameNumber(nsecs_t latchTime) = 0;



    virtual void setHwcLayerBuffer(DisplayId displayId) EXCLUDES(mStateMutex) = 0;

    virtual void setHwcLayerBuffer(DisplayId displayId) = 0;



protected:

    // Loads the corresponding system property once per process
@@ -166,15 +160,10 @@ protected: 
    // Check all of the local sync points to ensure that all transactions

    // which need to have been applied prior to the frame which is about to

    // be latched have signaled

    bool allTransactionsSignaled() REQUIRES(mStateMutex);

    bool allTransactionsSignaled();



    static bool getOpacityForFormat(uint32_t format);



    bool hasFrameUpdate() const EXCLUDES(mStateMutex) {

        Mutex::Autolock lock(mStateMutex);

        return hasFrameUpdateLocked();

    }



    // from GLES

    const uint32_t mTextureName;
@@ -183,12 +172,9 @@ private: 
    bool needsFiltering(const RenderArea& renderArea) const;



    // drawing

    void drawWithOpenGL(const RenderArea& renderArea, bool useIdentityTransform) const

            EXCLUDES(mStateMutex);



    uint64_t getHeadFrameNumber() const EXCLUDES(mStateMutex);

    void drawWithOpenGL(const RenderArea& renderArea, bool useIdentityTransform) const;



    uint64_t getHeadFrameNumberLocked() const REQUIRES(mStateMutex);

    uint64_t getHeadFrameNumber() const;



    uint32_t mCurrentScalingMode{NATIVE_WINDOW_SCALING_MODE_FREEZE};
@@ -200,7 +186,7 @@ private: 


    bool mRefreshPending{false};



    Rect getBufferSize(const State& s) const override REQUIRES(mStateMutex);

    Rect getBufferSize(const State& s) const override;

};



} // namespace android

services/surfaceflinger/BufferQueueLayer.cpp

+10 −13
Original line number Diff line number Diff line
@@ -51,7 +51,7 @@ std::vector<OccupancyTracker::Segment> BufferQueueLayer::getOccupancyHistory(boo 
    return history;

}



bool BufferQueueLayer::getTransformToDisplayInverseLocked() const {

bool BufferQueueLayer::getTransformToDisplayInverse() const {

    return mConsumer->getTransformToDisplayInverse();

}
@@ -131,7 +131,7 @@ nsecs_t BufferQueueLayer::getDesiredPresentTime() { 
    return mConsumer->getTimestamp();

}



std::shared_ptr<FenceTime> BufferQueueLayer::getCurrentFenceTimeLocked() const {

std::shared_ptr<FenceTime> BufferQueueLayer::getCurrentFenceTime() const {

    return mConsumer->getCurrentFenceTime();

}
@@ -192,7 +192,6 @@ bool BufferQueueLayer::getSidebandStreamChanged() const { 


std::optional<Region> BufferQueueLayer::latchSidebandStream(bool& recomputeVisibleRegions) {

    bool sidebandStreamChanged = true;

    Mutex::Autolock lock(mStateMutex);

    if (mSidebandStreamChanged.compare_exchange_strong(sidebandStreamChanged, false)) {

        // mSidebandStreamChanged was changed to false

        // replicated in LayerBE until FE/BE is ready to be synchronized
@@ -201,15 +200,15 @@ std::optional<Region> BufferQueueLayer::latchSidebandStream(bool& recomputeVisib 
            setTransactionFlags(eTransactionNeeded);

            mFlinger->setTransactionFlags(eTraversalNeeded);

        }



        recomputeVisibleRegions = true;



        const State& s(getDrawingState());

        return getTransformLocked().transform(Region(Rect(s.active_legacy.w, s.active_legacy.h)));

        return getTransform().transform(Region(Rect(s.active_legacy.w, s.active_legacy.h)));

    }

    return {};

}



bool BufferQueueLayer::hasFrameUpdateLocked() const {

bool BufferQueueLayer::hasFrameUpdate() const {

    return mQueuedFrames > 0;

}
@@ -229,18 +228,16 @@ status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t 
    // buffer mode.

    bool queuedBuffer = false;

    const int32_t layerID = getSequence();

    status_t updateResult;

    LayerRejecter r(mState.drawing, getCurrentState(), recomputeVisibleRegions,

    LayerRejecter r(mDrawingState, getCurrentState(), recomputeVisibleRegions,

                    getProducerStickyTransform() != 0, mName.string(), mOverrideScalingMode,

                    getTransformToDisplayInverseLocked(), mFreezeGeometryUpdates);

                    getTransformToDisplayInverse(), mFreezeGeometryUpdates);



    const nsecs_t expectedPresentTime = mFlinger->mUseScheduler

            ? mFlinger->mScheduler->mPrimaryDispSync->expectedPresentTime()

            : mFlinger->mPrimaryDispSync->expectedPresentTime();



    updateResult = mConsumer->updateTexImage(&r, expectedPresentTime, &mAutoRefresh, &queuedBuffer,

    status_t updateResult =

            mConsumer->updateTexImage(&r, expectedPresentTime, &mAutoRefresh, &queuedBuffer,

                                      mLastFrameNumberReceived, releaseFence);



    if (updateResult == BufferQueue::PRESENT_LATER) {

        // Producer doesn't want buffer to be displayed yet.  Signal a

        // layer update so we check again at the next opportunity.

services/surfaceflinger/BufferQueueLayer.h

+9 −10
Original line number Diff line number Diff line
@@ -44,7 +44,7 @@ public: 


    std::vector<OccupancyTracker::Segment> getOccupancyHistory(bool forceFlush) override;



    bool getTransformToDisplayInverseLocked() const override REQUIRES(mStateMutex);

    bool getTransformToDisplayInverse() const override;



    // If a buffer was replaced this frame, release the former buffer

    void releasePendingBuffer(nsecs_t dequeueReadyTime) override;
@@ -64,12 +64,12 @@ public: 


private:

    nsecs_t getDesiredPresentTime() override;

    std::shared_ptr<FenceTime> getCurrentFenceTimeLocked() const override REQUIRES(mStateMutex);

    std::shared_ptr<FenceTime> getCurrentFenceTime() const override;



    void getDrawingTransformMatrix(float *matrix) override;

    uint32_t getDrawingTransform() const override REQUIRES(mStateMutex);

    ui::Dataspace getDrawingDataSpace() const override REQUIRES(mStateMutex);

    Rect getDrawingCrop() const override REQUIRES(mStateMutex);

    uint32_t getDrawingTransform() const override;

    ui::Dataspace getDrawingDataSpace() const override;

    Rect getDrawingCrop() const override;

    uint32_t getDrawingScalingMode() const override;

    Region getDrawingSurfaceDamage() const override;

    const HdrMetadata& getDrawingHdrMetadata() const override;
@@ -81,18 +81,17 @@ private: 
    bool getAutoRefresh() const override;

    bool getSidebandStreamChanged() const override;



    std::optional<Region> latchSidebandStream(bool& recomputeVisibleRegions) override

            EXCLUDES(mStateMutex);

    std::optional<Region> latchSidebandStream(bool& recomputeVisibleRegions) override;



    bool hasFrameUpdateLocked() const override REQUIRES(mStateMutex);

    bool hasFrameUpdate() const override;



    void setFilteringEnabled(bool enabled) override;



    status_t bindTextureImage() override;

    status_t updateTexImage(bool& recomputeVisibleRegions, nsecs_t latchTime,

                            const sp<Fence>& releaseFence) override REQUIRES(mStateMutex);

                            const sp<Fence>& releaseFence) override;



    status_t updateActiveBuffer() override REQUIRES(mStateMutex);

    status_t updateActiveBuffer() override;

    status_t updateFrameNumber(nsecs_t latchTime) override;



    void setHwcLayerBuffer(DisplayId displayId) override;