From 9c16128241425c91249379aacc3bca31d04ef12f Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Wed, 7 Jul 2021 16:52:34 -0700 Subject: BlastBufferQueue: Keep transform hint in Surface consistent We should only update the transform hint in Surface when connecting to the queue or queuing a buffer. Otherwise, the transform hint value used when dequeuing the buffer will not align with the value queried from the Surface. This inconsistency was causing BBQ to reject buffers and we suspect this was causing an issue where BBQ will remain in this inconsistent state rejecting all buffers until the app changed orientation. Test: rotate device, check for rejected buffers, run through scenario in bug Bug: b/191841127 Change-Id: Id84a3a650ce68878e5638b942249c11558fd29eb Merged-In: Id84a3a650ce68878e5638b942249c11558fd29eb --- libs/gui/BLASTBufferQueue.cpp | 21 +++++++-------- libs/gui/tests/BLASTBufferQueue_test.cpp | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index ac8feaa0c2..7d57d8b02c 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -171,6 +171,8 @@ BLASTBufferQueue::BLASTBufferQueue(const std::string& name, const sp& surface, uint32_t width, uint32_t height, int32_t format) { std::unique_lock _lock{mMutex}; - BQA_LOGV("update width=%d height=%d format=%d", width, height, format); if (mFormat != format) { mFormat = format; mBufferItemConsumer->setDefaultBufferFormat(convertBufferFormat(format)); @@ -212,6 +213,8 @@ void BLASTBufferQueue::update(const sp& surface, uint32_t width, mTransformHint = mSurfaceControl->getTransformHint(); mBufferItemConsumer->setTransformHint(mTransformHint); } + BQA_LOGV("update width=%d height=%d format=%d mTransformHint=%d", width, height, format, + mTransformHint); ui::Size newSize(width, height); if (mRequestedSize != newSize) { @@ -267,6 +270,7 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const spsetTransformHint(mTransformHint); + BQA_LOGV("updated mTransformHint=%d", mTransformHint); // Update frametime stamps if the frame was latched and presented, indicated by a // valid latch time. if (stat.latchTime > 0) { @@ -339,6 +343,7 @@ void BLASTBufferQueue::releaseBufferCallback(const ReleaseCallbackId& id, mTransformHint = transformHint; mSurfaceControl->setTransformHint(transformHint); mBufferItemConsumer->setTransformHint(mTransformHint); + BQA_LOGV("updated mTransformHint=%d", mTransformHint); } // Calculate how many buffers we need to hold before we release them back @@ -422,7 +427,7 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { } if (rejectBuffer(bufferItem)) { - BQA_LOGE("rejecting buffer:active_size=%dx%d, requested_size=%dx%d" + BQA_LOGE("rejecting buffer:active_size=%dx%d, requested_size=%dx%d " "buffer{size=%dx%d transform=%d}", mSize.width, mSize.height, mRequestedSize.width, mRequestedSize.height, buffer->getWidth(), buffer->getHeight(), bufferItem.mTransform); @@ -515,11 +520,11 @@ void BLASTBufferQueue::processNextBufferLocked(bool useNextTransaction) { BQA_LOGV("processNextBufferLocked size=%dx%d mFrameNumber=%" PRIu64 " applyTransaction=%s mTimestamp=%" PRId64 "%s mPendingTransactions.size=%d" - " graphicBufferId=%" PRIu64 "%s", + " graphicBufferId=%" PRIu64 "%s transform=%d", mSize.width, mSize.height, bufferItem.mFrameNumber, toString(applyTransaction), bufferItem.mTimestamp, bufferItem.mIsAutoTimestamp ? "(auto)" : "", static_cast(mPendingTransactions.size()), bufferItem.mGraphicBuffer->getId(), - bufferItem.mAutoRefresh ? " mAutoRefresh" : ""); + bufferItem.mAutoRefresh ? " mAutoRefresh" : "", bufferItem.mTransform); } Rect BLASTBufferQueue::computeCrop(const BufferItem& item) { @@ -646,14 +651,6 @@ public: status_t setFrameTimelineInfo(const FrameTimelineInfo& frameTimelineInfo) override { return mBbq->setFrameTimelineInfo(frameTimelineInfo); } - protected: - uint32_t getTransformHint() const override { - if (mStickyTransform == 0 && !transformToDisplayInverse()) { - return mBbq->getLastTransformHint(); - } else { - return 0; - } - } }; // TODO: Can we coalesce this with frame updates? Need to confirm diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index 6ff67aa7cb..9082d275a2 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -71,6 +71,10 @@ public: return mBlastBufferQueueAdapter->mSurfaceControl; } + sp getSurface() { + return mBlastBufferQueueAdapter->getSurface(false /* includeSurfaceControlHandle */); + } + void waitForCallbacks() { std::unique_lock lock{mBlastBufferQueueAdapter->mMutex}; // Wait until all but one of the submitted buffers have been released. @@ -758,6 +762,48 @@ TEST_F(BLASTBufferQueueTest, OutOfOrderTransactionTest) { {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight / 2})); } +TEST_F(BLASTBufferQueueTest, TransformHint) { + // Transform hint is provided to BBQ via the surface control passed by WM + mSurfaceControl->setTransformHint(ui::Transform::ROT_90); + + BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + sp igbProducer = adapter.getIGraphicBufferProducer(); + ASSERT_NE(nullptr, igbProducer.get()); + ASSERT_EQ(NO_ERROR, igbProducer->setMaxDequeuedBufferCount(2)); + sp surface = adapter.getSurface(); + + // Before connecting to the surface, we do not get a valid transform hint + int transformHint; + surface->query(NATIVE_WINDOW_TRANSFORM_HINT, &transformHint); + ASSERT_EQ(ui::Transform::ROT_0, transformHint); + + ASSERT_EQ(NO_ERROR, + surface->connect(NATIVE_WINDOW_API_CPU, new TestProducerListener(igbProducer))); + + // After connecting to the surface, we should get the correct hint. + surface->query(NATIVE_WINDOW_TRANSFORM_HINT, &transformHint); + ASSERT_EQ(ui::Transform::ROT_90, transformHint); + + ANativeWindow_Buffer buffer; + surface->lock(&buffer, nullptr /* inOutDirtyBounds */); + + // Transform hint is updated via callbacks or surface control updates + mSurfaceControl->setTransformHint(ui::Transform::ROT_0); + adapter.update(mSurfaceControl, mDisplayWidth, mDisplayHeight); + + // The hint does not change and matches the value used when dequeueing the buffer. + surface->query(NATIVE_WINDOW_TRANSFORM_HINT, &transformHint); + ASSERT_EQ(ui::Transform::ROT_90, transformHint); + + surface->unlockAndPost(); + + // After queuing the buffer, we get the updated transform hint + surface->query(NATIVE_WINDOW_TRANSFORM_HINT, &transformHint); + ASSERT_EQ(ui::Transform::ROT_0, transformHint); + + adapter.waitForCallbacks(); +} + class BLASTBufferQueueTransformTest : public BLASTBufferQueueTest { public: void test(uint32_t tr) { -- cgit v1.2.3-59-g8ed1b