diff options
28 files changed, 813 insertions, 651 deletions
diff --git a/services/inputflinger/dispatcher/InputDispatcher.cpp b/services/inputflinger/dispatcher/InputDispatcher.cpp index 71afbccfab..8858f0caec 100644 --- a/services/inputflinger/dispatcher/InputDispatcher.cpp +++ b/services/inputflinger/dispatcher/InputDispatcher.cpp @@ -54,6 +54,7 @@ #include "InputDispatcher.h" #include "trace/InputTracer.h" #include "trace/InputTracingPerfettoBackend.h" +#include "trace/ThreadedBackend.h" #define INDENT " " #define INDENT2 " " @@ -86,6 +87,15 @@ bool isInputTracingEnabled() { return input_flags::enable_input_event_tracing() && isUserdebugOrEng; } +// Create the input tracing backend that writes to perfetto from a single thread. +std::unique_ptr<trace::InputTracingBackendInterface> createInputTracingBackendIfEnabled() { + if (!isInputTracingEnabled()) { + return nullptr; + } + return std::make_unique<trace::impl::ThreadedBackend<trace::impl::PerfettoBackend>>( + trace::impl::PerfettoBackend()); +} + template <class Entry> void ensureEventTraced(const Entry& entry) { if (!entry.traceTracker) { @@ -832,9 +842,7 @@ std::pair<bool /*cancelPointers*/, bool /*cancelNonPointers*/> expandCancellatio // --- InputDispatcher --- InputDispatcher::InputDispatcher(InputDispatcherPolicyInterface& policy) - : InputDispatcher(policy, - isInputTracingEnabled() ? std::make_unique<trace::impl::PerfettoBackend>() - : nullptr) {} + : InputDispatcher(policy, createInputTracingBackendIfEnabled()) {} InputDispatcher::InputDispatcher(InputDispatcherPolicyInterface& policy, std::unique_ptr<trace::InputTracingBackendInterface> traceBackend) @@ -1312,6 +1320,7 @@ bool InputDispatcher::enqueueInboundEventLocked(std::unique_ptr<EventEntry> newE ALOGD("Received a new pointer down event, stop waiting for events to process and " "just send the pending key event to the currently focused window."); mKeyIsWaitingForEventsTimeout = now(); + needWake = true; } break; } diff --git a/services/inputflinger/dispatcher/trace/InputTracer.cpp b/services/inputflinger/dispatcher/trace/InputTracer.cpp index 8a855c2035..be09013a83 100644 --- a/services/inputflinger/dispatcher/trace/InputTracer.cpp +++ b/services/inputflinger/dispatcher/trace/InputTracer.cpp @@ -19,12 +19,17 @@ #include "InputTracer.h" #include <android-base/logging.h> -#include <utils/AndroidThreads.h> namespace android::inputdispatcher::trace::impl { namespace { +// Helper to std::visit with lambdas. +template <typename... V> +struct Visitor : V... { + using V::operator()...; +}; + TracedEvent createTracedEvent(const MotionEntry& e) { return TracedMotionEvent{e.id, e.eventTime, @@ -59,19 +64,9 @@ TracedEvent createTracedEvent(const KeyEntry& e) { // --- InputTracer --- InputTracer::InputTracer(std::unique_ptr<InputTracingBackendInterface> backend) - : mTracerThread(&InputTracer::threadLoop, this), mBackend(std::move(backend)) {} - -InputTracer::~InputTracer() { - { - std::scoped_lock lock(mLock); - mThreadExit = true; - } - mThreadWakeCondition.notify_all(); - mTracerThread.join(); -} + : mBackend(std::move(backend)) {} std::unique_ptr<EventTrackerInterface> InputTracer::traceInboundEvent(const EventEntry& entry) { - std::scoped_lock lock(mLock); TracedEvent traced; if (entry.type == EventEntry::Type::MOTION) { @@ -89,7 +84,6 @@ std::unique_ptr<EventTrackerInterface> InputTracer::traceInboundEvent(const Even void InputTracer::dispatchToTargetHint(const EventTrackerInterface& cookie, const InputTarget& target) { - std::scoped_lock lock(mLock); auto& cookieState = getState(cookie); if (!cookieState) { LOG(FATAL) << "dispatchToTargetHint() should not be called after eventProcessingComplete()"; @@ -98,127 +92,72 @@ void InputTracer::dispatchToTargetHint(const EventTrackerInterface& cookie, } void InputTracer::eventProcessingComplete(const EventTrackerInterface& cookie) { - { - std::scoped_lock lock(mLock); - auto& cookieState = getState(cookie); - if (!cookieState) { - LOG(FATAL) << "Traced event was already logged. " - "eventProcessingComplete() was likely called more than once."; - } - mTraceQueue.emplace_back(std::move(*cookieState)); - cookieState.reset(); - } // release lock - - mThreadWakeCondition.notify_all(); + auto& cookieState = getState(cookie); + if (!cookieState) { + LOG(FATAL) << "Traced event was already logged. " + "eventProcessingComplete() was likely called more than once."; + } + + std::visit(Visitor{[&](const TracedMotionEvent& e) { mBackend->traceMotionEvent(e); }, + [&](const TracedKeyEvent& e) { mBackend->traceKeyEvent(e); }}, + cookieState->event); + cookieState.reset(); } void InputTracer::traceEventDispatch(const DispatchEntry& dispatchEntry, const EventTrackerInterface* cookie) { - { - std::scoped_lock lock(mLock); - const EventEntry& entry = *dispatchEntry.eventEntry; - - TracedEvent traced; - if (entry.type == EventEntry::Type::MOTION) { - const auto& motion = static_cast<const MotionEntry&>(entry); - traced = createTracedEvent(motion); - } else if (entry.type == EventEntry::Type::KEY) { - const auto& key = static_cast<const KeyEntry&>(entry); - traced = createTracedEvent(key); - } else { - LOG(FATAL) << "Cannot trace EventEntry of type: " << ftl::enum_string(entry.type); - } - - if (!cookie) { - // This event was not tracked as an inbound event, so trace it now. - mTraceQueue.emplace_back(traced); - } - - // The vsyncId only has meaning if the event is targeting a window. - const int32_t windowId = dispatchEntry.windowId.value_or(0); - const int32_t vsyncId = dispatchEntry.windowId.has_value() ? dispatchEntry.vsyncId : 0; - - mDispatchTraceQueue.emplace_back(std::move(traced), dispatchEntry.deliveryTime, - dispatchEntry.resolvedFlags, dispatchEntry.targetUid, - vsyncId, windowId, dispatchEntry.transform, - dispatchEntry.rawTransform); - } // release lock - - mThreadWakeCondition.notify_all(); -} + const EventEntry& entry = *dispatchEntry.eventEntry; -std::optional<InputTracer::EventState>& InputTracer::getState(const EventTrackerInterface& cookie) { - return static_cast<const EventTrackerImpl&>(cookie).mLockedState; -} - -void InputTracer::threadLoop() { - androidSetThreadName("InputTracer"); - - std::vector<const EventState> eventsToTrace; - std::vector<const WindowDispatchArgs> dispatchEventsToTrace; - - while (true) { - { // acquire lock - std::unique_lock lock(mLock); - base::ScopedLockAssertion assumeLocked(mLock); - - // Wait until we need to process more events or exit. - mThreadWakeCondition.wait(lock, [&]() REQUIRES(mLock) { - return mThreadExit || !mTraceQueue.empty() || !mDispatchTraceQueue.empty(); - }); - if (mThreadExit) { - return; - } - - mTraceQueue.swap(eventsToTrace); - mDispatchTraceQueue.swap(dispatchEventsToTrace); - } // release lock - - // Trace the events into the backend without holding the lock to reduce the amount of - // work performed in the critical section. - writeEventsToBackend(eventsToTrace, dispatchEventsToTrace); - eventsToTrace.clear(); - dispatchEventsToTrace.clear(); + TracedEvent traced; + if (entry.type == EventEntry::Type::MOTION) { + const auto& motion = static_cast<const MotionEntry&>(entry); + traced = createTracedEvent(motion); + } else if (entry.type == EventEntry::Type::KEY) { + const auto& key = static_cast<const KeyEntry&>(entry); + traced = createTracedEvent(key); + } else { + LOG(FATAL) << "Cannot trace EventEntry of type: " << ftl::enum_string(entry.type); } -} -void InputTracer::writeEventsToBackend( - const std::vector<const EventState>& events, - const std::vector<const WindowDispatchArgs>& dispatchEvents) { - for (const auto& event : events) { - if (auto* motion = std::get_if<TracedMotionEvent>(&event.event); motion != nullptr) { - mBackend->traceMotionEvent(*motion); - } else { - mBackend->traceKeyEvent(std::get<TracedKeyEvent>(event.event)); - } + if (!cookie) { + // This event was not tracked as an inbound event, so trace it now. + std::visit(Visitor{[&](const TracedMotionEvent& e) { mBackend->traceMotionEvent(e); }, + [&](const TracedKeyEvent& e) { mBackend->traceKeyEvent(e); }}, + traced); } - for (const auto& dispatchArgs : dispatchEvents) { - mBackend->traceWindowDispatch(dispatchArgs); - } + // The vsyncId only has meaning if the event is targeting a window. + const int32_t windowId = dispatchEntry.windowId.value_or(0); + const int32_t vsyncId = dispatchEntry.windowId.has_value() ? dispatchEntry.vsyncId : 0; + + mBackend->traceWindowDispatch({std::move(traced), dispatchEntry.deliveryTime, + dispatchEntry.resolvedFlags, dispatchEntry.targetUid, vsyncId, + windowId, dispatchEntry.transform, dispatchEntry.rawTransform, + /*hmac=*/{}}); +} + +std::optional<InputTracer::EventState>& InputTracer::getState(const EventTrackerInterface& cookie) { + return static_cast<const EventTrackerImpl&>(cookie).mState; } // --- InputTracer::EventTrackerImpl --- InputTracer::EventTrackerImpl::EventTrackerImpl(InputTracer& tracer, TracedEvent&& event) - : mTracer(tracer), mLockedState(event) {} + : mTracer(tracer), mState(event) {} InputTracer::EventTrackerImpl::~EventTrackerImpl() { - { - std::scoped_lock lock(mTracer.mLock); - if (!mLockedState) { - // This event has already been written to the trace as expected. - return; - } - // We're still holding on to the state, which means it hasn't yet been written to the trace. - // Write it to the trace now. - // TODO(b/210460522): Determine why/where the event is being destroyed before - // eventProcessingComplete() is called. - mTracer.mTraceQueue.emplace_back(std::move(*mLockedState)); - mLockedState.reset(); - } // release lock - - mTracer.mThreadWakeCondition.notify_all(); + if (!mState) { + // This event has already been written to the trace as expected. + return; + } + // We're still holding on to the state, which means it hasn't yet been written to the trace. + // Write it to the trace now. + // TODO(b/210460522): Determine why/where the event is being destroyed before + // eventProcessingComplete() is called. + std::visit(Visitor{[&](const TracedMotionEvent& e) { mTracer.mBackend->traceMotionEvent(e); }, + [&](const TracedKeyEvent& e) { mTracer.mBackend->traceKeyEvent(e); }}, + mState->event); + mState.reset(); } } // namespace android::inputdispatcher::trace::impl diff --git a/services/inputflinger/dispatcher/trace/InputTracer.h b/services/inputflinger/dispatcher/trace/InputTracer.h index 9fe395d397..c8b25c9961 100644 --- a/services/inputflinger/dispatcher/trace/InputTracer.h +++ b/services/inputflinger/dispatcher/trace/InputTracer.h @@ -18,14 +18,7 @@ #include "InputTracerInterface.h" -#include <android-base/thread_annotations.h> -#include <gui/WindowInfo.h> - #include <memory> -#include <mutex> -#include <thread> -#include <unordered_set> -#include <vector> #include "../Entry.h" #include "InputTracingBackendInterface.h" @@ -35,17 +28,16 @@ namespace android::inputdispatcher::trace::impl { /** * The tracer implementation for InputDispatcher. * - * InputTracer is thread-safe, so it can be called from any thread. Upon construction, InputTracer - * will start its own thread that it uses for write events into the tracing backend. That is the - * one and only thread that will interact with the tracing backend, since the Perfetto backend - * uses thread-local storage. + * InputTracer's responsibility is to keep track of events as they are processed by InputDispatcher, + * and to write the events to the tracing backend when enough information is collected. InputTracer + * is not thread-safe. * * See the documentation in InputTracerInterface for the API surface. */ class InputTracer : public InputTracerInterface { public: explicit InputTracer(std::unique_ptr<InputTracingBackendInterface>); - ~InputTracer() override; + ~InputTracer() = default; InputTracer(const InputTracer&) = delete; InputTracer& operator=(const InputTracer&) = delete; @@ -55,10 +47,6 @@ public: void traceEventDispatch(const DispatchEntry&, const EventTrackerInterface*) override; private: - std::mutex mLock; - std::thread mTracerThread; - bool mThreadExit GUARDED_BY(mLock){false}; - std::condition_variable mThreadWakeCondition; std::unique_ptr<InputTracingBackendInterface> mBackend; // The state of a tracked event. @@ -67,14 +55,12 @@ private: // TODO(b/210460522): Add additional args for tracking event sensitivity and // dispatch target UIDs. }; - std::vector<const EventState> mTraceQueue GUARDED_BY(mLock); - using WindowDispatchArgs = InputTracingBackendInterface::WindowDispatchArgs; - std::vector<const WindowDispatchArgs> mDispatchTraceQueue GUARDED_BY(mLock); - // Provides thread-safe access to the state from an event tracker cookie. - std::optional<EventState>& getState(const EventTrackerInterface&) REQUIRES(mLock); + // Get the event state associated with a tracking cookie. + std::optional<EventState>& getState(const EventTrackerInterface&); - // Implementation of the event tracker cookie. + // Implementation of the event tracker cookie. The cookie holds the event state directly for + // convenience to avoid the overhead of tracking the state separately in InputTracer. class EventTrackerImpl : public EventTrackerInterface { public: explicit EventTrackerImpl(InputTracer&, TracedEvent&& entry); @@ -84,16 +70,10 @@ private: InputTracer& mTracer; // This event tracker cookie will only hold the state as long as it has not been written // to the trace. The state is released when the event is written to the trace. - mutable std::optional<EventState> mLockedState; + mutable std::optional<EventState> mState; - // Only allow InputTracer access to the locked state through getTrackerState() to ensure - // that the InputTracer lock is held when this is accessed. friend std::optional<EventState>& InputTracer::getState(const EventTrackerInterface&); }; - - void threadLoop(); - void writeEventsToBackend(const std::vector<const EventState>& events, - const std::vector<const WindowDispatchArgs>& dispatchEvents); }; } // namespace android::inputdispatcher::trace::impl diff --git a/services/inputflinger/dispatcher/trace/InputTracingBackendInterface.h b/services/inputflinger/dispatcher/trace/InputTracingBackendInterface.h index bc47817817..b0eadfe51f 100644 --- a/services/inputflinger/dispatcher/trace/InputTracingBackendInterface.h +++ b/services/inputflinger/dispatcher/trace/InputTracingBackendInterface.h @@ -82,10 +82,10 @@ public: virtual ~InputTracingBackendInterface() = default; /** Trace a KeyEvent. */ - virtual void traceKeyEvent(const TracedKeyEvent&) const = 0; + virtual void traceKeyEvent(const TracedKeyEvent&) = 0; /** Trace a MotionEvent. */ - virtual void traceMotionEvent(const TracedMotionEvent&) const = 0; + virtual void traceMotionEvent(const TracedMotionEvent&) = 0; /** Trace an event being sent to a window. */ struct WindowDispatchArgs { @@ -99,7 +99,7 @@ public: ui::Transform rawTransform; std::array<uint8_t, 32> hmac; }; - virtual void traceWindowDispatch(const WindowDispatchArgs&) const = 0; + virtual void traceWindowDispatch(const WindowDispatchArgs&) = 0; }; } // namespace android::inputdispatcher::trace diff --git a/services/inputflinger/dispatcher/trace/InputTracingPerfettoBackend.cpp b/services/inputflinger/dispatcher/trace/InputTracingPerfettoBackend.cpp index 4442ad8586..46ad9e16d9 100644 --- a/services/inputflinger/dispatcher/trace/InputTracingPerfettoBackend.cpp +++ b/services/inputflinger/dispatcher/trace/InputTracingPerfettoBackend.cpp @@ -63,7 +63,7 @@ PerfettoBackend::PerfettoBackend() { }); } -void PerfettoBackend::traceMotionEvent(const TracedMotionEvent& event) const { +void PerfettoBackend::traceMotionEvent(const TracedMotionEvent& event) { InputEventDataSource::Trace([&](InputEventDataSource::TraceContext ctx) { auto tracePacket = ctx.NewTracePacket(); auto* inputEvent = tracePacket->set_android_input_event(); @@ -72,7 +72,7 @@ void PerfettoBackend::traceMotionEvent(const TracedMotionEvent& event) const { }); } -void PerfettoBackend::traceKeyEvent(const TracedKeyEvent& event) const { +void PerfettoBackend::traceKeyEvent(const TracedKeyEvent& event) { InputEventDataSource::Trace([&](InputEventDataSource::TraceContext ctx) { auto tracePacket = ctx.NewTracePacket(); auto* inputEvent = tracePacket->set_android_input_event(); @@ -81,7 +81,7 @@ void PerfettoBackend::traceKeyEvent(const TracedKeyEvent& event) const { }); } -void PerfettoBackend::traceWindowDispatch(const WindowDispatchArgs& dispatchArgs) const { +void PerfettoBackend::traceWindowDispatch(const WindowDispatchArgs& dispatchArgs) { InputEventDataSource::Trace([&](InputEventDataSource::TraceContext ctx) { auto tracePacket = ctx.NewTracePacket(); auto* inputEventProto = tracePacket->set_android_input_event(); diff --git a/services/inputflinger/dispatcher/trace/InputTracingPerfettoBackend.h b/services/inputflinger/dispatcher/trace/InputTracingPerfettoBackend.h index 2777cfe9fe..fefcfb3ae0 100644 --- a/services/inputflinger/dispatcher/trace/InputTracingPerfettoBackend.h +++ b/services/inputflinger/dispatcher/trace/InputTracingPerfettoBackend.h @@ -48,9 +48,9 @@ public: PerfettoBackend(); ~PerfettoBackend() override = default; - void traceKeyEvent(const TracedKeyEvent&) const override; - void traceMotionEvent(const TracedMotionEvent&) const override; - void traceWindowDispatch(const WindowDispatchArgs&) const override; + void traceKeyEvent(const TracedKeyEvent&) override; + void traceMotionEvent(const TracedMotionEvent&) override; + void traceWindowDispatch(const WindowDispatchArgs&) override; class InputEventDataSource : public perfetto::DataSource<InputEventDataSource> { public: diff --git a/services/inputflinger/dispatcher/trace/ThreadedBackend.cpp b/services/inputflinger/dispatcher/trace/ThreadedBackend.cpp new file mode 100644 index 0000000000..a58d52a08c --- /dev/null +++ b/services/inputflinger/dispatcher/trace/ThreadedBackend.cpp @@ -0,0 +1,114 @@ +/* + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#define LOG_TAG "InputTracer" + +#include "ThreadedBackend.h" + +#include "InputTracingPerfettoBackend.h" + +#include <android-base/logging.h> +#include <utils/AndroidThreads.h> + +namespace android::inputdispatcher::trace::impl { + +namespace { + +// Helper to std::visit with lambdas. +template <typename... V> +struct Visitor : V... { + using V::operator()...; +}; + +} // namespace + +// --- ThreadedBackend --- + +template <typename Backend> +ThreadedBackend<Backend>::ThreadedBackend(Backend&& innerBackend) + : mTracerThread(&ThreadedBackend::threadLoop, this), mBackend(std::move(innerBackend)) {} + +template <typename Backend> +ThreadedBackend<Backend>::~ThreadedBackend() { + { + std::scoped_lock lock(mLock); + mThreadExit = true; + } + mThreadWakeCondition.notify_all(); + mTracerThread.join(); +} + +template <typename Backend> +void ThreadedBackend<Backend>::traceMotionEvent(const TracedMotionEvent& event) { + std::scoped_lock lock(mLock); + mQueue.emplace_back(event); + mThreadWakeCondition.notify_all(); +} + +template <typename Backend> +void ThreadedBackend<Backend>::traceKeyEvent(const TracedKeyEvent& event) { + std::scoped_lock lock(mLock); + mQueue.emplace_back(event); + mThreadWakeCondition.notify_all(); +} + +template <typename Backend> +void ThreadedBackend<Backend>::traceWindowDispatch(const WindowDispatchArgs& dispatchArgs) { + std::scoped_lock lock(mLock); + mQueue.emplace_back(dispatchArgs); + mThreadWakeCondition.notify_all(); +} + +template <typename Backend> +void ThreadedBackend<Backend>::threadLoop() { + androidSetThreadName("InputTracer"); + + std::vector<std::variant<TracedKeyEvent, TracedMotionEvent, WindowDispatchArgs>> events; + + while (true) { + { // acquire lock + std::unique_lock lock(mLock); + base::ScopedLockAssertion assumeLocked(mLock); + + // Wait until we need to process more events or exit. + mThreadWakeCondition.wait(lock, [&]() REQUIRES(mLock) { + return mThreadExit || !mQueue.empty(); + }); + if (mThreadExit) { + return; + } + + mQueue.swap(events); + } // release lock + + // Trace the events into the backend without holding the lock to reduce the amount of + // work performed in the critical section. + for (const auto& event : events) { + std::visit(Visitor{[&](const TracedMotionEvent& e) { mBackend.traceMotionEvent(e); }, + [&](const TracedKeyEvent& e) { mBackend.traceKeyEvent(e); }, + [&](const WindowDispatchArgs& args) { + mBackend.traceWindowDispatch(args); + }}, + event); + } + events.clear(); + } +} + +// Explicit template instantiation for the PerfettoBackend. +template class ThreadedBackend<PerfettoBackend>; + +} // namespace android::inputdispatcher::trace::impl diff --git a/services/inputflinger/dispatcher/trace/ThreadedBackend.h b/services/inputflinger/dispatcher/trace/ThreadedBackend.h new file mode 100644 index 0000000000..c42f896ce0 --- /dev/null +++ b/services/inputflinger/dispatcher/trace/ThreadedBackend.h @@ -0,0 +1,59 @@ +/* + * Copyright 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include "InputTracingPerfettoBackend.h" + +#include <android-base/thread_annotations.h> +#include <mutex> +#include <thread> +#include <variant> +#include <vector> + +namespace android::inputdispatcher::trace::impl { + +/** + * A wrapper around an InputTracingBackend implementation that writes to the inner tracing backend + * from a single new thread that it creates. The new tracing thread is started when the + * ThreadedBackend is created, and is stopped when it is destroyed. The ThreadedBackend is + * thread-safe. + */ +template <typename Backend> +class ThreadedBackend : public InputTracingBackendInterface { +public: + ThreadedBackend(Backend&& innerBackend); + ~ThreadedBackend() override; + + void traceKeyEvent(const TracedKeyEvent&) override; + void traceMotionEvent(const TracedMotionEvent&) override; + void traceWindowDispatch(const WindowDispatchArgs&) override; + +private: + std::mutex mLock; + std::thread mTracerThread; + bool mThreadExit GUARDED_BY(mLock){false}; + std::condition_variable mThreadWakeCondition; + Backend mBackend; + std::vector<std::variant<TracedKeyEvent, TracedMotionEvent, WindowDispatchArgs>> mQueue + GUARDED_BY(mLock); + + using WindowDispatchArgs = InputTracingBackendInterface::WindowDispatchArgs; + + void threadLoop(); +}; + +} // namespace android::inputdispatcher::trace::impl diff --git a/services/inputflinger/tests/FakeInputTracingBackend.cpp b/services/inputflinger/tests/FakeInputTracingBackend.cpp index b7af356eba..4655ee8458 100644 --- a/services/inputflinger/tests/FakeInputTracingBackend.cpp +++ b/services/inputflinger/tests/FakeInputTracingBackend.cpp @@ -163,7 +163,7 @@ base::Result<void> VerifyingTrace::verifyEventTraced(const Event& expectedEvent, // --- FakeInputTracingBackend --- -void FakeInputTracingBackend::traceKeyEvent(const trace::TracedKeyEvent& event) const { +void FakeInputTracingBackend::traceKeyEvent(const trace::TracedKeyEvent& event) { { std::scoped_lock lock(mTrace->mLock); mTrace->mTracedEvents.emplace(event.id, event); @@ -171,7 +171,7 @@ void FakeInputTracingBackend::traceKeyEvent(const trace::TracedKeyEvent& event) mTrace->mEventTracedCondition.notify_all(); } -void FakeInputTracingBackend::traceMotionEvent(const trace::TracedMotionEvent& event) const { +void FakeInputTracingBackend::traceMotionEvent(const trace::TracedMotionEvent& event) { { std::scoped_lock lock(mTrace->mLock); mTrace->mTracedEvents.emplace(event.id, event); @@ -179,7 +179,7 @@ void FakeInputTracingBackend::traceMotionEvent(const trace::TracedMotionEvent& e mTrace->mEventTracedCondition.notify_all(); } -void FakeInputTracingBackend::traceWindowDispatch(const WindowDispatchArgs& args) const { +void FakeInputTracingBackend::traceWindowDispatch(const WindowDispatchArgs& args) { { std::scoped_lock lock(mTrace->mLock); mTrace->mTracedWindowDispatches.push_back(args); diff --git a/services/inputflinger/tests/FakeInputTracingBackend.h b/services/inputflinger/tests/FakeInputTracingBackend.h index 108419aaaf..1b3613d5c5 100644 --- a/services/inputflinger/tests/FakeInputTracingBackend.h +++ b/services/inputflinger/tests/FakeInputTracingBackend.h @@ -83,9 +83,9 @@ public: private: std::shared_ptr<VerifyingTrace> mTrace; - void traceKeyEvent(const trace::TracedKeyEvent& entry) const override; - void traceMotionEvent(const trace::TracedMotionEvent& entry) const override; - void traceWindowDispatch(const WindowDispatchArgs& entry) const override; + void traceKeyEvent(const trace::TracedKeyEvent& entry) override; + void traceMotionEvent(const trace::TracedMotionEvent& entry) override; + void traceWindowDispatch(const WindowDispatchArgs& entry) override; }; } // namespace android::inputdispatcher diff --git a/services/surfaceflinger/DisplayHardware/HWComposer.cpp b/services/surfaceflinger/DisplayHardware/HWComposer.cpp index cfa03397d6..776bcd3e32 100644 --- a/services/surfaceflinger/DisplayHardware/HWComposer.cpp +++ b/services/surfaceflinger/DisplayHardware/HWComposer.cpp @@ -92,7 +92,7 @@ HWComposer::HWComposer(std::unique_ptr<Hwc2::Composer> composer) mMaxVirtualDisplayDimension(static_cast<size_t>(sysprop::max_virtual_display_dimension(0))), mUpdateDeviceProductInfoOnHotplugReconnect( sysprop::update_device_product_info_on_hotplug_reconnect(false)), - mEnableVrrTimeout(base::GetBoolProperty("debug.sf.vrr_timeout_hint_enabled"s, false)) {} + mEnableVrrTimeout(base::GetBoolProperty("debug.sf.vrr_timeout_hint_enabled"s, true)) {} HWComposer::HWComposer(const std::string& composerServiceName) : HWComposer(Hwc2::Composer::create(composerServiceName)) {} diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp index a145e59eb5..ff88d71259 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.cpp +++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp @@ -17,7 +17,6 @@ #define ATRACE_TAG ATRACE_TAG_GRAPHICS #include <binder/IPCThreadState.h> -#include <gui/DisplayEventReceiver.h> #include <utils/Log.h> #include <utils/Timers.h> #include <utils/threads.h> @@ -66,7 +65,7 @@ void MessageQueue::vsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime, ns { std::lock_guard lock(mVsync.mutex); mVsync.lastCallbackTime = expectedVsyncTime; - mVsync.scheduledFrameTime.reset(); + mVsync.scheduledFrameTimeOpt.reset(); } const auto vsyncId = VsyncId{mVsync.tokenManager->generateTokenForPredictions( @@ -122,7 +121,7 @@ std::unique_ptr<scheduler::VSyncCallbackRegistration> MessageQueue::onNewVsyncSc std::placeholders::_3), "sf"); if (reschedule) { - mVsync.scheduledFrameTime = + mVsync.scheduledFrameTimeOpt = mVsync.registration->schedule({.workDuration = mVsync.workDuration.get().count(), .readyDuration = 0, .lastVsync = mVsync.lastCallbackTime.ns()}); @@ -140,7 +139,7 @@ void MessageQueue::setDuration(std::chrono::nanoseconds workDuration) { ATRACE_CALL(); std::lock_guard lock(mVsync.mutex); mVsync.workDuration = workDuration; - mVsync.scheduledFrameTime = + mVsync.scheduledFrameTimeOpt = mVsync.registration->update({.workDuration = mVsync.workDuration.get().count(), .readyDuration = 0, .lastVsync = mVsync.lastCallbackTime.ns()}); @@ -193,22 +192,20 @@ void MessageQueue::scheduleFrame() { ATRACE_CALL(); std::lock_guard lock(mVsync.mutex); - mVsync.scheduledFrameTime = + mVsync.scheduledFrameTimeOpt = mVsync.registration->schedule({.workDuration = mVsync.workDuration.get().count(), .readyDuration = 0, .lastVsync = mVsync.lastCallbackTime.ns()}); } -auto MessageQueue::getScheduledFrameTime() const -> std::optional<Clock::time_point> { +std::optional<scheduler::ScheduleResult> MessageQueue::getScheduledFrameResult() const { if (mHandler->isFramePending()) { - return Clock::now(); + return scheduler::ScheduleResult{TimePoint::now(), mHandler->getExpectedVsyncTime()}; } - std::lock_guard lock(mVsync.mutex); - if (const auto time = mVsync.scheduledFrameTime) { - return Clock::time_point(std::chrono::nanoseconds(*time)); + if (const auto scheduledFrameTimeline = mVsync.scheduledFrameTimeOpt) { + return scheduledFrameTimeline; } - return std::nullopt; } diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h index edb424b5b9..c5fc371d3a 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.h +++ b/services/surfaceflinger/Scheduler/MessageQueue.h @@ -76,8 +76,7 @@ public: virtual void scheduleConfigure() = 0; virtual void scheduleFrame() = 0; - using Clock = std::chrono::steady_clock; - virtual std::optional<Clock::time_point> getScheduledFrameTime() const = 0; + virtual std::optional<scheduler::ScheduleResult> getScheduledFrameResult() const = 0; }; namespace impl { @@ -95,7 +94,9 @@ protected: explicit Handler(MessageQueue& queue) : mQueue(queue) {} void handleMessage(const Message& message) override; - bool isFramePending() const; + virtual TimePoint getExpectedVsyncTime() const { return mExpectedVsyncTime.load(); } + + virtual bool isFramePending() const; virtual void dispatchFrame(VsyncId, TimePoint expectedVsyncTime); }; @@ -124,7 +125,7 @@ private: TracedOrdinal<std::chrono::nanoseconds> workDuration GUARDED_BY(mutex) = {"VsyncWorkDuration-sf", std::chrono::nanoseconds(0)}; TimePoint lastCallbackTime GUARDED_BY(mutex); - std::optional<nsecs_t> scheduledFrameTime GUARDED_BY(mutex); + std::optional<scheduler::ScheduleResult> scheduledFrameTimeOpt GUARDED_BY(mutex); TracedOrdinal<int> value = {"VSYNC-sf", 0}; }; @@ -150,7 +151,7 @@ public: void scheduleConfigure() override; void scheduleFrame() override; - std::optional<Clock::time_point> getScheduledFrameTime() const override; + std::optional<scheduler::ScheduleResult> getScheduledFrameResult() const override; }; } // namespace impl diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 6979f0399b..e6c58a304e 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -60,14 +60,6 @@ #include "VsyncController.h" #include "VsyncSchedule.h" -#define RETURN_IF_INVALID_HANDLE(handle, ...) \ - do { \ - if (mConnections.count(handle) == 0) { \ - ALOGE("Invalid connection handle %" PRIuPTR, handle.id); \ - return __VA_ARGS__; \ - } \ - } while (false) - namespace android::scheduler { Scheduler::Scheduler(ICompositor& compositor, ISchedulerCallback& callback, FeatureFlags features, @@ -344,40 +336,26 @@ void Scheduler::onExpectedPresentTimePosted(TimePoint expectedPresentTime) { } } -ConnectionHandle Scheduler::createEventThread(Cycle cycle, - frametimeline::TokenManager* tokenManager, - std::chrono::nanoseconds workDuration, - std::chrono::nanoseconds readyDuration) { +void Scheduler::createEventThread(Cycle cycle, frametimeline::TokenManager* tokenManager, + std::chrono::nanoseconds workDuration, + std::chrono::nanoseconds readyDuration) { auto eventThread = std::make_unique<android::impl::EventThread>(cycle == Cycle::Render ? "app" : "appSf", getVsyncSchedule(), tokenManager, *this, workDuration, readyDuration); - auto& handle = cycle == Cycle::Render ? mAppConnectionHandle : mSfConnectionHandle; - handle = createConnection(std::move(eventThread)); - return handle; -} - -ConnectionHandle Scheduler::createConnection(std::unique_ptr<EventThread> eventThread) { - const ConnectionHandle handle = ConnectionHandle{mNextConnectionHandleId++}; - ALOGV("Creating a connection handle with ID %" PRIuPTR, handle.id); - - auto connection = eventThread->createEventConnection(); - - std::lock_guard<std::mutex> lock(mConnectionsLock); - mConnections.emplace(handle, Connection{connection, std::move(eventThread)}); - return handle; + if (cycle == Cycle::Render) { + mRenderEventThread = std::move(eventThread); + mRenderEventConnection = mRenderEventThread->createEventConnection(); + } else { + mLastCompositeEventThread = std::move(eventThread); + mLastCompositeEventConnection = mLastCompositeEventThread->createEventConnection(); + } } sp<IDisplayEventConnection> Scheduler::createDisplayEventConnection( - ConnectionHandle handle, EventRegistrationFlags eventRegistration, - const sp<IBinder>& layerHandle) { - const auto connection = [&]() -> sp<EventThreadConnection> { - std::scoped_lock lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle, nullptr); - - return mConnections[handle].thread->createEventConnection(eventRegistration); - }(); + Cycle cycle, EventRegistrationFlags eventRegistration, const sp<IBinder>& layerHandle) { + const auto connection = eventThreadFor(cycle).createEventConnection(eventRegistration); const auto layerId = static_cast<int32_t>(LayerHandle::getLayerId(layerHandle)); if (layerId != static_cast<int32_t>(UNASSIGNED_LAYER_ID)) { @@ -397,85 +375,48 @@ sp<IDisplayEventConnection> Scheduler::createDisplayEventConnection( return connection; } -sp<EventThreadConnection> Scheduler::getEventConnection(ConnectionHandle handle) { - std::lock_guard<std::mutex> lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle, nullptr); - return mConnections[handle].connection; -} - -void Scheduler::onHotplugReceived(ConnectionHandle handle, PhysicalDisplayId displayId, - bool connected) { - android::EventThread* thread; - { - std::lock_guard<std::mutex> lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle); - thread = mConnections[handle].thread.get(); +void Scheduler::onHotplugReceived(Cycle cycle, PhysicalDisplayId displayId, bool connected) { + if (hasEventThreads()) { + eventThreadFor(cycle).onHotplugReceived(displayId, connected); } - - thread->onHotplugReceived(displayId, connected); } -void Scheduler::onHotplugConnectionError(ConnectionHandle handle, int32_t errorCode) { - android::EventThread* thread; - { - std::lock_guard<std::mutex> lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle); - thread = mConnections[handle].thread.get(); +void Scheduler::onHotplugConnectionError(Cycle cycle, int32_t errorCode) { + if (hasEventThreads()) { + eventThreadFor(cycle).onHotplugConnectionError(errorCode); } - - thread->onHotplugConnectionError(errorCode); } void Scheduler::enableSyntheticVsync(bool enable) { - // TODO(b/241285945): Remove connection handles. - const ConnectionHandle handle = mAppConnectionHandle; - android::EventThread* thread; - { - std::lock_guard<std::mutex> lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle); - thread = mConnections[handle].thread.get(); - } - thread->enableSyntheticVsync(enable); + eventThreadFor(Cycle::Render).enableSyntheticVsync(enable); } -void Scheduler::onFrameRateOverridesChanged(ConnectionHandle handle, PhysicalDisplayId displayId) { +void Scheduler::onFrameRateOverridesChanged(Cycle cycle, PhysicalDisplayId displayId) { const bool supportsFrameRateOverrideByContent = pacesetterSelectorPtr()->supportsAppFrameRateOverrideByContent(); std::vector<FrameRateOverride> overrides = mFrameRateOverrideMappings.getAllFrameRateOverrides(supportsFrameRateOverrideByContent); - android::EventThread* thread; - { - std::lock_guard lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle); - thread = mConnections[handle].thread.get(); - } - thread->onFrameRateOverridesChanged(displayId, std::move(overrides)); + eventThreadFor(cycle).onFrameRateOverridesChanged(displayId, std::move(overrides)); } -void Scheduler::onHdcpLevelsChanged(ConnectionHandle handle, PhysicalDisplayId displayId, +void Scheduler::onHdcpLevelsChanged(Cycle cycle, PhysicalDisplayId displayId, int32_t connectedLevel, int32_t maxLevel) { - android::EventThread* thread; - { - std::lock_guard<std::mutex> lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle); - thread = mConnections[handle].thread.get(); - } - thread->onHdcpLevelsChanged(displayId, connectedLevel, maxLevel); + eventThreadFor(cycle).onHdcpLevelsChanged(displayId, connectedLevel, maxLevel); } -void Scheduler::onPrimaryDisplayModeChanged(ConnectionHandle handle, const FrameRateMode& mode) { +void Scheduler::onPrimaryDisplayModeChanged(Cycle cycle, const FrameRateMode& mode) { { std::lock_guard<std::mutex> lock(mPolicyLock); // Cache the last reported modes for primary display. - mPolicy.cachedModeChangedParams = {handle, mode}; + mPolicy.cachedModeChangedParams = {cycle, mode}; // Invalidate content based refresh rate selection so it could be calculated // again for the new refresh rate. mPolicy.contentRequirements.clear(); } - onNonPrimaryDisplayModeChanged(handle, mode); + onNonPrimaryDisplayModeChanged(cycle, mode); } void Scheduler::dispatchCachedReportedMode() { @@ -502,39 +443,25 @@ void Scheduler::dispatchCachedReportedMode() { } mPolicy.cachedModeChangedParams->mode = *mPolicy.modeOpt; - onNonPrimaryDisplayModeChanged(mPolicy.cachedModeChangedParams->handle, + onNonPrimaryDisplayModeChanged(mPolicy.cachedModeChangedParams->cycle, mPolicy.cachedModeChangedParams->mode); } -void Scheduler::onNonPrimaryDisplayModeChanged(ConnectionHandle handle, const FrameRateMode& mode) { - android::EventThread* thread; - { - std::lock_guard<std::mutex> lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle); - thread = mConnections[handle].thread.get(); +void Scheduler::onNonPrimaryDisplayModeChanged(Cycle cycle, const FrameRateMode& mode) { + if (hasEventThreads()) { + eventThreadFor(cycle).onModeChanged(mode); } - thread->onModeChanged(mode); } -void Scheduler::dump(ConnectionHandle handle, std::string& result) const { - android::EventThread* thread; - { - std::lock_guard<std::mutex> lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle); - thread = mConnections.at(handle).thread.get(); - } - thread->dump(result); +void Scheduler::dump(Cycle cycle, std::string& result) const { + eventThreadFor(cycle).dump(result); } -void Scheduler::setDuration(ConnectionHandle handle, std::chrono::nanoseconds workDuration, +void Scheduler::setDuration(Cycle cycle, std::chrono::nanoseconds workDuration, std::chrono::nanoseconds readyDuration) { - android::EventThread* thread; - { - std::lock_guard<std::mutex> lock(mConnectionsLock); - RETURN_IF_INVALID_HANDLE(handle); - thread = mConnections[handle].thread.get(); + if (hasEventThreads()) { + eventThreadFor(cycle).setDuration(workDuration, readyDuration); } - thread->setDuration(workDuration, readyDuration); } void Scheduler::updatePhaseConfiguration(Fps refreshRate) { @@ -557,10 +484,10 @@ void Scheduler::setActiveDisplayPowerModeForRefreshRateStats(hal::PowerMode powe } void Scheduler::setVsyncConfig(const VsyncConfig& config, Period vsyncPeriod) { - setDuration(mAppConnectionHandle, + setDuration(Cycle::Render, /* workDuration */ config.appWorkDuration, /* readyDuration */ config.sfWorkDuration); - setDuration(mSfConnectionHandle, + setDuration(Cycle::LastComposite, /* workDuration */ vsyncPeriod, /* readyDuration */ config.sfWorkDuration); setDuration(config.sfWorkDuration); @@ -1027,16 +954,10 @@ std::shared_ptr<VsyncSchedule> Scheduler::promotePacesetterDisplayLocked( void Scheduler::applyNewVsyncSchedule(std::shared_ptr<VsyncSchedule> vsyncSchedule) { onNewVsyncSchedule(vsyncSchedule->getDispatch()); - std::vector<android::EventThread*> threads; - { - std::lock_guard<std::mutex> lock(mConnectionsLock); - threads.reserve(mConnections.size()); - for (auto& [_, connection] : mConnections) { - threads.push_back(connection.thread.get()); - } - } - for (auto* thread : threads) { - thread->onNewVsyncSchedule(vsyncSchedule); + + if (hasEventThreads()) { + eventThreadFor(Cycle::Render).onNewVsyncSchedule(vsyncSchedule); + eventThreadFor(Cycle::LastComposite).onNewVsyncSchedule(vsyncSchedule); } } diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 9f29e9f696..dc322254c0 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -56,35 +56,6 @@ #include <FrontEnd/LayerHierarchy.h> -namespace android::scheduler { - -// Opaque handle to scheduler connection. -struct ConnectionHandle { - using Id = std::uintptr_t; - static constexpr Id INVALID_ID = static_cast<Id>(-1); - - Id id = INVALID_ID; - - explicit operator bool() const { return id != INVALID_ID; } -}; - -inline bool operator==(ConnectionHandle lhs, ConnectionHandle rhs) { - return lhs.id == rhs.id; -} - -} // namespace android::scheduler - -namespace std { - -template <> -struct hash<android::scheduler::ConnectionHandle> { - size_t operator()(android::scheduler::ConnectionHandle handle) const { - return hash<android::scheduler::ConnectionHandle::Id>()(handle.id); - } -}; - -} // namespace std - namespace android { class FenceTime; @@ -106,6 +77,11 @@ class RefreshRateStats; class VsyncConfiguration; class VsyncSchedule; +enum class Cycle { + Render, // Surface rendering. + LastComposite // Ahead of display compositing by one refresh period. +}; + class Scheduler : public IEventThreadCallback, android::impl::MessageQueue { using Impl = android::impl::MessageQueue; @@ -133,9 +109,9 @@ public: void initVsync(frametimeline::TokenManager&, std::chrono::nanoseconds workDuration); - using Impl::getScheduledFrameTime; using Impl::setDuration; + using Impl::getScheduledFrameResult; using Impl::scheduleConfigure; using Impl::scheduleFrame; @@ -154,36 +130,32 @@ public: return std::move(future); } - enum class Cycle { - Render, // Surface rendering. - LastComposite // Ahead of display compositing by one refresh period. - }; - - ConnectionHandle createEventThread(Cycle, frametimeline::TokenManager*, - std::chrono::nanoseconds workDuration, - std::chrono::nanoseconds readyDuration); + void createEventThread(Cycle, frametimeline::TokenManager*, + std::chrono::nanoseconds workDuration, + std::chrono::nanoseconds readyDuration); sp<IDisplayEventConnection> createDisplayEventConnection( - ConnectionHandle, EventRegistrationFlags eventRegistration = {}, + Cycle, EventRegistrationFlags eventRegistration = {}, const sp<IBinder>& layerHandle = nullptr) EXCLUDES(mChoreographerLock); - sp<EventThreadConnection> getEventConnection(ConnectionHandle); + const sp<EventThreadConnection>& getEventConnection(Cycle cycle) const { + return cycle == Cycle::Render ? mRenderEventConnection : mLastCompositeEventConnection; + } - void onHotplugReceived(ConnectionHandle, PhysicalDisplayId, bool connected); - void onHotplugConnectionError(ConnectionHandle, int32_t errorCode); + void onHotplugReceived(Cycle, PhysicalDisplayId, bool connected); + void onHotplugConnectionError(Cycle, int32_t errorCode); - void onPrimaryDisplayModeChanged(ConnectionHandle, const FrameRateMode&) EXCLUDES(mPolicyLock); - void onNonPrimaryDisplayModeChanged(ConnectionHandle, const FrameRateMode&); + void onPrimaryDisplayModeChanged(Cycle, const FrameRateMode&) EXCLUDES(mPolicyLock); + void onNonPrimaryDisplayModeChanged(Cycle, const FrameRateMode&); void enableSyntheticVsync(bool = true) REQUIRES(kMainThreadContext); - void onFrameRateOverridesChanged(ConnectionHandle, PhysicalDisplayId) - EXCLUDES(mConnectionsLock); + void onFrameRateOverridesChanged(Cycle, PhysicalDisplayId); - void onHdcpLevelsChanged(ConnectionHandle, PhysicalDisplayId, int32_t, int32_t); + void onHdcpLevelsChanged(Cycle, PhysicalDisplayId, int32_t, int32_t); // Modifies work duration in the event thread. - void setDuration(ConnectionHandle, std::chrono::nanoseconds workDuration, + void setDuration(Cycle, std::chrono::nanoseconds workDuration, std::chrono::nanoseconds readyDuration); VsyncModulator& vsyncModulator() { return *mVsyncModulator; } @@ -288,7 +260,7 @@ public: bool isVsyncInPhase(TimePoint expectedVsyncTime, Fps frameRate) const; void dump(utils::Dumper&) const; - void dump(ConnectionHandle, std::string&) const; + void dump(Cycle, std::string&) const; void dumpVsync(std::string&) const EXCLUDES(mDisplayLock); // Returns the preferred refresh rate and frame rate for the pacesetter display. @@ -369,8 +341,15 @@ private: void onFrameSignal(ICompositor&, VsyncId, TimePoint expectedVsyncTime) override REQUIRES(kMainThreadContext, mDisplayLock); - // Create a connection on the given EventThread. - ConnectionHandle createConnection(std::unique_ptr<EventThread>); + // Used to skip event dispatch before EventThread creation during boot. + // TODO: b/241285191 - Reorder Scheduler initialization to avoid this. + bool hasEventThreads() const { + return CC_LIKELY(mRenderEventThread && mLastCompositeEventThread); + } + + EventThread& eventThreadFor(Cycle cycle) const { + return *(cycle == Cycle::Render ? mRenderEventThread : mLastCompositeEventThread); + } // Update feature state machine to given state when corresponding timer resets or expires. void kernelIdleTimerCallback(TimerState) EXCLUDES(mDisplayLock); @@ -460,18 +439,11 @@ private: void resync() override EXCLUDES(mDisplayLock); void onExpectedPresentTimePosted(TimePoint expectedPresentTime) override EXCLUDES(mDisplayLock); - // Stores EventThread associated with a given VSyncSource, and an initial EventThreadConnection. - struct Connection { - sp<EventThreadConnection> connection; - std::unique_ptr<EventThread> thread; - }; - - ConnectionHandle::Id mNextConnectionHandleId = 0; - mutable std::mutex mConnectionsLock; - std::unordered_map<ConnectionHandle, Connection> mConnections GUARDED_BY(mConnectionsLock); + std::unique_ptr<EventThread> mRenderEventThread; + sp<EventThreadConnection> mRenderEventConnection; - ConnectionHandle mAppConnectionHandle; - ConnectionHandle mSfConnectionHandle; + std::unique_ptr<EventThread> mLastCompositeEventThread; + sp<EventThreadConnection> mLastCompositeEventConnection; std::atomic<nsecs_t> mLastResyncTime = 0; @@ -585,7 +557,7 @@ private: ftl::Optional<FrameRateMode> modeOpt; struct ModeChangedParams { - ConnectionHandle handle; + Cycle cycle; FrameRateMode mode; }; diff --git a/services/surfaceflinger/Scheduler/VSyncDispatch.h b/services/surfaceflinger/Scheduler/VSyncDispatch.h index ed8f8fef93..0c43ffbc04 100644 --- a/services/surfaceflinger/Scheduler/VSyncDispatch.h +++ b/services/surfaceflinger/Scheduler/VSyncDispatch.h @@ -21,11 +21,15 @@ #include <string> #include <ftl/mixins.h> +#include <scheduler/Time.h> #include <utils/Timers.h> namespace android::scheduler { -using ScheduleResult = std::optional<nsecs_t>; +struct ScheduleResult { + TimePoint callbackTime; + TimePoint vsyncTime; +}; enum class CancelResult { Cancelled, TooLate, Error }; @@ -124,10 +128,12 @@ public: * * \param [in] token The callback to schedule. * \param [in] scheduleTiming The timing information for this schedule call - * \return The expected callback time if a callback was scheduled. + * \return The expected callback time if a callback was scheduled, + * along with VSYNC time for the callback scheduled. * std::nullopt if the callback is not registered. */ - virtual ScheduleResult schedule(CallbackToken token, ScheduleTiming scheduleTiming) = 0; + virtual std::optional<ScheduleResult> schedule(CallbackToken token, + ScheduleTiming scheduleTiming) = 0; /* * Update the timing information for a scheduled callback. @@ -135,10 +141,12 @@ public: * * \param [in] token The callback to schedule. * \param [in] scheduleTiming The timing information for this schedule call - * \return The expected callback time if a callback was scheduled. + * \return The expected callback time if a callback was scheduled, + * along with VSYNC time for the callback scheduled. * std::nullopt if the callback is not registered. */ - virtual ScheduleResult update(CallbackToken token, ScheduleTiming scheduleTiming) = 0; + virtual std::optional<ScheduleResult> update(CallbackToken token, + ScheduleTiming scheduleTiming) = 0; /* Cancels a scheduled callback, if possible. * @@ -168,10 +176,10 @@ public: VSyncCallbackRegistration& operator=(VSyncCallbackRegistration&&); // See documentation for VSyncDispatch::schedule. - ScheduleResult schedule(VSyncDispatch::ScheduleTiming scheduleTiming); + std::optional<ScheduleResult> schedule(VSyncDispatch::ScheduleTiming scheduleTiming); // See documentation for VSyncDispatch::update. - ScheduleResult update(VSyncDispatch::ScheduleTiming scheduleTiming); + std::optional<ScheduleResult> update(VSyncDispatch::ScheduleTiming scheduleTiming); // See documentation for VSyncDispatch::cancel. CancelResult cancel(); diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp index b92fa24670..84ccf8e938 100644 --- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp +++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp @@ -38,9 +38,10 @@ using base::StringAppendF; namespace { -nsecs_t getExpectedCallbackTime(nsecs_t nextVsyncTime, - const VSyncDispatch::ScheduleTiming& timing) { - return nextVsyncTime - timing.readyDuration - timing.workDuration; +ScheduleResult getExpectedCallbackTime(nsecs_t nextVsyncTime, + const VSyncDispatch::ScheduleTiming& timing) { + return {TimePoint::fromNs(nextVsyncTime - timing.readyDuration - timing.workDuration), + TimePoint::fromNs(nextVsyncTime)}; } } // namespace @@ -115,14 +116,15 @@ ScheduleResult VSyncDispatchTimerQueueEntry::schedule(VSyncDispatch::ScheduleTim auto const nextReadyTime = nextVsyncTime - timing.readyDuration; mScheduleTiming = timing; mArmedInfo = {nextWakeupTime, nextVsyncTime, nextReadyTime}; - return nextWakeupTime; + return ScheduleResult{TimePoint::fromNs(nextWakeupTime), TimePoint::fromNs(nextVsyncTime)}; } -nsecs_t VSyncDispatchTimerQueueEntry::addPendingWorkloadUpdate( +ScheduleResult VSyncDispatchTimerQueueEntry::addPendingWorkloadUpdate( VSyncTracker& tracker, nsecs_t now, VSyncDispatch::ScheduleTiming timing) { mWorkloadUpdateInfo = timing; const auto armedInfo = update(tracker, now, timing, mArmedInfo); - return armedInfo.mActualWakeupTime; + return {TimePoint::fromNs(armedInfo.mActualWakeupTime), + TimePoint::fromNs(armedInfo.mActualVsyncTime)}; } bool VSyncDispatchTimerQueueEntry::hasPendingWorkloadUpdate() const { @@ -383,14 +385,14 @@ void VSyncDispatchTimerQueue::unregisterCallback(CallbackToken token) { } } -ScheduleResult VSyncDispatchTimerQueue::schedule(CallbackToken token, - ScheduleTiming scheduleTiming) { +std::optional<ScheduleResult> VSyncDispatchTimerQueue::schedule(CallbackToken token, + ScheduleTiming scheduleTiming) { std::lock_guard lock(mMutex); return scheduleLocked(token, scheduleTiming); } -ScheduleResult VSyncDispatchTimerQueue::scheduleLocked(CallbackToken token, - ScheduleTiming scheduleTiming) { +std::optional<ScheduleResult> VSyncDispatchTimerQueue::scheduleLocked( + CallbackToken token, ScheduleTiming scheduleTiming) { auto it = mCallbacks.find(token); if (it == mCallbacks.end()) { return {}; @@ -405,10 +407,7 @@ ScheduleResult VSyncDispatchTimerQueue::scheduleLocked(CallbackToken token, return callback->addPendingWorkloadUpdate(*mTracker, now, scheduleTiming); } - const ScheduleResult result = callback->schedule(scheduleTiming, *mTracker, now); - if (!result.has_value()) { - return {}; - } + const auto result = callback->schedule(scheduleTiming, *mTracker, now); if (callback->wakeupTime() < mIntendedWakeupTime - mTimerSlack) { rearmTimerSkippingUpdateFor(now, it); @@ -417,7 +416,8 @@ ScheduleResult VSyncDispatchTimerQueue::scheduleLocked(CallbackToken token, return result; } -ScheduleResult VSyncDispatchTimerQueue::update(CallbackToken token, ScheduleTiming scheduleTiming) { +std::optional<ScheduleResult> VSyncDispatchTimerQueue::update(CallbackToken token, + ScheduleTiming scheduleTiming) { std::lock_guard lock(mMutex); const auto it = mCallbacks.find(token); if (it == mCallbacks.end()) { @@ -494,14 +494,16 @@ VSyncCallbackRegistration::~VSyncCallbackRegistration() { if (mToken) mDispatch->unregisterCallback(*mToken); } -ScheduleResult VSyncCallbackRegistration::schedule(VSyncDispatch::ScheduleTiming scheduleTiming) { +std::optional<ScheduleResult> VSyncCallbackRegistration::schedule( + VSyncDispatch::ScheduleTiming scheduleTiming) { if (!mToken) { return std::nullopt; } return mDispatch->schedule(*mToken, scheduleTiming); } -ScheduleResult VSyncCallbackRegistration::update(VSyncDispatch::ScheduleTiming scheduleTiming) { +std::optional<ScheduleResult> VSyncCallbackRegistration::update( + VSyncDispatch::ScheduleTiming scheduleTiming) { if (!mToken) { return std::nullopt; } diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h index b5ddd25293..252c09ce53 100644 --- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h +++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.h @@ -69,7 +69,8 @@ public: // Adds a pending upload of the earliestVSync and workDuration that will be applied on the next // call to update() - nsecs_t addPendingWorkloadUpdate(VSyncTracker&, nsecs_t now, VSyncDispatch::ScheduleTiming); + ScheduleResult addPendingWorkloadUpdate(VSyncTracker&, nsecs_t now, + VSyncDispatch::ScheduleTiming); // Checks if there is a pending update to the workload, returning true if so. bool hasPendingWorkloadUpdate() const; @@ -128,8 +129,8 @@ public: CallbackToken registerCallback(Callback, std::string callbackName) final; void unregisterCallback(CallbackToken) final; - ScheduleResult schedule(CallbackToken, ScheduleTiming) final; - ScheduleResult update(CallbackToken, ScheduleTiming) final; + std::optional<ScheduleResult> schedule(CallbackToken, ScheduleTiming) final; + std::optional<ScheduleResult> update(CallbackToken, ScheduleTiming) final; CancelResult cancel(CallbackToken) final; void dump(std::string&) const final; @@ -147,7 +148,7 @@ private: void rearmTimerSkippingUpdateFor(nsecs_t now, CallbackMap::const_iterator skipUpdate) REQUIRES(mMutex); void cancelTimer() REQUIRES(mMutex); - ScheduleResult scheduleLocked(CallbackToken, ScheduleTiming) REQUIRES(mMutex); + std::optional<ScheduleResult> scheduleLocked(CallbackToken, ScheduleTiming) REQUIRES(mMutex); std::mutex mutable mMutex; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index e44f841a25..08a78cc54a 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2085,12 +2085,11 @@ status_t SurfaceFlinger::getDisplayDecorationSupport( sp<IDisplayEventConnection> SurfaceFlinger::createDisplayEventConnection( gui::ISurfaceComposer::VsyncSource vsyncSource, EventRegistrationFlags eventRegistration, const sp<IBinder>& layerHandle) { - const auto& handle = - vsyncSource == gui::ISurfaceComposer::VsyncSource::eVsyncSourceSurfaceFlinger - ? mSfConnectionHandle - : mAppConnectionHandle; + const auto cycle = vsyncSource == gui::ISurfaceComposer::VsyncSource::eVsyncSourceSurfaceFlinger + ? scheduler::Cycle::LastComposite + : scheduler::Cycle::Render; - return mScheduler->createDisplayEventConnection(handle, eventRegistration, layerHandle); + return mScheduler->createDisplayEventConnection(cycle, eventRegistration, layerHandle); } void SurfaceFlinger::scheduleCommit(FrameHint hint) { @@ -2132,7 +2131,7 @@ void SurfaceFlinger::onComposerHalVsync(hal::HWDisplayId hwcDisplayId, int64_t t const int32_t hotplugErrorCode = static_cast<int32_t>(-timestamp); ALOGD("SurfaceFlinger got hotplugErrorCode=%d for display %" PRIu64, hotplugErrorCode, hwcDisplayId); - mScheduler->onHotplugConnectionError(mAppConnectionHandle, hotplugErrorCode); + mScheduler->onHotplugConnectionError(scheduler::Cycle::Render, hotplugErrorCode); return; } @@ -2183,7 +2182,7 @@ void SurfaceFlinger::onComposerHalHotplugEvent(hal::HWDisplayId hwcDisplayId, if (FlagManager::getInstance().hotplug2()) { ALOGD("SurfaceFlinger got hotplug event=%d", static_cast<int32_t>(event)); // TODO(b/311403559): use enum type instead of int - mScheduler->onHotplugConnectionError(mAppConnectionHandle, static_cast<int32_t>(event)); + mScheduler->onHotplugConnectionError(scheduler::Cycle::Render, static_cast<int32_t>(event)); } } @@ -2752,7 +2751,11 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( // TODO(b/255601557) Update frameInterval per display refreshArgs.frameInterval = mScheduler->getNextFrameInterval(pacesetterId, pacesetterTarget.expectedPresentTime()); - refreshArgs.scheduledFrameTime = mScheduler->getScheduledFrameTime(); + const auto scheduledFrameResultOpt = mScheduler->getScheduledFrameResult(); + const auto scheduledFrameTimeOpt = scheduledFrameResultOpt + ? std::optional{scheduledFrameResultOpt->callbackTime} + : std::nullopt; + refreshArgs.scheduledFrameTime = scheduledFrameTimeOpt; refreshArgs.hasTrustedPresentationListener = mNumTrustedPresentationListeners > 0; // Store the present time just before calling to the composition engine so we could notify // the scheduler. @@ -3473,7 +3476,7 @@ const char* SurfaceFlinger::processHotplug(PhysicalDisplayId displayId, if (!activeMode) { ALOGE("Failed to hotplug display %s", to_string(displayId).c_str()); if (FlagManager::getInstance().hotplug2()) { - mScheduler->onHotplugConnectionError(mAppConnectionHandle, + mScheduler->onHotplugConnectionError(scheduler::Cycle::Render, static_cast<int32_t>( DisplayHotplugEvent::ERROR_UNKNOWN)); } @@ -3526,8 +3529,8 @@ const char* SurfaceFlinger::processHotplug(PhysicalDisplayId displayId, } void SurfaceFlinger::dispatchDisplayHotplugEvent(PhysicalDisplayId displayId, bool connected) { - mScheduler->onHotplugReceived(mAppConnectionHandle, displayId, connected); - mScheduler->onHotplugReceived(mSfConnectionHandle, displayId, connected); + mScheduler->onHotplugReceived(scheduler::Cycle::Render, displayId, connected); + mScheduler->onHotplugReceived(scheduler::Cycle::LastComposite, displayId, connected); } void SurfaceFlinger::dispatchDisplayModeChangeEvent(PhysicalDisplayId displayId, @@ -3537,7 +3540,7 @@ void SurfaceFlinger::dispatchDisplayModeChangeEvent(PhysicalDisplayId displayId, ? &scheduler::Scheduler::onPrimaryDisplayModeChanged : &scheduler::Scheduler::onNonPrimaryDisplayModeChanged; - ((*mScheduler).*onDisplayModeChanged)(mAppConnectionHandle, mode); + ((*mScheduler).*onDisplayModeChanged)(scheduler::Cycle::Render, mode); } sp<DisplayDevice> SurfaceFlinger::setupNewDisplayDeviceInternal( @@ -4189,7 +4192,7 @@ void SurfaceFlinger::triggerOnFrameRateOverridesChanged() { return getDefaultDisplayDeviceLocked()->getPhysicalId(); }(); - mScheduler->onFrameRateOverridesChanged(mAppConnectionHandle, displayId); + mScheduler->onFrameRateOverridesChanged(scheduler::Cycle::Render, displayId); } void SurfaceFlinger::notifyCpuLoadUp() { @@ -4251,7 +4254,8 @@ void SurfaceFlinger::notifyExpectedPresentIfRequired(PhysicalDisplayId displayId auto hintStatus = data.hintStatus.load(); if (!expectedPresentWithinTimeout) { - if (hintStatus != NotifyExpectedPresentHintStatus::Sent || + if ((hintStatus != NotifyExpectedPresentHintStatus::Sent && + hintStatus != NotifyExpectedPresentHintStatus::ScheduleOnTx) || (timeoutOpt && timeoutOpt->ns() == 0)) { // Send the hint immediately if timeout, as the hint gets // delayed otherwise, as the frame is scheduled close @@ -4260,11 +4264,16 @@ void SurfaceFlinger::notifyExpectedPresentIfRequired(PhysicalDisplayId displayId .compare_exchange_strong(hintStatus, NotifyExpectedPresentHintStatus::ScheduleOnTx)) { scheduleNotifyExpectedPresentHint(displayId); + return; } - return; } } + if (hintStatus == NotifyExpectedPresentHintStatus::Sent && + data.hintStatus.compare_exchange_strong(hintStatus, + NotifyExpectedPresentHintStatus::ScheduleOnTx)) { + return; + } if (hintStatus != NotifyExpectedPresentHintStatus::Start) { return; } @@ -4272,7 +4281,8 @@ void SurfaceFlinger::notifyExpectedPresentIfRequired(PhysicalDisplayId displayId mScheduler->scheduleFrame(); } -void SurfaceFlinger::scheduleNotifyExpectedPresentHint(PhysicalDisplayId displayId) { +void SurfaceFlinger::scheduleNotifyExpectedPresentHint(PhysicalDisplayId displayId, + VsyncId vsyncId) { auto itr = mNotifyExpectedPresentMap.find(displayId); if (itr == mNotifyExpectedPresentMap.end()) { return; @@ -4281,10 +4291,30 @@ void SurfaceFlinger::scheduleNotifyExpectedPresentHint(PhysicalDisplayId display const char* const whence = __func__; const auto sendHint = [=, this]() { auto& data = mNotifyExpectedPresentMap.at(displayId); - data.hintStatus.store(NotifyExpectedPresentHintStatus::Sent); - const auto status = - getHwComposer().notifyExpectedPresent(displayId, data.lastExpectedPresentTimestamp, - data.lastFrameInterval); + TimePoint expectedPresentTime = data.lastExpectedPresentTimestamp; + if (ftl::to_underlying(vsyncId) != FrameTimelineInfo::INVALID_VSYNC_ID) { + const auto predictionOpt = mFrameTimeline->getTokenManager()->getPredictionsForToken( + ftl::to_underlying(vsyncId)); + const auto expectedPresentTimeOnPredictor = TimePoint::fromNs( + predictionOpt ? predictionOpt->presentTime : expectedPresentTime.ns()); + const auto scheduledFrameResultOpt = mScheduler->getScheduledFrameResult(); + const auto expectedPresentTimeOnScheduler = scheduledFrameResultOpt.has_value() + ? scheduledFrameResultOpt->vsyncTime + : TimePoint::fromNs(0); + expectedPresentTime = + std::max(expectedPresentTimeOnPredictor, expectedPresentTimeOnScheduler); + } + + if (expectedPresentTime < TimePoint::now()) { + expectedPresentTime = + mScheduler->getVsyncSchedule()->vsyncDeadlineAfter(TimePoint::now()); + if (mScheduler->vsyncModulator().getVsyncConfig().sfWorkDuration > + mScheduler->getVsyncSchedule(displayId)->period()) { + expectedPresentTime += mScheduler->getVsyncSchedule(displayId)->period(); + } + } + const auto status = getHwComposer().notifyExpectedPresent(displayId, expectedPresentTime, + data.lastFrameInterval); if (status != NO_ERROR) { ALOGE("%s failed to notifyExpectedPresentHint for display %" PRId64, whence, displayId.value); @@ -4302,7 +4332,11 @@ void SurfaceFlinger::scheduleNotifyExpectedPresentHint(PhysicalDisplayId display } })); } - sendHint(); + auto scheduleHintOnPresent = NotifyExpectedPresentHintStatus::ScheduleOnPresent; + if (itr->second.hintStatus.compare_exchange_strong(scheduleHintOnPresent, + NotifyExpectedPresentHintStatus::Sent)) { + sendHint(); + } } void SurfaceFlinger::sendNotifyExpectedPresentHint(PhysicalDisplayId displayId) { @@ -4354,24 +4388,22 @@ void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { mScheduler = std::make_unique<Scheduler>(static_cast<ICompositor&>(*this), static_cast<ISchedulerCallback&>(*this), features, getFactory(), activeRefreshRate, *mTimeStats); + + // The pacesetter must be registered before EventThread creation below. mScheduler->registerDisplay(display->getPhysicalId(), display->holdRefreshRateSelector()); if (FlagManager::getInstance().vrr_config()) { mScheduler->setRenderRate(display->getPhysicalId(), activeMode.fps); } - mScheduler->startTimers(); const auto configs = mScheduler->getVsyncConfiguration().getCurrentConfigs(); - mAppConnectionHandle = - mScheduler->createEventThread(Scheduler::Cycle::Render, - mFrameTimeline->getTokenManager(), - /* workDuration */ configs.late.appWorkDuration, - /* readyDuration */ configs.late.sfWorkDuration); - mSfConnectionHandle = - mScheduler->createEventThread(Scheduler::Cycle::LastComposite, - mFrameTimeline->getTokenManager(), - /* workDuration */ activeRefreshRate.getPeriod(), - /* readyDuration */ configs.late.sfWorkDuration); + mScheduler->createEventThread(scheduler::Cycle::Render, mFrameTimeline->getTokenManager(), + /* workDuration */ configs.late.appWorkDuration, + /* readyDuration */ configs.late.sfWorkDuration); + mScheduler->createEventThread(scheduler::Cycle::LastComposite, + mFrameTimeline->getTokenManager(), + /* workDuration */ activeRefreshRate.getPeriod(), + /* readyDuration */ configs.late.sfWorkDuration); mScheduler->initVsync(*mFrameTimeline->getTokenManager(), configs.late.sfWorkDuration); @@ -4379,6 +4411,9 @@ void SurfaceFlinger::initScheduler(const sp<const DisplayDevice>& display) { sp<RegionSamplingThread>::make(*this, RegionSamplingThread::EnvironmentTimingTunables()); mFpsReporter = sp<FpsReporter>::make(*mFrameTimeline); + + // Timer callbacks may fire, so do this last. + mScheduler->startTimers(); } void SurfaceFlinger::doCommitTransactions() { @@ -5105,6 +5140,11 @@ status_t SurfaceFlinger::setTransactionState( const auto frameHint = state.isFrameActive() ? FrameHint::kActive : FrameHint::kNone; mTransactionHandler.queueTransaction(std::move(state)); + for (const auto& [displayId, data] : mNotifyExpectedPresentMap) { + if (data.hintStatus.load() == NotifyExpectedPresentHintStatus::ScheduleOnTx) { + scheduleNotifyExpectedPresentHint(displayId, VsyncId{frameTimelineInfo.vsyncId}); + } + } setTransactionFlags(eTransactionFlushNeeded, schedule, applyToken, frameHint); return NO_ERROR; } @@ -6315,7 +6355,7 @@ void SurfaceFlinger::dumpScheduler(std::string& result) const { } void SurfaceFlinger::dumpEvents(std::string& result) const { - mScheduler->dump(mAppConnectionHandle, result); + mScheduler->dump(scheduler::Cycle::Render, result); } void SurfaceFlinger::dumpVsync(std::string& result) const { @@ -7087,14 +7127,15 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r mForceFullDamage = n != 0; return NO_ERROR; } - case 1018: { // Modify Choreographer's duration + case 1018: { // Set the render deadline as a duration until VSYNC. n = data.readInt32(); - mScheduler->setDuration(mAppConnectionHandle, std::chrono::nanoseconds(n), 0ns); + mScheduler->setDuration(scheduler::Cycle::Render, std::chrono::nanoseconds(n), 0ns); return NO_ERROR; } - case 1019: { // Modify SurfaceFlinger's duration + case 1019: { // Set the deadline of the last composite as a duration until VSYNC. n = data.readInt32(); - mScheduler->setDuration(mSfConnectionHandle, std::chrono::nanoseconds(n), 0ns); + mScheduler->setDuration(scheduler::Cycle::LastComposite, + std::chrono::nanoseconds(n), 0ns); return NO_ERROR; } case 1020: { // Unused @@ -7326,7 +7367,7 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r auto inUid = static_cast<uid_t>(data.readInt32()); const auto refreshRate = data.readFloat(); mScheduler->setPreferredRefreshRateForUid(FrameRateOverride{inUid, refreshRate}); - mScheduler->onFrameRateOverridesChanged(mAppConnectionHandle, displayId); + mScheduler->onFrameRateOverridesChanged(scheduler::Cycle::Render, displayId); return NO_ERROR; } // Toggle caching feature @@ -8466,10 +8507,10 @@ status_t SurfaceFlinger::applyRefreshRateSelectorPolicy( // TODO(b/140204874): Leave the event in until we do proper testing with all apps that might // be depending in this callback. if (const auto activeMode = selector.getActiveMode(); displayId == mActiveDisplayId) { - mScheduler->onPrimaryDisplayModeChanged(mAppConnectionHandle, activeMode); + mScheduler->onPrimaryDisplayModeChanged(scheduler::Cycle::Render, activeMode); toggleKernelIdleTimer(); } else { - mScheduler->onNonPrimaryDisplayModeChanged(mAppConnectionHandle, activeMode); + mScheduler->onNonPrimaryDisplayModeChanged(scheduler::Cycle::Render, activeMode); } auto preferredModeOpt = getPreferredDisplayMode(displayId, currentPolicy.defaultMode); @@ -8657,7 +8698,7 @@ status_t SurfaceFlinger::setGameModeFrameRateOverride(uid_t uid, float frameRate }(); mScheduler->setGameModeFrameRateForUid(FrameRateOverride{static_cast<uid_t>(uid), frameRate}); - mScheduler->onFrameRateOverridesChanged(mAppConnectionHandle, displayId); + mScheduler->onFrameRateOverridesChanged(scheduler::Cycle::Render, displayId); return NO_ERROR; } @@ -8813,7 +8854,11 @@ void SurfaceFlinger::sample() { return; } - mRegionSamplingThread->onCompositionComplete(mScheduler->getScheduledFrameTime()); + const auto scheduledFrameResultOpt = mScheduler->getScheduledFrameResult(); + const auto scheduleFrameTimeOpt = scheduledFrameResultOpt + ? std::optional{scheduledFrameResultOpt->callbackTime} + : std::nullopt; + mRegionSamplingThread->onCompositionComplete(scheduleFrameTimeOpt); } void SurfaceFlinger::onActiveDisplaySizeChanged(const DisplayDevice& activeDisplay) { @@ -8919,7 +8964,8 @@ void SurfaceFlinger::updateHdcpLevels(hal::HWDisplayId hwcDisplayId, int32_t con Mutex::Autolock lock(mStateLock); display->setSecure(connectedLevel >= 2 /* HDCP_V1 */); } - mScheduler->onHdcpLevelsChanged(mAppConnectionHandle, displayId, connectedLevel, maxLevel); + mScheduler->onHdcpLevelsChanged(scheduler::Cycle::Render, displayId, connectedLevel, + maxLevel); })); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index be057979f9..bada8292fd 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1356,12 +1356,7 @@ private: const std::string mHwcServiceName; - /* - * Scheduler - */ std::unique_ptr<scheduler::Scheduler> mScheduler; - scheduler::ConnectionHandle mAppConnectionHandle; - scheduler::ConnectionHandle mSfConnectionHandle; scheduler::PresentLatencyTracker mPresentLatencyTracker GUARDED_BY(kMainThreadContext); @@ -1500,7 +1495,9 @@ private: std::unordered_map<PhysicalDisplayId, NotifyExpectedPresentData> mNotifyExpectedPresentMap; void sendNotifyExpectedPresentHint(PhysicalDisplayId displayId) override REQUIRES(kMainThreadContext); - void scheduleNotifyExpectedPresentHint(PhysicalDisplayId displayId); + void scheduleNotifyExpectedPresentHint(PhysicalDisplayId displayId, + VsyncId vsyncId = VsyncId{ + FrameTimelineInfo::INVALID_VSYNC_ID}); void notifyExpectedPresentIfRequired(PhysicalDisplayId, Period vsyncPeriod, TimePoint expectedPresentTime, Fps frameInterval, std::optional<Period> timeoutOpt); diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index d5ec654c59..3eabe1f362 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -117,14 +117,16 @@ protected: mThread->onVsync(expectedPresentationTime, timestamp, deadlineTimestamp); } + static constexpr scheduler::ScheduleResult kScheduleResult{TimePoint::fromNs(0), + TimePoint::fromNs(0)}; AsyncCallRecorderWithCannedReturn< scheduler::ScheduleResult (*)(scheduler::VSyncDispatch::CallbackToken, scheduler::VSyncDispatch::ScheduleTiming)> - mVSyncCallbackScheduleRecorder{0}; + mVSyncCallbackScheduleRecorder{kScheduleResult}; AsyncCallRecorderWithCannedReturn< scheduler::ScheduleResult (*)(scheduler::VSyncDispatch::CallbackToken, scheduler::VSyncDispatch::ScheduleTiming)> - mVSyncCallbackUpdateRecorder{0}; + mVSyncCallbackUpdateRecorder{kScheduleResult}; AsyncCallRecorderWithCannedReturn< scheduler::VSyncDispatch::CallbackToken (*)(scheduler::VSyncDispatch::Callback, std::string)> diff --git a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp index f5661fccf5..71f9f88ba7 100644 --- a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp +++ b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp @@ -25,6 +25,7 @@ #include "FrameTimeline.h" #include "Scheduler/MessageQueue.h" #include "mock/MockVSyncDispatch.h" +#include "utils/Timers.h" namespace android { @@ -49,6 +50,8 @@ class TestableMessageQueue : public impl::MessageQueue { using MessageQueue::Handler::Handler; MOCK_METHOD(void, dispatchFrame, (VsyncId, TimePoint), (override)); + MOCK_METHOD(bool, isFramePending, (), (const, override)); + MOCK_METHOD(TimePoint, getExpectedVsyncTime, (), (const override)); }; explicit TestableMessageQueue(sp<MockHandler> handler) @@ -94,13 +97,17 @@ TEST_F(MessageQueueTest, commit) { const auto timing = scheduler::VSyncDispatch::ScheduleTiming{.workDuration = kDuration.ns(), .readyDuration = 0, .lastVsync = 0}; - EXPECT_FALSE(mEventQueue.getScheduledFrameTime()); + EXPECT_FALSE(mEventQueue.getScheduledFrameResult()); - EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(1234)); + const auto timePoint = TimePoint::fromNs(1234); + const auto scheduleResult = scheduler::ScheduleResult{timePoint, timePoint}; + EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(scheduleResult)); EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleFrame()); - ASSERT_TRUE(mEventQueue.getScheduledFrameTime()); - EXPECT_EQ(1234, mEventQueue.getScheduledFrameTime()->time_since_epoch().count()); + const auto scheduledFrameResult = mEventQueue.getScheduledFrameResult(); + ASSERT_TRUE(scheduledFrameResult); + EXPECT_EQ(1234, scheduledFrameResult->callbackTime.ns()); + EXPECT_EQ(1234, scheduledFrameResult->vsyncTime.ns()); } TEST_F(MessageQueueTest, commitTwice) { @@ -109,17 +116,25 @@ TEST_F(MessageQueueTest, commitTwice) { .readyDuration = 0, .lastVsync = 0}; - EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(1234)); + auto timePoint = TimePoint::fromNs(1234); + auto scheduleResult = scheduler::ScheduleResult{timePoint, timePoint}; + EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(scheduleResult)); EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleFrame()); - ASSERT_TRUE(mEventQueue.getScheduledFrameTime()); - EXPECT_EQ(1234, mEventQueue.getScheduledFrameTime()->time_since_epoch().count()); + auto scheduledFrameResult = mEventQueue.getScheduledFrameResult(); + ASSERT_TRUE(scheduledFrameResult); + EXPECT_EQ(1234, scheduledFrameResult->callbackTime.ns()); + EXPECT_EQ(1234, scheduledFrameResult->vsyncTime.ns()); - EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(4567)); + timePoint = TimePoint::fromNs(4567); + scheduleResult = scheduler::ScheduleResult{timePoint, timePoint}; + EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(scheduleResult)); EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleFrame()); - ASSERT_TRUE(mEventQueue.getScheduledFrameTime()); - EXPECT_EQ(4567, mEventQueue.getScheduledFrameTime()->time_since_epoch().count()); + scheduledFrameResult = mEventQueue.getScheduledFrameResult(); + ASSERT_TRUE(scheduledFrameResult); + EXPECT_EQ(4567, scheduledFrameResult->callbackTime.ns()); + EXPECT_EQ(4567, scheduledFrameResult->vsyncTime.ns()); } TEST_F(MessageQueueTest, commitTwiceWithCallback) { @@ -128,11 +143,15 @@ TEST_F(MessageQueueTest, commitTwiceWithCallback) { .readyDuration = 0, .lastVsync = 0}; - EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(1234)); + const auto timePoint = TimePoint::fromNs(1234); + auto scheduleResult = scheduler::ScheduleResult{timePoint, timePoint}; + EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(scheduleResult)); EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleFrame()); - ASSERT_TRUE(mEventQueue.getScheduledFrameTime()); - EXPECT_EQ(1234, mEventQueue.getScheduledFrameTime()->time_since_epoch().count()); + auto scheduledFrameResult = mEventQueue.getScheduledFrameResult(); + ASSERT_TRUE(scheduledFrameResult); + EXPECT_EQ(1234, scheduledFrameResult->callbackTime.ns()); + EXPECT_EQ(1234, scheduledFrameResult->vsyncTime.ns()); constexpr TimePoint kStartTime = TimePoint::fromNs(100); constexpr TimePoint kEndTime = kStartTime + kDuration; @@ -148,14 +167,15 @@ TEST_F(MessageQueueTest, commitTwiceWithCallback) { EXPECT_NO_FATAL_FAILURE( mEventQueue.vsyncCallback(kPresentTime.ns(), kStartTime.ns(), kEndTime.ns())); - EXPECT_FALSE(mEventQueue.getScheduledFrameTime()); + EXPECT_FALSE(mEventQueue.getScheduledFrameResult()); const auto timingAfterCallback = scheduler::VSyncDispatch::ScheduleTiming{.workDuration = kDuration.ns(), .readyDuration = 0, .lastVsync = kPresentTime.ns()}; - - EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timingAfterCallback)).WillOnce(Return(0)); + scheduleResult = scheduler::ScheduleResult{TimePoint::fromNs(0), TimePoint::fromNs(0)}; + EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timingAfterCallback)) + .WillOnce(Return(scheduleResult)); EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleFrame()); } @@ -167,9 +187,24 @@ TEST_F(MessageQueueTest, commitWithDurationChange) { .readyDuration = 0, .lastVsync = 0}; - EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(0)); + const auto scheduleResult = + scheduler::ScheduleResult{TimePoint::fromNs(0), TimePoint::fromNs(0)}; + EXPECT_CALL(*mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(scheduleResult)); EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleFrame()); } +TEST_F(MessageQueueTest, scheduleResultWhenFrameIsPending) { + const auto timePoint = TimePoint::now(); + EXPECT_CALL(*mEventQueue.mHandler, isFramePending()).WillOnce(Return(true)); + EXPECT_CALL(*mEventQueue.mHandler, getExpectedVsyncTime()).WillRepeatedly(Return(timePoint)); + + const auto scheduledFrameResult = mEventQueue.getScheduledFrameResult(); + + ASSERT_TRUE(scheduledFrameResult); + EXPECT_NEAR(static_cast<double>(TimePoint::now().ns()), + static_cast<double>(scheduledFrameResult->callbackTime.ns()), ms2ns(1)); + EXPECT_EQ(timePoint, scheduledFrameResult->vsyncTime); +} + } // namespace } // namespace android diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp index b0595257a9..10e2220ece 100644 --- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp +++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp @@ -99,7 +99,6 @@ protected: TestableScheduler* mScheduler = new TestableScheduler{mSelector, mFlinger, mSchedulerCallback}; surfaceflinger::frontend::LayerHierarchyBuilder mLayerHierarchyBuilder; - ConnectionHandle mConnectionHandle; MockEventThread* mEventThread; sp<MockEventThreadConnection> mEventThreadConnection; }; @@ -116,54 +115,13 @@ SchedulerTest::SchedulerTest() { EXPECT_CALL(*mEventThread, createEventConnection(_, _)) .WillRepeatedly(Return(mEventThreadConnection)); - mConnectionHandle = mScheduler->createConnection(std::move(eventThread)); - EXPECT_TRUE(mConnectionHandle); + mScheduler->setEventThread(Cycle::Render, std::move(eventThread)); mFlinger.resetScheduler(mScheduler); } } // namespace -TEST_F(SchedulerTest, invalidConnectionHandle) { - ConnectionHandle handle; - - const sp<IDisplayEventConnection> connection = mScheduler->createDisplayEventConnection(handle); - - EXPECT_FALSE(connection); - EXPECT_FALSE(mScheduler->getEventConnection(handle)); - - // The EXPECT_CALLS make sure we don't call the functions on the subsequent event threads. - EXPECT_CALL(*mEventThread, onHotplugReceived(_, _)).Times(0); - mScheduler->onHotplugReceived(handle, kDisplayId1, false); - - std::string output; - EXPECT_CALL(*mEventThread, dump(_)).Times(0); - mScheduler->dump(handle, output); - EXPECT_TRUE(output.empty()); - - EXPECT_CALL(*mEventThread, setDuration(10ns, 20ns)).Times(0); - mScheduler->setDuration(handle, 10ns, 20ns); -} - -TEST_F(SchedulerTest, validConnectionHandle) { - const sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle); - - ASSERT_EQ(mEventThreadConnection, connection); - EXPECT_TRUE(mScheduler->getEventConnection(mConnectionHandle)); - - EXPECT_CALL(*mEventThread, onHotplugReceived(kDisplayId1, false)).Times(1); - mScheduler->onHotplugReceived(mConnectionHandle, kDisplayId1, false); - - std::string output("dump"); - EXPECT_CALL(*mEventThread, dump(output)).Times(1); - mScheduler->dump(mConnectionHandle, output); - EXPECT_FALSE(output.empty()); - - EXPECT_CALL(*mEventThread, setDuration(10ns, 20ns)).Times(1); - mScheduler->setDuration(mConnectionHandle, 10ns, 20ns); -} - TEST_F(SchedulerTest, registerDisplay) FTL_FAKE_GUARD(kMainThreadContext) { // Hardware VSYNC should not change if the display is already registered. EXPECT_CALL(mSchedulerCallback, requestHardwareVsync(kDisplayId1, false)).Times(0); @@ -235,22 +193,6 @@ TEST_F(SchedulerTest, dispatchCachedReportedMode) { EXPECT_NO_FATAL_FAILURE(mScheduler->dispatchCachedReportedMode()); } -TEST_F(SchedulerTest, onNonPrimaryDisplayModeChanged_invalidParameters) { - const auto mode = DisplayMode::Builder(hal::HWConfigId(0)) - .setId(DisplayModeId(111)) - .setPhysicalDisplayId(kDisplayId1) - .setVsyncPeriod(111111) - .build(); - - // If the handle is incorrect, the function should return before - // onModeChange is called. - ConnectionHandle invalidHandle = {.id = 123}; - EXPECT_NO_FATAL_FAILURE( - mScheduler->onNonPrimaryDisplayModeChanged(invalidHandle, - {90_Hz, ftl::as_non_null(mode)})); - EXPECT_CALL(*mEventThread, onModeChanged(_)).Times(0); -} - TEST_F(SchedulerTest, calculateMaxAcquiredBufferCount) { EXPECT_EQ(1, mFlinger.calculateMaxAcquiredBufferCount(60_Hz, 30ms)); EXPECT_EQ(2, mFlinger.calculateMaxAcquiredBufferCount(90_Hz, 30ms)); @@ -753,7 +695,7 @@ TEST_F(AttachedChoreographerTest, registerSingle) { EXPECT_CALL(mSchedulerCallback, onChoreographerAttached); const sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, layer->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, layer->getHandle()); EXPECT_EQ(1u, mScheduler->mutableAttachedChoreographers().size()); ASSERT_EQ(1u, mScheduler->mutableAttachedChoreographers().count(layer->getSequence())); @@ -782,9 +724,9 @@ TEST_F(AttachedChoreographerTest, registerMultipleOnSameLayer) { .WillOnce(Return(mockConnection2)); const sp<IDisplayEventConnection> connection1 = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, handle); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, handle); const sp<IDisplayEventConnection> connection2 = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, handle); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, handle); EXPECT_EQ(1u, mScheduler->mutableAttachedChoreographers().size()); ASSERT_EQ(1u, mScheduler->mutableAttachedChoreographers().count(layer->getSequence())); @@ -802,9 +744,9 @@ TEST_F(AttachedChoreographerTest, registerMultipleOnDifferentLayers) { EXPECT_CALL(mSchedulerCallback, onChoreographerAttached).Times(2); const sp<IDisplayEventConnection> connection1 = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, layer1->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, layer1->getHandle()); const sp<IDisplayEventConnection> connection2 = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, layer2->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, layer2->getHandle()); EXPECT_EQ(2u, mScheduler->mutableAttachedChoreographers().size()); @@ -831,7 +773,7 @@ TEST_F(AttachedChoreographerTest, removedWhenConnectionIsGone) { EXPECT_CALL(mSchedulerCallback, onChoreographerAttached); sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, layer->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, layer->getHandle()); ASSERT_EQ(1u, mScheduler->mutableAttachedChoreographers().count(layer->getSequence())); EXPECT_EQ(1u, @@ -861,7 +803,7 @@ TEST_F(AttachedChoreographerTest, removedWhenLayerIsGone) { EXPECT_CALL(mSchedulerCallback, onChoreographerAttached); const sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, layer->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, layer->getHandle()); layer.clear(); mFlinger.mutableLayersPendingRemoval().clear(); @@ -875,7 +817,7 @@ void AttachedChoreographerTest::frameRateTestScenario(Fps layerFps, int8_t frame EXPECT_CALL(mSchedulerCallback, onChoreographerAttached); sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, layer->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, layer->getHandle()); RequestedLayerState layerState(LayerCreationArgs(layer->getSequence())); LayerHierarchy hierarchy(&layerState); @@ -935,7 +877,7 @@ TEST_F(AttachedChoreographerTest, setsFrameRateParent) { EXPECT_CALL(mSchedulerCallback, onChoreographerAttached); sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, parent->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, parent->getHandle()); RequestedLayerState parentState(LayerCreationArgs(parent->getSequence())); LayerHierarchy parentHierarchy(&parentState); @@ -962,7 +904,7 @@ TEST_F(AttachedChoreographerTest, setsFrameRateParent2Children) { EXPECT_CALL(mSchedulerCallback, onChoreographerAttached); sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, parent->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, parent->getHandle()); RequestedLayerState parentState(LayerCreationArgs(parent->getSequence())); LayerHierarchy parentHierarchy(&parentState); @@ -997,7 +939,7 @@ TEST_F(AttachedChoreographerTest, setsFrameRateParentConflictingChildren) { EXPECT_CALL(mSchedulerCallback, onChoreographerAttached); sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, parent->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, parent->getHandle()); RequestedLayerState parentState(LayerCreationArgs(parent->getSequence())); LayerHierarchy parentHierarchy(&parentState); @@ -1031,7 +973,7 @@ TEST_F(AttachedChoreographerTest, setsFrameRateChild) { EXPECT_CALL(mSchedulerCallback, onChoreographerAttached); sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, layer->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, layer->getHandle()); RequestedLayerState parentState(LayerCreationArgs(parent->getSequence())); LayerHierarchy parentHierarchy(&parentState); @@ -1057,7 +999,7 @@ TEST_F(AttachedChoreographerTest, setsFrameRateChildNotOverriddenByParent) { EXPECT_CALL(mSchedulerCallback, onChoreographerAttached); sp<IDisplayEventConnection> connection = - mScheduler->createDisplayEventConnection(mConnectionHandle, {}, layer->getHandle()); + mScheduler->createDisplayEventConnection(Cycle::Render, {}, layer->getHandle()); RequestedLayerState parentState(LayerCreationArgs(parent->getSequence())); LayerHierarchy parentHierarchy(&parentState); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp index 91b901838c..20a3315169 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_NotifyExpectedPresentTest.cpp @@ -41,6 +41,33 @@ public: } protected: + void setTransactionState() { + ASSERT_TRUE(mFlinger.getTransactionQueue().isEmpty()); + TransactionInfo transaction; + mFlinger.setTransactionState(FrameTimelineInfo{}, transaction.states, transaction.displays, + transaction.flags, transaction.applyToken, + transaction.inputWindowCommands, + TimePoint::now().ns() + s2ns(1), transaction.isAutoTimestamp, + transaction.unCachedBuffers, + /*HasListenerCallbacks=*/false, transaction.callbacks, + transaction.id, transaction.mergedTransactionIds); + } + + struct TransactionInfo { + Vector<ComposerState> states; + Vector<DisplayState> displays; + uint32_t flags = 0; + sp<IBinder> applyToken = IInterface::asBinder(TransactionCompletedListener::getIInstance()); + InputWindowCommands inputWindowCommands; + int64_t desiredPresentTime = 0; + bool isAutoTimestamp = false; + FrameTimelineInfo frameTimelineInfo{}; + std::vector<client_cache_t> unCachedBuffers; + uint64_t id = static_cast<uint64_t>(-1); + std::vector<uint64_t> mergedTransactionIds; + std::vector<ListenerCallbacks> callbacks; + }; + struct Compositor final : ICompositor { explicit Compositor(PhysicalDisplayId displayId, TestableSurfaceFlinger& surfaceFlinger) : displayId(displayId), surfaceFlinger(surfaceFlinger) {} @@ -102,7 +129,7 @@ protected: }; TEST_F(NotifyExpectedPresentTest, noNotifyExpectedPresentHintCall_absentTimeout) { - auto expectedPresentTime = systemTime() + ms2ns(10); + auto expectedPresentTime = TimePoint::now().ns() + ms2ns(10); ASSERT_NO_FATAL_FAILURE( mFlinger.setNotifyExpectedPresentData(mPhysicalDisplayId, TimePoint::fromNs(expectedPresentTime), @@ -120,7 +147,7 @@ TEST_F(NotifyExpectedPresentTest, noNotifyExpectedPresentHintCall_absentTimeout) } TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentHint_zeroTimeout) { - auto expectedPresentTime = systemTime() + ms2ns(10); + auto expectedPresentTime = TimePoint::now().ns() + ms2ns(10); { // Very first ExpectedPresent after idle, no previous timestamp. EXPECT_CALL(*mComposer, @@ -139,6 +166,10 @@ TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentHint_zeroTimeout) { { mCompositor->committed = false; expectedPresentTime += kFrameInterval60HzNs; + EXPECT_CALL(static_cast<mock::VSyncTracker&>( + mFlinger.scheduler()->getVsyncSchedule()->getTracker()), + nextAnticipatedVSyncTimeFrom(_, _)) + .WillRepeatedly(Return(expectedPresentTime)); EXPECT_CALL(*mComposer, notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, kFrameInterval60HzNs)) .WillOnce(Return(Error::NONE)); @@ -154,6 +185,10 @@ TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentHint_zeroTimeout) { } { expectedPresentTime += kFrameInterval60HzNs; + EXPECT_CALL(static_cast<mock::VSyncTracker&>( + mFlinger.scheduler()->getVsyncSchedule()->getTracker()), + nextAnticipatedVSyncTimeFrom(_, _)) + .WillRepeatedly(Return(expectedPresentTime)); EXPECT_CALL(*mComposer, notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, kFrameInterval60HzNs)) .WillOnce(Return(Error::NONE)); @@ -168,9 +203,8 @@ TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentHint_zeroTimeout) { ASSERT_TRUE(mFlinger.verifyHintIsSent(mPhysicalDisplayId)); } } - TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentTimeout) { - auto expectedPresentTime = systemTime() + ms2ns(10); + auto expectedPresentTime = TimePoint::now().ns() + ms2ns(10); { // Very first ExpectedPresent after idle, no previous timestamp mCompositor->committed = false; @@ -185,6 +219,27 @@ TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentTimeout) { ASSERT_TRUE(mFlinger.verifyHintIsSent(mPhysicalDisplayId)); } { + EXPECT_CALL(*mComposer, notifyExpectedPresent(kHwcDisplayId, _, _)).Times(0); + expectedPresentTime += 2 * kFrameInterval5HzNs; + mFlinger.notifyExpectedPresentIfRequired(mPhysicalDisplayId, kVsyncPeriod, + TimePoint::fromNs(expectedPresentTime), kFps60Hz, + kTimeoutNs); + EXPECT_TRUE( + mFlinger.verifyLastExpectedPresentTime(mPhysicalDisplayId, expectedPresentTime)); + ASSERT_TRUE(mFlinger.verifyHintStatusIsScheduledOnTx(mPhysicalDisplayId)); + mFlinger.scheduler()->doFrameSignal(*mCompositor, VsyncId{42}); + ASSERT_TRUE(mFlinger.verifyHintStatusIsScheduledOnTx(mPhysicalDisplayId)); + { + EXPECT_CALL(*mComposer, + notifyExpectedPresent(kHwcDisplayId, expectedPresentTime, + kFrameInterval60HzNs)) + .WillOnce(Return(Error::NONE)); + // Hint sent with the setTransactionState + setTransactionState(); + ASSERT_TRUE(mFlinger.verifyHintIsSent(mPhysicalDisplayId)); + } + } + { // ExpectedPresentTime is after the timeoutNs mCompositor->committed = true; expectedPresentTime += 2 * kFrameInterval5HzNs; @@ -194,7 +249,7 @@ TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentTimeout) { kTimeoutNs); EXPECT_TRUE( mFlinger.verifyLastExpectedPresentTime(mPhysicalDisplayId, expectedPresentTime)); - ASSERT_TRUE(mFlinger.verifyHintIsSent(mPhysicalDisplayId)); + ASSERT_TRUE(mFlinger.verifyHintStatusIsScheduledOnTx(mPhysicalDisplayId)); mFlinger.scheduler()->doFrameSignal(*mCompositor, VsyncId{42}); // Present happens notifyExpectedPresentHintStatus is Start ASSERT_TRUE(mFlinger.verifyHintStatusIsStart(mPhysicalDisplayId)); @@ -259,7 +314,7 @@ TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentTimeout) { } TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentRenderRateChanged) { - const auto now = systemTime(); + const auto now = TimePoint::now().ns(); auto expectedPresentTime = now; static constexpr Period kTimeoutNs = Period::fromNs(static_cast<Fps>(1_Hz).getPeriodNsecs()); @@ -298,6 +353,10 @@ TEST_F(NotifyExpectedPresentTest, notifyExpectedPresentRenderRateChanged) { TimePoint::fromNs(expectedPresentTime), Fps::fromPeriodNsecs(frameIntervalNs), kTimeoutNs); + EXPECT_CALL(static_cast<mock::VSyncTracker&>( + mFlinger.scheduler()->getVsyncSchedule()->getTracker()), + nextAnticipatedVSyncTimeFrom(_, _)) + .WillRepeatedly(Return(expectedPresentTime)); if (callNotifyExpectedPresentHint) { mCompositor->committed = false; ASSERT_TRUE(mFlinger.verifyHintIsScheduledOnPresent(mPhysicalDisplayId)) diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h index 25a85dfa20..1472ebf009 100644 --- a/services/surfaceflinger/tests/unittests/TestableScheduler.h +++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h @@ -71,9 +71,14 @@ public: Scheduler::onFrameSignal(compositor, vsyncId, TimePoint()); } - // Used to inject mock event thread. - ConnectionHandle createConnection(std::unique_ptr<EventThread> eventThread) { - return Scheduler::createConnection(std::move(eventThread)); + void setEventThread(Cycle cycle, std::unique_ptr<EventThread> eventThreadPtr) { + if (cycle == Cycle::Render) { + mRenderEventThread = std::move(eventThreadPtr); + mRenderEventConnection = mRenderEventThread->createEventConnection(); + } else { + mLastCompositeEventThread = std::move(eventThreadPtr); + mLastCompositeEventConnection = mLastCompositeEventThread->createEventConnection(); + } } auto refreshRateSelector() { return pacesetterSelectorPtr(); } @@ -124,7 +129,6 @@ public: using Scheduler::resyncAllToHardwareVsync; - auto& mutableAppConnectionHandle() { return mAppConnectionHandle; } auto& mutableLayerHistory() { return mLayerHistory; } auto& mutableAttachedChoreographers() { return mAttachedChoreographers; } @@ -180,10 +184,6 @@ public: mPolicy.cachedModeChangedParams.reset(); } - void onNonPrimaryDisplayModeChanged(ConnectionHandle handle, const FrameRateMode& mode) { - Scheduler::onNonPrimaryDisplayModeChanged(handle, mode); - } - void setInitialHwVsyncEnabled(PhysicalDisplayId id, bool enabled) { auto schedule = getVsyncSchedule(id); std::lock_guard<std::mutex> lock(schedule->mHwVsyncLock); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 46a079cfa1..bce7729d80 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -258,11 +258,9 @@ public: mScheduler->initVsync(*mTokenManager, 0ms); - mScheduler->mutableAppConnectionHandle() = - mScheduler->createConnection(std::move(appEventThread)); + mScheduler->setEventThread(scheduler::Cycle::Render, std::move(appEventThread)); + mScheduler->setEventThread(scheduler::Cycle::LastComposite, std::move(sfEventThread)); - mFlinger->mAppConnectionHandle = mScheduler->mutableAppConnectionHandle(); - mFlinger->mSfConnectionHandle = mScheduler->createConnection(std::move(sfEventThread)); resetScheduler(mScheduler); } diff --git a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp index eb4e84ef4f..9b70d92eac 100644 --- a/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp +++ b/services/surfaceflinger/tests/unittests/VSyncDispatchTimerQueueTest.cpp @@ -247,7 +247,8 @@ TEST_F(VSyncDispatchTimerQueueTest, unregistersSetAlarmOnDestruction) { mDispatch->schedule(cb, {.workDuration = 100, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(900, *result); + EXPECT_EQ(900, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); } } @@ -260,7 +261,8 @@ TEST_F(VSyncDispatchTimerQueueTest, basicAlarmSettingFuture) { mDispatch->schedule(cb, {.workDuration = 100, .readyDuration = 0, .lastVsync = intended}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(900, *result); + EXPECT_EQ(900, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); advanceToNextCallback(); @@ -279,12 +281,14 @@ TEST_F(VSyncDispatchTimerQueueTest, updateAlarmSettingFuture) { mDispatch->schedule(cb, {.workDuration = 100, .readyDuration = 0, .lastVsync = intended}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(900, *result); + EXPECT_EQ(900, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); result = mDispatch->update(cb, {.workDuration = 300, .readyDuration = 0, .lastVsync = intended}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(700, *result); + EXPECT_EQ(700, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); advanceToNextCallback(); @@ -332,7 +336,8 @@ TEST_F(VSyncDispatchTimerQueueTest, basicAlarmSettingAdjustmentPast) { .readyDuration = 0, .lastVsync = mPeriod}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(mPeriod, *result); + EXPECT_EQ(mPeriod, result->callbackTime.ns()); + EXPECT_EQ(workDuration + mPeriod, result->vsyncTime.ns()); } TEST_F(VSyncDispatchTimerQueueTest, basicAlarmCancel) { @@ -344,7 +349,8 @@ TEST_F(VSyncDispatchTimerQueueTest, basicAlarmCancel) { mDispatch->schedule(cb, {.workDuration = 100, .readyDuration = 0, .lastVsync = mPeriod}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(mPeriod - 100, *result); + EXPECT_EQ(mPeriod - 100, result->callbackTime.ns()); + EXPECT_EQ(mPeriod, result->vsyncTime.ns()); EXPECT_EQ(mDispatch->cancel(cb), CancelResult::Cancelled); } @@ -357,7 +363,8 @@ TEST_F(VSyncDispatchTimerQueueTest, basicAlarmCancelTooLate) { mDispatch->schedule(cb, {.workDuration = 100, .readyDuration = 0, .lastVsync = mPeriod}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(mPeriod - 100, *result); + EXPECT_EQ(mPeriod - 100, result->callbackTime.ns()); + EXPECT_EQ(mPeriod, result->vsyncTime.ns()); mMockClock.advanceBy(950); EXPECT_EQ(mDispatch->cancel(cb), CancelResult::TooLate); } @@ -371,7 +378,8 @@ TEST_F(VSyncDispatchTimerQueueTest, basicAlarmCancelTooLateWhenRunning) { mDispatch->schedule(cb, {.workDuration = 100, .readyDuration = 0, .lastVsync = mPeriod}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(mPeriod - 100, *result); + EXPECT_EQ(mPeriod - 100, result->callbackTime.ns()); + EXPECT_EQ(mPeriod, result->vsyncTime.ns()); std::thread pausingThread([&] { mMockClock.advanceToNextCallback(); }); EXPECT_TRUE(cb.waitForPause()); @@ -392,7 +400,8 @@ TEST_F(VSyncDispatchTimerQueueTest, unregisterSynchronizes) { mDispatch->schedule(cb, {.workDuration = 100, .readyDuration = 0, .lastVsync = mPeriod}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(mPeriod - 100, *result); + EXPECT_EQ(mPeriod - 100, result->callbackTime.ns()); + EXPECT_EQ(mPeriod, result->vsyncTime.ns()); std::thread pausingThread([&] { mMockClock.advanceToNextCallback(); }); EXPECT_TRUE(cb.waitForPause()); @@ -625,19 +634,22 @@ TEST_F(VSyncDispatchTimerQueueTest, callbackReentrantWithPastWakeup) { .readyDuration = 0, .lastVsync = timestamp - mVsyncMoveThreshold}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(mPeriod + timestamp - 400, *result); + EXPECT_EQ(mPeriod + timestamp - 400, result->callbackTime.ns()); + EXPECT_EQ(mPeriod + timestamp, result->vsyncTime.ns()); result = mDispatch->schedule(tmp, {.workDuration = 400, .readyDuration = 0, .lastVsync = timestamp}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(mPeriod + timestamp - 400, *result); + EXPECT_EQ(mPeriod + timestamp - 400, result->callbackTime.ns()); + EXPECT_EQ(mPeriod + timestamp, result->vsyncTime.ns()); result = mDispatch->schedule(tmp, {.workDuration = 400, .readyDuration = 0, .lastVsync = timestamp + mVsyncMoveThreshold}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(mPeriod + timestamp - 400, *result); + EXPECT_EQ(mPeriod + timestamp - 400, result->callbackTime.ns()); + EXPECT_EQ(mPeriod + timestamp, result->vsyncTime.ns()); lastTarget = timestamp; }, "oo"); @@ -726,10 +738,12 @@ TEST_F(VSyncDispatchTimerQueueTest, canMoveCallbackBackwardsInTime) { auto result = mDispatch->schedule(cb0, {.workDuration = 500, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(500, *result); + EXPECT_EQ(500, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); result = mDispatch->schedule(cb0, {.workDuration = 100, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(900, *result); + EXPECT_EQ(900, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); } // b/1450138150 @@ -741,12 +755,14 @@ TEST_F(VSyncDispatchTimerQueueTest, doesNotMoveCallbackBackwardsAndSkipASchedule auto result = mDispatch->schedule(cb, {.workDuration = 500, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(500, *result); + EXPECT_EQ(500, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.advanceBy(400); result = mDispatch->schedule(cb, {.workDuration = 800, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(1200, *result); + EXPECT_EQ(1200, result->callbackTime.ns()); + EXPECT_EQ(2000, result->vsyncTime.ns()); advanceToNextCallback(); ASSERT_THAT(cb.mCalls.size(), Eq(1)); @@ -763,12 +779,14 @@ TEST_F(VSyncDispatchTimerQueueTest, movesCallbackBackwardsAndSkipAScheduledTarge auto result = mDispatch->schedule(cb, {.workDuration = 500, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(500, *result); + EXPECT_EQ(500, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.advanceBy(400); result = mDispatch->schedule(cb, {.workDuration = 800, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(400, *result); + EXPECT_EQ(400, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); advanceToNextCallback(); ASSERT_THAT(cb.mCalls.size(), Eq(1)); @@ -784,11 +802,13 @@ TEST_F(VSyncDispatchTimerQueueTest, targetOffsetMovingBackALittleCanStillSchedul auto result = mDispatch->schedule(cb, {.workDuration = 500, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(500, *result); + EXPECT_EQ(500, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.advanceBy(400); result = mDispatch->schedule(cb, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(602, *result); + EXPECT_EQ(602, result->callbackTime.ns()); + EXPECT_EQ(1002, result->vsyncTime.ns()); } TEST_F(VSyncDispatchTimerQueueTest, canScheduleNegativeOffsetAgainstDifferentPeriods) { @@ -796,12 +816,14 @@ TEST_F(VSyncDispatchTimerQueueTest, canScheduleNegativeOffsetAgainstDifferentPer auto result = mDispatch->schedule(cb0, {.workDuration = 500, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(500, *result); + EXPECT_EQ(500, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); advanceToNextCallback(); result = mDispatch->schedule(cb0, {.workDuration = 1100, .readyDuration = 0, .lastVsync = 2000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(900, *result); + EXPECT_EQ(900, result->callbackTime.ns()); + EXPECT_EQ(2000, result->vsyncTime.ns()); } TEST_F(VSyncDispatchTimerQueueTest, canScheduleLargeNegativeOffset) { @@ -812,12 +834,14 @@ TEST_F(VSyncDispatchTimerQueueTest, canScheduleLargeNegativeOffset) { auto result = mDispatch->schedule(cb0, {.workDuration = 500, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(500, *result); + EXPECT_EQ(500, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); advanceToNextCallback(); result = mDispatch->schedule(cb0, {.workDuration = 1900, .readyDuration = 0, .lastVsync = 2000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(1100, *result); + EXPECT_EQ(1100, result->callbackTime.ns()); + EXPECT_EQ(3000, result->vsyncTime.ns()); } TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesNotAffectSchedulingState) { @@ -829,11 +853,13 @@ TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesNotAffectSchedulingState) auto result = mDispatch->schedule(cb, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); result = mDispatch->schedule(cb, {.workDuration = 1400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(2000, result->vsyncTime.ns()); advanceToNextCallback(); } @@ -849,11 +875,13 @@ TEST_F(VSyncDispatchTimerQueueTest, scheduleUpdatesDoesAffectSchedulingState) { auto result = mDispatch->schedule(cb, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); result = mDispatch->schedule(cb, {.workDuration = 1400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(0, *result); + EXPECT_EQ(0, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); advanceToNextCallback(); } @@ -899,14 +927,16 @@ TEST_F(VSyncDispatchTimerQueueTest, skipsSchedulingIfTimerReschedulingIsImminent auto result = mDispatch->schedule(cb1, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.setLag(100); mMockClock.advanceBy(620); result = mDispatch->schedule(cb2, {.workDuration = 100, .readyDuration = 0, .lastVsync = 2000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(1900, *result); + EXPECT_EQ(1900, result->callbackTime.ns()); + EXPECT_EQ(2000, result->vsyncTime.ns()); mMockClock.advanceBy(80); EXPECT_THAT(cb1.mCalls.size(), Eq(1)); @@ -927,14 +957,16 @@ TEST_F(VSyncDispatchTimerQueueTest, skipsSchedulingIfTimerReschedulingIsImminent auto result = mDispatch->schedule(cb, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.setLag(100); mMockClock.advanceBy(620); result = mDispatch->schedule(cb, {.workDuration = 370, .readyDuration = 0, .lastVsync = 2000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(1630, *result); + EXPECT_EQ(1630, result->callbackTime.ns()); + EXPECT_EQ(2000, result->vsyncTime.ns()); mMockClock.advanceBy(80); EXPECT_THAT(cb.mCalls.size(), Eq(1)); @@ -954,14 +986,16 @@ TEST_F(VSyncDispatchTimerQueueTest, doesntSkipSchedulingIfTimerReschedulingIsImm auto result = mDispatch->schedule(cb, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.setLag(100); mMockClock.advanceBy(620); result = mDispatch->schedule(cb, {.workDuration = 370, .readyDuration = 0, .lastVsync = 2000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.advanceBy(80); ASSERT_EQ(1, cb.mCalls.size()); @@ -982,10 +1016,12 @@ TEST_F(VSyncDispatchTimerQueueTest, skipsRearmingWhenNotNextScheduled) { auto result = mDispatch->schedule(cb1, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); result = mDispatch->schedule(cb2, {.workDuration = 100, .readyDuration = 0, .lastVsync = 2000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(1900, *result); + EXPECT_EQ(1900, result->callbackTime.ns()); + EXPECT_EQ(2000, result->vsyncTime.ns()); mMockClock.setLag(100); mMockClock.advanceBy(620); @@ -1009,10 +1045,12 @@ TEST_F(VSyncDispatchTimerQueueTest, rearmsWhenCancelledAndIsNextScheduled) { auto result = mDispatch->schedule(cb1, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); result = mDispatch->schedule(cb2, {.workDuration = 100, .readyDuration = 0, .lastVsync = 2000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(1900, *result); + EXPECT_EQ(1900, result->callbackTime.ns()); + EXPECT_EQ(2000, result->vsyncTime.ns()); mMockClock.setLag(100); mMockClock.advanceBy(620); @@ -1045,10 +1083,12 @@ TEST_F(VSyncDispatchTimerQueueTest, laggedTimerGroupsCallbacksWithinLag) { auto result = mDispatch->schedule(cb1, {.workDuration = 400, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(600, *result); + EXPECT_EQ(600, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); result = mDispatch->schedule(cb2, {.workDuration = 390, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(610, *result); + EXPECT_EQ(610, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.setLag(100); mMockClock.advanceBy(700); @@ -1072,7 +1112,8 @@ TEST_F(VSyncDispatchTimerQueueTest, basicAlarmSettingFutureWithReadyDuration) { mDispatch->schedule(cb, {.workDuration = 70, .readyDuration = 30, .lastVsync = intended}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(900, *result); + EXPECT_EQ(900, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); advanceToNextCallback(); ASSERT_THAT(cb.mCalls.size(), Eq(1)); @@ -1138,12 +1179,14 @@ TEST_F(VSyncDispatchTimerQueueTest, skipAVsyc) { auto result = mDispatch->schedule(cb, {.workDuration = 500, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(500, *result); + EXPECT_EQ(500, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.advanceBy(300); result = mDispatch->schedule(cb, {.workDuration = 800, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(1200, *result); + EXPECT_EQ(1200, result->callbackTime.ns()); + EXPECT_EQ(2000, result->vsyncTime.ns()); advanceToNextCallback(); ASSERT_THAT(cb.mCalls.size(), Eq(1)); @@ -1159,12 +1202,14 @@ TEST_F(VSyncDispatchTimerQueueTest, dontskipAVsyc) { auto result = mDispatch->schedule(cb, {.workDuration = 500, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(500, *result); + EXPECT_EQ(500, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); mMockClock.advanceBy(300); result = mDispatch->schedule(cb, {.workDuration = 800, .readyDuration = 0, .lastVsync = 1000}); EXPECT_TRUE(result.has_value()); - EXPECT_EQ(300, *result); + EXPECT_EQ(300, result->callbackTime.ns()); + EXPECT_EQ(1000, result->vsyncTime.ns()); advanceToNextCallback(); ASSERT_THAT(cb.mCalls.size(), Eq(1)); @@ -1197,9 +1242,11 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, stateScheduling) { "test", [](auto, auto, auto) {}, mVsyncMoveThreshold); EXPECT_FALSE(entry.wakeupTime()); - EXPECT_TRUE(entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + const auto scheduleResult = + entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0); + EXPECT_EQ(900, scheduleResult.callbackTime.ns()); + EXPECT_EQ(1000, scheduleResult.vsyncTime.ns()); auto const wakeup = entry.wakeupTime(); ASSERT_TRUE(wakeup); EXPECT_THAT(*wakeup, Eq(900)); @@ -1220,9 +1267,11 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, stateSchedulingReallyLongWakeupLatency) "test", [](auto, auto, auto) {}, mVsyncMoveThreshold); EXPECT_FALSE(entry.wakeupTime()); - EXPECT_TRUE(entry.schedule({.workDuration = 500, .readyDuration = 0, .lastVsync = 994}, - *mStubTracker.get(), now) - .has_value()); + const auto scheduleResult = + entry.schedule({.workDuration = 500, .readyDuration = 0, .lastVsync = 994}, + *mStubTracker, now); + EXPECT_EQ(9500, scheduleResult.callbackTime.ns()); + EXPECT_EQ(10000, scheduleResult.vsyncTime.ns()); auto const wakeup = entry.wakeupTime(); ASSERT_TRUE(wakeup); EXPECT_THAT(*wakeup, Eq(9500)); @@ -1243,9 +1292,11 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, runCallback) { }, mVsyncMoveThreshold); - EXPECT_TRUE(entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + const auto scheduleResult = + entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0); + EXPECT_EQ(900, scheduleResult.callbackTime.ns()); + EXPECT_EQ(1000, scheduleResult.vsyncTime.ns()); auto const wakeup = entry.wakeupTime(); ASSERT_TRUE(wakeup); EXPECT_THAT(*wakeup, Eq(900)); @@ -1275,17 +1326,19 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, updateCallback) { "test", [](auto, auto, auto) {}, mVsyncMoveThreshold); EXPECT_FALSE(entry.wakeupTime()); - entry.update(*mStubTracker.get(), 0); + entry.update(*mStubTracker, 0); EXPECT_FALSE(entry.wakeupTime()); - EXPECT_TRUE(entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + const auto scheduleResult = + entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0); + EXPECT_EQ(900, scheduleResult.callbackTime.ns()); + EXPECT_EQ(1000, scheduleResult.vsyncTime.ns()); auto wakeup = entry.wakeupTime(); ASSERT_TRUE(wakeup); EXPECT_THAT(wakeup, Eq(900)); - entry.update(*mStubTracker.get(), 0); + entry.update(*mStubTracker, 0); wakeup = entry.wakeupTime(); ASSERT_TRUE(wakeup); EXPECT_THAT(*wakeup, Eq(920)); @@ -1294,9 +1347,10 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, updateCallback) { TEST_F(VSyncDispatchTimerQueueEntryTest, skipsUpdateIfJustScheduled) { VSyncDispatchTimerQueueEntry entry( "test", [](auto, auto, auto) {}, mVsyncMoveThreshold); - EXPECT_TRUE(entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + EXPECT_EQ(900, + entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker.get(), 0) + .callbackTime.ns()); entry.update(*mStubTracker.get(), 0); auto const wakeup = entry.wakeupTime(); @@ -1307,26 +1361,32 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, skipsUpdateIfJustScheduled) { TEST_F(VSyncDispatchTimerQueueEntryTest, willSnapToNextTargettableVSync) { VSyncDispatchTimerQueueEntry entry( "test", [](auto, auto, auto) {}, mVsyncMoveThreshold); - EXPECT_TRUE(entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + auto scheduleResult = + entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0); + + EXPECT_EQ(900, scheduleResult.callbackTime.ns()); + EXPECT_EQ(1000, scheduleResult.vsyncTime.ns()); entry.executing(); // 1000 is executing // had 1000 not been executing, this could have been scheduled for time 800. - EXPECT_TRUE(entry.schedule({.workDuration = 200, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + scheduleResult = entry.schedule({.workDuration = 200, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0); + EXPECT_EQ(1800, scheduleResult.callbackTime.ns()); + EXPECT_EQ(2000, scheduleResult.vsyncTime.ns()); EXPECT_THAT(*entry.wakeupTime(), Eq(1800)); EXPECT_THAT(*entry.readyTime(), Eq(2000)); - EXPECT_TRUE(entry.schedule({.workDuration = 50, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + scheduleResult = entry.schedule({.workDuration = 50, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0); + EXPECT_EQ(1950, scheduleResult.callbackTime.ns()); + EXPECT_EQ(2000, scheduleResult.vsyncTime.ns()); EXPECT_THAT(*entry.wakeupTime(), Eq(1950)); EXPECT_THAT(*entry.readyTime(), Eq(2000)); - EXPECT_TRUE(entry.schedule({.workDuration = 200, .readyDuration = 0, .lastVsync = 1001}, - *mStubTracker.get(), 0) - .has_value()); + scheduleResult = entry.schedule({.workDuration = 200, .readyDuration = 0, .lastVsync = 1001}, + *mStubTracker, 0); + EXPECT_EQ(1800, scheduleResult.callbackTime.ns()); + EXPECT_EQ(2000, scheduleResult.vsyncTime.ns()); EXPECT_THAT(*entry.wakeupTime(), Eq(1800)); EXPECT_THAT(*entry.readyTime(), Eq(2000)); } @@ -1349,32 +1409,48 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, .InSequence(seq) .WillOnce(Return(2000)); - EXPECT_TRUE(entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + auto scheduleResult = + entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0); + EXPECT_EQ(900, scheduleResult.callbackTime.ns()); + EXPECT_EQ(1000, scheduleResult.vsyncTime.ns()); entry.executing(); // 1000 is executing - EXPECT_TRUE(entry.schedule({.workDuration = 200, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + scheduleResult = entry.schedule({.workDuration = 200, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0); + EXPECT_EQ(1800, scheduleResult.callbackTime.ns()); + EXPECT_EQ(2000, scheduleResult.vsyncTime.ns()); } TEST_F(VSyncDispatchTimerQueueEntryTest, reportsScheduledIfStillTime) { - VSyncDispatchTimerQueueEntry entry( - "test", [](auto, auto, auto) {}, mVsyncMoveThreshold); - EXPECT_TRUE(entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); - EXPECT_TRUE(entry.schedule({.workDuration = 200, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); - EXPECT_TRUE(entry.schedule({.workDuration = 50, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); - EXPECT_TRUE(entry.schedule({.workDuration = 1200, .readyDuration = 0, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + VSyncDispatchTimerQueueEntry entry("test", [](auto, auto, auto) {}, mVsyncMoveThreshold); + EXPECT_EQ(900, + entry.schedule({.workDuration = 100, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0) + .callbackTime.ns()); + EXPECT_EQ(800, + entry.schedule({.workDuration = 200, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0) + .callbackTime.ns()); + EXPECT_EQ(950, + entry.schedule({.workDuration = 50, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0) + .callbackTime.ns()); + { + SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, true); + EXPECT_EQ(0, + entry.schedule({.workDuration = 1200, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0) + .callbackTime.ns()); + } + { + SET_FLAG_FOR_TEST(flags::dont_skip_on_early_ro, false); + EXPECT_EQ(800, + entry.schedule({.workDuration = 1200, .readyDuration = 0, .lastVsync = 500}, + *mStubTracker, 0) + .callbackTime.ns()); + } } TEST_F(VSyncDispatchTimerQueueEntryTest, storesPendingUpdatesUntilUpdateAndDontSkip) { @@ -1389,7 +1465,7 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, storesPendingUpdatesUntilUpdateAndDontS .readyDuration = 0, .lastVsync = 400}); EXPECT_TRUE(entry.hasPendingWorkloadUpdate()); - entry.update(*mStubTracker.get(), 0); + entry.update(*mStubTracker, 0); EXPECT_FALSE(entry.hasPendingWorkloadUpdate()); EXPECT_THAT(*entry.wakeupTime(), Eq(mPeriod - effectualOffset)); } @@ -1409,9 +1485,11 @@ TEST_F(VSyncDispatchTimerQueueEntryTest, runCallbackWithReadyDuration) { }, mVsyncMoveThreshold); - EXPECT_TRUE(entry.schedule({.workDuration = 70, .readyDuration = 30, .lastVsync = 500}, - *mStubTracker.get(), 0) - .has_value()); + const auto scheduleResult = + entry.schedule({.workDuration = 70, .readyDuration = 30, .lastVsync = 500}, + *mStubTracker, 0); + EXPECT_EQ(900, scheduleResult.callbackTime.ns()); + EXPECT_EQ(mPeriod, scheduleResult.vsyncTime.ns()); auto const wakeup = entry.wakeupTime(); ASSERT_TRUE(wakeup); EXPECT_THAT(*wakeup, Eq(900)); diff --git a/services/surfaceflinger/tests/unittests/mock/MockVSyncDispatch.h b/services/surfaceflinger/tests/unittests/mock/MockVSyncDispatch.h index dc32ff969c..1dc2ef42d6 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockVSyncDispatch.h +++ b/services/surfaceflinger/tests/unittests/mock/MockVSyncDispatch.h @@ -29,8 +29,10 @@ public: MOCK_METHOD(CallbackToken, registerCallback, (Callback, std::string), (override)); MOCK_METHOD(void, unregisterCallback, (CallbackToken), (override)); - MOCK_METHOD(scheduler::ScheduleResult, schedule, (CallbackToken, ScheduleTiming), (override)); - MOCK_METHOD(scheduler::ScheduleResult, update, (CallbackToken, ScheduleTiming), (override)); + MOCK_METHOD(std::optional<scheduler::ScheduleResult>, schedule, (CallbackToken, ScheduleTiming), + (override)); + MOCK_METHOD(std::optional<scheduler::ScheduleResult>, update, (CallbackToken, ScheduleTiming), + (override)); MOCK_METHOD(scheduler::CancelResult, cancel, (CallbackToken token), (override)); MOCK_METHOD(void, dump, (std::string&), (const, override)); }; |