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

Commit 959a7ff7 authored by Leon Scroggins III's avatar Leon Scroggins III
Browse files

Make onVsync return a display id 

The caller of this method then turns around and asks HWComposer for the
display id, so we might as well return the id in the first place. Use an
optional so it can return the equivalent of false in the old version.

Use ftl::Concat to save memory allocations.

Factored out from If60218e85292c786b9fa70ecb33ee374d3a385e0 after it was
reverted.

Bug: 255601557
Bug: 256196556
Test: atest libsurfaceflinger_unittest:HWComposerTest
Change-Id: I21be76e20776d8a3f49e5bd295a0042de9e2dde9
parent db16a2be

services/surfaceflinger/CompositionEngine/tests/MockHWComposer.h

+1 −1
Original line number Diff line number Diff line
@@ -93,7 +93,7 @@ public: 
    MOCK_METHOD2(onHotplug,

                 std::optional<DisplayIdentificationInfo>(hal::HWDisplayId, hal::Connection));

    MOCK_CONST_METHOD0(updatesDeviceProductInfoOnHotplugReconnect, bool());

    MOCK_METHOD2(onVsync, bool(hal::HWDisplayId, int64_t));

    MOCK_METHOD(std::optional<PhysicalDisplayId>, onVsync, (hal::HWDisplayId, int64_t));

    MOCK_METHOD2(setVsyncEnabled, void(PhysicalDisplayId, hal::Vsync));

    MOCK_CONST_METHOD1(isConnected, bool(PhysicalDisplayId));

    MOCK_CONST_METHOD1(getModes, std::vector<HWComposer::HWCDisplayMode>(PhysicalDisplayId));

services/surfaceflinger/DisplayHardware/HWComposer.cpp

+15 −13
Original line number Diff line number Diff line
@@ -30,6 +30,7 @@ 
#include <compositionengine/Output.h>

#include <compositionengine/OutputLayer.h>

#include <compositionengine/impl/OutputLayerCompositionState.h>

#include <ftl/concat.h>

#include <log/log.h>

#include <ui/DebugUtils.h>

#include <ui/GraphicBuffer.h>
@@ -148,16 +149,17 @@ bool HWComposer::updatesDeviceProductInfoOnHotplugReconnect() const { 
    return mUpdateDeviceProductInfoOnHotplugReconnect;

}



