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

Commit ec7aa8a7 authored by Ady Abraham's avatar Ady Abraham
Browse files

SF: check previous present fence to avoid early presentation 

If SurfaceFlinger finishes composition a vsyncPeriod or more before it
planned to, it sleeps before calling composer to present the frame to
avoid early presentation. In this CL we are removing the sleep if the
previous present fence is still pending, as early presentation is not
possible (composer has already a frame queued).

Bug: 190842189
Test: Bouncy ball while forcing client composition
Change-Id: Id913a21b3489973d82498c6154843656972b6e1a
parent b3c68852

services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h

+5 −0
Original line number Original line Diff line number Diff line
@@ -24,6 +24,7 @@ 
#include <compositionengine/LayerFE.h>

#include <compositionengine/LayerFE.h>

#include <compositionengine/OutputColorSetting.h>

#include <compositionengine/OutputColorSetting.h>

#include <math/mat4.h>

#include <math/mat4.h>

#include <ui/FenceTime.h>

#include <ui/Transform.h>

#include <ui/Transform.h>





namespace android::compositionengine {

namespace android::compositionengine {
@@ -83,6 +84,10 @@ struct CompositionRefreshArgs { 
    // The earliest time to send the present command to the HAL

    // The earliest time to send the present command to the HAL

    std::chrono::steady_clock::time_point earliestPresentTime;

    std::chrono::steady_clock::time_point earliestPresentTime;





    // The previous present fence. Used together with earliestPresentTime

    // to prevent an early presentation of a frame.

    std::shared_ptr<FenceTime> previousPresentFence;



    // The predicted next invalidation time

    // The predicted next invalidation time

    std::optional<std::chrono::steady_clock::time_point> nextInvalidateTime;

    std::optional<std::chrono::steady_clock::time_point> nextInvalidateTime;

};

};

services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h

+5 −0
Original line number Original line Diff line number Diff line
@@ -19,6 +19,7 @@ 
#include <cstdint>

#include <cstdint>





#include <math/mat4.h>

#include <math/mat4.h>

#include <ui/FenceTime.h>





// TODO(b/129481165): remove the #pragma below and fix conversion issues

// TODO(b/129481165): remove the #pragma below and fix conversion issues

#pragma clang diagnostic push

#pragma clang diagnostic push
@@ -118,6 +119,10 @@ struct OutputCompositionState { 
    // The earliest time to send the present command to the HAL

    // The earliest time to send the present command to the HAL

    std::chrono::steady_clock::time_point earliestPresentTime;

    std::chrono::steady_clock::time_point earliestPresentTime;





    // The previous present fence. Used together with earliestPresentTime

    // to prevent an early presentation of a frame.

    std::shared_ptr<FenceTime> previousPresentFence;



    // Current display brightness

    // Current display brightness

    float displayBrightnessNits{-1.f};

    float displayBrightnessNits{-1.f};

services/surfaceflinger/CompositionEngine/src/Display.cpp

+4 −2
Original line number Original line Diff line number Diff line
@@ -229,7 +229,8 @@ void Display::chooseCompositionStrategy() { 
    auto& hwc = getCompositionEngine().getHwComposer();

    auto& hwc = getCompositionEngine().getHwComposer();

    if (status_t result =

    if (status_t result =

                hwc.getDeviceCompositionChanges(*halDisplayId, anyLayersRequireClientComposition(),

                hwc.getDeviceCompositionChanges(*halDisplayId, anyLayersRequireClientComposition(),

                                                getState().earliestPresentTime, &changes);

                                                getState().earliestPresentTime,

                                                getState().previousPresentFence, &changes);

        result != NO_ERROR) {

        result != NO_ERROR) {

        ALOGE("chooseCompositionStrategy failed for %s: %d (%s)", getName().c_str(), result,

        ALOGE("chooseCompositionStrategy failed for %s: %d (%s)", getName().c_str(), result,

              strerror(-result));

              strerror(-result));
@@ -330,7 +331,8 @@ compositionengine::Output::FrameFences Display::presentAndGetFrameFences() { 
    }

    }





    auto& hwc = getCompositionEngine().getHwComposer();

    auto& hwc = getCompositionEngine().getHwComposer();

    hwc.presentAndGetReleaseFences(*halDisplayIdOpt, getState().earliestPresentTime);

    hwc.presentAndGetReleaseFences(*halDisplayIdOpt, getState().earliestPresentTime,

                                   getState().previousPresentFence);





    fences.presentFence = hwc.getPresentFence(*halDisplayIdOpt);

    fences.presentFence = hwc.getPresentFence(*halDisplayIdOpt);

services/surfaceflinger/CompositionEngine/src/Output.cpp

+1 −0
Original line number Original line Diff line number Diff line
@@ -729,6 +729,7 @@ void Output::writeCompositionState(const compositionengine::CompositionRefreshAr 
    }

    }





    editState().earliestPresentTime = refreshArgs.earliestPresentTime;

    editState().earliestPresentTime = refreshArgs.earliestPresentTime;

    editState().previousPresentFence = refreshArgs.previousPresentFence;





    compositionengine::OutputLayer* peekThroughLayer = nullptr;

    compositionengine::OutputLayer* peekThroughLayer = nullptr;

    sp<GraphicBuffer> previousOverride = nullptr;

    sp<GraphicBuffer> previousOverride = nullptr;

services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp

+6 −6
Original line number Original line Diff line number Diff line
@@ -579,7 +579,7 @@ TEST_F(DisplayChooseCompositionStrategyTest, takesEarlyOutIfGpuDisplay) { 
TEST_F(DisplayChooseCompositionStrategyTest, takesEarlyOutOnHwcError) {

TEST_F(DisplayChooseCompositionStrategyTest, takesEarlyOutOnHwcError) {

    EXPECT_CALL(*mDisplay, anyLayersRequireClientComposition()).WillOnce(Return(false));

    EXPECT_CALL(*mDisplay, anyLayersRequireClientComposition()).WillOnce(Return(false));

    EXPECT_CALL(mHwComposer,

    EXPECT_CALL(mHwComposer,

                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), false, _, _))

                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), false, _, _, _))

            .WillOnce(Return(INVALID_OPERATION));

            .WillOnce(Return(INVALID_OPERATION));





    mDisplay->chooseCompositionStrategy();

    mDisplay->chooseCompositionStrategy();
@@ -602,7 +602,7 @@ TEST_F(DisplayChooseCompositionStrategyTest, normalOperation) { 
            .WillOnce(Return(false));

            .WillOnce(Return(false));





    EXPECT_CALL(mHwComposer,

    EXPECT_CALL(mHwComposer,

                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _))

                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _))

