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

Commit 59dd2ea9 authored by John Reck's avatar John Reck
Browse files

Fix ReliableSurface to be more reliable

Handle TIMED_OUT better by rescheduling (TODO: give up after N
attempts?)

Fix SYNC_SURFACE_LOST_REWARD_IF_FOUND path to actually go fetch
a new surface.

Bug: 137509524
Test: Injected errors randomly, verified nothing got permanently dead.
Change-Id: Id30f8ad1dd7196041ee84c16c8cf5c814002a6ce
parent 04fbf352
Loading
Loading
Loading
Loading
+6 −5
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import android.graphics.Rect;
import android.graphics.RenderNode;
import android.os.SystemProperties;
import android.os.Trace;
import android.util.Log;
import android.view.Surface.OutOfResourcesException;
import android.view.View.AttachInfo;
import android.view.animation.AnimationUtils;
@@ -674,11 +675,11 @@ public final class ThreadedRenderer extends HardwareRenderer {

        int syncResult = syncAndDrawFrame(choreographer.mFrameInfo);
        if ((syncResult & SYNC_LOST_SURFACE_REWARD_IF_FOUND) != 0) {
            setEnabled(false);
            attachInfo.mViewRootImpl.mSurface.release();
            // Invalidate since we failed to draw. This should fetch a Surface
            // if it is still needed or do nothing if we are no longer drawing
            attachInfo.mViewRootImpl.invalidate();
            Log.w("OpenGLRenderer", "Surface lost, forcing relayout");
            // We lost our surface. For a relayout next frame which should give us a new
            // surface from WindowManager, which hopefully will work.
            attachInfo.mViewRootImpl.mForceNextWindowRelayout = true;
            attachInfo.mViewRootImpl.requestLayout();
        }
        if ((syncResult & SYNC_REDRAW_REQUESTED) != 0) {
            attachInfo.mViewRootImpl.invalidate();
+21 −3
Original line number Diff line number Diff line
@@ -450,20 +450,38 @@ void CanvasContext::draw() {
    waitOnFences();

    bool requireSwap = false;
    int error = OK;
    bool didSwap =
            mRenderPipeline->swapBuffers(frame, drew, windowDirty, mCurrentFrameInfo, &requireSwap);

    mIsDirty = false;

    if (requireSwap) {
        if (!didSwap) {  // some error happened
        bool didDraw = true;
        // Handle any swapchain errors
        error = mNativeSurface->getAndClearError();
        if (error == TIMED_OUT) {
            // Try again
            mRenderThread.postFrameCallback(this);
            // But since this frame didn't happen, we need to mark full damage in the swap
            // history
            didDraw = false;

        } else if (error != OK || !didSwap) {
            // Unknown error, abandon the surface
            setSurface(nullptr);
            didDraw = false;
        }

        SwapHistory& swap = mSwapHistory.next();
        if (didDraw) {
            swap.damage = windowDirty;
        } else {
            swap.damage = SkRect::MakeWH(INT_MAX, INT_MAX);
        }
        swap.swapCompletedTime = systemTime(SYSTEM_TIME_MONOTONIC);
        swap.vsyncTime = mRenderThread.timeLord().latestVsync();
        if (mNativeSurface.get()) {
        if (didDraw) {
            int durationUs;
            nsecs_t dequeueStart = mNativeSurface->getLastDequeueStartTime();
            if (dequeueStart < mCurrentFrameInfo->get(FrameInfoIndex::SyncStart)) {
+8 −7
Original line number Diff line number Diff line
@@ -87,21 +87,21 @@ void ReliableSurface::perform(int operation, va_list args) {
}

int ReliableSurface::reserveNext() {
    if constexpr (DISABLE_BUFFER_PREFETCH) {
        return OK;
    }
    {
        std::lock_guard _lock{mMutex};
        if (mReservedBuffer) {
            ALOGW("reserveNext called but there was already a buffer reserved?");
            return OK;
        }
        if (mInErrorState) {
        if (mBufferQueueState != OK) {
            return UNKNOWN_ERROR;
        }
        if (mHasDequeuedBuffer) {
            return OK;
        }
        if constexpr (DISABLE_BUFFER_PREFETCH) {
            return OK;
        }
    }

    // TODO: Update this to better handle when requested dimensions have changed
@@ -165,10 +165,11 @@ int ReliableSurface::dequeueBuffer(ANativeWindowBuffer** buffer, int* fenceFd) {
        }
    }


    int result = callProtected(mSurface, dequeueBuffer, buffer, fenceFd);
    if (result != OK) {
        ALOGW("dequeueBuffer failed, error = %d; switching to fallback", result);
        *buffer = acquireFallbackBuffer();
        *buffer = acquireFallbackBuffer(result);
        *fenceFd = -1;
        return *buffer ? OK : INVALID_OPERATION;
    } else {
@@ -201,9 +202,9 @@ bool ReliableSurface::isFallbackBuffer(const ANativeWindowBuffer* windowBuffer)
    return windowBuffer == scratchBuffer;
}

ANativeWindowBuffer* ReliableSurface::acquireFallbackBuffer() {
ANativeWindowBuffer* ReliableSurface::acquireFallbackBuffer(int error) {
    std::lock_guard _lock{mMutex};
    mInErrorState = true;
    mBufferQueueState = error;

    if (mScratchBuffer) {
        return AHardwareBuffer_to_ANativeWindowBuffer(mScratchBuffer.get());
+8 −2
Original line number Diff line number Diff line
@@ -43,6 +43,12 @@ public:

    uint64_t getNextFrameNumber() const { return mSurface->getNextFrameNumber(); }

    int getAndClearError() {
        int ret = mBufferQueueState;
        mBufferQueueState = OK;
        return ret;
    }

private:
    const sp<Surface> mSurface;

@@ -55,10 +61,10 @@ private:
    ANativeWindowBuffer* mReservedBuffer = nullptr;
    base::unique_fd mReservedFenceFd;
    bool mHasDequeuedBuffer = false;
    bool mInErrorState = false;
    int mBufferQueueState = OK;

    bool isFallbackBuffer(const ANativeWindowBuffer* windowBuffer) const;
    ANativeWindowBuffer* acquireFallbackBuffer();
    ANativeWindowBuffer* acquireFallbackBuffer(int error);
    void clearReservedBuffer();

    void perform(int operation, va_list args);