diff options
26 files changed, 286 insertions, 169 deletions
diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp index 325a91178a..b973211966 100644 --- a/libs/renderengine/skia/SkiaRenderEngine.cpp +++ b/libs/renderengine/skia/SkiaRenderEngine.cpp @@ -91,7 +91,6 @@ namespace { // Debugging settings static const bool kPrintLayerSettings = false; static const bool kGaneshFlushAfterEveryLayer = kPrintLayerSettings; -static constexpr bool kEnableLayerBrightening = true; } // namespace @@ -714,8 +713,7 @@ void SkiaRenderEngine::drawLayersInternal( // ...and compute the dimming ratio if dimming is requested const float displayDimmingRatio = display.targetLuminanceNits > 0.f && - maxLayerWhitePoint > 0.f && - (kEnableLayerBrightening || display.targetLuminanceNits > maxLayerWhitePoint) + maxLayerWhitePoint > 0.f && display.targetLuminanceNits > maxLayerWhitePoint ? maxLayerWhitePoint / display.targetLuminanceNits : 1.f; diff --git a/services/inputflinger/include/InputReaderBase.h b/services/inputflinger/include/InputReaderBase.h index 6cf5a7e1e5..f2a627035c 100644 --- a/services/inputflinger/include/InputReaderBase.h +++ b/services/inputflinger/include/InputReaderBase.h @@ -310,9 +310,6 @@ public: /* Called by the heartbeat to ensures that the reader has not deadlocked. */ virtual void monitor() = 0; - /* Returns true if the input device is enabled. */ - virtual bool isInputDeviceEnabled(int32_t deviceId) = 0; - /* Makes the reader start processing events from the kernel. */ virtual status_t start() = 0; diff --git a/services/inputflinger/reader/InputReader.cpp b/services/inputflinger/reader/InputReader.cpp index 69555f8961..b9523ef332 100644 --- a/services/inputflinger/reader/InputReader.cpp +++ b/services/inputflinger/reader/InputReader.cpp @@ -874,17 +874,6 @@ std::optional<std::string> InputReader::getBluetoothAddress(int32_t deviceId) co return std::nullopt; } -bool InputReader::isInputDeviceEnabled(int32_t deviceId) { - std::scoped_lock _l(mLock); - - InputDevice* device = findInputDeviceLocked(deviceId); - if (device) { - return device->isEnabled(); - } - ALOGW("Ignoring invalid device id %" PRId32 ".", deviceId); - return false; -} - bool InputReader::canDispatchToDisplay(int32_t deviceId, ui::LogicalDisplayId displayId) { std::scoped_lock _l(mLock); diff --git a/services/inputflinger/reader/include/InputReader.h b/services/inputflinger/reader/include/InputReader.h index 92a778a5bb..7e701c5341 100644 --- a/services/inputflinger/reader/include/InputReader.h +++ b/services/inputflinger/reader/include/InputReader.h @@ -61,8 +61,6 @@ public: std::vector<InputDeviceInfo> getInputDevices() const override; - bool isInputDeviceEnabled(int32_t deviceId) override; - int32_t getScanCodeState(int32_t deviceId, uint32_t sourceMask, int32_t scanCode) override; int32_t getKeyCodeState(int32_t deviceId, uint32_t sourceMask, int32_t keyCode) override; int32_t getSwitchState(int32_t deviceId, uint32_t sourceMask, int32_t sw) override; diff --git a/services/inputflinger/tests/InputDispatcher_test.cpp b/services/inputflinger/tests/InputDispatcher_test.cpp index 8ad235f3a3..8de28c680f 100644 --- a/services/inputflinger/tests/InputDispatcher_test.cpp +++ b/services/inputflinger/tests/InputDispatcher_test.cpp @@ -7814,6 +7814,7 @@ TEST_F(InputDispatcherTest, MultiDeviceSpyWindowSlipTest) { * If device B reports more ACTION_MOVE events, the middle window should receive remaining events. */ TEST_F(InputDispatcherTest, MultiDeviceSlipperyWindowTest) { + SCOPED_FLAG_OVERRIDE(enable_multi_device_same_window_stream, true); std::shared_ptr<FakeApplicationHandle> application = std::make_shared<FakeApplicationHandle>(); sp<FakeWindowHandle> leftWindow = sp<FakeWindowHandle>::make(application, mDispatcher, "Left window", diff --git a/services/inputflinger/tests/fuzzers/InputReaderFuzzer.cpp b/services/inputflinger/tests/fuzzers/InputReaderFuzzer.cpp index a19726a479..7d26a43440 100644 --- a/services/inputflinger/tests/fuzzers/InputReaderFuzzer.cpp +++ b/services/inputflinger/tests/fuzzers/InputReaderFuzzer.cpp @@ -55,8 +55,6 @@ public: void monitor() { reader->monitor(); } - bool isInputDeviceEnabled(int32_t deviceId) { return reader->isInputDeviceEnabled(deviceId); } - status_t start() { return reader->start(); } status_t stop() { return reader->stop(); } @@ -206,7 +204,6 @@ extern "C" int LLVMFuzzerTestOneInput(uint8_t* data, size_t size) { }, [&]() -> void { reader->monitor(); }, [&]() -> void { reader->getInputDevices(); }, - [&]() -> void { reader->isInputDeviceEnabled(fdp->ConsumeIntegral<int32_t>()); }, [&]() -> void { reader->getScanCodeState(fdp->ConsumeIntegral<int32_t>(), fdp->ConsumeIntegral<uint32_t>(), diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp index f62562ce9d..9c4d1ace15 100644 --- a/services/sensorservice/SensorDevice.cpp +++ b/services/sensorservice/SensorDevice.cpp @@ -429,14 +429,18 @@ void SensorDevice::onDynamicSensorsConnected(const std::vector<sensor_t>& dynami } void SensorDevice::onDynamicSensorsDisconnected( - const std::vector<int32_t>& dynamicSensorHandlesRemoved) { - if (sensorservice_flags::sensor_device_on_dynamic_sensor_disconnected()) { - for (auto handle : dynamicSensorHandlesRemoved) { - auto it = mConnectedDynamicSensors.find(handle); - if (it != mConnectedDynamicSensors.end()) { - mConnectedDynamicSensors.erase(it); - } - } + const std::vector<int32_t>& /*dynamicSensorHandlesRemoved*/) { + // This function is currently a no-op has removing data in mConnectedDynamicSensors here will + // cause a race condition between when this callback is invoked and when the dynamic sensor meta + // event is processed by polling. The clean up should only happen after processing the meta + // event. See the call stack of cleanupDisconnectedDynamicSensor. +} + +void SensorDevice::cleanupDisconnectedDynamicSensor(int handle) { + std::lock_guard<std::mutex> lock(mDynamicSensorsMutex); + auto it = mConnectedDynamicSensors.find(handle); + if (it != mConnectedDynamicSensors.end()) { + mConnectedDynamicSensors.erase(it); } } diff --git a/services/sensorservice/SensorDevice.h b/services/sensorservice/SensorDevice.h index 52f7cf2de8..b7b04b5d00 100644 --- a/services/sensorservice/SensorDevice.h +++ b/services/sensorservice/SensorDevice.h @@ -63,6 +63,14 @@ public: std::vector<int32_t> getDynamicSensorHandles(); void handleDynamicSensorConnection(int handle, bool connected); + /** + * Removes handle from connected dynamic sensor list. Note that this method must be called after + * SensorService has done using sensor data. + * + * @param handle of the disconnected dynamic sensor. + */ + void cleanupDisconnectedDynamicSensor(int handle); + status_t initCheck() const; int getHalDeviceVersion() const; diff --git a/services/sensorservice/SensorService.cpp b/services/sensorservice/SensorService.cpp index 69e430901a..70ca7025d4 100644 --- a/services/sensorservice/SensorService.cpp +++ b/services/sensorservice/SensorService.cpp @@ -1273,6 +1273,7 @@ bool SensorService::threadLoop() { } else { int handle = mSensorEventBuffer[i].dynamic_sensor_meta.handle; disconnectDynamicSensor(handle, activeConnections); + device.cleanupDisconnectedDynamicSensor(handle); } } } diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp index 8369a1ec64..c9ed15781d 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.cpp @@ -543,12 +543,14 @@ std::string SurfaceFrame::miniDump() const { } void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& refreshRate, - Fps displayFrameRenderRate, nsecs_t& deadlineDelta) { + Fps displayFrameRenderRate, nsecs_t* outDeadlineDelta) { if (mActuals.presentTime == Fence::SIGNAL_TIME_INVALID) { // Cannot do any classification for invalid present time. mJankType = JankType::Unknown; mJankSeverityType = JankSeverityType::Unknown; - deadlineDelta = -1; + if (outDeadlineDelta) { + *outDeadlineDelta = -1; + } return; } @@ -559,7 +561,9 @@ void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& r mJankType = mPresentState != PresentState::Presented ? JankType::Dropped : JankType::AppDeadlineMissed; mJankSeverityType = JankSeverityType::Unknown; - deadlineDelta = -1; + if (outDeadlineDelta) { + *outDeadlineDelta = -1; + } return; } @@ -568,11 +572,14 @@ void SurfaceFrame::classifyJankLocked(int32_t displayFrameJankType, const Fps& r return; } - deadlineDelta = mActuals.endTime - mPredictions.endTime; const nsecs_t presentDelta = mActuals.presentTime - mPredictions.presentTime; const nsecs_t deltaToVsync = refreshRate.getPeriodNsecs() > 0 ? std::abs(presentDelta) % refreshRate.getPeriodNsecs() : 0; + const nsecs_t deadlineDelta = mActuals.endTime - mPredictions.endTime; + if (outDeadlineDelta) { + *outDeadlineDelta = deadlineDelta; + } if (deadlineDelta > mJankClassificationThresholds.deadlineThreshold) { mFrameReadyMetadata = FrameReadyMetadata::LateFinish; @@ -671,7 +678,7 @@ void SurfaceFrame::onPresent(nsecs_t presentTime, int32_t displayFrameJankType, mActuals.presentTime = presentTime; nsecs_t deadlineDelta = 0; - classifyJankLocked(displayFrameJankType, refreshRate, displayFrameRenderRate, deadlineDelta); + classifyJankLocked(displayFrameJankType, refreshRate, displayFrameRenderRate, &deadlineDelta); if (mPredictionState != PredictionState::None) { // Only update janky frames if the app used vsync predictions @@ -686,8 +693,7 @@ void SurfaceFrame::onCommitNotComposited(Fps refreshRate, Fps displayFrameRender mDisplayFrameRenderRate = displayFrameRenderRate; mActuals.presentTime = mPredictions.presentTime; - nsecs_t deadlineDelta = 0; - classifyJankLocked(JankType::None, refreshRate, displayFrameRenderRate, deadlineDelta); + classifyJankLocked(JankType::None, refreshRate, displayFrameRenderRate, nullptr); } void SurfaceFrame::tracePredictions(int64_t displayFrameToken, nsecs_t monoBootOffset) const { diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h index 21bc95a4c5..94cfcb40b9 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h @@ -237,7 +237,7 @@ private: void tracePredictions(int64_t displayFrameToken, nsecs_t monoBootOffset) const; void traceActuals(int64_t displayFrameToken, nsecs_t monoBootOffset) const; void classifyJankLocked(int32_t displayFrameJankType, const Fps& refreshRate, - Fps displayFrameRenderRate, nsecs_t& deadlineDelta) REQUIRES(mMutex); + Fps displayFrameRenderRate, nsecs_t* outDeadlineDelta) REQUIRES(mMutex); const int64_t mToken; const int32_t mInputEventId; diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp index caeb575c4e..f10bb33a2f 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp @@ -313,6 +313,21 @@ void updateMetadata(LayerSnapshot& snapshot, const RequestedLayerState& requeste } } +void updateMetadataAndGameMode(LayerSnapshot& snapshot, const RequestedLayerState& requested, + const LayerSnapshotBuilder::Args& args, + const LayerSnapshot& parentSnapshot) { + if (snapshot.changes.test(RequestedLayerState::Changes::GameMode)) { + snapshot.gameMode = requested.metadata.has(gui::METADATA_GAME_MODE) + ? requested.gameMode + : parentSnapshot.gameMode; + } + updateMetadata(snapshot, requested, args); + if (args.includeMetadata) { + snapshot.layerMetadata = parentSnapshot.layerMetadata; + snapshot.layerMetadata.merge(requested.metadata); + } +} + void clearChanges(LayerSnapshot& snapshot) { snapshot.changes.clear(); snapshot.clientChanges = 0; @@ -746,6 +761,11 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a RequestedLayerState::Changes::Input)) { updateInput(snapshot, requested, parentSnapshot, path, args); } + if (forceUpdate || + (args.includeMetadata && + snapshot.changes.test(RequestedLayerState::Changes::Metadata))) { + updateMetadataAndGameMode(snapshot, requested, args, parentSnapshot); + } return; } @@ -786,16 +806,7 @@ void LayerSnapshotBuilder::updateSnapshot(LayerSnapshot& snapshot, const Args& a } if (forceUpdate || snapshot.changes.test(RequestedLayerState::Changes::Metadata)) { - if (snapshot.changes.test(RequestedLayerState::Changes::GameMode)) { - snapshot.gameMode = requested.metadata.has(gui::METADATA_GAME_MODE) - ? requested.gameMode - : parentSnapshot.gameMode; - } - updateMetadata(snapshot, requested, args); - if (args.includeMetadata) { - snapshot.layerMetadata = parentSnapshot.layerMetadata; - snapshot.layerMetadata.merge(requested.metadata); - } + updateMetadataAndGameMode(snapshot, requested, args, parentSnapshot); } if (forceUpdate || snapshot.clientChanges & layer_state_t::eFixedTransformHintChanged || @@ -1164,6 +1175,15 @@ void LayerSnapshotBuilder::forEachVisibleSnapshot(const Visitor& visitor) { } } +void LayerSnapshotBuilder::forEachSnapshot(const Visitor& visitor, + const ConstPredicate& predicate) { + for (int i = 0; i < mNumInterestingSnapshots; i++) { + std::unique_ptr<LayerSnapshot>& snapshot = mSnapshots.at((size_t)i); + if (!predicate(*snapshot)) continue; + visitor(snapshot); + } +} + void LayerSnapshotBuilder::forEachInputSnapshot(const ConstVisitor& visitor) const { for (int i = mNumInterestingSnapshots - 1; i >= 0; i--) { LayerSnapshot& snapshot = *mSnapshots[(size_t)i]; diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h index 1cec0183b9..dbbad7664a 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h @@ -86,6 +86,11 @@ public: // Visit each visible snapshot in z-order and move the snapshot if needed void forEachVisibleSnapshot(const Visitor& visitor); + typedef std::function<bool(const LayerSnapshot& snapshot)> ConstPredicate; + // Visit each snapshot that satisfies the predicate and move the snapshot if needed with visible + // snapshots in z-order + void forEachSnapshot(const Visitor& visitor, const ConstPredicate& predicate); + // Visit each snapshot interesting to input reverse z-order void forEachInputSnapshot(const ConstVisitor& visitor) const; diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.h b/services/surfaceflinger/FrontEnd/RequestedLayerState.h index 09f33de63b..48b9640486 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.h +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.h @@ -26,6 +26,7 @@ #include "TransactionState.h" namespace android::surfaceflinger::frontend { +using namespace ftl::flag_operators; // Stores client requested states for a layer. // This struct does not store any other states or states pertaining to @@ -58,6 +59,13 @@ struct RequestedLayerState : layer_state_t { GameMode = 1u << 19, BufferUsageFlags = 1u << 20, }; + + static constexpr ftl::Flags<Changes> kMustComposite = Changes::Created | Changes::Destroyed | + Changes::Hierarchy | Changes::Geometry | Changes::Content | Changes::Input | + Changes::Z | Changes::Mirror | Changes::Parent | Changes::RelativeParent | + Changes::Metadata | Changes::Visibility | Changes::VisibleRegion | Changes::Buffer | + Changes::SidebandStream | Changes::Animation | Changes::BufferSize | Changes::GameMode | + Changes::BufferUsageFlags; static Rect reduce(const Rect& win, const Region& exclude); RequestedLayerState(const LayerCreationArgs&); void merge(const ResolvedComposerState&); diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index f72e7a05d5..60681a2bc8 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -1153,8 +1153,10 @@ auto Scheduler::chooseDisplayModes() const -> DisplayModeChoiceMap { return pacesetterFps; }(); + // Choose a mode for powered-on follower displays. for (const auto& [id, display] : mDisplays) { if (id == *mPacesetterDisplayId) continue; + if (display.powerMode != hal::PowerMode::ON) continue; auto rankedFrameRates = display.selectorPtr->getRankedFrameRates(mPolicy.contentRequirements, globalSignals, diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 33dbab01df..cf67f67d78 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -1411,11 +1411,12 @@ void SurfaceFlinger::applyActiveMode(const sp<DisplayDevice>& display) { } } -void SurfaceFlinger::initiateDisplayModeChanges() { +bool SurfaceFlinger::initiateDisplayModeChanges() { ATRACE_CALL(); std::optional<PhysicalDisplayId> displayToUpdateImmediately; + bool mustComposite = false; for (const auto& [id, physical] : mPhysicalDisplays) { const auto display = getDisplayDeviceLocked(id); if (!display) continue; @@ -1425,11 +1426,6 @@ void SurfaceFlinger::initiateDisplayModeChanges() { continue; } - if (!shouldApplyRefreshRateSelectorPolicy(*display)) { - dropModeRequest(display); - continue; - } - const auto desiredModeId = desiredModeOpt->mode.modePtr->getId(); const auto displayModePtrOpt = physical.snapshot().displayModes().get(desiredModeId); @@ -1472,7 +1468,11 @@ void SurfaceFlinger::initiateDisplayModeChanges() { mScheduler->onNewVsyncPeriodChangeTimeline(outTimeline); if (outTimeline.refreshRequired) { - scheduleComposite(FrameHint::kNone); + if (FlagManager::getInstance().vrr_bugfix_24q4()) { + mustComposite = true; + } else { + scheduleComposite(FrameHint::kNone); + } } else { // TODO(b/255635711): Remove `displayToUpdateImmediately` to `finalizeDisplayModeChange` // for all displays. This was only needed when the loop iterated over `mDisplays` rather @@ -1490,6 +1490,8 @@ void SurfaceFlinger::initiateDisplayModeChanges() { applyActiveMode(display); } } + + return mustComposite; } void SurfaceFlinger::disableExpensiveRendering() { @@ -2439,7 +2441,12 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs, mUpdateAttachedChoreographer = true; } outTransactionsAreEmpty = mLayerLifecycleManager.getGlobalChanges().get() == 0; - mustComposite |= mLayerLifecycleManager.getGlobalChanges().get() != 0; + if (FlagManager::getInstance().vrr_bugfix_24q4()) { + mustComposite |= mLayerLifecycleManager.getGlobalChanges().any( + frontend::RequestedLayerState::kMustComposite); + } else { + mustComposite |= mLayerLifecycleManager.getGlobalChanges().get() != 0; + } bool newDataLatched = false; ATRACE_NAME("DisplayCallbackAndStatsUpdates"); @@ -2664,7 +2671,7 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, ? &mLayerHierarchyBuilder.getHierarchy() : nullptr, updateAttachedChoreographer); - initiateDisplayModeChanges(); + mustComposite |= initiateDisplayModeChanges(); } updateCursorAsync(); @@ -4228,12 +4235,6 @@ void SurfaceFlinger::requestDisplayModes(std::vector<display::DisplayModeRequest if (!display) continue; - if (ftl::FakeGuard guard(kMainThreadContext); - !shouldApplyRefreshRateSelectorPolicy(*display)) { - ALOGV("%s(%s): Skipped applying policy", __func__, to_string(displayId).c_str()); - continue; - } - if (display->refreshRateSelector().isModeAllowed(request.mode)) { setDesiredMode(std::move(request)); } else { @@ -7507,14 +7508,11 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r auto future = mScheduler->schedule( [&]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(kMainThreadContext) { n = data.readInt32(); - mHdrSdrRatioOverlay = n != 0; - switch (n) { - case 0: - case 1: - enableHdrSdrRatioOverlay(mHdrSdrRatioOverlay); - break; - default: - reply->writeBool(isHdrSdrRatioOverlayEnabled()); + if (n == 0 || n == 1) { + mHdrSdrRatioOverlay = n != 0; + enableHdrSdrRatioOverlay(mHdrSdrRatioOverlay); + } else { + reply->writeBool(isHdrSdrRatioOverlayEnabled()); } }); future.wait(); @@ -8334,10 +8332,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl( renderArea->getHintForSeamlessTransition()); sdrWhitePointNits = state.sdrWhitePointNits; - // TODO(b/298219334): Clean this up once we verify this doesn't break anything - static constexpr bool kScreenshotsDontDim = true; - - if (kScreenshotsDontDim && !captureResults.capturedHdrLayers) { + if (!captureResults.capturedHdrLayers) { displayBrightnessNits = sdrWhitePointNits; } else { displayBrightnessNits = state.displayBrightnessNits; @@ -8569,35 +8564,11 @@ status_t SurfaceFlinger::setDesiredDisplayModeSpecsInternal( break; } - if (!shouldApplyRefreshRateSelectorPolicy(*display)) { - ALOGV("%s(%s): Skipped applying policy", __func__, to_string(displayId).c_str()); - return NO_ERROR; - } - return applyRefreshRateSelectorPolicy(displayId, selector); } -bool SurfaceFlinger::shouldApplyRefreshRateSelectorPolicy(const DisplayDevice& display) const { - if (display.isPoweredOn() || mPhysicalDisplays.size() == 1) return true; - - LOG_ALWAYS_FATAL_IF(display.isVirtual()); - const auto displayId = display.getPhysicalId(); - - // The display is powered off, and this is a multi-display device. If the display is the - // inactive internal display of a dual-display foldable, then the policy will be applied - // when it becomes active upon powering on. - // - // TODO(b/255635711): Remove this function (i.e. returning `false` as a special case) once - // concurrent mode setting across multiple (potentially powered off) displays is supported. - // - return displayId == mActiveDisplayId || - !mPhysicalDisplays.get(displayId) - .transform(&PhysicalDisplay::isInternal) - .value_or(false); -} - status_t SurfaceFlinger::applyRefreshRateSelectorPolicy( - PhysicalDisplayId displayId, const scheduler::RefreshRateSelector& selector, bool force) { + PhysicalDisplayId displayId, const scheduler::RefreshRateSelector& selector) { const scheduler::RefreshRateSelector::Policy currentPolicy = selector.getCurrentPolicy(); ALOGV("Setting desired display mode specs: %s", currentPolicy.toString().c_str()); @@ -8628,7 +8599,7 @@ status_t SurfaceFlinger::applyRefreshRateSelectorPolicy( return INVALID_OPERATION; } - setDesiredMode({std::move(preferredMode), .emitEvent = true, .force = force}); + setDesiredMode({std::move(preferredMode), .emitEvent = true}); // Update the frameRateOverride list as the display render rate might have changed if (mScheduler->updateFrameRateOverrides(scheduler::GlobalSignals{}, preferredFps)) { @@ -8986,13 +8957,8 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD const DisplayDevice& activeDisplay) { ATRACE_CALL(); - // For the first display activated during boot, there is no need to force setDesiredMode, - // because DM is about to send its policy via setDesiredDisplayModeSpecs. - bool forceApplyPolicy = false; - if (inactiveDisplayPtr) { inactiveDisplayPtr->getCompositionDisplay()->setLayerCachingTexturePoolEnabled(false); - forceApplyPolicy = true; } mActiveDisplayId = activeDisplay.getPhysicalId(); @@ -9009,12 +8975,11 @@ void SurfaceFlinger::onActiveDisplayChangedLocked(const DisplayDevice* inactiveD mActiveDisplayTransformHint = activeDisplay.getTransformHint(); sActiveDisplayRotationFlags = ui::Transform::toRotationFlags(activeDisplay.getOrientation()); - // The policy of the new active/pacesetter display may have changed while it was inactive. In - // that case, its preferred mode has not been propagated to HWC (via setDesiredMode). In either - // case, the Scheduler's cachedModeChangedParams must be initialized to the newly active mode, - // and the kernel idle timer of the newly active display must be toggled. - applyRefreshRateSelectorPolicy(mActiveDisplayId, activeDisplay.refreshRateSelector(), - forceApplyPolicy); + // Whether or not the policy of the new active/pacesetter display changed while it was inactive + // (in which case its preferred mode has already been propagated to HWC via setDesiredMode), the + // Scheduler's cachedModeChangedParams must be initialized to the newly active mode, and the + // kernel idle timer of the newly active display must be toggled. + applyRefreshRateSelectorPolicy(mActiveDisplayId, activeDisplay.refreshRateSelector()); } status_t SurfaceFlinger::addWindowInfosListener(const sp<IWindowInfosListener>& windowInfosListener, @@ -9233,7 +9198,9 @@ std::vector<std::pair<Layer*, LayerFE*>> SurfaceFlinger::moveSnapshotsToComposit std::vector<std::pair<Layer*, LayerFE*>> layers; if (mLayerLifecycleManagerEnabled) { nsecs_t currentTime = systemTime(); - mLayerSnapshotBuilder.forEachVisibleSnapshot( + const bool needsMetadata = mCompositionEngine->getFeatureFlags().test( + compositionengine::Feature::kSnapshotLayerMetadata); + mLayerSnapshotBuilder.forEachSnapshot( [&](std::unique_ptr<frontend::LayerSnapshot>& snapshot) FTL_FAKE_GUARD( kMainThreadContext) { if (cursorOnly && @@ -9256,6 +9223,12 @@ std::vector<std::pair<Layer*, LayerFE*>> SurfaceFlinger::moveSnapshotsToComposit layerFE->mSnapshot = std::move(snapshot); refreshArgs.layers.push_back(layerFE); layers.emplace_back(legacyLayer.get(), layerFE.get()); + }, + [needsMetadata](const frontend::LayerSnapshot& snapshot) { + return snapshot.isVisible || + (needsMetadata && + snapshot.changes.test( + frontend::RequestedLayerState::Changes::Metadata)); }); } if (!mLayerLifecycleManagerEnabled) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index edcc8d3206..278dee74fe 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -736,7 +736,7 @@ private: status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId, Fps minFps, Fps maxFps); - void initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext); + bool initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext); void finalizeDisplayModeChange(DisplayDevice&) REQUIRES(mStateLock, kMainThreadContext); // TODO(b/241285191): Replace DisplayDevice with DisplayModeRequest, and move to Scheduler. @@ -756,13 +756,9 @@ private: const sp<DisplayDevice>&, const scheduler::RefreshRateSelector::PolicyVariant&) EXCLUDES(mStateLock) REQUIRES(kMainThreadContext); - bool shouldApplyRefreshRateSelectorPolicy(const DisplayDevice&) const - REQUIRES(mStateLock, kMainThreadContext); - // TODO(b/241285191): Look up RefreshRateSelector on Scheduler to remove redundant parameter. status_t applyRefreshRateSelectorPolicy(PhysicalDisplayId, - const scheduler::RefreshRateSelector&, - bool force = false) + const scheduler::RefreshRateSelector&) REQUIRES(mStateLock, kMainThreadContext); void commitTransactionsLegacy() EXCLUDES(mStateLock) REQUIRES(kMainThreadContext); diff --git a/services/surfaceflinger/TransactionState.h b/services/surfaceflinger/TransactionState.h index 89a8f9238b..e5d648194f 100644 --- a/services/surfaceflinger/TransactionState.h +++ b/services/surfaceflinger/TransactionState.h @@ -23,6 +23,7 @@ #include "FrontEnd/LayerCreationArgs.h" #include "renderengine/ExternalTexture.h" +#include <common/FlagManager.h> #include <gui/LayerState.h> #include <system/window.h> @@ -108,9 +109,22 @@ struct TransactionState { for (const auto& state : states) { const bool frameRateChanged = state.state.what & layer_state_t::eFrameRateChanged; - if (!frameRateChanged || - state.state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) { - return true; + if (FlagManager::getInstance().vrr_bugfix_24q4()) { + const bool frameRateIsNoVote = frameRateChanged && + state.state.frameRateCompatibility == ANATIVEWINDOW_FRAME_RATE_NO_VOTE; + const bool frameRateCategoryChanged = + state.state.what & layer_state_t::eFrameRateCategoryChanged; + const bool frameRateCategoryIsNoPreference = frameRateCategoryChanged && + state.state.frameRateCategory == + ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE; + if (!frameRateIsNoVote && !frameRateCategoryIsNoPreference) { + return true; + } + } else { + if (!frameRateChanged || + state.state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) { + return true; + } } } diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp index e73cf5d785..121629fd10 100644 --- a/services/surfaceflinger/common/FlagManager.cpp +++ b/services/surfaceflinger/common/FlagManager.cpp @@ -134,6 +134,7 @@ void FlagManager::dump(std::string& result) const { DUMP_READ_ONLY_FLAG(screenshot_fence_preservation); DUMP_READ_ONLY_FLAG(vulkan_renderengine); DUMP_READ_ONLY_FLAG(renderable_buffer_usage); + DUMP_READ_ONLY_FLAG(vrr_bugfix_24q4); DUMP_READ_ONLY_FLAG(restore_blur_step); DUMP_READ_ONLY_FLAG(dont_skip_on_early_ro); DUMP_READ_ONLY_FLAG(protected_if_client); @@ -145,6 +146,7 @@ void FlagManager::dump(std::string& result) const { DUMP_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter); DUMP_READ_ONLY_FLAG(detached_mirror); DUMP_READ_ONLY_FLAG(commit_not_composited); + DUMP_READ_ONLY_FLAG(local_tonemap_screenshots); #undef DUMP_READ_ONLY_FLAG #undef DUMP_SERVER_FLAG @@ -233,6 +235,7 @@ FLAG_MANAGER_READ_ONLY_FLAG(renderable_buffer_usage, "") FLAG_MANAGER_READ_ONLY_FLAG(restore_blur_step, "debug.renderengine.restore_blur_step") FLAG_MANAGER_READ_ONLY_FLAG(dont_skip_on_early_ro, "") FLAG_MANAGER_READ_ONLY_FLAG(protected_if_client, "") +FLAG_MANAGER_READ_ONLY_FLAG(vrr_bugfix_24q4, ""); FLAG_MANAGER_READ_ONLY_FLAG(ce_fence_promise, ""); FLAG_MANAGER_READ_ONLY_FLAG(graphite_renderengine, "debug.renderengine.graphite") FLAG_MANAGER_READ_ONLY_FLAG(latch_unsignaled_with_auto_refresh_changed, ""); @@ -240,6 +243,7 @@ FLAG_MANAGER_READ_ONLY_FLAG(deprecate_vsync_sf, ""); FLAG_MANAGER_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter, ""); FLAG_MANAGER_READ_ONLY_FLAG(detached_mirror, ""); FLAG_MANAGER_READ_ONLY_FLAG(commit_not_composited, ""); +FLAG_MANAGER_READ_ONLY_FLAG(local_tonemap_screenshots, "debug.sf.local_tonemap_screenshots"); /// Trunk stable server flags /// FLAG_MANAGER_SERVER_FLAG(refresh_rate_overlay_on_external_display, "") diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h index 327cc4ae41..4cf4453980 100644 --- a/services/surfaceflinger/common/include/common/FlagManager.h +++ b/services/surfaceflinger/common/include/common/FlagManager.h @@ -72,6 +72,7 @@ public: bool enable_layer_command_batching() const; bool screenshot_fence_preservation() const; bool vulkan_renderengine() const; + bool vrr_bugfix_24q4() const; bool renderable_buffer_usage() const; bool restore_blur_step() const; bool dont_skip_on_early_ro() const; @@ -84,6 +85,7 @@ public: bool allow_n_vsyncs_in_targeter() const; bool detached_mirror() const; bool commit_not_composited() const; + bool local_tonemap_screenshots() const; protected: // overridden for unit tests diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig index f1ec3e1ec2..5b94f07dff 100644 --- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig +++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig @@ -76,4 +76,23 @@ flag { } } # latch_unsignaled_with_auto_refresh_changed +flag { + name: "local_tonemap_screenshots" + namespace: "core_graphics" + description: "Enables local tonemapping when capturing screenshots" + bug: "329464641" + is_fixed_read_only: true +} # local_tonemap_screenshots + +flag { + name: "vrr_bugfix_24q4" + namespace: "core_graphics" + description: "bug fixes for VRR" + bug: "331513837" + is_fixed_read_only: true + metadata { + purpose: PURPOSE_BUGFIX + } +} # vrr_bugfix_24q4 + # IMPORTANT - please keep alphabetize to reduce merge conflicts diff --git a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp index 867ff55632..cfc8e99f49 100644 --- a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp @@ -461,8 +461,8 @@ TEST_F(LayerLifecycleManagerTest, layerOpacityChangesSetsVisibilityChangeFlag) { HAL_PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_PROTECTED /*usage*/)); EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(), - ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer | - RequestedLayerState::Changes::Content) + ftl::Flags<RequestedLayerState::Changes>( + RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content) .get()); mLifecycleManager.commitChanges(); @@ -493,10 +493,10 @@ TEST_F(LayerLifecycleManagerTest, bufferFormatChangesSetsVisibilityChangeFlag) { HAL_PIXEL_FORMAT_RGB_888, GRALLOC_USAGE_PROTECTED /*usage*/)); EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(), - ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer | - RequestedLayerState::Changes::Content | - RequestedLayerState::Changes::VisibleRegion | - RequestedLayerState::Changes::Visibility) + ftl::Flags<RequestedLayerState::Changes>( + RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content | + RequestedLayerState::Changes::VisibleRegion | + RequestedLayerState::Changes::Visibility) .get()); mLifecycleManager.commitChanges(); } @@ -580,8 +580,8 @@ TEST_F(LayerLifecycleManagerTest, layerSecureChangesSetsVisibilityChangeFlag) { HAL_PIXEL_FORMAT_RGBA_8888, GRALLOC_USAGE_SW_READ_NEVER /*usage*/)); EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(), - ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer | - RequestedLayerState::Changes::Content) + ftl::Flags<RequestedLayerState::Changes>( + RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content) .get()); mLifecycleManager.commitChanges(); diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp index 5ff6417482..23b6851bed 100644 --- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp @@ -254,7 +254,8 @@ TEST_F(LayerSnapshotTest, UpdateClearsPreviousChangeStates) { TEST_F(LayerSnapshotTest, FastPathClearsPreviousChangeStates) { setColor(11, {1._hf, 0._hf, 0._hf}); UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); - EXPECT_EQ(getSnapshot(11)->changes, RequestedLayerState::Changes::Content); + EXPECT_EQ(getSnapshot(11)->changes, + RequestedLayerState::Changes::Content); EXPECT_EQ(getSnapshot(11)->clientChanges, layer_state_t::eColorChanged); EXPECT_EQ(getSnapshot(1)->changes.get(), 0u); UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); @@ -264,7 +265,8 @@ TEST_F(LayerSnapshotTest, FastPathClearsPreviousChangeStates) { TEST_F(LayerSnapshotTest, FastPathSetsChangeFlagToContent) { setColor(1, {1._hf, 0._hf, 0._hf}); UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); - EXPECT_EQ(getSnapshot(1)->changes, RequestedLayerState::Changes::Content); + EXPECT_EQ(getSnapshot(1)->changes, + RequestedLayerState::Changes::Content); EXPECT_EQ(getSnapshot(1)->clientChanges, layer_state_t::eColorChanged); } @@ -329,6 +331,55 @@ TEST_F(LayerSnapshotTest, UpdateMetadata) { EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_CALLING_UID, -1), 789); } +TEST_F(LayerSnapshotTest, UpdateMetadataOfHiddenLayers) { + hideLayer(1); + + std::vector<TransactionState> transactions; + transactions.emplace_back(); + transactions.back().states.push_back({}); + transactions.back().states.front().state.what = layer_state_t::eMetadataChanged; + // This test focuses on metadata used by ARC++ to ensure LayerMetadata is updated correctly, + // and not using stale data. + transactions.back().states.front().state.metadata = LayerMetadata(); + transactions.back().states.front().state.metadata.setInt32(METADATA_OWNER_UID, 123); + transactions.back().states.front().state.metadata.setInt32(METADATA_WINDOW_TYPE, 234); + transactions.back().states.front().state.metadata.setInt32(METADATA_TASK_ID, 345); + transactions.back().states.front().state.metadata.setInt32(METADATA_MOUSE_CURSOR, 456); + transactions.back().states.front().state.metadata.setInt32(METADATA_ACCESSIBILITY_ID, 567); + transactions.back().states.front().state.metadata.setInt32(METADATA_OWNER_PID, 678); + transactions.back().states.front().state.metadata.setInt32(METADATA_CALLING_UID, 789); + + transactions.back().states.front().layerId = 1; + transactions.back().states.front().state.layerId = static_cast<int32_t>(1); + + mLifecycleManager.applyTransactions(transactions); + EXPECT_EQ(mLifecycleManager.getGlobalChanges(), + RequestedLayerState::Changes::Metadata | RequestedLayerState::Changes::Visibility | + RequestedLayerState::Changes::VisibleRegion | + RequestedLayerState::Changes::AffectsChildren); + + // Setting includeMetadata=true to ensure metadata update is applied to LayerSnapshot + LayerSnapshotBuilder::Args args{.root = mHierarchyBuilder.getHierarchy(), + .layerLifecycleManager = mLifecycleManager, + .includeMetadata = true, + .displays = mFrontEndDisplayInfos, + .globalShadowSettings = globalShadowSettings, + .supportsBlur = true, + .supportedLayerGenericMetadata = {}, + .genericLayerMetadataKeyMap = {}}; + update(mSnapshotBuilder, args); + + EXPECT_EQ(static_cast<int64_t>(getSnapshot(1)->clientChanges), + layer_state_t::eMetadataChanged | layer_state_t::eFlagsChanged); + EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_OWNER_UID, -1), 123); + EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_WINDOW_TYPE, -1), 234); + EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_TASK_ID, -1), 345); + EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_MOUSE_CURSOR, -1), 456); + EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_ACCESSIBILITY_ID, -1), 567); + EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_OWNER_PID, -1), 678); + EXPECT_EQ(getSnapshot(1)->layerMetadata.getInt32(METADATA_CALLING_UID, -1), 789); +} + TEST_F(LayerSnapshotTest, NoLayerVoteForParentWithChildVotes) { // ROOT // ├── 1 @@ -1325,6 +1376,17 @@ TEST_F(LayerSnapshotTest, NonVisibleLayerWithInput) { EXPECT_TRUE(foundInputLayer); } +TEST_F(LayerSnapshotTest, ForEachSnapshotsWithPredicate) { + std::vector<uint32_t> visitedUniqueSequences; + mSnapshotBuilder.forEachSnapshot( + [&](const std::unique_ptr<frontend::LayerSnapshot>& snapshot) { + visitedUniqueSequences.push_back(snapshot->uniqueSequence); + }, + [](const frontend::LayerSnapshot& snapshot) { return snapshot.uniqueSequence == 111; }); + EXPECT_EQ(visitedUniqueSequences.size(), 1u); + EXPECT_EQ(visitedUniqueSequences[0], 111u); +} + TEST_F(LayerSnapshotTest, canOccludePresentation) { setFlags(12, layer_state_t::eCanOccludePresentation, layer_state_t::eCanOccludePresentation); LayerSnapshotBuilder::Args args{.root = mHierarchyBuilder.getHierarchy(), diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp index 7479a4f890..4fb06907d0 100644 --- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp +++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp @@ -350,6 +350,9 @@ TEST_F(SchedulerTest, chooseDisplayModesMultipleDisplays) { std::make_shared<RefreshRateSelector>(kDisplay2Modes, kDisplay2Mode60->getId())); + mScheduler->setDisplayPowerMode(kDisplayId1, hal::PowerMode::ON); + mScheduler->setDisplayPowerMode(kDisplayId2, hal::PowerMode::ON); + using DisplayModeChoice = TestableScheduler::DisplayModeChoice; TestableScheduler::DisplayModeChoiceMap expectedChoices; @@ -412,6 +415,7 @@ TEST_F(SchedulerTest, chooseDisplayModesMultipleDisplays) { ->registerDisplay(kDisplayId3, std::make_shared<RefreshRateSelector>(kDisplay3Modes, kDisplay3Mode60->getId())); + mScheduler->setDisplayPowerMode(kDisplayId3, hal::PowerMode::ON); const GlobalSignals globalSignals = {.touch = true}; mScheduler->replaceTouchTimer(10); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp index 15a6db626a..078b4fe15e 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DisplayModeSwitching.cpp @@ -327,7 +327,9 @@ MATCHER_P2(ModeSwitchingTo, flinger, modeId, "") { return false; } - if (!flinger->scheduler()->vsyncModulator().isVsyncConfigEarly()) { + // VsyncModulator should react to mode switches on the pacesetter display. + if (arg->getPhysicalId() == flinger->scheduler()->pacesetterDisplayId() && + !flinger->scheduler()->vsyncModulator().isVsyncConfigEarly()) { *result_listener << "VsyncModulator did not shift to early phase"; return false; } @@ -368,8 +370,8 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); - // Only the inner display is powered on. - mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); @@ -385,40 +387,44 @@ TEST_F(DisplayModeSwitchingTest, innerXorOuterDisplay) { 0.f, 120.f))); EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90); + EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60); mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); - - innerDisplay->setPowerMode(hal::PowerMode::OFF); - outerDisplay->setPowerMode(hal::PowerMode::ON); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); - // Only the outer display is powered on. - mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); - EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60); + EXPECT_EQ(NO_ERROR, + mFlinger.setDesiredDisplayModeSpecs(innerDisplay->getDisplayToken().promote(), + mock::createDisplayModeSpecs(kModeId60, false, + 0.f, 120.f))); + + EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId60); mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); mFlinger.commit(); - EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); + EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); } @@ -437,10 +443,8 @@ TEST_F(DisplayModeSwitchingTest, innerAndOuterDisplay) { EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); - outerDisplay->setPowerMode(hal::PowerMode::ON); - - // Both displays are powered on. - mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); @@ -485,7 +489,7 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringModeSet) { EXPECT_THAT(mDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); // Power off the display before the mode has been set. - mDisplay->setPowerMode(hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(mDisplay, hal::PowerMode::OFF); const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90); @@ -517,10 +521,8 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); - outerDisplay->setPowerMode(hal::PowerMode::ON); - - // Both displays are powered on. - mFlinger.onActiveDisplayChanged(nullptr, *innerDisplay); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::ON); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId60)); EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); @@ -539,40 +541,42 @@ TEST_F(DisplayModeSwitchingTest, powerOffDuringConcurrentModeSet) { EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); // Power off the outer display before the mode has been set. - outerDisplay->setPowerMode(hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::OFF); const VsyncPeriodChangeTimeline timeline{.refreshRequired = true}; EXPECT_SET_ACTIVE_CONFIG(kInnerDisplayHwcId, kModeId90); + EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60); mFlinger.commit(); - // Powering off the inactive display should abort the mode set. + // Powering off the inactive display should not abort the mode set. EXPECT_THAT(innerDisplay, ModeSwitchingTo(&mFlinger, kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); - innerDisplay->setPowerMode(hal::PowerMode::OFF); - outerDisplay->setPowerMode(hal::PowerMode::ON); + mFlinger.setPowerModeInternal(innerDisplay, hal::PowerMode::OFF); + mFlinger.setPowerModeInternal(outerDisplay, hal::PowerMode::ON); - // Only the outer display is powered on. - mFlinger.onActiveDisplayChanged(innerDisplay.get(), *outerDisplay); + EXPECT_EQ(NO_ERROR, + mFlinger.setDesiredDisplayModeSpecs(outerDisplay->getDisplayToken().promote(), + mock::createDisplayModeSpecs(kModeId120, false, + 0.f, 120.f))); - EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId60); + EXPECT_SET_ACTIVE_CONFIG(kOuterDisplayHwcId, kModeId120); mFlinger.commit(); - // The mode set should resume once the display becomes active. EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId60)); + EXPECT_THAT(outerDisplay, ModeSwitchingTo(&mFlinger, kModeId120)); mFlinger.commit(); EXPECT_THAT(innerDisplay, ModeSettledTo(kModeId90)); - EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId60)); + EXPECT_THAT(outerDisplay, ModeSettledTo(kModeId120)); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h index 1e02c67d91..198a5def8c 100644 --- a/services/surfaceflinger/tests/unittests/TestableScheduler.h +++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h @@ -111,6 +111,11 @@ public: Scheduler::unregisterDisplay(displayId); } + void setDisplayPowerMode(PhysicalDisplayId displayId, hal::PowerMode powerMode) { + ftl::FakeGuard guard(kMainThreadContext); + Scheduler::setDisplayPowerMode(displayId, powerMode); + } + std::optional<PhysicalDisplayId> pacesetterDisplayId() const NO_THREAD_SAFETY_ANALYSIS { return mPacesetterDisplayId; } |