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

Commit 127a2d9b authored by Kevin DuBois's avatar Kevin DuBois
Browse files

SF: VSyncPredictor correct calculation 

Correct a miscalculation in vsync predictions that led to
a missed callback.

Bug: 144707443
Fixes: 145667109
Test: boot to home, scroll shade and news
Test: 1 new unit test that forces the error condition:

Change-Id: Idef8018326f965fab42a450e8a954cb93fcad905
parent 75bed1e6

services/surfaceflinger/Scheduler/Timer.cpp

+1 −1
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ void Timer::alarmIn(std::function<void()> const& cb, nsecs_t fireIn) { 
    };



    if (timerfd_settime(mTimerFd, 0, &new_timer, &old_timer)) {

        ALOGW("Failed to set timerfd");

        ALOGW("Failed to set timerfd %s (%i)", strerror(errno), errno);

    }

}

services/surfaceflinger/Scheduler/VSyncPredictor.cpp

+17 −4
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ 
#include <utils/Trace.h>

#include <algorithm>

#include <chrono>

#include <sstream>

#include "SchedulerUtils.h"



namespace android::scheduler {
@@ -153,12 +154,24 @@ nsecs_t VSyncPredictor::nextAnticipatedVSyncTimeFrom(nsecs_t timePoint) const { 
    }



    auto const oldest = *std::min_element(timestamps.begin(), timestamps.end());

    auto const ordinalRequest = (timePoint - oldest + slope) / slope;



    // See b/145667109, the ordinal calculation must take into account the intercept.

    auto const zeroPoint = oldest + intercept;

    auto const ordinalRequest = (timePoint - zeroPoint + slope) / slope;

    auto const prediction = (ordinalRequest * slope) + intercept + oldest;



    ALOGV("prediction made from: %" PRId64 " prediction: %" PRId64 " (+%" PRId64 ") slope: %" PRId64

          " intercept: %" PRId64,

          timePoint, prediction, prediction - timePoint, slope, intercept);

    auto const printer = [&, slope = slope, intercept = intercept] {

        std::stringstream str;

        str << "prediction made from: " << timePoint << "prediction: " << prediction << " (+"

            << prediction - timePoint << ") slope: " << slope << " intercept: " << intercept

            << "oldestTS: " << oldest << " ordinal: " << ordinalRequest;

        return str.str();

    };



    ALOGV("%s", printer().c_str());

    LOG_ALWAYS_FATAL_IF(prediction < timePoint, "VSyncPredictor: model miscalculation: %s",

                        printer().c_str());



    return prediction;

}

services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp

+32 −0
Original line number Diff line number Diff line
@@ -319,4 +319,36 @@ TEST_F(VSyncPredictorTest, idealModelPredictionsBeforeRegressionModelIsBuilt) { 
    }

}



// See b/145667109, and comment in prod code under test.

TEST_F(VSyncPredictorTest, doesNotPredictBeforeTimePointWithHigherIntercept) {

    std::vector<nsecs_t> const simulatedVsyncs{

            158929578733000,

            158929306806205, // oldest TS in ringbuffer

            158929650879052,

            158929661969209,

            158929684198847,

            158929695268171,

            158929706370359,

    };

    auto const idealPeriod = 11111111;

    auto const expectedPeriod = 11113500;

    auto const expectedIntercept = -395335;



    tracker.setPeriod(idealPeriod);

    for (auto const& timestamp : simulatedVsyncs) {

        tracker.addVsyncTimestamp(timestamp);

    }



    auto [slope, intercept] = tracker.getVSyncPredictionModel();

    EXPECT_THAT(slope, IsCloseTo(expectedPeriod, mMaxRoundingError));

    EXPECT_THAT(intercept, IsCloseTo(expectedIntercept, mMaxRoundingError));



    // (timePoint - oldestTS) % expectedPeriod works out to be: 395334

    // (timePoint - oldestTS) / expectedPeriod works out to be: 38.96

    // so failure to account for the offset will floor the ordinal to 38, which was in the past.

    auto const timePoint = 158929728723871;

    auto const prediction = tracker.nextAnticipatedVSyncTimeFrom(timePoint);

    EXPECT_THAT(prediction, Ge(timePoint));

}



} // namespace android::scheduler