diff options
9 files changed, 177 insertions, 31 deletions
diff --git a/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp b/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp index 41a8426e42..cc323ad42e 100644 --- a/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/MultiTouchInputMapper.cpp @@ -48,11 +48,8 @@ void MultiTouchMotionAccumulator::configure(InputDeviceContext& deviceContext, s delete[] mSlots; mSlots = new Slot[slotCount]; -} -void MultiTouchMotionAccumulator::reset(InputDeviceContext& deviceContext) { - // Unfortunately there is no way to read the initial contents of the slots. - // So when we reset the accumulator, we must assume they are all zeroes. + mCurrentSlot = -1; if (mUsingSlotsProtocol) { // Query the driver for the current slot index and use it as the initial slot // before we start reading events from the device. It is possible that the @@ -64,24 +61,22 @@ void MultiTouchMotionAccumulator::reset(InputDeviceContext& deviceContext) { // This can cause the touch point to "jump", but at least there will be // no stuck touches. int32_t initialSlot; - status_t status = deviceContext.getAbsoluteAxisValue(ABS_MT_SLOT, &initialSlot); - if (status) { - ALOGD("Could not retrieve current multitouch slot index. status=%d", status); - initialSlot = -1; + if (const auto status = deviceContext.getAbsoluteAxisValue(ABS_MT_SLOT, &initialSlot); + status == OK) { + mCurrentSlot = initialSlot; + } else { + ALOGD("Could not retrieve current multi-touch slot index. status=%d", status); } - clearSlots(initialSlot); - } else { - clearSlots(-1); } } -void MultiTouchMotionAccumulator::clearSlots(int32_t initialSlot) { +void MultiTouchMotionAccumulator::resetSlots() { if (mSlots) { for (size_t i = 0; i < mSlotCount; i++) { mSlots[i].clear(); } } - mCurrentSlot = initialSlot; + mCurrentSlot = -1; } void MultiTouchMotionAccumulator::process(const RawEvent* rawEvent) { @@ -169,7 +164,7 @@ void MultiTouchMotionAccumulator::process(const RawEvent* rawEvent) { void MultiTouchMotionAccumulator::finishSync() { if (!mUsingSlotsProtocol) { - clearSlots(-1); + resetSlots(); } } @@ -230,10 +225,12 @@ MultiTouchInputMapper::MultiTouchInputMapper(InputDeviceContext& deviceContext) MultiTouchInputMapper::~MultiTouchInputMapper() {} void MultiTouchInputMapper::reset(nsecs_t when) { - mMultiTouchMotionAccumulator.reset(getDeviceContext()); - - mPointerIdBits.clear(); - + // The evdev multi-touch protocol does not allow userspace applications to query the initial or + // current state of the pointers at any time. This means if we clear our accumulated state when + // resetting the input mapper, there's no way to rebuild the full initial state of the pointers. + // We can only wait for updates to all the pointers and axes. Rather than clearing the state and + // rebuilding the state from scratch, we work around this kernel API limitation by never + // fully clearing any state specific to the multi-touch protocol. TouchInputMapper::reset(when); } diff --git a/services/inputflinger/reader/mapper/MultiTouchInputMapper.h b/services/inputflinger/reader/mapper/MultiTouchInputMapper.h index b7c3457285..e7d935082a 100644 --- a/services/inputflinger/reader/mapper/MultiTouchInputMapper.h +++ b/services/inputflinger/reader/mapper/MultiTouchInputMapper.h @@ -71,7 +71,6 @@ public: ~MultiTouchMotionAccumulator(); void configure(InputDeviceContext& deviceContext, size_t slotCount, bool usingSlotsProtocol); - void reset(InputDeviceContext& deviceContext); void process(const RawEvent* rawEvent); void finishSync(); bool hasStylus() const; @@ -86,7 +85,7 @@ private: bool mUsingSlotsProtocol; bool mHaveStylus; - void clearSlots(int32_t initialSlot); + void resetSlots(); void warnIfNotInUse(const RawEvent& event, const Slot& slot); }; diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.cpp b/services/inputflinger/reader/mapper/TouchInputMapper.cpp index 2ddacef02d..3c88bac5d6 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.cpp +++ b/services/inputflinger/reader/mapper/TouchInputMapper.cpp @@ -1473,6 +1473,10 @@ void TouchInputMapper::process(const RawEvent* rawEvent) { } void TouchInputMapper::sync(nsecs_t when, nsecs_t readTime) { + if (mDeviceMode == DeviceMode::DISABLED) { + // Only save the last pending state when the device is disabled. + mRawStatesPending.clear(); + } // Push a new state. mRawStatesPending.emplace_back(); diff --git a/services/inputflinger/reader/mapper/TouchInputMapper.h b/services/inputflinger/reader/mapper/TouchInputMapper.h index c948f565d9..718d275606 100644 --- a/services/inputflinger/reader/mapper/TouchInputMapper.h +++ b/services/inputflinger/reader/mapper/TouchInputMapper.h @@ -327,6 +327,8 @@ protected: int32_t rawVScroll; int32_t rawHScroll; + explicit inline RawState() { clear(); } + void copyFrom(const RawState& other) { when = other.when; readTime = other.readTime; diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp index 3afa52cf2c..697fb207c9 100644 --- a/services/inputflinger/tests/InputReader_test.cpp +++ b/services/inputflinger/tests/InputReader_test.cpp @@ -6787,6 +6787,33 @@ TEST_F(SingleTouchInputMapperTest, Process_WhenAbsPressureIsPresent_HoversIfItsV toDisplayX(150), toDisplayY(250), 0, 0, 0, 0, 0, 0, 0, 0)); } +TEST_F(SingleTouchInputMapperTest, Reset_RecreatesTouchState) { + addConfigurationProperty("touch.deviceType", "touchScreen"); + prepareDisplay(DISPLAY_ORIENTATION_0); + prepareButtons(); + prepareAxes(POSITION | PRESSURE); + SingleTouchInputMapper& mapper = addMapperAndConfigure<SingleTouchInputMapper>(); + NotifyMotionArgs motionArgs; + + // Set the initial state for the touch pointer. + mFakeEventHub->setAbsoluteAxisValue(EVENTHUB_ID, ABS_X, 100); + mFakeEventHub->setAbsoluteAxisValue(EVENTHUB_ID, ABS_Y, 200); + mFakeEventHub->setAbsoluteAxisValue(EVENTHUB_ID, ABS_PRESSURE, RAW_PRESSURE_MAX); + mFakeEventHub->setScanCodeState(EVENTHUB_ID, BTN_TOUCH, 1); + + // Reset the mapper. When the mapper is reset, we expect it to attempt to recreate the touch + // state by reading the current axis values. + mapper.reset(ARBITRARY_TIME); + + // Send a sync to simulate an empty touch frame where nothing changes. The mapper should use + // the recreated touch state to generate a down event. + processSync(mapper); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs)); + ASSERT_EQ(AMOTION_EVENT_ACTION_DOWN, motionArgs.action); + + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasNotCalled()); +} + TEST_F(SingleTouchInputMapperTest, Process_WhenViewportDisplayIdChanged_TouchIsCanceledAndDeviceIsReset) { addConfigurationProperty("touch.deviceType", "touchScreen"); @@ -6848,10 +6875,17 @@ TEST_F(SingleTouchInputMapperTest, // No events are generated while the viewport is inactive. processMove(mapper, 101, 201); processSync(mapper); - processDown(mapper, 102, 202); + processUp(mapper); processSync(mapper); ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasNotCalled()); + // Start a new gesture while the viewport is still inactive. + processDown(mapper, 300, 400); + mFakeEventHub->setAbsoluteAxisValue(EVENTHUB_ID, ABS_X, 300); + mFakeEventHub->setAbsoluteAxisValue(EVENTHUB_ID, ABS_Y, 400); + mFakeEventHub->setScanCodeState(EVENTHUB_ID, BTN_TOUCH, 1); + processSync(mapper); + // Make the viewport active again. The device should resume processing events. viewport->isActive = true; mFakePolicy->updateViewport(*viewport); @@ -6861,8 +6895,7 @@ TEST_F(SingleTouchInputMapperTest, ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyDeviceResetWasCalled()); ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasNotCalled()); - // Start a new gesture. - processDown(mapper, 100, 200); + // In the next sync, the touch state that was recreated when the device was reset is reported. processSync(mapper); ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs)); ASSERT_EQ(AMOTION_EVENT_ACTION_DOWN, motionArgs.action); @@ -9397,6 +9430,80 @@ TEST_F(MultiTouchInputMapperTest, Process_MultiTouch_WithInvalidTrackingId) { ASSERT_EQ(uint32_t(1), motionArgs.pointerCount); } +TEST_F(MultiTouchInputMapperTest, Reset_PreservesLastTouchState) { + addConfigurationProperty("touch.deviceType", "touchScreen"); + prepareDisplay(DISPLAY_ORIENTATION_0); + prepareAxes(POSITION | ID | SLOT | PRESSURE); + MultiTouchInputMapper& mapper = addMapperAndConfigure<MultiTouchInputMapper>(); + + NotifyMotionArgs motionArgs; + + // First finger down. + processId(mapper, FIRST_TRACKING_ID); + processPosition(mapper, 100, 200); + processPressure(mapper, RAW_PRESSURE_MAX); + processSync(mapper); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs)); + ASSERT_EQ(AMOTION_EVENT_ACTION_DOWN, motionArgs.action); + + // Second finger down. + processSlot(mapper, SECOND_SLOT); + processId(mapper, SECOND_TRACKING_ID); + processPosition(mapper, 300, 400); + processPressure(mapper, RAW_PRESSURE_MAX); + processSync(mapper); + ASSERT_NO_FATAL_FAILURE( + mFakeListener->assertNotifyMotionWasCalled(&motionArgs)); + ASSERT_EQ(ACTION_POINTER_1_DOWN, motionArgs.action); + + // Reset the mapper. When the mapper is reset, we expect the current multi-touch state to be + // preserved. Resetting should not generate any events. + mapper.reset(ARBITRARY_TIME); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasNotCalled()); + + // Send a sync to simulate an empty touch frame where nothing changes. The mapper should use + // the existing touch state to generate a down event. + processPosition(mapper, 301, 302); + processSync(mapper); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs)); + ASSERT_EQ(AMOTION_EVENT_ACTION_DOWN, motionArgs.action); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs)); + ASSERT_EQ(ACTION_POINTER_1_DOWN, motionArgs.action); + + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasNotCalled()); +} + +TEST_F(MultiTouchInputMapperTest, Reset_PreservesLastTouchState_NoPointersDown) { + addConfigurationProperty("touch.deviceType", "touchScreen"); + prepareDisplay(DISPLAY_ORIENTATION_0); + prepareAxes(POSITION | ID | SLOT | PRESSURE); + MultiTouchInputMapper& mapper = addMapperAndConfigure<MultiTouchInputMapper>(); + + NotifyMotionArgs motionArgs; + + // First finger touches down and releases. + processId(mapper, FIRST_TRACKING_ID); + processPosition(mapper, 100, 200); + processPressure(mapper, RAW_PRESSURE_MAX); + processSync(mapper); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasCalled(&motionArgs)); + ASSERT_EQ(AMOTION_EVENT_ACTION_DOWN, motionArgs.action); + processId(mapper, INVALID_TRACKING_ID); + processSync(mapper); + ASSERT_NO_FATAL_FAILURE( + mFakeListener->assertNotifyMotionWasCalled(&motionArgs)); + ASSERT_EQ(AMOTION_EVENT_ACTION_UP, motionArgs.action); + + // Reset the mapper. When the mapper is reset, we expect it to restore the latest + // raw state where no pointers are down. + mapper.reset(ARBITRARY_TIME); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasNotCalled()); + + // Send an empty sync frame. Since there are no pointers, no events are generated. + processSync(mapper); + ASSERT_NO_FATAL_FAILURE(mFakeListener->assertNotifyMotionWasNotCalled()); +} + // --- MultiTouchInputMapperTest_ExternalDevice --- class MultiTouchInputMapperTest_ExternalDevice : public MultiTouchInputMapperTest { diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h index df721cdc89..428c19fe02 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h @@ -152,6 +152,8 @@ protected: virtual const compositionengine::CompositionEngine& getCompositionEngine() const = 0; virtual void dumpState(std::string& out) const = 0; + bool mustRecompose() const; + private: void dirtyEntireOutput(); compositionengine::OutputLayer* findLayerRequestingBackgroundComposition() const; @@ -170,6 +172,9 @@ private: std::unique_ptr<ClientCompositionRequestCache> mClientCompositionRequestCache; std::unique_ptr<planner::Planner> mPlanner; std::unique_ptr<HwcAsyncWorker> mHwComposerAsyncWorker; + + // Whether the content must be recomposed this frame. + bool mMustRecompose = false; }; // This template factory function standardizes the implementation details of the diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp index 1ec6449ed0..163d9a3748 100644 --- a/services/surfaceflinger/CompositionEngine/src/Display.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp @@ -426,7 +426,7 @@ void Display::finishFrame(const compositionengine::CompositionRefreshArgs& refre // 1) It is being handled by hardware composer, which may need this to // keep its virtual display state machine in sync, or // 2) There is work to be done (the dirty region isn't empty) - if (GpuVirtualDisplayId::tryCast(mId) && getDirtyRegion().isEmpty()) { + if (GpuVirtualDisplayId::tryCast(mId) && !mustRecompose()) { ALOGV("Skipping display composition"); return; } diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index b724daa8ce..d0c5803d42 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -977,17 +977,17 @@ void Output::beginFrame() { // frame, then nothing more until we get new layers. // - When a display is created with a private layer stack, we won't // emit any black frames until a layer is added to the layer stack. - const bool mustRecompose = dirty && !(empty && wasEmpty); + mMustRecompose = dirty && !(empty && wasEmpty); const char flagPrefix[] = {'-', '+'}; static_cast<void>(flagPrefix); - ALOGV_IF("%s: %s composition for %s (%cdirty %cempty %cwasEmpty)", __FUNCTION__, - mustRecompose ? "doing" : "skipping", getName().c_str(), flagPrefix[dirty], - flagPrefix[empty], flagPrefix[wasEmpty]); + ALOGV("%s: %s composition for %s (%cdirty %cempty %cwasEmpty)", __func__, + mMustRecompose ? "doing" : "skipping", getName().c_str(), flagPrefix[dirty], + flagPrefix[empty], flagPrefix[wasEmpty]); - mRenderSurface->beginFrame(mustRecompose); + mRenderSurface->beginFrame(mMustRecompose); - if (mustRecompose) { + if (mMustRecompose) { outputState.lastCompositionHadVisibleLayers = !empty; } } @@ -1590,5 +1590,9 @@ void Output::finishPrepareFrame() { mRenderSurface->prepareFrame(state.usesClientComposition, state.usesDeviceComposition); } +bool Output::mustRecompose() const { + return mMustRecompose; +} + } // namespace impl } // namespace android::compositionengine diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp index 344fea3331..5369642a1b 100644 --- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp @@ -971,16 +971,40 @@ TEST_F(DisplayFinishFrameTest, skipsCompositionIfNotDirty) { // We expect no calls to queueBuffer if composition was skipped. EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(0); + EXPECT_CALL(*renderSurface, beginFrame(false)); gpuDisplay->editState().isEnabled = true; gpuDisplay->editState().usesClientComposition = false; gpuDisplay->editState().layerStackSpace.setContent(Rect(0, 0, 1, 1)); gpuDisplay->editState().dirtyRegion = Region::INVALID_REGION; + gpuDisplay->editState().lastCompositionHadVisibleLayers = true; + gpuDisplay->beginFrame(); gpuDisplay->finishFrame({}, std::move(mResultWithoutBuffer)); } -TEST_F(DisplayFinishFrameTest, performsCompositionIfDirty) { +TEST_F(DisplayFinishFrameTest, skipsCompositionIfEmpty) { + auto args = getDisplayCreationArgsForGpuVirtualDisplay(); + std::shared_ptr<impl::Display> gpuDisplay = impl::createDisplay(mCompositionEngine, args); + + mock::RenderSurface* renderSurface = new StrictMock<mock::RenderSurface>(); + gpuDisplay->setRenderSurfaceForTest(std::unique_ptr<RenderSurface>(renderSurface)); + + // We expect no calls to queueBuffer if composition was skipped. + EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(0); + EXPECT_CALL(*renderSurface, beginFrame(false)); + + gpuDisplay->editState().isEnabled = true; + gpuDisplay->editState().usesClientComposition = false; + gpuDisplay->editState().layerStackSpace.setContent(Rect(0, 0, 1, 1)); + gpuDisplay->editState().dirtyRegion = Region(Rect(0, 0, 1, 1)); + gpuDisplay->editState().lastCompositionHadVisibleLayers = false; + + gpuDisplay->beginFrame(); + gpuDisplay->finishFrame({}, std::move(mResultWithoutBuffer)); +} + +TEST_F(DisplayFinishFrameTest, performsCompositionIfDirtyAndNotEmpty) { auto args = getDisplayCreationArgsForGpuVirtualDisplay(); std::shared_ptr<impl::Display> gpuDisplay = impl::createDisplay(mCompositionEngine, args); @@ -989,11 +1013,15 @@ TEST_F(DisplayFinishFrameTest, performsCompositionIfDirty) { // We expect a single call to queueBuffer when composition is not skipped. EXPECT_CALL(*renderSurface, queueBuffer(_)).Times(1); + EXPECT_CALL(*renderSurface, beginFrame(true)); gpuDisplay->editState().isEnabled = true; gpuDisplay->editState().usesClientComposition = false; gpuDisplay->editState().layerStackSpace.setContent(Rect(0, 0, 1, 1)); gpuDisplay->editState().dirtyRegion = Region(Rect(0, 0, 1, 1)); + gpuDisplay->editState().lastCompositionHadVisibleLayers = true; + + gpuDisplay->beginFrame(); gpuDisplay->finishFrame({}, std::move(mResultWithBuffer)); } |