diff options
Diffstat (limited to 'libs')
| -rw-r--r-- | libs/gui/BLASTBufferQueue.cpp | 70 | ||||
| -rw-r--r-- | libs/gui/SurfaceComposerClient.cpp | 10 | ||||
| -rw-r--r-- | libs/gui/include/gui/BLASTBufferQueue.h | 12 | ||||
| -rw-r--r-- | libs/gui/include/gui/SurfaceComposerClient.h | 11 | ||||
| -rw-r--r-- | libs/gui/include/gui/test/CallbackUtils.h | 3 | ||||
| -rw-r--r-- | libs/gui/tests/BLASTBufferQueue_test.cpp | 37 | ||||
| -rw-r--r-- | libs/gui/tests/EndToEndNativeInputTest.cpp | 11 | ||||
| -rw-r--r-- | libs/input/Input.cpp | 24 | ||||
| -rw-r--r-- | libs/input/tests/InputEvent_test.cpp | 10 | 
9 files changed, 152 insertions, 36 deletions
| diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index dbccf30fae..a51bbb1553 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -287,18 +287,17 @@ void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/,                  // We need to check if we were waiting for a transaction callback in order to                  // process any pending buffers and unblock. It's possible to get transaction -                // callbacks for previous requests so we need to ensure the frame from this -                // transaction callback matches the last acquired buffer. Since acquireNextBuffer -                // will stop processing buffers when mWaitForTransactionCallback is set, we know -                // that mLastAcquiredFrameNumber is the frame we're waiting on. -                // We also want to check if mNextTransaction is null because it's possible another +                // callbacks for previous requests so we need to ensure that there are no pending +                // frame numbers that were in a sync. We remove the frame from mSyncedFrameNumbers +                // set and then check if it's empty. If there are no more pending syncs, we can +                // proceed with flushing the shadow queue. +                // We also want to check if mSyncTransaction is null because it's possible another                  // sync request came in while waiting, but it hasn't started processing yet. In that                  // case, we don't actually want to flush the frames in between since they will get                  // processed and merged with the sync transaction and released earlier than if they                  // were sent to SF -                if (mWaitForTransactionCallback && mSyncTransaction == nullptr && -                    currFrameNumber >= mLastAcquiredFrameNumber) { -                    mWaitForTransactionCallback = false; +                mSyncedFrameNumbers.erase(currFrameNumber); +                if (mSyncedFrameNumbers.empty() && mSyncTransaction == nullptr) {                      flushShadowQueue();                  }              } else { @@ -308,7 +307,6 @@ void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/,              BQA_LOGE("No matching SurfaceControls found: mSurfaceControlsWithPendingCallback was "                       "empty.");          } -          decStrong((void*)transactionCommittedCallbackThunk);      }  } @@ -351,6 +349,20 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence                                                      stat.latchTime,                                                      stat.frameEventStats.dequeueReadyTime);                  } +                auto currFrameNumber = stat.frameEventStats.frameNumber; +                std::vector<ReleaseCallbackId> staleReleases; +                for (const auto& [key, value]: mSubmitted) { +                    if (currFrameNumber > key.framenumber) { +                        staleReleases.push_back(key); +                    } +                } +                for (const auto& staleRelease : staleReleases) { +                    BQA_LOGE("Faking releaseBufferCallback from transactionCompleteCallback"); +                    BBQ_TRACE("FakeReleaseCallback"); +                    releaseBufferCallbackLocked(staleRelease, +                        stat.previousReleaseFence ? stat.previousReleaseFence : Fence::NO_FENCE, +                        stat.currentMaxAcquiredBufferCount); +                }              } else {                  BQA_LOGE("Failed to find matching SurfaceControl in transactionCallback");              } @@ -391,7 +403,14 @@ void BLASTBufferQueue::releaseBufferCallback(          const ReleaseCallbackId& id, const sp<Fence>& releaseFence,          std::optional<uint32_t> currentMaxAcquiredBufferCount) {      BBQ_TRACE(); +      std::unique_lock _lock{mMutex}; +    releaseBufferCallbackLocked(id, releaseFence, currentMaxAcquiredBufferCount); +} + +void BLASTBufferQueue::releaseBufferCallbackLocked(const ReleaseCallbackId& id, +        const sp<Fence>& releaseFence, std::optional<uint32_t> currentMaxAcquiredBufferCount) { +    ATRACE_CALL();      BQA_LOGV("releaseBufferCallback %s", id.to_string().c_str());      // Calculate how many buffers we need to hold before we release them back @@ -409,16 +428,22 @@ void BLASTBufferQueue::releaseBufferCallback(      const auto numPendingBuffersToHold =              isEGL ? std::max(0u, mMaxAcquiredBuffers - mCurrentMaxAcquiredBufferCount) : 0; -    mPendingRelease.emplace_back(ReleasedBuffer{id, releaseFence}); + +    auto rb = ReleasedBuffer{id, releaseFence}; +    if (std::find(mPendingRelease.begin(), mPendingRelease.end(), rb) == mPendingRelease.end()) { +        mPendingRelease.emplace_back(rb); +    }      // Release all buffers that are beyond the ones that we need to hold      while (mPendingRelease.size() > numPendingBuffersToHold) {          const auto releasedBuffer = mPendingRelease.front();          mPendingRelease.pop_front();          releaseBuffer(releasedBuffer.callbackId, releasedBuffer.releaseFence); -        // Don't process the transactions here if mWaitForTransactionCallback is set. Instead, let -        // onFrameAvailable handle processing them since it will merge with the syncTransaction. -        if (!mWaitForTransactionCallback) { +        // Don't process the transactions here if mSyncedFrameNumbers is not empty. That means +        // are still transactions that have sync buffers in them that have not been applied or +        // dropped. Instead, let onFrameAvailable handle processing them since it will merge with +        // the syncTransaction. +        if (mSyncedFrameNumbers.empty()) {              acquireNextBufferLocked(std::nullopt);          }      } @@ -442,6 +467,9 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId,      BQA_LOGV("released %s", callbackId.to_string().c_str());      mBufferItemConsumer->releaseBuffer(it->second, releaseFence);      mSubmitted.erase(it); +    // Remove the frame number from mSyncedFrameNumbers since we can get a release callback +    // without getting a transaction committed if the buffer was dropped. +    mSyncedFrameNumbers.erase(callbackId.framenumber);  }  void BLASTBufferQueue::acquireNextBufferLocked( @@ -608,7 +636,7 @@ void BLASTBufferQueue::acquireAndReleaseBuffer() {  }  void BLASTBufferQueue::flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& lock) { -    if (mWaitForTransactionCallback && mNumFrameAvailable > 0) { +    if (!mSyncedFrameNumbers.empty() && mNumFrameAvailable > 0) {          // We are waiting on a previous sync's transaction callback so allow another sync          // transaction to proceed.          // @@ -635,6 +663,8 @@ void BLASTBufferQueue::flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& l  void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {      std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr;      SurfaceComposerClient::Transaction* prevTransaction = nullptr; +    bool waitForTransactionCallback = !mSyncedFrameNumbers.empty(); +      {          BBQ_TRACE();          std::unique_lock _lock{mMutex}; @@ -666,7 +696,7 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {          // add to shadow queue          mNumFrameAvailable++; -        if (mWaitForTransactionCallback && mNumFrameAvailable >= 2) { +        if (waitForTransactionCallback && mNumFrameAvailable >= 2) {              acquireAndReleaseBuffer();          }          ATRACE_INT(mQueuedBufferTrace.c_str(), @@ -683,14 +713,14 @@ void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {              incStrong((void*)transactionCommittedCallbackThunk);              mSyncTransaction->addTransactionCommittedCallback(transactionCommittedCallbackThunk,                                                                static_cast<void*>(this)); -            mWaitForTransactionCallback = true; +            mSyncedFrameNumbers.emplace(item.mFrameNumber);              if (mAcquireSingleBuffer) {                  prevCallback = mTransactionReadyCallback;                  prevTransaction = mSyncTransaction;                  mTransactionReadyCallback = nullptr;                  mSyncTransaction = nullptr;              } -        } else if (!mWaitForTransactionCallback) { +        } else if (!waitForTransactionCallback) {              acquireNextBufferLocked(std::nullopt);          }      } @@ -1097,9 +1127,9 @@ void BLASTBufferQueue::abandon() {      }      // Clear sync states -    if (mWaitForTransactionCallback) { -        BQA_LOGD("mWaitForTransactionCallback cleared"); -        mWaitForTransactionCallback = false; +    if (!mSyncedFrameNumbers.empty()) { +        BQA_LOGD("mSyncedFrameNumbers cleared"); +        mSyncedFrameNumbers.clear();      }      if (mSyncTransaction != nullptr) { diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 47d801a78d..0f5192d41c 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -352,7 +352,8 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener                                        transactionStats.latchTime, surfaceStats.acquireTimeOrFence,                                        transactionStats.presentFence,                                        surfaceStats.previousReleaseFence, surfaceStats.transformHint, -                                      surfaceStats.eventStats); +                                      surfaceStats.eventStats, +                                      surfaceStats.currentMaxAcquiredBufferCount);              }              callbackFunction(transactionStats.latchTime, transactionStats.presentFence, @@ -377,7 +378,8 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener                                        transactionStats.latchTime, surfaceStats.acquireTimeOrFence,                                        transactionStats.presentFence,                                        surfaceStats.previousReleaseFence, surfaceStats.transformHint, -                                      surfaceStats.eventStats); +                                      surfaceStats.eventStats, +                                      surfaceStats.currentMaxAcquiredBufferCount);                  if (callbacksMap[callbackId].surfaceControls[surfaceStats.surfaceControl]) {                      callbacksMap[callbackId]                              .surfaceControls[surfaceStats.surfaceControl] @@ -897,6 +899,10 @@ void SurfaceComposerClient::Transaction::clear() {      mApplyToken = nullptr;  } +uint64_t SurfaceComposerClient::Transaction::getId() { +    return mId; +} +  void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) {      sp<ISurfaceComposer> sf(ComposerService::getComposerService()); diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 9328a54184..f5898d20f1 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -95,9 +95,12 @@ public:                                       const std::vector<SurfaceControlStats>& stats);      void releaseBufferCallback(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,                                 std::optional<uint32_t> currentMaxAcquiredBufferCount); +    void releaseBufferCallbackLocked(const ReleaseCallbackId& id, const sp<Fence>& releaseFence, +                               std::optional<uint32_t> currentMaxAcquiredBufferCount);      void syncNextTransaction(std::function<void(SurfaceComposerClient::Transaction*)> callback,                               bool acquireSingleBuffer = true);      void stopContinuousSyncTransaction(); +      void mergeWithNextTransaction(SurfaceComposerClient::Transaction* t, uint64_t frameNumber);      void applyPendingTransactions(uint64_t frameNumber);      SurfaceComposerClient::Transaction* gatherPendingTransactions(uint64_t frameNumber); @@ -177,6 +180,12 @@ private:      struct ReleasedBuffer {          ReleaseCallbackId callbackId;          sp<Fence> releaseFence; +        bool operator==(const ReleasedBuffer& rhs) const { +            // Only compare Id so if we somehow got two callbacks +            // with different fences we don't decrement mNumAcquired +            // too far. +            return rhs.callbackId == callbackId; +        }      };      std::deque<ReleasedBuffer> mPendingRelease GUARDED_BY(mMutex); @@ -251,7 +260,6 @@ private:      std::queue<sp<SurfaceControl>> mSurfaceControlsWithPendingCallback GUARDED_BY(mMutex);      uint32_t mCurrentMaxAcquiredBufferCount; -    bool mWaitForTransactionCallback GUARDED_BY(mMutex) = false;      // Flag to determine if syncTransaction should only acquire a single buffer and then clear or      // continue to acquire buffers until explicitly cleared @@ -279,6 +287,8 @@ private:      uint64_t mLastAppliedFrameNumber = 0;      std::function<void(bool)> mTransactionHangCallback; + +    std::unordered_set<uint64_t> mSyncedFrameNumbers GUARDED_BY(mMutex);  };  } // namespace android diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index efbdb36fef..9033e17c53 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -65,14 +65,16 @@ struct SurfaceControlStats {      SurfaceControlStats(const sp<SurfaceControl>& sc, nsecs_t latchTime,                          std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence,                          const sp<Fence>& presentFence, const sp<Fence>& prevReleaseFence, -                        uint32_t hint, FrameEventHistoryStats eventStats) +                        uint32_t hint, FrameEventHistoryStats eventStats, +                        uint32_t currentMaxAcquiredBufferCount)            : surfaceControl(sc),              latchTime(latchTime),              acquireTimeOrFence(std::move(acquireTimeOrFence)),              presentFence(presentFence),              previousReleaseFence(prevReleaseFence),              transformHint(hint), -            frameEventStats(eventStats) {} +            frameEventStats(eventStats), +            currentMaxAcquiredBufferCount(currentMaxAcquiredBufferCount) {}      sp<SurfaceControl> surfaceControl;      nsecs_t latchTime = -1; @@ -81,6 +83,7 @@ struct SurfaceControlStats {      sp<Fence> previousReleaseFence;      uint32_t transformHint = 0;      FrameEventHistoryStats frameEventStats; +    uint32_t currentMaxAcquiredBufferCount = 0;  };  using TransactionCompletedCallbackTakesContext = @@ -461,6 +464,10 @@ public:          // Clears the contents of the transaction without applying it.          void clear(); +        // Returns the current id of the transaction. +        // The id is updated every time the transaction is applied. +        uint64_t getId(); +          status_t apply(bool synchronous = false, bool oneWay = false);          // Merge another transaction in to this one, clearing other          // as if it had been applied. diff --git a/libs/gui/include/gui/test/CallbackUtils.h b/libs/gui/include/gui/test/CallbackUtils.h index 62d1496ccd..08785b49c1 100644 --- a/libs/gui/include/gui/test/CallbackUtils.h +++ b/libs/gui/include/gui/test/CallbackUtils.h @@ -135,7 +135,8 @@ private:          void verifySurfaceControlStats(const SurfaceControlStats& surfaceControlStats,                                         nsecs_t latchTime) const {              const auto& [surfaceControl, latch, acquireTimeOrFence, presentFence, -                         previousReleaseFence, transformHint, frameEvents] = surfaceControlStats; +                         previousReleaseFence, transformHint, frameEvents, ignore] = +                surfaceControlStats;              ASSERT_TRUE(std::holds_alternative<nsecs_t>(acquireTimeOrFence));              ASSERT_EQ(std::get<nsecs_t>(acquireTimeOrFence) > 0, diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index cb7e94c932..b993289e6a 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -161,6 +161,10 @@ public:          ASSERT_EQ(numFramesSubmitted, mBlastBufferQueueAdapter->mSubmitted.size());      } +    void mergeWithNextTransaction(Transaction* merge, uint64_t frameNumber) { +        mBlastBufferQueueAdapter->mergeWithNextTransaction(merge, frameNumber); +    } +  private:      sp<TestBLASTBufferQueue> mBlastBufferQueueAdapter;  }; @@ -1111,6 +1115,39 @@ TEST_F(BLASTBufferQueueTest, SyncNextTransactionOverwrite) {      ASSERT_TRUE(receivedCallback);  } +TEST_F(BLASTBufferQueueTest, SyncNextTransactionDropBuffer) { +    uint8_t r = 255; +    uint8_t g = 0; +    uint8_t b = 0; + +    BLASTBufferQueueHelper adapter(mSurfaceControl, mDisplayWidth, mDisplayHeight); + +    sp<IGraphicBufferProducer> igbProducer; +    setUpProducer(adapter, igbProducer); + +    Transaction sync; +    adapter.setSyncTransaction(sync); +    queueBuffer(igbProducer, 0, 255, 0, 0); + +    // Merge a transaction that has a complete callback into the next frame so we can get notified +    // when to take a screenshot +    CallbackHelper transactionCallback; +    Transaction t; +    t.addTransactionCompletedCallback(transactionCallback.function, +                                      transactionCallback.getContext()); +    adapter.mergeWithNextTransaction(&t, 2); +    queueBuffer(igbProducer, r, g, b, 0); + +    // Drop the buffer, but ensure the next one continues to get processed. +    sync.setBuffer(mSurfaceControl, nullptr); + +    CallbackData callbackData; +    transactionCallback.getCallbackData(&callbackData); +    ASSERT_EQ(NO_ERROR, captureDisplay(mCaptureArgs, mCaptureResults)); +    ASSERT_NO_FATAL_FAILURE( +            checkScreenCapture(r, g, b, {0, 0, (int32_t)mDisplayWidth, (int32_t)mDisplayHeight})); +} +  // This test will currently fail because the old surfacecontrol will steal the last presented buffer  // until the old surface control is destroyed. This is not necessarily a bug but to document a  // limitation with the update API and to test any changes to make the api more robust. The current diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp index c6cdeb7706..2637f59b5e 100644 --- a/libs/gui/tests/EndToEndNativeInputTest.cpp +++ b/libs/gui/tests/EndToEndNativeInputTest.cpp @@ -1184,18 +1184,23 @@ public:      std::vector<sp<IGraphicBufferProducer>> mProducers;  }; -TEST_F(MultiDisplayTests, drop_input_if_layer_on_invalid_display) { +TEST_F(MultiDisplayTests, drop_touch_if_layer_on_invalid_display) {      ui::LayerStack layerStack = ui::LayerStack::fromValue(42);      // Do not create a display associated with the LayerStack.      std::unique_ptr<InputSurface> surface = makeSurface(100, 100);      surface->doTransaction([&](auto &t, auto &sc) { t.setLayerStack(sc, layerStack); });      surface->showAt(100, 100); +    // Touches should be dropped if the layer is on an invalid display.      injectTapOnDisplay(101, 101, layerStack.id); +    EXPECT_EQ(surface->consumeEvent(100), nullptr); + +    // However, we still let the window be focused and receive keys.      surface->requestFocus(layerStack.id); -    injectKeyOnDisplay(AKEYCODE_V, layerStack.id); +    surface->assertFocusChange(true); -    EXPECT_EQ(surface->consumeEvent(100), nullptr); +    injectKeyOnDisplay(AKEYCODE_V, layerStack.id); +    surface->expectKey(AKEYCODE_V);  }  TEST_F(MultiDisplayTests, virtual_display_receives_input) { diff --git a/libs/input/Input.cpp b/libs/input/Input.cpp index 4127f7ce10..2b7483d27d 100644 --- a/libs/input/Input.cpp +++ b/libs/input/Input.cpp @@ -64,9 +64,10 @@ float transformAngle(const ui::Transform& transform, float angleRadians) {  }  bool shouldDisregardTransformation(uint32_t source) { -    // Do not apply any transformations to axes from joysticks or touchpads. +    // Do not apply any transformations to axes from joysticks, touchpads, or relative mice.      return isFromSource(source, AINPUT_SOURCE_CLASS_JOYSTICK) || -            isFromSource(source, AINPUT_SOURCE_CLASS_POSITION); +            isFromSource(source, AINPUT_SOURCE_CLASS_POSITION) || +            isFromSource(source, AINPUT_SOURCE_MOUSE_RELATIVE);  }  bool shouldDisregardOffset(uint32_t source) { @@ -89,6 +90,25 @@ const char* motionClassificationToString(MotionClassification classification) {      }  } +const char* motionToolTypeToString(int32_t toolType) { +    switch (toolType) { +        case AMOTION_EVENT_TOOL_TYPE_UNKNOWN: +            return "UNKNOWN"; +        case AMOTION_EVENT_TOOL_TYPE_FINGER: +            return "FINGER"; +        case AMOTION_EVENT_TOOL_TYPE_STYLUS: +            return "STYLUS"; +        case AMOTION_EVENT_TOOL_TYPE_MOUSE: +            return "MOUSE"; +        case AMOTION_EVENT_TOOL_TYPE_ERASER: +            return "ERASER"; +        case AMOTION_EVENT_TOOL_TYPE_PALM: +            return "PALM"; +        default: +            return "INVALID"; +    } +} +  // --- IdGenerator ---  IdGenerator::IdGenerator(Source source) : mSource(source) {} diff --git a/libs/input/tests/InputEvent_test.cpp b/libs/input/tests/InputEvent_test.cpp index a92016ba3b..4b3124636b 100644 --- a/libs/input/tests/InputEvent_test.cpp +++ b/libs/input/tests/InputEvent_test.cpp @@ -715,10 +715,10 @@ TEST_F(MotionEventTest, ApplyTransform) {  }  TEST_F(MotionEventTest, JoystickAndTouchpadAreNotTransformed) { -    constexpr static std::array kNonTransformedSources = {std::pair(AINPUT_SOURCE_TOUCHPAD, -                                                                    AMOTION_EVENT_ACTION_DOWN), -                                                          std::pair(AINPUT_SOURCE_JOYSTICK, -                                                                    AMOTION_EVENT_ACTION_MOVE)}; +    constexpr static std::array kNonTransformedSources = +            {std::pair(AINPUT_SOURCE_TOUCHPAD, AMOTION_EVENT_ACTION_DOWN), +             std::pair(AINPUT_SOURCE_JOYSTICK, AMOTION_EVENT_ACTION_MOVE), +             std::pair(AINPUT_SOURCE_MOUSE_RELATIVE, AMOTION_EVENT_ACTION_MOVE)};      // Create a rotate-90 transform with an offset (like a window which isn't fullscreen).      ui::Transform transform(ui::Transform::ROT_90, 800, 400);      transform.set(transform.tx() + 20, transform.ty() + 40); @@ -738,7 +738,7 @@ TEST_F(MotionEventTest, JoystickAndTouchpadAreNotTransformed) {  TEST_F(MotionEventTest, NonPointerSourcesAreNotTranslated) {      constexpr static std::array kNonPointerSources = {std::pair(AINPUT_SOURCE_TRACKBALL,                                                                  AMOTION_EVENT_ACTION_DOWN), -                                                      std::pair(AINPUT_SOURCE_MOUSE_RELATIVE, +                                                      std::pair(AINPUT_SOURCE_TOUCH_NAVIGATION,                                                                  AMOTION_EVENT_ACTION_MOVE)};      // Create a rotate-90 transform with an offset (like a window which isn't fullscreen).      ui::Transform transform(ui::Transform::ROT_90, 800, 400); |