diff options
22 files changed, 287 insertions, 103 deletions
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 7aa7068538..8572c944da 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -265,6 +265,7 @@ struct layer_state_t { // Changes affecting child states. static constexpr uint64_t AFFECTS_CHILDREN = layer_state_t::GEOMETRY_CHANGES | layer_state_t::HIERARCHY_CHANGES | layer_state_t::eAlphaChanged | + layer_state_t::eBackgroundBlurRadiusChanged | layer_state_t::eBlurRegionsChanged | layer_state_t::eColorTransformChanged | layer_state_t::eCornerRadiusChanged | layer_state_t::eFlagsChanged | layer_state_t::eTrustedOverlayChanged | layer_state_t::eFrameRateChanged | layer_state_t::eFrameRateSelectionPriority | diff --git a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp index 4fe6927d80..22db247cc9 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputLayer.cpp @@ -327,10 +327,6 @@ void OutputLayer::updateCompositionState( ? outputState.dataspace : layerFEState->dataspace; - // re-get HdrRenderType after the dataspace gets changed. - hdrRenderType = - getHdrRenderType(state.dataspace, pixelFormat, layerFEState->desiredHdrSdrRatio); - // Override the dataspace transfer from 170M to sRGB if the device configuration requests this. // We do this here instead of in buffer info so that dumpsys can still report layers that are // using the 170M transfer. Also we only do this if the colorspace is not agnostic for the @@ -342,6 +338,10 @@ void OutputLayer::updateCompositionState( (state.dataspace & HAL_DATASPACE_RANGE_MASK) | HAL_DATASPACE_TRANSFER_SRGB); } + // re-get HdrRenderType after the dataspace gets changed. + hdrRenderType = + getHdrRenderType(state.dataspace, pixelFormat, layerFEState->desiredHdrSdrRatio); + // For hdr content, treat the white point as the display brightness - HDR content should not be // boosted or dimmed. // If the layer explicitly requests to disable dimming, then don't dim either. diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp index ab4c15d670..962dc09680 100644 --- a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp +++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp @@ -60,9 +60,9 @@ void LayerHierarchy::traverse(const Visitor& visitor, return; } } - if (traversalPath.hasRelZLoop()) { - LOG_ALWAYS_FATAL("Found relative z loop layerId:%d", traversalPath.invalidRelativeRootId); - } + + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(traversalPath.hasRelZLoop(), "Found relative z loop layerId:%d", + traversalPath.invalidRelativeRootId); for (auto& [child, childVariant] : mChildren) { ScopedAddToTraversalPath addChildToTraversalPath(traversalPath, child->mLayer->id, childVariant); @@ -104,9 +104,7 @@ void LayerHierarchy::removeChild(LayerHierarchy* child) { [child](const std::pair<LayerHierarchy*, Variant>& x) { return x.first == child; }); - if (it == mChildren.end()) { - LOG_ALWAYS_FATAL("Could not find child!"); - } + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(it == mChildren.end(), "Could not find child!"); mChildren.erase(it); } @@ -119,11 +117,8 @@ void LayerHierarchy::updateChild(LayerHierarchy* hierarchy, LayerHierarchy::Vari [hierarchy](std::pair<LayerHierarchy*, Variant>& child) { return child.first == hierarchy; }); - if (it == mChildren.end()) { - LOG_ALWAYS_FATAL("Could not find child!"); - } else { - it->second = variant; - } + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(it == mChildren.end(), "Could not find child!"); + it->second = variant; } const RequestedLayerState* LayerHierarchy::getLayer() const { @@ -422,9 +417,8 @@ LayerHierarchy LayerHierarchyBuilder::getPartialHierarchy(uint32_t layerId, LayerHierarchy* LayerHierarchyBuilder::getHierarchyFromId(uint32_t layerId, bool crashOnFailure) { auto it = mLayerIdToHierarchy.find(layerId); if (it == mLayerIdToHierarchy.end()) { - if (crashOnFailure) { - LOG_ALWAYS_FATAL("Could not find hierarchy for layer id %d", layerId); - } + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(crashOnFailure, "Could not find hierarchy for layer id %d", + layerId); return nullptr; }; @@ -460,7 +454,7 @@ std::string LayerHierarchy::TraversalPath::toString() const { } LayerHierarchy::TraversalPath LayerHierarchy::TraversalPath::getMirrorRoot() const { - LOG_ALWAYS_FATAL_IF(!isClone(), "Cannot get mirror root of a non cloned node"); + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(!isClone(), "Cannot get mirror root of a non cloned node"); TraversalPath mirrorRootPath = *this; mirrorRootPath.id = mirrorRootId; return mirrorRootPath; diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp index 1712137a7e..a826ec18a1 100644 --- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp +++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp @@ -45,11 +45,11 @@ void LayerLifecycleManager::addLayers(std::vector<std::unique_ptr<RequestedLayer for (auto& newLayer : newLayers) { RequestedLayerState& layer = *newLayer.get(); auto [it, inserted] = mIdToLayer.try_emplace(layer.id, References{.owner = layer}); - if (!inserted) { - LOG_ALWAYS_FATAL("Duplicate layer id found. New layer: %s Existing layer: %s", - layer.getDebugString().c_str(), - it->second.owner.getDebugString().c_str()); - } + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(!inserted, + "Duplicate layer id found. New layer: %s Existing layer: " + "%s", + layer.getDebugString().c_str(), + it->second.owner.getDebugString().c_str()); mAddedLayers.push_back(newLayer.get()); mChangedLayers.push_back(newLayer.get()); layer.parentId = linkLayer(layer.parentId, layer.id); @@ -85,14 +85,15 @@ void LayerLifecycleManager::addLayers(std::vector<std::unique_ptr<RequestedLayer } } -void LayerLifecycleManager::onHandlesDestroyed(const std::vector<uint32_t>& destroyedHandles, - bool ignoreUnknownHandles) { +void LayerLifecycleManager::onHandlesDestroyed( + const std::vector<std::pair<uint32_t, std::string /* debugName */>>& destroyedHandles, + bool ignoreUnknownHandles) { std::vector<uint32_t> layersToBeDestroyed; - for (const auto& layerId : destroyedHandles) { + for (const auto& [layerId, name] : destroyedHandles) { auto it = mIdToLayer.find(layerId); if (it == mIdToLayer.end()) { - LOG_ALWAYS_FATAL_IF(!ignoreUnknownHandles, "%s Layerid not found %d", __func__, - layerId); + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(!ignoreUnknownHandles, "%s Layerid not found %s[%d]", + __func__, name.c_str(), layerId); continue; } RequestedLayerState& layer = it->second.owner; @@ -113,10 +114,8 @@ void LayerLifecycleManager::onHandlesDestroyed(const std::vector<uint32_t>& dest for (size_t i = 0; i < layersToBeDestroyed.size(); i++) { uint32_t layerId = layersToBeDestroyed[i]; auto it = mIdToLayer.find(layerId); - if (it == mIdToLayer.end()) { - LOG_ALWAYS_FATAL("%s Layer with id %d not found", __func__, layerId); - continue; - } + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(it == mIdToLayer.end(), "%s Layer with id %d not found", + __func__, layerId); RequestedLayerState& layer = it->second.owner; @@ -135,11 +134,9 @@ void LayerLifecycleManager::onHandlesDestroyed(const std::vector<uint32_t>& dest auto& references = it->second.references; for (uint32_t linkedLayerId : references) { RequestedLayerState* linkedLayer = getLayerFromId(linkedLayerId); - if (!linkedLayer) { - LOG_ALWAYS_FATAL("%s Layerid reference %d not found for %d", __func__, - linkedLayerId, layer.id); - continue; - }; + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(!linkedLayer, + "%s Layerid reference %d not found for %d", __func__, + linkedLayerId, layer.id); if (linkedLayer->parentId == layer.id) { linkedLayer->parentId = UNASSIGNED_LAYER_ID; if (linkedLayer->canBeDestroyed()) { @@ -191,17 +188,17 @@ void LayerLifecycleManager::applyTransactions(const std::vector<TransactionState RequestedLayerState* layer = getLayerFromId(layerId); if (layer == nullptr) { - LOG_ALWAYS_FATAL_IF(!ignoreUnknownLayers, "%s Layer with layerid=%d not found", - __func__, layerId); + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(!ignoreUnknownLayers, + "%s Layer with layerid=%d not found", __func__, + layerId); continue; } - if (!layer->handleAlive) { - LOG_ALWAYS_FATAL("%s Layer's with layerid=%d) is not alive. Possible out of " - "order LayerLifecycleManager updates", - __func__, layerId); - continue; - } + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(!layer->handleAlive, + "%s Layer's with layerid=%d) is not alive. Possible " + "out of " + "order LayerLifecycleManager updates", + __func__, layerId); if (layer->changes.get() == 0) { mChangedLayers.push_back(layer); @@ -241,7 +238,7 @@ void LayerLifecycleManager::applyTransactions(const std::vector<TransactionState RequestedLayerState* bgColorLayer = getLayerFromId(layer->bgColorLayerId); layer->bgColorLayerId = UNASSIGNED_LAYER_ID; bgColorLayer->parentId = unlinkLayer(bgColorLayer->parentId, bgColorLayer->id); - onHandlesDestroyed({bgColorLayer->id}); + onHandlesDestroyed({{bgColorLayer->id, bgColorLayer->debugName}}); } else if (layer->bgColorLayerId != UNASSIGNED_LAYER_ID) { RequestedLayerState* bgColorLayer = getLayerFromId(layer->bgColorLayerId); bgColorLayer->color = layer->bgColor; diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h index 48571bf923..9aff78e463 100644 --- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h +++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h @@ -47,7 +47,8 @@ public: // Ignore unknown handles when iteroping with legacy front end. In the old world, we // would create child layers which are not necessary with the new front end. This means // we will get notified for handle changes that don't exist in the new front end. - void onHandlesDestroyed(const std::vector<uint32_t>&, bool ignoreUnknownHandles = false); + void onHandlesDestroyed(const std::vector<std::pair<uint32_t, std::string /* debugName */>>&, + bool ignoreUnknownHandles = false); // Detaches the layer from its relative parent to prevent a loop in the // layer hierarchy. This overrides the RequestedLayerState and leaves diff --git a/services/surfaceflinger/FrontEnd/LayerLog.h b/services/surfaceflinger/FrontEnd/LayerLog.h index 4943483e62..3845dfeb26 100644 --- a/services/surfaceflinger/FrontEnd/LayerLog.h +++ b/services/surfaceflinger/FrontEnd/LayerLog.h @@ -16,6 +16,8 @@ #pragma once +#include "Tracing/TransactionTracing.h" + // Uncomment to trace layer updates for a single layer // #define LOG_LAYER 1 @@ -27,3 +29,17 @@ #endif #define LLOGD(LAYER_ID, x, ...) ALOGD("[%d] %s " x, (LAYER_ID), __func__, ##__VA_ARGS__); + +#define LLOG_ALWAYS_FATAL_WITH_TRACE(...) \ + do { \ + TransactionTraceWriter::getInstance().invoke(__func__, /* overwrite= */ false); \ + LOG_ALWAYS_FATAL(##__VA_ARGS__); \ + } while (false) + +#define LLOG_ALWAYS_FATAL_WITH_TRACE_IF(cond, ...) \ + do { \ + if (__predict_false(cond)) { \ + TransactionTraceWriter::getInstance().invoke(__func__, /* overwrite= */ false); \ + } \ + LOG_ALWAYS_FATAL_IF(cond, ##__VA_ARGS__); \ + } while (false)
\ No newline at end of file diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp index 159d0f028d..21f1a4a53f 100644 --- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp +++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp @@ -523,12 +523,9 @@ const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy( const Args& args, const LayerHierarchy& hierarchy, LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot, int depth) { - if (depth > 50) { - TransactionTraceWriter::getInstance().invoke("layer_builder_stack_overflow_", - /*overwrite=*/false); - LOG_ALWAYS_FATAL("Cycle detected in LayerSnapshotBuilder. See " - "builder_stack_overflow_transactions.winscope"); - } + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(depth > 50, + "Cycle detected in LayerSnapshotBuilder. See " + "builder_stack_overflow_transactions.winscope"); const RequestedLayerState* layer = hierarchy.getLayer(); LayerSnapshot* snapshot = getSnapshot(traversalPath); diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp index a5d556328d..8a39cd6330 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp @@ -150,7 +150,7 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta const bool hadSideStream = sidebandStream != nullptr; const layer_state_t& clientState = resolvedComposerState.state; - const bool hadBlur = hasBlur(); + const bool hadSomethingToDraw = hasSomethingToDraw(); uint64_t clientChanges = what | layer_state_t::diff(clientState); layer_state_t::merge(clientState); what = clientChanges; @@ -228,11 +228,10 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta RequestedLayerState::Changes::VisibleRegion; } } - if (what & (layer_state_t::eBackgroundBlurRadiusChanged | layer_state_t::eBlurRegionsChanged)) { - if (hadBlur != hasBlur()) { - changes |= RequestedLayerState::Changes::Visibility | - RequestedLayerState::Changes::VisibleRegion; - } + + if (hadSomethingToDraw != hasSomethingToDraw()) { + changes |= RequestedLayerState::Changes::Visibility | + RequestedLayerState::Changes::VisibleRegion; } if (clientChanges & layer_state_t::HIERARCHY_CHANGES) changes |= RequestedLayerState::Changes::Hierarchy; @@ -579,6 +578,12 @@ bool RequestedLayerState::isProtected() const { return externalTexture && externalTexture->getUsage() & GRALLOC_USAGE_PROTECTED; } +bool RequestedLayerState::hasSomethingToDraw() const { + return externalTexture != nullptr || sidebandStream != nullptr || shadowRadius > 0.f || + backgroundBlurRadius > 0 || blurRegions.size() > 0 || + (color.r >= 0.0_hf && color.g >= 0.0_hf && color.b >= 0.0_hf); +} + void RequestedLayerState::clearChanges() { what = 0; changes.clear(); diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.h b/services/surfaceflinger/FrontEnd/RequestedLayerState.h index 8eff22bbe4..09f33de63b 100644 --- a/services/surfaceflinger/FrontEnd/RequestedLayerState.h +++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.h @@ -87,6 +87,7 @@ struct RequestedLayerState : layer_state_t { bool backpressureEnabled() const; bool isSimpleBufferUpdate(const layer_state_t&) const; bool isProtected() const; + bool hasSomethingToDraw() const; // Layer serial number. This gives layers an explicit ordering, so we // have a stable sort order when their layer stack and Z-order are diff --git a/services/surfaceflinger/FrontEnd/TransactionHandler.cpp b/services/surfaceflinger/FrontEnd/TransactionHandler.cpp index ca7c3c25f7..d3d950974f 100644 --- a/services/surfaceflinger/FrontEnd/TransactionHandler.cpp +++ b/services/surfaceflinger/FrontEnd/TransactionHandler.cpp @@ -22,6 +22,7 @@ #include <cutils/trace.h> #include <utils/Log.h> #include <utils/Trace.h> +#include "FrontEnd/LayerLog.h" #include "TransactionHandler.h" @@ -87,8 +88,8 @@ void TransactionHandler::applyUnsignaledBufferTransaction( } auto it = mPendingTransactionQueues.find(flushState.queueWithUnsignaledBuffer); - LOG_ALWAYS_FATAL_IF(it == mPendingTransactionQueues.end(), - "Could not find queue with unsignaled buffer!"); + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(it == mPendingTransactionQueues.end(), + "Could not find queue with unsignaled buffer!"); auto& queue = it->second; popTransactionFromPending(transactions, flushState, queue); diff --git a/services/surfaceflinger/FrontEnd/Update.h b/services/surfaceflinger/FrontEnd/Update.h index e1449b6e1b..e5cca8fa95 100644 --- a/services/surfaceflinger/FrontEnd/Update.h +++ b/services/surfaceflinger/FrontEnd/Update.h @@ -46,7 +46,7 @@ struct Update { std::vector<LayerCreatedState> layerCreatedStates; std::vector<std::unique_ptr<frontend::RequestedLayerState>> newLayers; std::vector<LayerCreationArgs> layerCreationArgs; - std::vector<uint32_t> destroyedHandles; + std::vector<std::pair<uint32_t, std::string /* debugName */>> destroyedHandles; }; } // namespace android::surfaceflinger::frontend diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 1ef381c417..32bb46c6fd 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -134,6 +134,7 @@ #include "FrontEnd/LayerCreationArgs.h" #include "FrontEnd/LayerHandle.h" #include "FrontEnd/LayerLifecycleManager.h" +#include "FrontEnd/LayerLog.h" #include "FrontEnd/LayerSnapshot.h" #include "HdrLayerInfoReporter.h" #include "Layer.h" @@ -863,30 +864,32 @@ void SurfaceFlinger::init() FTL_FAKE_GUARD(kMainThreadContext) { ALOGE("Run StartPropertySetThread failed!"); } - if (mTransactionTracing) { - TransactionTraceWriter::getInstance().setWriterFunction([&](const std::string& prefix, - bool overwrite) { - auto writeFn = [&]() { - const std::string filename = - TransactionTracing::DIR_NAME + prefix + TransactionTracing::FILE_NAME; - if (!overwrite && access(filename.c_str(), F_OK) == 0) { - ALOGD("TransactionTraceWriter: file=%s already exists", filename.c_str()); - return; - } - mTransactionTracing->flush(); - mTransactionTracing->writeToFile(filename); - }; - if (std::this_thread::get_id() == mMainThreadId) { - writeFn(); - } else { - mScheduler->schedule(writeFn).get(); - } - }); - } - + initTransactionTraceWriter(); ALOGV("Done initializing"); } +void SurfaceFlinger::initTransactionTraceWriter() { + if (!mTransactionTracing) { + return; + } + TransactionTraceWriter::getInstance().setWriterFunction( + [&](const std::string& filename, bool overwrite) { + auto writeFn = [&]() { + if (!overwrite && access(filename.c_str(), F_OK) == 0) { + ALOGD("TransactionTraceWriter: file=%s already exists", filename.c_str()); + return; + } + mTransactionTracing->flush(); + mTransactionTracing->writeToFile(filename); + }; + if (std::this_thread::get_id() == mMainThreadId) { + writeFn(); + } else { + mScheduler->schedule(writeFn).get(); + } + }); +} + void SurfaceFlinger::readPersistentProperties() { Mutex::Autolock _l(mStateLock); @@ -2618,6 +2621,23 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( constexpr bool kCursorOnly = false; const auto layers = moveSnapshotsToCompositionArgs(refreshArgs, kCursorOnly); + if (mLayerLifecycleManagerEnabled && !refreshArgs.updatingGeometryThisFrame) { + for (const auto& [token, display] : FTL_FAKE_GUARD(mStateLock, mDisplays)) { + auto compositionDisplay = display->getCompositionDisplay(); + if (!compositionDisplay->getState().isEnabled) continue; + for (auto outputLayer : compositionDisplay->getOutputLayersOrderedByZ()) { + LLOG_ALWAYS_FATAL_WITH_TRACE_IF(outputLayer->getLayerFE().getCompositionState() == + nullptr, + "Output layer %s for display %s %" PRIu64 + " has a null " + "snapshot.", + outputLayer->getLayerFE().getDebugName(), + compositionDisplay->getName().c_str(), + compositionDisplay->getId().value); + } + } + } + mCompositionEngine->present(refreshArgs); moveSnapshotsFromCompositionArgs(refreshArgs, layers); @@ -5525,7 +5545,7 @@ void SurfaceFlinger::markLayerPendingRemovalLocked(const sp<Layer>& layer) { void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32_t layerId) { { std::scoped_lock<std::mutex> lock(mCreatedLayersLock); - mDestroyedHandles.emplace_back(layerId); + mDestroyedHandles.emplace_back(layerId, layer->getDebugName()); } mTransactionHandler.onLayerDestroyed(layerId); diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 4d17fa7d2a..7b64489028 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -1134,7 +1134,7 @@ private: ui::Rotation getPhysicalDisplayOrientation(DisplayId, bool isPrimary) const REQUIRES(mStateLock); void traverseLegacyLayers(const LayerVector::Visitor& visitor) const; - + void initTransactionTraceWriter(); sp<StartPropertySetThread> mStartPropertySetThread; surfaceflinger::Factory& mFactory; pid_t mPid; @@ -1425,7 +1425,7 @@ private: frontend::LayerHierarchyBuilder mLayerHierarchyBuilder{{}}; frontend::LayerSnapshotBuilder mLayerSnapshotBuilder; - std::vector<uint32_t> mDestroyedHandles; + std::vector<std::pair<uint32_t, std::string>> mDestroyedHandles; std::vector<std::unique_ptr<frontend::RequestedLayerState>> mNewLayers; std::vector<LayerCreationArgs> mNewLayerArgs; // These classes do not store any client state but help with managing transaction callbacks diff --git a/services/surfaceflinger/Tracing/TransactionTracing.cpp b/services/surfaceflinger/Tracing/TransactionTracing.cpp index 7e330b97dd..bc69191cc1 100644 --- a/services/surfaceflinger/Tracing/TransactionTracing.cpp +++ b/services/surfaceflinger/Tracing/TransactionTracing.cpp @@ -111,7 +111,7 @@ void TransactionTracing::addCommittedTransactions(int64_t vsyncId, nsecs_t commi update.createdLayers = std::move(newUpdate.layerCreationArgs); newUpdate.layerCreationArgs.clear(); update.destroyedLayerHandles.reserve(newUpdate.destroyedHandles.size()); - for (uint32_t handle : newUpdate.destroyedHandles) { + for (auto& [handle, _] : newUpdate.destroyedHandles) { update.destroyedLayerHandles.push_back(handle); } mPendingUpdates.emplace_back(update); diff --git a/services/surfaceflinger/Tracing/TransactionTracing.h b/services/surfaceflinger/Tracing/TransactionTracing.h index 31bca5fc66..422b5f3689 100644 --- a/services/surfaceflinger/Tracing/TransactionTracing.h +++ b/services/surfaceflinger/Tracing/TransactionTracing.h @@ -73,12 +73,16 @@ public: static constexpr auto TRACING_VERSION = 1; private: + friend class TransactionTraceWriter; friend class TransactionTracingTest; friend class SurfaceFlinger; static constexpr auto DIR_NAME = "/data/misc/wmtrace/"; static constexpr auto FILE_NAME = "transactions_trace.winscope"; static constexpr auto FILE_PATH = "/data/misc/wmtrace/transactions_trace.winscope"; + static std::string getFilePath(const std::string& prefix) { + return DIR_NAME + prefix + FILE_NAME; + } mutable std::mutex mTraceLock; TransactionRingBuffer<proto::TransactionTraceFile, proto::TransactionTraceEntry> mBuffer @@ -138,10 +142,16 @@ class TransactionTraceWriter : public Singleton<TransactionTraceWriter> { public: void setWriterFunction( - std::function<void(const std::string& prefix, bool overwrite)> function) { + std::function<void(const std::string& filename, bool overwrite)> function) { mWriterFunction = std::move(function); } - void invoke(const std::string& prefix, bool overwrite) { mWriterFunction(prefix, overwrite); } + void invoke(const std::string& prefix, bool overwrite) { + mWriterFunction(TransactionTracing::getFilePath(prefix), overwrite); + } + /* pass in a complete file path for testing */ + void invokeForTest(const std::string& filename, bool overwrite) { + mWriterFunction(filename, overwrite); + } }; } // namespace android diff --git a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp index 519ef44552..321b8baccc 100644 --- a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp +++ b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp @@ -109,11 +109,11 @@ bool LayerTraceGenerator::generate(const proto::TransactionTraceFile& traceFile, ALOGV(" destroyedHandles=%d", entry.destroyed_layers(j)); } - std::vector<uint32_t> destroyedHandles; + std::vector<std::pair<uint32_t, std::string>> destroyedHandles; destroyedHandles.reserve((size_t)entry.destroyed_layer_handles_size()); for (int j = 0; j < entry.destroyed_layer_handles_size(); j++) { ALOGV(" destroyedHandles=%d", entry.destroyed_layer_handles(j)); - destroyedHandles.push_back(entry.destroyed_layer_handles(j)); + destroyedHandles.push_back({entry.destroyed_layer_handles(j), ""}); } bool displayChanged = entry.displays_changed(); diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index 3dd33b9957..7d8796f71c 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -66,6 +66,7 @@ cc_test { // option to false temporarily. address: true, }, + static_libs: ["libc++fs"], srcs: [ ":libsurfaceflinger_mock_sources", ":libsurfaceflinger_sources", @@ -128,6 +129,7 @@ cc_test { "TransactionFrameTracerTest.cpp", "TransactionProtoParserTest.cpp", "TransactionSurfaceFrameTest.cpp", + "TransactionTraceWriterTest.cpp", "TransactionTracingTest.cpp", "TunnelModeEnabledReporterTest.cpp", "StrongTypingTest.cpp", diff --git a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h index f64ba2a439..902f2b9405 100644 --- a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h +++ b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h @@ -170,7 +170,7 @@ protected: mLifecycleManager.applyTransactions(transactions); } - void destroyLayerHandle(uint32_t id) { mLifecycleManager.onHandlesDestroyed({id}); } + void destroyLayerHandle(uint32_t id) { mLifecycleManager.onHandlesDestroyed({{id, "test"}}); } void updateAndVerify(LayerHierarchyBuilder& hierarchyBuilder) { if (mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy)) { diff --git a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp index 97ef5a268d..d65277a999 100644 --- a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp @@ -84,7 +84,7 @@ TEST_F(LayerLifecycleManagerTest, addLayers) { layers.emplace_back(rootLayer(2)); layers.emplace_back(rootLayer(3)); lifecycleManager.addLayers(std::move(layers)); - lifecycleManager.onHandlesDestroyed({1, 2, 3}); + lifecycleManager.onHandlesDestroyed({{1, "1"}, {2, "2"}, {3, "3"}}); EXPECT_TRUE(lifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy)); lifecycleManager.commitChanges(); EXPECT_FALSE(lifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy)); @@ -133,7 +133,7 @@ TEST_F(LayerLifecycleManagerTest, layerWithoutHandleIsDestroyed) { layers.emplace_back(rootLayer(1)); layers.emplace_back(rootLayer(2)); lifecycleManager.addLayers(std::move(layers)); - lifecycleManager.onHandlesDestroyed({1}); + lifecycleManager.onHandlesDestroyed({{1, "1"}}); lifecycleManager.commitChanges(); SCOPED_TRACE("layerWithoutHandleIsDestroyed"); @@ -149,7 +149,7 @@ TEST_F(LayerLifecycleManagerTest, rootLayerWithoutHandleIsDestroyed) { layers.emplace_back(rootLayer(1)); layers.emplace_back(rootLayer(2)); lifecycleManager.addLayers(std::move(layers)); - lifecycleManager.onHandlesDestroyed({1}); + lifecycleManager.onHandlesDestroyed({{1, "1"}}); lifecycleManager.commitChanges(); listener->expectLayersAdded({1, 2}); listener->expectLayersDestroyed({1}); @@ -173,7 +173,7 @@ TEST_F(LayerLifecycleManagerTest, offscreenLayerIsDestroyed) { listener->expectLayersAdded({}); listener->expectLayersDestroyed({}); - lifecycleManager.onHandlesDestroyed({3}); + lifecycleManager.onHandlesDestroyed({{3, "3"}}); lifecycleManager.commitChanges(); listener->expectLayersAdded({}); listener->expectLayersDestroyed({3}); @@ -194,7 +194,7 @@ TEST_F(LayerLifecycleManagerTest, offscreenChildLayerWithHandleIsNotDestroyed) { listener->expectLayersDestroyed({}); lifecycleManager.applyTransactions(reparentLayerTransaction(3, UNASSIGNED_LAYER_ID)); - lifecycleManager.onHandlesDestroyed({3}); + lifecycleManager.onHandlesDestroyed({{3, "3"}}); lifecycleManager.commitChanges(); listener->expectLayersAdded({}); listener->expectLayersDestroyed({3}); @@ -215,7 +215,7 @@ TEST_F(LayerLifecycleManagerTest, offscreenChildLayerWithoutHandleIsDestroyed) { listener->expectLayersDestroyed({}); lifecycleManager.applyTransactions(reparentLayerTransaction(3, UNASSIGNED_LAYER_ID)); - lifecycleManager.onHandlesDestroyed({3, 4}); + lifecycleManager.onHandlesDestroyed({{3, "3"}, {4, "4"}}); lifecycleManager.commitChanges(); listener->expectLayersAdded({}); listener->expectLayersDestroyed({3, 4}); @@ -376,7 +376,7 @@ TEST_F(LayerLifecycleManagerTest, onParentDestroyDestroysBackgroundLayer) { transactions.back().states.front().layerId = 1; transactions.emplace_back(); lifecycleManager.applyTransactions(transactions); - lifecycleManager.onHandlesDestroyed({1}); + lifecycleManager.onHandlesDestroyed({{1, "1"}}); ASSERT_EQ(lifecycleManager.getLayers().size(), 0u); ASSERT_EQ(lifecycleManager.getDestroyedLayers().size(), 2u); @@ -389,4 +389,52 @@ TEST_F(LayerLifecycleManagerTest, onParentDestroyDestroysBackgroundLayer) { listener->expectLayersDestroyed({1, bgLayerId}); } +TEST_F(LayerLifecycleManagerTest, blurSetsVisibilityChangeFlag) { + // clear default color on layer so we start with a layer that does not draw anything. + setColor(1, {-1.f, -1.f, -1.f}); + mLifecycleManager.commitChanges(); + + // layer has something to draw + setBackgroundBlurRadius(1, 2); + EXPECT_TRUE( + mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility)); + mLifecycleManager.commitChanges(); + + // layer still has something to draw, so visibility shouldn't change + setBackgroundBlurRadius(1, 3); + EXPECT_FALSE( + mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility)); + mLifecycleManager.commitChanges(); + + // layer has nothing to draw + setBackgroundBlurRadius(1, 0); + EXPECT_TRUE( + mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility)); + mLifecycleManager.commitChanges(); +} + +TEST_F(LayerLifecycleManagerTest, colorSetsVisibilityChangeFlag) { + // clear default color on layer so we start with a layer that does not draw anything. + setColor(1, {-1.f, -1.f, -1.f}); + mLifecycleManager.commitChanges(); + + // layer has something to draw + setColor(1, {2.f, 3.f, 4.f}); + EXPECT_TRUE( + mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility)); + mLifecycleManager.commitChanges(); + + // layer still has something to draw, so visibility shouldn't change + setColor(1, {0.f, 0.f, 0.f}); + EXPECT_FALSE( + mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility)); + mLifecycleManager.commitChanges(); + + // layer has nothing to draw + setColor(1, {-1.f, -1.f, -1.f}); + EXPECT_TRUE( + mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility)); + mLifecycleManager.commitChanges(); +} + } // namespace android::surfaceflinger::frontend diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp index 84c37759ba..c8eda12d9a 100644 --- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp @@ -349,16 +349,22 @@ TEST_F(LayerSnapshotTest, CanCropTouchableRegion) { } TEST_F(LayerSnapshotTest, blurUpdatesWhenAlphaChanges) { - static constexpr int blurRadius = 42; - setBackgroundBlurRadius(1221, blurRadius); + int blurRadius = 42; + setBackgroundBlurRadius(1221, static_cast<uint32_t>(blurRadius)); UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); EXPECT_EQ(getSnapshot({.id = 1221})->backgroundBlurRadius, blurRadius); + blurRadius = 21; + setBackgroundBlurRadius(1221, static_cast<uint32_t>(blurRadius)); + UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); + EXPECT_EQ(getSnapshot({.id = 1221})->backgroundBlurRadius, blurRadius); + static constexpr float alpha = 0.5; setAlpha(12, alpha); UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER); - EXPECT_EQ(getSnapshot({.id = 1221})->backgroundBlurRadius, blurRadius * alpha); + EXPECT_EQ(getSnapshot({.id = 1221})->backgroundBlurRadius, + static_cast<int>(static_cast<float>(blurRadius) * alpha)); } // Display Mirroring Tests diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 02fa4153ed..8458aa3ce0 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -651,6 +651,11 @@ public: auto fromHandle(const sp<IBinder>& handle) { return LayerHandle::getLayer(handle); } + auto initTransactionTraceWriter() { + mFlinger->mTransactionTracing.emplace(); + return mFlinger->initTransactionTraceWriter(); + } + ~TestableSurfaceFlinger() { // All these pointer and container clears help ensure that GMock does // not report a leaked object, since the SurfaceFlinger instance may @@ -664,6 +669,7 @@ public: mFlinger->mCompositionEngine->setHwComposer(std::unique_ptr<HWComposer>()); mFlinger->mRenderEngine = std::unique_ptr<renderengine::RenderEngine>(); mFlinger->mCompositionEngine->setRenderEngine(mFlinger->mRenderEngine.get()); + mFlinger->mTransactionTracing.reset(); } /* ------------------------------------------------------------------------ diff --git a/services/surfaceflinger/tests/unittests/TransactionTraceWriterTest.cpp b/services/surfaceflinger/tests/unittests/TransactionTraceWriterTest.cpp new file mode 100644 index 0000000000..379135e270 --- /dev/null +++ b/services/surfaceflinger/tests/unittests/TransactionTraceWriterTest.cpp @@ -0,0 +1,79 @@ +/* + * Copyright 2023 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. + */ + +#undef LOG_TAG +#define LOG_TAG "TransactionTraceWriterTest" + +#include <gmock/gmock.h> +#include <gtest/gtest.h> +#include <log/log.h> +#include <filesystem> + +#include "TestableSurfaceFlinger.h" + +namespace android { + +class TransactionTraceWriterTest : public testing::Test { +protected: + std::string mFilename = "/data/local/tmp/testfile_transaction_trace.winscope"; + + void SetUp() { mFlinger.initTransactionTraceWriter(); } + void TearDown() { std::filesystem::remove(mFilename); } + + void verifyTraceFile() { + std::fstream file(mFilename, std::ios::in); + ASSERT_TRUE(file.is_open()); + std::string line; + char magicNumber[8]; + file.read(magicNumber, 8); + EXPECT_EQ("\tTNXTRAC", std::string(magicNumber, magicNumber + 8)); + } + + TestableSurfaceFlinger mFlinger; +}; + +TEST_F(TransactionTraceWriterTest, canWriteToFile) { + TransactionTraceWriter::getInstance().invokeForTest(mFilename, /* overwrite */ true); + EXPECT_EQ(access(mFilename.c_str(), F_OK), 0); + verifyTraceFile(); +} + +TEST_F(TransactionTraceWriterTest, canOverwriteFile) { + std::string testLine = "test"; + { + std::ofstream file(mFilename, std::ios::out); + file << testLine; + } + TransactionTraceWriter::getInstance().invokeForTest(mFilename, /* overwrite */ true); + verifyTraceFile(); +} + +TEST_F(TransactionTraceWriterTest, doNotOverwriteFile) { + std::string testLine = "test"; + { + std::ofstream file(mFilename, std::ios::out); + file << testLine; + } + TransactionTraceWriter::getInstance().invokeForTest(mFilename, /* overwrite */ false); + { + std::fstream file(mFilename, std::ios::in); + ASSERT_TRUE(file.is_open()); + std::string line; + std::getline(file, line); + EXPECT_EQ(line, testLine); + } +} +} // namespace android
\ No newline at end of file |