bool HWComposer::onVsync(hal::HWDisplayId hwcDisplayId, nsecs_t timestamp) {

    const auto displayId = toPhysicalDisplayId(hwcDisplayId);

    if (!displayId) {

std::optional<PhysicalDisplayId> HWComposer::onVsync(hal::HWDisplayId hwcDisplayId,

                                                     nsecs_t timestamp) {

    const auto displayIdOpt = toPhysicalDisplayId(hwcDisplayId);

    if (!displayIdOpt) {

        LOG_HWC_DISPLAY_ERROR(hwcDisplayId, "Invalid HWC display");

        return false;

        return {};

    }



    RETURN_IF_INVALID_DISPLAY(*displayId, false);

    RETURN_IF_INVALID_DISPLAY(*displayIdOpt, {});



    auto& displayData = mDisplayData[*displayId];

    auto& displayData = mDisplayData[*displayIdOpt];



    {

        // There have been reports of HWCs that signal several vsync events
@@ -166,18 +168,18 @@ bool HWComposer::onVsync(hal::HWDisplayId hwcDisplayId, nsecs_t timestamp) { 
        // out here so they don't cause havoc downstream.

        if (timestamp == displayData.lastPresentTimestamp) {

            ALOGW("Ignoring duplicate VSYNC event from HWC for display %s (t=%" PRId64 ")",

                  to_string(*displayId).c_str(), timestamp);

            return false;

                  to_string(*displayIdOpt).c_str(), timestamp);

            return {};

        }



        displayData.lastPresentTimestamp = timestamp;

    }



    const auto tag = "HW_VSYNC_" + to_string(*displayId);

    ATRACE_INT(tag.c_str(), displayData.vsyncTraceToggle);

    ATRACE_INT(ftl::Concat("HW_VSYNC_", displayIdOpt->value).c_str(),

               displayData.vsyncTraceToggle);

    displayData.vsyncTraceToggle = !displayData.vsyncTraceToggle;



    return true;

    return displayIdOpt;

}



size_t HWComposer::getMaxVirtualDisplayCount() const {
@@ -375,8 +377,8 @@ void HWComposer::setVsyncEnabled(PhysicalDisplayId displayId, hal::Vsync enabled 


    displayData.vsyncEnabled = enabled;



    const auto tag = "HW_VSYNC_ON_" + to_string(displayId);

    ATRACE_INT(tag.c_str(), enabled == hal::Vsync::ENABLE ? 1 : 0);

    ATRACE_INT(ftl::Concat("HW_VSYNC_ON_", displayId.value).c_str(),

               enabled == hal::Vsync::ENABLE ? 1 : 0);

}



status_t HWComposer::setClientTarget(HalDisplayId displayId, uint32_t slot,

services/surfaceflinger/DisplayHardware/HWComposer.h

+5 −2
Original line number Diff line number Diff line
@@ -221,7 +221,10 @@ public: 
    // TODO(b/157555476): Remove when the framework has proper support for headless mode

    virtual bool updatesDeviceProductInfoOnHotplugReconnect() const = 0;



    virtual bool onVsync(hal::HWDisplayId, nsecs_t timestamp) = 0;

    // Called when a vsync happens. If the vsync is valid, returns the

    // corresponding PhysicalDisplayId. Otherwise returns nullopt.

    virtual std::optional<PhysicalDisplayId> onVsync(hal::HWDisplayId, nsecs_t timestamp) = 0;



    virtual void setVsyncEnabled(PhysicalDisplayId, hal::Vsync enabled) = 0;



    virtual bool isConnected(PhysicalDisplayId) const = 0;
@@ -402,7 +405,7 @@ public: 


    bool updatesDeviceProductInfoOnHotplugReconnect() const override;



    bool onVsync(hal::HWDisplayId, nsecs_t timestamp) override;

    std::optional<PhysicalDisplayId> onVsync(hal::HWDisplayId, nsecs_t timestamp) override;

    void setVsyncEnabled(PhysicalDisplayId, hal::Vsync enabled) override;



    bool isConnected(PhysicalDisplayId) const override;

services/surfaceflinger/SurfaceFlinger.cpp

+3 −7
Original line number Diff line number Diff line
@@ -2031,13 +2031,9 @@ void SurfaceFlinger::onComposerHalVsync(hal::HWDisplayId hwcDisplayId, int64_t t 


    Mutex::Autolock lock(mStateLock);



    if (!getHwComposer().onVsync(hwcDisplayId, timestamp)) {

        return;

    }



    if (const auto displayId = getHwComposer().toPhysicalDisplayId(hwcDisplayId);

        displayId != mActiveDisplayId) {

        // For now, we don't do anything with non active display vsyncs.

    if (const auto displayIdOpt = getHwComposer().onVsync(hwcDisplayId, timestamp);

        displayIdOpt != mActiveDisplayId) {

        // Ignore VSYNC for invalid/inactive displays.

        return;

    }

services/surfaceflinger/tests/unittests/HWComposerTest.cpp

+28 −0
Original line number Diff line number Diff line
@@ -118,6 +118,34 @@ TEST_F(HWComposerTest, getActiveMode) { 
    }

}



TEST_F(HWComposerTest, onVsync) {

    constexpr hal::HWDisplayId kHwcDisplayId = 1;

    expectHotplugConnect(kHwcDisplayId);



    const auto info = mHwc.onHotplug(kHwcDisplayId, hal::Connection::CONNECTED);

    ASSERT_TRUE(info);



    const auto physicalDisplayId = info->id;



    // Deliberately chosen not to match DisplayData.lastPresentTimestamp's

    // initial value.

    constexpr nsecs_t kTimestamp = 1;

    auto displayIdOpt = mHwc.onVsync(kHwcDisplayId, kTimestamp);

    ASSERT_TRUE(displayIdOpt);

    EXPECT_EQ(physicalDisplayId, displayIdOpt);



    // Attempt to send the same time stamp again.

    displayIdOpt = mHwc.onVsync(kHwcDisplayId, kTimestamp);

    EXPECT_FALSE(displayIdOpt);

}



TEST_F(HWComposerTest, onVsyncInvalid) {

    constexpr hal::HWDisplayId kInvalidHwcDisplayId = 2;

    constexpr nsecs_t kTimestamp = 1;

    const auto displayIdOpt = mHwc.onVsync(kInvalidHwcDisplayId, kTimestamp);

    EXPECT_FALSE(displayIdOpt);

}



struct MockHWC2ComposerCallback final : StrictMock<HWC2::ComposerCallback> {

    MOCK_METHOD2(onComposerHalHotplug, void(hal::HWDisplayId, hal::Connection));

    MOCK_METHOD1(onComposerHalRefresh, void(hal::HWDisplayId));