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

Commit bc9e5290 authored by Pablo Gamito's avatar Pablo Gamito
Browse files

Pass surface control id to callback to accurately identify surface metrics belongs to

Avoid getting the wrong frame info when duplicate frame numbers are found in the ring buffer. Will ensure there isn't a mismatch in the metrics data reported.

Test: Existing tests
Bug: 197515602
Change-Id: Iff9ba01f575f94e5a9872ee48c0dd1e5067880c3
parent 88660d77
Loading
Loading
Loading
Loading
+19 −17
Original line number Diff line number Diff line
@@ -203,9 +203,10 @@ void CanvasContext::setSurfaceControl(ASurfaceControl* surfaceControl) {
    mSurfaceControl = surfaceControl;
    mSurfaceControlGenerationId++;
    mExpectSurfaceStats = surfaceControl != nullptr;
    if (mSurfaceControl != nullptr) {
    if (mExpectSurfaceStats) {
        funcs.acquireFunc(mSurfaceControl);
        funcs.registerListenerFunc(surfaceControl, this, &onSurfaceStatsAvailable);
        funcs.registerListenerFunc(surfaceControl, mSurfaceControlGenerationId, this,
                                   &onSurfaceStatsAvailable);
    }
}

@@ -613,11 +614,13 @@ nsecs_t CanvasContext::draw() {
    if (requireSwap) {
        if (mExpectSurfaceStats) {
            reportMetricsWithPresentTime();
            {  // acquire lock
                std::lock_guard lock(mLast4FrameMetricsInfosMutex);
                FrameMetricsInfo& next = mLast4FrameMetricsInfos.next();
                next.frameInfo = mCurrentFrameInfo;
                next.frameNumber = frameCompleteNr;
                next.surfaceId = mSurfaceControlGenerationId;
            }  // release lock
        } else {
            mCurrentFrameInfo->markFrameCompleted();
            mCurrentFrameInfo->set(FrameInfoIndex::GpuCompleted)
@@ -716,19 +719,20 @@ void CanvasContext::removeFrameMetricsObserver(FrameMetricsObserver* observer) {
    }
}

CanvasContext::FrameMetricsInfo CanvasContext::getFrameMetricsInfoFromLast4(uint64_t frameNumber) {
FrameInfo* CanvasContext::getFrameInfoFromLast4(uint64_t frameNumber, uint32_t surfaceControlId) {
    std::scoped_lock lock(mLast4FrameMetricsInfosMutex);
    for (size_t i = 0; i < mLast4FrameMetricsInfos.size(); i++) {
        if (mLast4FrameMetricsInfos[i].frameNumber == frameNumber) {
            return mLast4FrameMetricsInfos[i];
        if (mLast4FrameMetricsInfos[i].frameNumber == frameNumber &&
            mLast4FrameMetricsInfos[i].surfaceId == surfaceControlId) {
            return mLast4FrameMetricsInfos[i].frameInfo;
        }
    }

    return {};
    return nullptr;
}

void CanvasContext::onSurfaceStatsAvailable(void* context, ASurfaceControl* control,
            ASurfaceControlStats* stats) {
                                            int32_t surfaceControlId, ASurfaceControlStats* stats) {
    auto* instance = static_cast<CanvasContext*>(context);

    const ASurfaceControlFunctions& functions =
@@ -737,17 +741,15 @@ void CanvasContext::onSurfaceStatsAvailable(void* context, ASurfaceControl* cont
    nsecs_t gpuCompleteTime = functions.getAcquireTimeFunc(stats);
    uint64_t frameNumber = functions.getFrameNumberFunc(stats);

    FrameMetricsInfo frameMetricsInfo = instance->getFrameMetricsInfoFromLast4(frameNumber);
    FrameInfo* frameInfo = instance->getFrameInfoFromLast4(frameNumber, surfaceControlId);

    FrameInfo* frameInfo = frameMetricsInfo.frameInfo;
    if (frameInfo != nullptr) {
        frameInfo->set(FrameInfoIndex::FrameCompleted) = std::max(gpuCompleteTime,
                frameInfo->get(FrameInfoIndex::SwapBuffersCompleted));
        frameInfo->set(FrameInfoIndex::GpuCompleted) = gpuCompleteTime;
        std::scoped_lock lock(instance->mFrameMetricsReporterMutex);
        instance->mJankTracker.finishFrame(*frameInfo, instance->mFrameMetricsReporter,
                                           frameMetricsInfo.frameNumber,
                                           frameMetricsInfo.surfaceId);
        instance->mJankTracker.finishFrame(*frameInfo, instance->mFrameMetricsReporter, frameNumber,
                                           surfaceControlId);
    }
}

+3 −2
Original line number Diff line number Diff line
@@ -197,7 +197,7 @@ public:

    // Called when SurfaceStats are available.
    static void onSurfaceStatsAvailable(void* context, ASurfaceControl* control,
            ASurfaceControlStats* stats);
                                        int32_t surfaceControlId, ASurfaceControlStats* stats);

    void setASurfaceTransactionCallback(
            const std::function<bool(int64_t, int64_t, int64_t)>& callback) {
@@ -244,7 +244,7 @@ private:
        int32_t surfaceId;
    };

    CanvasContext::FrameMetricsInfo getFrameMetricsInfoFromLast4(uint64_t frameNumber);
    FrameInfo* getFrameInfoFromLast4(uint64_t frameNumber, uint32_t surfaceControlId);

    // The same type as Frame.mWidth and Frame.mHeight
    int32_t mLastFrameWidth = 0;
@@ -259,6 +259,7 @@ private:
    // whether reparenting is needed also used by FrameMetricsReporter to determine
    // if a frame is from an "old" surface (i.e. one that existed before the
    // observer was attched) and therefore shouldn't be reported.
    // NOTE: It is important that this is an increasing counter.
    int32_t mSurfaceControlGenerationId = 0;
    // stopped indicates the CanvasContext will reject actual redraw operations,
    // and defer repaint until it is un-stopped
+3 −2
Original line number Diff line number Diff line
@@ -83,7 +83,8 @@ typedef ASurfaceControl* (*ASC_create)(ASurfaceControl* parent, const char* debu
typedef void (*ASC_acquire)(ASurfaceControl* control);
typedef void (*ASC_release)(ASurfaceControl* control);

typedef void (*ASC_registerSurfaceStatsListener)(ASurfaceControl* control, void* context,
typedef void (*ASC_registerSurfaceStatsListener)(ASurfaceControl* control, int32_t id,
                                                 void* context,
                                                 ASurfaceControl_SurfaceStatsListener func);
typedef void (*ASC_unregisterSurfaceStatsListener)(void* context,
                                                   ASurfaceControl_SurfaceStatsListener func);
+8 −11
Original line number Diff line number Diff line
@@ -146,28 +146,25 @@ struct ASurfaceControlStats {
    uint64_t frameNumber;
};

void ASurfaceControl_registerSurfaceStatsListener(ASurfaceControl* control, void* context,
void ASurfaceControl_registerSurfaceStatsListener(ASurfaceControl* control, int32_t id,
                                                  void* context,
                                                  ASurfaceControl_SurfaceStatsListener func) {
    SurfaceStatsCallback callback = [func](void* callback_context,
                                                               nsecs_t,
    SurfaceStatsCallback callback = [func, control, id](void* callback_context, nsecs_t,
                                                        const sp<Fence>&,
                                                        const SurfaceStats& surfaceStats) {

        ASurfaceControlStats aSurfaceControlStats;

        ASurfaceControl* aSurfaceControl =
                reinterpret_cast<ASurfaceControl*>(surfaceStats.surfaceControl.get());
        aSurfaceControlStats.acquireTime = surfaceStats.acquireTime;
        aSurfaceControlStats.previousReleaseFence = surfaceStats.previousReleaseFence;
        aSurfaceControlStats.frameNumber = surfaceStats.eventStats.frameNumber;

        (*func)(callback_context, aSurfaceControl, &aSurfaceControlStats);
        (*func)(callback_context, control, id, &aSurfaceControlStats);
    };

    TransactionCompletedListener::getInstance()->addSurfaceStatsListener(context,
            reinterpret_cast<void*>(func), ASurfaceControl_to_SurfaceControl(control), callback);
}


void ASurfaceControl_unregisterSurfaceStatsListener(void* context,
        ASurfaceControl_SurfaceStatsListener func) {
    TransactionCompletedListener::getInstance()->removeSurfaceStatsListener(context,