            .WillOnce(Return(NO_ERROR));

            .WillOnce(Return(NO_ERROR));

    EXPECT_CALL(*mDisplay, allLayersRequireClientComposition()).WillOnce(Return(false));

    EXPECT_CALL(*mDisplay, allLayersRequireClientComposition()).WillOnce(Return(false));
@@ -633,8 +633,8 @@ TEST_F(DisplayChooseCompositionStrategyTest, normalOperationWithChanges) { 
            .WillOnce(Return(false));

            .WillOnce(Return(false));





    EXPECT_CALL(mHwComposer,

    EXPECT_CALL(mHwComposer,

                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _))

                getDeviceCompositionChanges(HalDisplayId(DEFAULT_DISPLAY_ID), true, _, _, _))

            .WillOnce(DoAll(SetArgPointee<3>(changes), Return(NO_ERROR)));

            .WillOnce(DoAll(SetArgPointee<4>(changes), Return(NO_ERROR)));

    EXPECT_CALL(*mDisplay, applyChangedTypesToLayers(changes.changedTypes)).Times(1);

    EXPECT_CALL(*mDisplay, applyChangedTypesToLayers(changes.changedTypes)).Times(1);

    EXPECT_CALL(*mDisplay, applyDisplayRequests(changes.displayRequests)).Times(1);

    EXPECT_CALL(*mDisplay, applyDisplayRequests(changes.displayRequests)).Times(1);

    EXPECT_CALL(*mDisplay, applyLayerRequestsToLayers(changes.layerRequests)).Times(1);

    EXPECT_CALL(*mDisplay, applyLayerRequestsToLayers(changes.layerRequests)).Times(1);
@@ -844,7 +844,7 @@ TEST_F(DisplayPresentAndGetFrameFencesTest, returnsPresentAndLayerFences) { 
    sp<Fence> layer1Fence = new Fence();

    sp<Fence> layer1Fence = new Fence();

    sp<Fence> layer2Fence = new Fence();

    sp<Fence> layer2Fence = new Fence();





    EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(HalDisplayId(DEFAULT_DISPLAY_ID), _))

    EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(HalDisplayId(DEFAULT_DISPLAY_ID), _, _))

            .Times(1);

            .Times(1);

    EXPECT_CALL(mHwComposer, getPresentFence(HalDisplayId(DEFAULT_DISPLAY_ID)))

    EXPECT_CALL(mHwComposer, getPresentFence(HalDisplayId(DEFAULT_DISPLAY_ID)))

            .WillOnce(Return(presentFence));

            .WillOnce(Return(presentFence));
@@ -1020,7 +1020,7 @@ TEST_F(DisplayFunctionalTest, postFramebufferCriticalCallsAreOrdered) { 




    mDisplay->editState().isEnabled = true;

    mDisplay->editState().isEnabled = true;





    EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(_, _));

    EXPECT_CALL(mHwComposer, presentAndGetReleaseFences(_, _, _));

    EXPECT_CALL(*mDisplaySurface, onFrameCommitted());

    EXPECT_CALL(*mDisplaySurface, onFrameCommitted());





    mDisplay->postFramebuffer();

    mDisplay->postFramebuffer();