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

Commit e2fad810 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

Merge "SurfaceFlinger: setGeometryAppliesWithResize crop latching fixes." into oc-dev

parents ef21fd2b 7bf247e2
Loading
Loading
Loading
Loading
+36 −13
Original line number Original line Diff line number Diff line
@@ -103,7 +103,7 @@ Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& client,
        mLastFrameNumberReceived(0),
        mLastFrameNumberReceived(0),
        mUpdateTexImageFailed(false),
        mUpdateTexImageFailed(false),
        mAutoRefresh(false),
        mAutoRefresh(false),
        mFreezePositionUpdates(false)
        mFreezeGeometryUpdates(false)
{
{
#ifdef USE_HWC2
#ifdef USE_HWC2
    ALOGV("Creating Layer %s", name.string());
    ALOGV("Creating Layer %s", name.string());
@@ -131,6 +131,8 @@ Layer::Layer(SurfaceFlinger* flinger, const sp<Client>& client,
    mCurrentState.active.transform.set(0, 0);
    mCurrentState.active.transform.set(0, 0);
    mCurrentState.crop.makeInvalid();
    mCurrentState.crop.makeInvalid();
    mCurrentState.finalCrop.makeInvalid();
    mCurrentState.finalCrop.makeInvalid();
    mCurrentState.requestedFinalCrop = mCurrentState.finalCrop;
    mCurrentState.requestedCrop = mCurrentState.crop;
    mCurrentState.z = 0;
    mCurrentState.z = 0;
#ifdef USE_HWC2
#ifdef USE_HWC2
    mCurrentState.alpha = 1.0f;
    mCurrentState.alpha = 1.0f;
@@ -1641,12 +1643,25 @@ uint32_t Layer::doTransaction(uint32_t flags) {
        }
        }
    }
    }


    // always set active to requested, unless we're asked not to
    // Here we apply various requested geometry states, depending on our
    // this is used by Layer, which special cases resizes.
    // latching configuration. See Layer.h for a detailed discussion of
    if (flags & eDontUpdateGeometryState)  {
    // how geometry latching is controlled.
    } else {
    if (!(flags & eDontUpdateGeometryState)) {
        Layer::State& editCurrentState(getCurrentState());
        Layer::State& editCurrentState(getCurrentState());
        if (mFreezePositionUpdates) {

        // If mFreezeGeometryUpdates is true we are in the setGeometryAppliesWithResize
        // mode, which causes attributes which normally latch regardless of scaling mode,
        // to be delayed. We copy the requested state to the active state making sure
        // to respect these rules (again see Layer.h for a detailed discussion).
        //
        // There is an awkward asymmetry in the handling of the crop states in the position
        // states, as can be seen below. Largely this arises from position and transform
        // being stored in the same data structure while having different latching rules.
        // b/38182305
        //
        // Careful that "c" and editCurrentState may not begin as equivalent due to
        // applyPendingStates in the presence of deferred transactions.
        if (mFreezeGeometryUpdates) {
            float tx = c.active.transform.tx();
            float tx = c.active.transform.tx();
            float ty = c.active.transform.ty();
            float ty = c.active.transform.ty();
            c.active = c.requested;
            c.active = c.requested;
@@ -1707,10 +1722,14 @@ bool Layer::setPosition(float x, float y, bool immediate) {
    // we want to apply the position portion of the transform matrix immediately,
    // we want to apply the position portion of the transform matrix immediately,
    // but still delay scaling when resizing a SCALING_MODE_FREEZE layer.
    // but still delay scaling when resizing a SCALING_MODE_FREEZE layer.
    mCurrentState.requested.transform.set(x, y);
    mCurrentState.requested.transform.set(x, y);
    if (immediate && !mFreezePositionUpdates) {
    if (immediate && !mFreezeGeometryUpdates) {
        // Here we directly update the active state
        // unlike other setters, because we store it within
        // the transform, but use different latching rules.
        // b/38182305
        mCurrentState.active.transform.set(x, y);
        mCurrentState.active.transform.set(x, y);
    }
    }
    mFreezePositionUpdates = mFreezePositionUpdates || !immediate;
    mFreezeGeometryUpdates = mFreezeGeometryUpdates || !immediate;


    mCurrentState.modified = true;
    mCurrentState.modified = true;
    setTransactionFlags(eTransactionNeeded);
    setTransactionFlags(eTransactionNeeded);
@@ -1833,26 +1852,30 @@ bool Layer::setFlags(uint8_t flags, uint8_t mask) {
}
}


bool Layer::setCrop(const Rect& crop, bool immediate) {
bool Layer::setCrop(const Rect& crop, bool immediate) {
    if (mCurrentState.crop == crop)
    if (mCurrentState.requestedCrop == crop)
        return false;
        return false;
    mCurrentState.sequence++;
    mCurrentState.sequence++;
    mCurrentState.requestedCrop = crop;
    mCurrentState.requestedCrop = crop;
    if (immediate) {
    if (immediate && !mFreezeGeometryUpdates) {
        mCurrentState.crop = crop;
        mCurrentState.crop = crop;
    }
    }
    mFreezeGeometryUpdates = mFreezeGeometryUpdates || !immediate;

    mCurrentState.modified = true;
    mCurrentState.modified = true;
    setTransactionFlags(eTransactionNeeded);
    setTransactionFlags(eTransactionNeeded);
    return true;
    return true;
}
}


bool Layer::setFinalCrop(const Rect& crop, bool immediate) {
bool Layer::setFinalCrop(const Rect& crop, bool immediate) {
    if (mCurrentState.finalCrop == crop)
    if (mCurrentState.requestedFinalCrop == crop)
        return false;
        return false;
    mCurrentState.sequence++;
    mCurrentState.sequence++;
    mCurrentState.requestedFinalCrop = crop;
    mCurrentState.requestedFinalCrop = crop;
    if (immediate) {
    if (immediate && !mFreezeGeometryUpdates) {
        mCurrentState.finalCrop = crop;
        mCurrentState.finalCrop = crop;
    }
    }
    mFreezeGeometryUpdates = mFreezeGeometryUpdates || !immediate;

    mCurrentState.modified = true;
    mCurrentState.modified = true;
    setTransactionFlags(eTransactionNeeded);
    setTransactionFlags(eTransactionNeeded);
    return true;
    return true;
@@ -2142,7 +2165,7 @@ Region Layer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime)
    bool queuedBuffer = false;
    bool queuedBuffer = false;
    LayerRejecter r(mDrawingState, getCurrentState(), recomputeVisibleRegions,
    LayerRejecter r(mDrawingState, getCurrentState(), recomputeVisibleRegions,
                    getProducerStickyTransform() != 0, mName.string(),
                    getProducerStickyTransform() != 0, mName.string(),
                    mOverrideScalingMode, mFreezePositionUpdates);
                    mOverrideScalingMode, mFreezeGeometryUpdates);
    status_t updateResult = mSurfaceFlingerConsumer->updateTexImage(&r,
    status_t updateResult = mSurfaceFlingerConsumer->updateTexImage(&r,
            mFlinger->mPrimaryDispSync, &mAutoRefresh, &queuedBuffer,
            mFlinger->mPrimaryDispSync, &mAutoRefresh, &queuedBuffer,
            mLastFrameNumberReceived);
            mLastFrameNumberReceived);
+46 −7
Original line number Original line Diff line number Diff line
@@ -169,25 +169,64 @@ public:
    // the this layer's size and format
    // the this layer's size and format
    status_t setBuffers(uint32_t w, uint32_t h, PixelFormat format, uint32_t flags);
    status_t setBuffers(uint32_t w, uint32_t h, PixelFormat format, uint32_t flags);


    // modify current state
    // ------------------------------------------------------------------------
    // Geometry setting functions.
    //
    // The following group of functions are used to specify the layers
    // bounds, and the mapping of the texture on to those bounds. According
    // to various settings changes to them may apply immediately, or be delayed until
    // a pending resize is completed by the producer submitting a buffer. For example
    // if we were to change the buffer size, and update the matrix ahead of the
    // new buffer arriving, then we would be stretching the buffer to a different
    // aspect before and after the buffer arriving, which probably isn't what we wanted.
    //
    // The first set of geometry functions are controlled by the scaling mode, described
    // in window.h. The scaling mode may be set by the client, as it submits buffers.
    // This value may be overriden through SurfaceControl, with setOverrideScalingMode.
    //
    // Put simply, if our scaling mode is SCALING_MODE_FREEZE, then
    // matrix updates will not be applied while a resize is pending
    // and the size and transform will remain in their previous state
    // until a new buffer is submitted. If the scaling mode is another value
    // then the old-buffer will immediately be scaled to the pending size
    // and the new matrix will be immediately applied following this scaling
    // transformation.

    // Set the default buffer size for the assosciated Producer, in pixels. This is
    // also the rendered size of the layer prior to any transformations. Parent
    // or local matrix transformations will not affect the size of the buffer,
    // but may affect it's on-screen size or clipping.
    bool setSize(uint32_t w, uint32_t h);
    // Set a 2x2 transformation matrix on the layer. This transform
    // will be applied after parent transforms, but before any final
    // producer specified transform.
    bool setMatrix(const layer_state_t::matrix22_t& matrix);

    // This second set of geometry attributes are controlled by
    // setGeometryAppliesWithResize, and their default mode is to be
    // immediate. If setGeometryAppliesWithResize is specified
    // while a resize is pending, then update of these attributes will
    // be delayed until the resize completes.


    // These members of state (position, crop, and finalCrop)
    // setPosition operates in parent buffer space (pre parent-transform) or display
    // may be updated immediately or have the update delayed
    // space for top-level layers.
    // until a pending surface resize completes (if applicable).
    bool setPosition(float x, float y, bool immediate);
    bool setPosition(float x, float y, bool immediate);
    // Buffer space
    bool setCrop(const Rect& crop, bool immediate);
    bool setCrop(const Rect& crop, bool immediate);
    // Parent buffer space/display space
    bool setFinalCrop(const Rect& crop, bool immediate);
    bool setFinalCrop(const Rect& crop, bool immediate);


    // TODO(b/38182121): Could we eliminate the various latching modes by
    // using the layer hierarchy?
    // -----------------------------------------------------------------------
    bool setLayer(int32_t z);
    bool setLayer(int32_t z);
    bool setRelativeLayer(const sp<IBinder>& relativeToHandle, int32_t relativeZ);
    bool setRelativeLayer(const sp<IBinder>& relativeToHandle, int32_t relativeZ);


    bool setSize(uint32_t w, uint32_t h);
#ifdef USE_HWC2
#ifdef USE_HWC2
    bool setAlpha(float alpha);
    bool setAlpha(float alpha);
#else
#else
    bool setAlpha(uint8_t alpha);
    bool setAlpha(uint8_t alpha);
#endif
#endif
    bool setMatrix(const layer_state_t::matrix22_t& matrix);
    bool setTransparentRegionHint(const Region& transparent);
    bool setTransparentRegionHint(const Region& transparent);
    bool setFlags(uint8_t flags, uint8_t mask);
    bool setFlags(uint8_t flags, uint8_t mask);
    bool setLayerStack(uint32_t layerStack);
    bool setLayerStack(uint32_t layerStack);
@@ -754,7 +793,7 @@ private:
    bool mUpdateTexImageFailed; // This is only accessed on the main thread.
    bool mUpdateTexImageFailed; // This is only accessed on the main thread.


    bool mAutoRefresh;
    bool mAutoRefresh;
    bool mFreezePositionUpdates;
    bool mFreezeGeometryUpdates;


    // Child list about to be committed/used for editing.
    // Child list about to be committed/used for editing.
    LayerVector mCurrentChildren;
    LayerVector mCurrentChildren;
+18 −13
Original line number Original line Diff line number Diff line
@@ -37,7 +37,7 @@ LayerRejecter::LayerRejecter(Layer::State& front,
    mStickyTransformSet(stickySet),
    mStickyTransformSet(stickySet),
    mName(name),
    mName(name),
    mOverrideScalingMode(overrideScalingMode),
    mOverrideScalingMode(overrideScalingMode),
    mFreezePositionUpdates(freezePositionUpdates) {}
    mFreezeGeometryUpdates(freezePositionUpdates) {}


bool LayerRejecter::reject(const sp<GraphicBuffer>& buf, const BufferItem& item) {
bool LayerRejecter::reject(const sp<GraphicBuffer>& buf, const BufferItem& item) {
    if (buf == NULL) {
    if (buf == NULL) {
@@ -73,6 +73,19 @@ bool LayerRejecter::reject(const sp<GraphicBuffer>& buf, const BufferItem& item)


            // recompute visible region
            // recompute visible region
            mRecomputeVisibleRegions = true;
            mRecomputeVisibleRegions = true;

            mFreezeGeometryUpdates = false;

            if (mFront.crop != mFront.requestedCrop) {
                mFront.crop = mFront.requestedCrop;
                mCurrent.crop = mFront.requestedCrop;
                mRecomputeVisibleRegions = true;
            }
            if (mFront.finalCrop != mFront.requestedFinalCrop) {
                mFront.finalCrop = mFront.requestedFinalCrop;
                mCurrent.finalCrop = mFront.requestedFinalCrop;
                mRecomputeVisibleRegions = true;
            }
        }
        }


        ALOGD_IF(DEBUG_RESIZE,
        ALOGD_IF(DEBUG_RESIZE,
@@ -100,6 +113,10 @@ bool LayerRejecter::reject(const sp<GraphicBuffer>& buf, const BufferItem& item)
    // conservative, but that's fine, worst case we're doing
    // conservative, but that's fine, worst case we're doing
    // a bit of extra work), we latch the new one and we
    // a bit of extra work), we latch the new one and we
    // trigger a visible-region recompute.
    // trigger a visible-region recompute.
    //
    // We latch the transparent region here, instead of above where we latch
    // the rest of the geometry because it is only content but not necessarily
    // resize dependent.
    if (!mFront.activeTransparentRegion.isTriviallyEqual(mFront.requestedTransparentRegion)) {
    if (!mFront.activeTransparentRegion.isTriviallyEqual(mFront.requestedTransparentRegion)) {
        mFront.activeTransparentRegion = mFront.requestedTransparentRegion;
        mFront.activeTransparentRegion = mFront.requestedTransparentRegion;


@@ -115,18 +132,6 @@ bool LayerRejecter::reject(const sp<GraphicBuffer>& buf, const BufferItem& item)
        mRecomputeVisibleRegions = true;
        mRecomputeVisibleRegions = true;
    }
    }


    if (mFront.crop != mFront.requestedCrop) {
        mFront.crop = mFront.requestedCrop;
        mCurrent.crop = mFront.requestedCrop;
        mRecomputeVisibleRegions = true;
    }
    if (mFront.finalCrop != mFront.requestedFinalCrop) {
        mFront.finalCrop = mFront.requestedFinalCrop;
        mCurrent.finalCrop = mFront.requestedFinalCrop;
        mRecomputeVisibleRegions = true;
    }
    mFreezePositionUpdates = false;

    return false;
    return false;
}
}


+2 −2
Original line number Original line Diff line number Diff line
@@ -40,7 +40,7 @@ namespace android {
        bool mStickyTransformSet;
        bool mStickyTransformSet;
        const char *mName;
        const char *mName;
        int32_t mOverrideScalingMode;
        int32_t mOverrideScalingMode;
        bool &mFreezePositionUpdates;
        bool &mFreezeGeometryUpdates;
    };
    };
}  // namespace android
}  // namespace android


+74 −2
Original line number Original line Diff line number Diff line
@@ -33,7 +33,7 @@ namespace android {


// Fill an RGBA_8888 formatted surface with a single color.
// Fill an RGBA_8888 formatted surface with a single color.
static void fillSurfaceRGBA8(const sp<SurfaceControl>& sc,
static void fillSurfaceRGBA8(const sp<SurfaceControl>& sc,
        uint8_t r, uint8_t g, uint8_t b) {
        uint8_t r, uint8_t g, uint8_t b, bool unlock=true) {
    ANativeWindow_Buffer outBuffer;
    ANativeWindow_Buffer outBuffer;
    sp<Surface> s = sc->getSurface();
    sp<Surface> s = sc->getSurface();
    ASSERT_TRUE(s != NULL);
    ASSERT_TRUE(s != NULL);
@@ -48,8 +48,10 @@ static void fillSurfaceRGBA8(const sp<SurfaceControl>& sc,
            pixel[3] = 255;
            pixel[3] = 255;
        }
        }
    }
    }
    if (unlock) {
        ASSERT_EQ(NO_ERROR, s->unlockAndPost());
        ASSERT_EQ(NO_ERROR, s->unlockAndPost());
    }
    }
}


// A ScreenCapture is a screenshot from SurfaceFlinger that can be used to check
// A ScreenCapture is a screenshot from SurfaceFlinger that can be used to check
// individual pixel values for testing purposes.
// individual pixel values for testing purposes.
@@ -479,6 +481,17 @@ protected:
        sc->expectFGColor(127, 127);
        sc->expectFGColor(127, 127);
        sc->expectBGColor(128, 128);
        sc->expectBGColor(128, 128);
    }
    }

    void lockAndFillFGBuffer() {
        fillSurfaceRGBA8(mFGSurfaceControl, 195, 63, 63, false);
    }

    void unlockFGBuffer() {
        sp<Surface> s = mFGSurfaceControl->getSurface();
        ASSERT_EQ(NO_ERROR, s->unlockAndPost());
        waitForPostedBuffers();
    }

    void completeFGResize() {
    void completeFGResize() {
        fillSurfaceRGBA8(mFGSurfaceControl, 195, 63, 63);
        fillSurfaceRGBA8(mFGSurfaceControl, 195, 63, 63);
        waitForPostedBuffers();
        waitForPostedBuffers();
@@ -605,6 +618,65 @@ TEST_F(CropLatchingTest, FinalCropLatching) {
    EXPECT_CROPPED_STATE("after the resize finishes");
    EXPECT_CROPPED_STATE("after the resize finishes");
}
}


// In this test we ensure that setGeometryAppliesWithResize actually demands
// a buffer of the new size, and not just any size.
TEST_F(CropLatchingTest, FinalCropLatchingBufferOldSize) {
    EXPECT_INITIAL_STATE("before anything");
    // Normally the crop applies immediately even while a resize is pending.
    SurfaceComposerClient::openGlobalTransaction();
    mFGSurfaceControl->setSize(128, 128);
    mFGSurfaceControl->setFinalCrop(Rect(64, 64, 127, 127));
    SurfaceComposerClient::closeGlobalTransaction(true);

    EXPECT_CROPPED_STATE("after setting crop (without geometryAppliesWithResize)");

    restoreInitialState();

    // In order to prepare to submit a buffer at the wrong size, we acquire it prior to
    // initiating the resize.
    lockAndFillFGBuffer();

    SurfaceComposerClient::openGlobalTransaction();
    mFGSurfaceControl->setSize(128, 128);
    mFGSurfaceControl->setGeometryAppliesWithResize();
    mFGSurfaceControl->setFinalCrop(Rect(64, 64, 127, 127));
    SurfaceComposerClient::closeGlobalTransaction(true);

    EXPECT_INITIAL_STATE("after setting crop (with geometryAppliesWithResize)");

    // We now submit our old buffer, at the old size, and ensure it doesn't
    // trigger geometry latching.
    unlockFGBuffer();

    EXPECT_INITIAL_STATE("after unlocking FG buffer (with geometryAppliesWithResize)");

    completeFGResize();

    EXPECT_CROPPED_STATE("after the resize finishes");
}

TEST_F(CropLatchingTest, FinalCropLatchingRegressionForb37531386) {
    EXPECT_INITIAL_STATE("before anything");
    // In this scenario, we attempt to set the final crop a second time while the resize
    // is still pending, and ensure we are successful. Success meaning the second crop
    // is the one which eventually latches and not the first.
    SurfaceComposerClient::openGlobalTransaction();
    mFGSurfaceControl->setSize(128, 128);
    mFGSurfaceControl->setGeometryAppliesWithResize();
    mFGSurfaceControl->setFinalCrop(Rect(64, 64, 127, 127));
    SurfaceComposerClient::closeGlobalTransaction(true);

    SurfaceComposerClient::openGlobalTransaction();
    mFGSurfaceControl->setFinalCrop(Rect(0, 0, -1, -1));
    SurfaceComposerClient::closeGlobalTransaction(true);

    EXPECT_INITIAL_STATE("after setting crops with geometryAppliesWithResize");

    completeFGResize();

    EXPECT_INITIAL_STATE("after the resize finishes");
}

TEST_F(LayerUpdateTest, DeferredTransactionTest) {
TEST_F(LayerUpdateTest, DeferredTransactionTest) {
    sp<ScreenCapture> sc;
    sp<ScreenCapture> sc;
    {
    {