diff options
author | 2018-06-06 16:42:02 -0700 | |
---|---|---|
committer | 2018-06-19 14:56:39 -0700 | |
commit | 00a6fa2f33e2a9e42e4108677fb4abacfca6fd11 (patch) | |
tree | 5aef0b9fef2743f2d493435f4b7df79f1f0bc30b | |
parent | a2edf615f9a185918654867e28c5d999f9d74900 (diff) |
SF: Decouple EventThread from DisplayDevice
EventThread uses DisplayDevice::DisplayType constants, which will be
removed in a follow-up CL. This CL replaces them with local constants as
a stopgap until stable display IDs are propagated through the SF/WM
interface.
Bug: 74619554
Test: libsurfaceflinger_unittest
Change-Id: I68be363dc58c5e3aa17d05fba156d520a01fa775
6 files changed, 69 insertions, 53 deletions
diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp index bc271c8ec5..5a8fd25270 100644 --- a/services/surfaceflinger/EventThread.cpp +++ b/services/surfaceflinger/EventThread.cpp @@ -155,20 +155,17 @@ void EventThread::onVSyncEvent(nsecs_t timestamp) { mCondition.notify_all(); } -void EventThread::onHotplugReceived(int type, bool connected) { - ALOGE_IF(type >= DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES, - "received hotplug event for an invalid display (id=%d)", type); - +void EventThread::onHotplugReceived(DisplayType displayType, bool connected) { std::lock_guard<std::mutex> lock(mMutex); - if (type < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES) { - DisplayEventReceiver::Event event; - event.header.type = DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG; - event.header.id = type; - event.header.timestamp = systemTime(); - event.hotplug.connected = connected; - mPendingEvents.add(event); - mCondition.notify_all(); - } + + DisplayEventReceiver::Event event; + event.header.type = DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG; + event.header.id = displayType == DisplayType::Primary ? 0 : 1; + event.header.timestamp = systemTime(); + event.hotplug.connected = connected; + + mPendingEvents.add(event); + mCondition.notify_all(); } void EventThread::threadMain() NO_THREAD_SAFETY_ANALYSIS { @@ -205,7 +202,7 @@ void EventThread::threadMain() NO_THREAD_SAFETY_ANALYSIS { // This will return when (1) a vsync event has been received, and (2) there was // at least one connection interested in receiving it when we started waiting. Vector<sp<EventThread::Connection> > EventThread::waitForEventLocked( - std::unique_lock<std::mutex>* lock, DisplayEventReceiver::Event* event) { + std::unique_lock<std::mutex>* lock, DisplayEventReceiver::Event* outEvent) { Vector<sp<EventThread::Connection> > signalConnections; while (signalConnections.isEmpty() && mKeepRunning) { @@ -214,16 +211,16 @@ Vector<sp<EventThread::Connection> > EventThread::waitForEventLocked( size_t vsyncCount = 0; nsecs_t timestamp = 0; - for (int32_t i = 0; i < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES; i++) { - timestamp = mVSyncEvent[i].header.timestamp; + for (auto& event : mVSyncEvent) { + timestamp = event.header.timestamp; if (timestamp) { // we have a vsync event to dispatch if (mInterceptVSyncsCallback) { mInterceptVSyncsCallback(timestamp); } - *event = mVSyncEvent[i]; - mVSyncEvent[i].header.timestamp = 0; - vsyncCount = mVSyncEvent[i].vsync.count; + *outEvent = event; + event.header.timestamp = 0; + vsyncCount = event.vsync.count; break; } } @@ -233,7 +230,7 @@ Vector<sp<EventThread::Connection> > EventThread::waitForEventLocked( eventPending = !mPendingEvents.isEmpty(); if (eventPending) { // we have some other event to dispatch - *event = mPendingEvents[0]; + *outEvent = mPendingEvents[0]; mPendingEvents.removeAt(0); } } @@ -319,7 +316,7 @@ Vector<sp<EventThread::Connection> > EventThread::waitForEventLocked( // FIXME: how do we decide which display id the fake // vsync came from ? mVSyncEvent[0].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC; - mVSyncEvent[0].header.id = DisplayDevice::DISPLAY_PRIMARY; + mVSyncEvent[0].header.id = 0; mVSyncEvent[0].header.timestamp = systemTime(SYSTEM_TIME_MONOTONIC); mVSyncEvent[0].vsync.count++; } @@ -364,8 +361,7 @@ void EventThread::dump(String8& result) const { result.appendFormat("VSYNC state: %s\n", mDebugVsyncEnabled ? "enabled" : "disabled"); result.appendFormat(" soft-vsync: %s\n", mUseSoftwareVSync ? "enabled" : "disabled"); result.appendFormat(" numListeners=%zu,\n events-delivered: %u\n", - mDisplayEventConnections.size(), - mVSyncEvent[DisplayDevice::DISPLAY_PRIMARY].vsync.count); + mDisplayEventConnections.size(), mVSyncEvent[0].vsync.count); for (size_t i = 0; i < mDisplayEventConnections.size(); i++) { sp<Connection> connection = mDisplayEventConnections.itemAt(i).promote(); result.appendFormat(" %p: count=%d\n", connection.get(), diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h index 9c13ed2755..a0262b2ad9 100644 --- a/services/surfaceflinger/EventThread.h +++ b/services/surfaceflinger/EventThread.h @@ -16,9 +16,11 @@ #pragma once -#include <stdint.h> #include <sys/types.h> + +#include <array> #include <condition_variable> +#include <cstdint> #include <mutex> #include <thread> @@ -31,8 +33,6 @@ #include <utils/Errors.h> #include <utils/SortedVector.h> -#include "DisplayDevice.h" - // --------------------------------------------------------------------------- namespace android { // --------------------------------------------------------------------------- @@ -59,6 +59,9 @@ public: class EventThread { public: + // TODO: Remove once stable display IDs are plumbed through SF/WM interface. + enum class DisplayType { Primary, External }; + virtual ~EventThread(); virtual sp<BnDisplayEventConnection> createEventConnection() const = 0; @@ -70,7 +73,7 @@ public: virtual void onScreenAcquired() = 0; // called when receiving a hotplug event - virtual void onHotplugReceived(int type, bool connected) = 0; + virtual void onHotplugReceived(DisplayType displayType, bool connected) = 0; virtual void dump(String8& result) const = 0; @@ -122,7 +125,7 @@ public: void onScreenAcquired() override; // called when receiving a hotplug event - void onHotplugReceived(int type, bool connected) override; + void onHotplugReceived(DisplayType displayType, bool connected) override; void dump(String8& result) const override; @@ -155,8 +158,7 @@ private: // protected by mLock SortedVector<wp<Connection>> mDisplayEventConnections GUARDED_BY(mMutex); Vector<DisplayEventReceiver::Event> mPendingEvents GUARDED_BY(mMutex); - DisplayEventReceiver::Event mVSyncEvent[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES] GUARDED_BY( - mMutex); + std::array<DisplayEventReceiver::Event, 2> mVSyncEvent GUARDED_BY(mMutex); bool mUseSoftwareVSync GUARDED_BY(mMutex) = false; bool mVsyncEnabled GUARDED_BY(mMutex) = false; bool mKeepRunning GUARDED_BY(mMutex) = true; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index db534cc60a..5acf2b8f85 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2293,8 +2293,11 @@ void SurfaceFlinger::processDisplayChangesLocked() { if (const auto display = getDisplayDeviceLocked(draw.keyAt(i))) { display->disconnect(getHwComposer()); } - if (draw[i].type < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES) - mEventThread->onHotplugReceived(draw[i].type, false); + if (draw[i].type == DisplayDevice::DISPLAY_PRIMARY) { + mEventThread->onHotplugReceived(EventThread::DisplayType::Primary, false); + } else if (draw[i].type == DisplayDevice::DISPLAY_EXTERNAL) { + mEventThread->onHotplugReceived(EventThread::DisplayType::External, false); + } mDisplays.erase(draw.keyAt(i)); } else { // this display is in both lists. see if something changed. @@ -2395,7 +2398,13 @@ void SurfaceFlinger::processDisplayChangesLocked() { setupNewDisplayDeviceInternal(displayToken, displayId, state, dispSurface, producer)); if (!state.isVirtual()) { - mEventThread->onHotplugReceived(state.type, true); + if (state.type == DisplayDevice::DISPLAY_PRIMARY) { + mEventThread->onHotplugReceived(EventThread::DisplayType::Primary, + true); + } else if (state.type == DisplayDevice::DISPLAY_EXTERNAL) { + mEventThread->onHotplugReceived(EventThread::DisplayType::External, + true); + } } } } diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp index 7928cba108..8d18d76fff 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp @@ -1165,13 +1165,23 @@ void HandleTransactionLockedTest::setupCommonCallExpectationsForConnectProcessin Case::PerFrameMetadataSupport::setupComposerCallExpectations(this); EXPECT_CALL(*mSurfaceInterceptor, saveDisplayCreation(_)).Times(1); - EXPECT_CALL(*mEventThread, onHotplugReceived(Case::Display::TYPE, true)).Times(1); + EXPECT_CALL(*mEventThread, + onHotplugReceived(Case::Display::TYPE == DisplayDevice::DISPLAY_PRIMARY + ? EventThread::DisplayType::Primary + : EventThread::DisplayType::External, + true)) + .Times(1); } template <typename Case> void HandleTransactionLockedTest::setupCommonCallExpectationsForDisconnectProcessing() { EXPECT_CALL(*mSurfaceInterceptor, saveDisplayDeletion(_)).Times(1); - EXPECT_CALL(*mEventThread, onHotplugReceived(Case::Display::TYPE, false)).Times(1); + EXPECT_CALL(*mEventThread, + onHotplugReceived(Case::Display::TYPE == DisplayDevice::DISPLAY_PRIMARY + ? EventThread::DisplayType::Primary + : EventThread::DisplayType::External, + false)) + .Times(1); } template <typename Case> diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index 80fdb80264..19747bd442 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -71,7 +71,8 @@ protected: ConnectionEventRecorder& connectionEventRecorder, nsecs_t expectedTimestamp, unsigned expectedCount); void expectVsyncEventReceivedByConnection(nsecs_t expectedTimestamp, unsigned expectedCount); - void expectHotplugEventReceivedByConnection(int expectedDisplayType, bool expectedConnected); + void expectHotplugEventReceivedByConnection(EventThread::DisplayType expectedDisplayType, + bool expectedConnected); AsyncCallRecorder<void (*)(bool)> mVSyncSetEnabledCallRecorder; AsyncCallRecorder<void (*)(VSyncSource::Callback*)> mVSyncSetCallbackCallRecorder; @@ -169,13 +170,16 @@ void EventThreadTest::expectVsyncEventReceivedByConnection(nsecs_t expectedTimes expectedCount); } -void EventThreadTest::expectHotplugEventReceivedByConnection(int expectedDisplayType, - bool expectedConnected) { +void EventThreadTest::expectHotplugEventReceivedByConnection( + EventThread::DisplayType expectedDisplayType, bool expectedConnected) { + const uint32_t expectedDisplayId = + expectedDisplayType == EventThread::DisplayType::Primary ? 0 : 1; + auto args = mConnectionEventCallRecorder.waitForCall(); ASSERT_TRUE(args.has_value()); const auto& event = std::get<0>(args.value()); EXPECT_EQ(DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG, event.header.type); - EXPECT_EQ(static_cast<unsigned>(expectedDisplayType), event.header.id); + EXPECT_EQ(expectedDisplayId, event.header.id); EXPECT_EQ(expectedConnected, event.hotplug.connected); } @@ -394,28 +398,23 @@ TEST_F(EventThreadTest, setPhaseOffsetForwardsToVSyncSource) { } TEST_F(EventThreadTest, postHotplugPrimaryDisconnect) { - mThread->onHotplugReceived(DisplayDevice::DISPLAY_PRIMARY, false); - expectHotplugEventReceivedByConnection(DisplayDevice::DISPLAY_PRIMARY, false); + mThread->onHotplugReceived(EventThread::DisplayType::Primary, false); + expectHotplugEventReceivedByConnection(EventThread::DisplayType::Primary, false); } TEST_F(EventThreadTest, postHotplugPrimaryConnect) { - mThread->onHotplugReceived(DisplayDevice::DISPLAY_PRIMARY, true); - expectHotplugEventReceivedByConnection(DisplayDevice::DISPLAY_PRIMARY, true); + mThread->onHotplugReceived(EventThread::DisplayType::Primary, true); + expectHotplugEventReceivedByConnection(EventThread::DisplayType::Primary, true); } TEST_F(EventThreadTest, postHotplugExternalDisconnect) { - mThread->onHotplugReceived(DisplayDevice::DISPLAY_EXTERNAL, false); - expectHotplugEventReceivedByConnection(DisplayDevice::DISPLAY_EXTERNAL, false); + mThread->onHotplugReceived(EventThread::DisplayType::External, false); + expectHotplugEventReceivedByConnection(EventThread::DisplayType::External, false); } TEST_F(EventThreadTest, postHotplugExternalConnect) { - mThread->onHotplugReceived(DisplayDevice::DISPLAY_EXTERNAL, true); - expectHotplugEventReceivedByConnection(DisplayDevice::DISPLAY_EXTERNAL, true); -} - -TEST_F(EventThreadTest, postHotplugVirtualDisconnectIsFilteredOut) { - mThread->onHotplugReceived(DisplayDevice::DISPLAY_VIRTUAL, false); - EXPECT_FALSE(mConnectionEventCallRecorder.waitForUnexpectedCall().has_value()); + mThread->onHotplugReceived(EventThread::DisplayType::External, true); + expectHotplugEventReceivedByConnection(EventThread::DisplayType::External, true); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h index e6ea6634c0..df9bfc6d96 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h +++ b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h @@ -31,7 +31,7 @@ public: MOCK_CONST_METHOD0(createEventConnection, sp<BnDisplayEventConnection>()); MOCK_METHOD0(onScreenReleased, void()); MOCK_METHOD0(onScreenAcquired, void()); - MOCK_METHOD2(onHotplugReceived, void(int, bool)); + MOCK_METHOD2(onHotplugReceived, void(DisplayType, bool)); MOCK_CONST_METHOD1(dump, void(String8&)); MOCK_METHOD1(setPhaseOffset, void(nsecs_t phaseOffset)); }; |