From e47d243cb1c209873f9847e69d5dce2df2566702 Mon Sep 17 00:00:00 2001 From: rnlee Date: Wed, 4 Aug 2021 12:28:05 -0700 Subject: Enable -Wconversion for Client. Bug: 129481165 Test: atest libsurfaceflinger_unittest Change-Id: I7e7ade6a38a02bcc94ff96d90f902b96afa2da9b --- services/surfaceflinger/Client.cpp | 7 ------- 1 file changed, 7 deletions(-) (limited to 'services/surfaceflinger/Client.cpp') diff --git a/services/surfaceflinger/Client.cpp b/services/surfaceflinger/Client.cpp index aac6c913cf..8da2e24aa4 100644 --- a/services/surfaceflinger/Client.cpp +++ b/services/surfaceflinger/Client.cpp @@ -14,10 +14,6 @@ * limitations under the License. */ -// TODO(b/129481165): remove the #pragma below and fix conversion issues -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wconversion" - #include #include @@ -132,6 +128,3 @@ status_t Client::getLayerFrameStats(const sp& handle, FrameStats* outSt // --------------------------------------------------------------------------- }; // namespace android - -// TODO(b/129481165): remove the #pragma below and fix conversion issues -#pragma clang diagnostic pop // ignored "-Wconversion" -- cgit v1.2.3-59-g8ed1b From 7fb9e5a047292114552ba6523bd790269b6cf937 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Mon, 8 Nov 2021 12:44:05 -0800 Subject: SF: Create layers with layerid When recreating layer states from transaction traces we need to create layers with a specific layer id. Add the layer id as part of LayerCreationArgs and pass the struct around instead of individual args. Test: presubmit Bug: 200284593 Change-Id: I029cdb5362d1926deaf2ce64f70a1882a418705b --- services/surfaceflinger/BufferQueueLayer.cpp | 2 +- services/surfaceflinger/BufferQueueLayer.h | 5 + services/surfaceflinger/BufferStateLayer.cpp | 2 +- services/surfaceflinger/Client.cpp | 45 +++---- services/surfaceflinger/ContainerLayer.cpp | 3 +- services/surfaceflinger/EffectLayer.cpp | 3 +- services/surfaceflinger/Layer.cpp | 15 +-- services/surfaceflinger/Layer.h | 10 +- services/surfaceflinger/LayerRenderArea.cpp | 4 +- services/surfaceflinger/MonitoredProducer.cpp | 8 +- services/surfaceflinger/SurfaceFlinger.cpp | 132 +++++---------------- services/surfaceflinger/SurfaceFlinger.h | 44 ++----- .../tests/SurfaceInterceptor_test.cpp | 4 +- .../tests/unittests/CompositionTest.cpp | 5 +- .../tests/unittests/FpsReporterTest.cpp | 3 +- .../tests/unittests/GameModeTest.cpp | 2 +- .../tests/unittests/RefreshRateSelectionTest.cpp | 7 +- .../tests/unittests/SetFrameRateTest.cpp | 6 +- .../tests/unittests/TransactionFrameTracerTest.cpp | 2 +- .../unittests/TransactionSurfaceFrameTest.cpp | 2 +- .../unittests/TunnelModeEnabledReporterTest.cpp | 3 +- .../tests/unittests/mock/MockLayer.h | 2 +- 22 files changed, 99 insertions(+), 210 deletions(-) (limited to 'services/surfaceflinger/Client.cpp') diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index 28c387e5bd..dec7cc0806 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -523,7 +523,7 @@ void BufferQueueLayer::gatherBufferInfo() { } sp BufferQueueLayer::createClone() { - LayerCreationArgs args(mFlinger.get(), nullptr, mName + " (Mirror)", 0, 0, 0, LayerMetadata()); + LayerCreationArgs args(mFlinger.get(), nullptr, mName + " (Mirror)", 0, LayerMetadata()); args.textureName = mTextureName; sp layer = mFlinger->getFactory().createBufferQueueLayer(args); layer->setInitialValuesForClone(this); diff --git a/services/surfaceflinger/BufferQueueLayer.h b/services/surfaceflinger/BufferQueueLayer.h index dfdb5c055d..c6e0727806 100644 --- a/services/surfaceflinger/BufferQueueLayer.h +++ b/services/surfaceflinger/BufferQueueLayer.h @@ -62,6 +62,11 @@ public: status_t setDefaultBufferProperties(uint32_t w, uint32_t h, PixelFormat format); sp getProducer() const; + void setSizeForTest(uint32_t w, uint32_t h) { + mDrawingState.active_legacy.w = w; + mDrawingState.active_legacy.h = h; + } + protected: void gatherBufferInfo() override; diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index b6cbbb658f..88e3fb6744 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -890,7 +890,7 @@ Rect BufferStateLayer::computeBufferCrop(const State& s) { } sp BufferStateLayer::createClone() { - LayerCreationArgs args(mFlinger.get(), nullptr, mName + " (Mirror)", 0, 0, 0, LayerMetadata()); + LayerCreationArgs args(mFlinger.get(), nullptr, mName + " (Mirror)", 0, LayerMetadata()); args.textureName = mTextureName; sp layer = mFlinger->getFactory().createBufferStateLayer(args); layer->mHwcSlotGenerator = mHwcSlotGenerator; diff --git a/services/surfaceflinger/Client.cpp b/services/surfaceflinger/Client.cpp index 8da2e24aa4..0a8ebec9f3 100644 --- a/services/surfaceflinger/Client.cpp +++ b/services/surfaceflinger/Client.cpp @@ -72,35 +72,28 @@ sp Client::getLayerUser(const sp& handle) const return lbc; } -status_t Client::createSurface(const String8& name, uint32_t w, uint32_t h, PixelFormat format, - uint32_t flags, const sp& parentHandle, - LayerMetadata metadata, sp* handle, - sp* gbp, int32_t* outLayerId, - uint32_t* outTransformHint) { +status_t Client::createSurface(const String8& name, uint32_t /* w */, uint32_t /* h */, + PixelFormat /* format */, uint32_t flags, + const sp& parentHandle, LayerMetadata metadata, + sp* outHandle, sp* /* gbp */, + int32_t* outLayerId, uint32_t* outTransformHint) { // We rely on createLayer to check permissions. - return mFlinger->createLayer(name, this, w, h, format, flags, std::move(metadata), handle, gbp, - parentHandle, outLayerId, nullptr, outTransformHint); + LayerCreationArgs args(mFlinger.get(), this, name.c_str(), flags, std::move(metadata)); + return mFlinger->createLayer(args, outHandle, parentHandle, outLayerId, nullptr, + outTransformHint); } -status_t Client::createWithSurfaceParent(const String8& name, uint32_t w, uint32_t h, - PixelFormat format, uint32_t flags, - const sp& parent, - LayerMetadata metadata, sp* handle, - sp* gbp, int32_t* outLayerId, - uint32_t* outTransformHint) { - if (mFlinger->authenticateSurfaceTexture(parent) == false) { - ALOGE("failed to authenticate surface texture"); - return BAD_VALUE; - } - - const auto& layer = (static_cast(parent.get()))->getLayer(); - if (layer == nullptr) { - ALOGE("failed to find parent layer"); - return BAD_VALUE; - } - - return mFlinger->createLayer(name, this, w, h, format, flags, std::move(metadata), handle, gbp, - nullptr, outLayerId, layer, outTransformHint); +status_t Client::createWithSurfaceParent(const String8& /* name */, uint32_t /* w */, + uint32_t /* h */, PixelFormat /* format */, + uint32_t /* flags */, + const sp& /* parent */, + LayerMetadata /* metadata */, sp* /* handle */, + sp* /* gbp */, + int32_t* /* outLayerId */, + uint32_t* /* outTransformHint */) { + // This api does not make sense with blast since SF no longer tracks IGBP. This api should be + // removed. + return BAD_VALUE; } status_t Client::mirrorSurface(const sp& mirrorFromHandle, sp* outHandle, diff --git a/services/surfaceflinger/ContainerLayer.cpp b/services/surfaceflinger/ContainerLayer.cpp index 841e79f8af..3ccc229261 100644 --- a/services/surfaceflinger/ContainerLayer.cpp +++ b/services/surfaceflinger/ContainerLayer.cpp @@ -36,8 +36,7 @@ bool ContainerLayer::isVisible() const { sp ContainerLayer::createClone() { sp layer = mFlinger->getFactory().createContainerLayer( - LayerCreationArgs(mFlinger.get(), nullptr, mName + " (Mirror)", 0, 0, 0, - LayerMetadata())); + LayerCreationArgs(mFlinger.get(), nullptr, mName + " (Mirror)", 0, LayerMetadata())); layer->setInitialValuesForClone(this); return layer; } diff --git a/services/surfaceflinger/EffectLayer.cpp b/services/surfaceflinger/EffectLayer.cpp index 86c6b2161c..845176c112 100644 --- a/services/surfaceflinger/EffectLayer.cpp +++ b/services/surfaceflinger/EffectLayer.cpp @@ -136,8 +136,7 @@ ui::Dataspace EffectLayer::getDataSpace() const { sp EffectLayer::createClone() { sp layer = mFlinger->getFactory().createEffectLayer( - LayerCreationArgs(mFlinger.get(), nullptr, mName + " (Mirror)", 0, 0, 0, - LayerMetadata())); + LayerCreationArgs(mFlinger.get(), nullptr, mName + " (Mirror)", 0, LayerMetadata())); layer->setInitialValuesForClone(this); return layer; } diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index d85e843d83..3d189d681b 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -87,11 +87,12 @@ using gui::WindowInfo; std::atomic Layer::sSequence{1}; Layer::Layer(const LayerCreationArgs& args) - : mFlinger(args.flinger), + : sequence(args.sequence.value_or(sSequence++)), + mFlinger(args.flinger), mName(base::StringPrintf("%s#%d", args.name.c_str(), sequence)), mClientRef(args.client), - mWindowType( - static_cast(args.metadata.getInt32(METADATA_WINDOW_TYPE, 0))) { + mWindowType(static_cast(args.metadata.getInt32(METADATA_WINDOW_TYPE, 0))), + mLayerCreationFlags(args.flags) { uint32_t layerFlags = 0; if (args.flags & ISurfaceComposerClient::eHidden) layerFlags |= layer_state_t::eLayerHidden; if (args.flags & ISurfaceComposerClient::eOpaque) layerFlags |= layer_state_t::eLayerOpaque; @@ -99,8 +100,6 @@ Layer::Layer(const LayerCreationArgs& args) if (args.flags & ISurfaceComposerClient::eSkipScreenshot) layerFlags |= layer_state_t::eLayerSkipScreenshot; - mDrawingState.active_legacy.w = args.w; - mDrawingState.active_legacy.h = args.h; mDrawingState.flags = layerFlags; mDrawingState.active_legacy.transform.set(0, 0); mDrawingState.crop.makeInvalid(); @@ -185,12 +184,10 @@ Layer::~Layer() { } LayerCreationArgs::LayerCreationArgs(SurfaceFlinger* flinger, sp client, std::string name, - uint32_t w, uint32_t h, uint32_t flags, LayerMetadata metadata) + uint32_t flags, LayerMetadata metadata) : flinger(flinger), client(std::move(client)), name(std::move(name)), - w(w), - h(h), flags(flags), metadata(std::move(metadata)) { IPCThreadState* ipc = IPCThreadState::self(); @@ -887,7 +884,7 @@ bool Layer::setBackgroundColor(const half3& color, float alpha, ui::Dataspace da uint32_t flags = ISurfaceComposerClient::eFXSurfaceEffect; std::string name = mName + "BackgroundColorLayer"; mDrawingState.bgColorLayer = mFlinger->getFactory().createEffectLayer( - LayerCreationArgs(mFlinger.get(), nullptr, std::move(name), 0, 0, flags, + LayerCreationArgs(mFlinger.get(), nullptr, std::move(name), flags, LayerMetadata())); // add to child list diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index b79903d11b..3da07e824d 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -85,20 +85,18 @@ class SurfaceFrame; } // namespace frametimeline struct LayerCreationArgs { - LayerCreationArgs(SurfaceFlinger*, sp, std::string name, uint32_t w, uint32_t h, - uint32_t flags, LayerMetadata); + LayerCreationArgs(SurfaceFlinger*, sp, std::string name, uint32_t flags, LayerMetadata); SurfaceFlinger* flinger; const sp client; std::string name; - uint32_t w; - uint32_t h; uint32_t flags; LayerMetadata metadata; pid_t callingPid; uid_t callingUid; uint32_t textureName; + std::optional sequence = std::nullopt; }; class Layer : public virtual RefBase, compositionengine::LayerFE { @@ -879,7 +877,7 @@ public: // Layer serial number. This gives layers an explicit ordering, so we // have a stable sort order when their layer stack and Z-order are // the same. - int32_t sequence{sSequence++}; + const int32_t sequence; bool mPendingHWCDestroy{false}; @@ -1117,6 +1115,8 @@ private: const std::vector getBlurRegions() const; bool mIsAtRoot = false; + + uint32_t mLayerCreationFlags; }; std::ostream& operator<<(std::ostream& stream, const Layer::FrameRate& rate); diff --git a/services/surfaceflinger/LayerRenderArea.cpp b/services/surfaceflinger/LayerRenderArea.cpp index 11fe6d0755..a1e14559e9 100644 --- a/services/surfaceflinger/LayerRenderArea.cpp +++ b/services/surfaceflinger/LayerRenderArea.cpp @@ -112,12 +112,10 @@ void LayerRenderArea::render(std::function drawLayers) { } drawLayers(); } else { - uint32_t w = static_cast(getWidth()); - uint32_t h = static_cast(getHeight()); // In the "childrenOnly" case we reparent the children to a screenshot // layer which has no properties set and which does not draw. sp screenshotParentLayer = mFlinger.getFactory().createContainerLayer( - {&mFlinger, nullptr, "Screenshot Parent"s, w, h, 0, LayerMetadata()}); + {&mFlinger, nullptr, "Screenshot Parent"s, 0, LayerMetadata()}); ReparentForDrawing reparent(mLayer, screenshotParentLayer, sourceCrop); drawLayers(); diff --git a/services/surfaceflinger/MonitoredProducer.cpp b/services/surfaceflinger/MonitoredProducer.cpp index 6b2d745998..df76f50112 100644 --- a/services/surfaceflinger/MonitoredProducer.cpp +++ b/services/surfaceflinger/MonitoredProducer.cpp @@ -33,13 +33,7 @@ MonitoredProducer::MonitoredProducer(const sp& producer, mFlinger(flinger), mLayer(layer) {} -MonitoredProducer::~MonitoredProducer() { - // Remove ourselves from SurfaceFlinger's list. We do this asynchronously - // because we don't know where this destructor is called from. It could be - // called with the mStateLock held, leading to a dead-lock (it actually - // happens). - mFlinger->removeGraphicBufferProducerAsync(onAsBinder()); -} +MonitoredProducer::~MonitoredProducer() {} status_t MonitoredProducer::requestBuffer(int slot, sp* buf) { return mProducer->requestBuffer(slot, buf); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c2dcd70166..5a6a8ce05a 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -904,9 +904,8 @@ bool SurfaceFlinger::authenticateSurfaceTexture( } bool SurfaceFlinger::authenticateSurfaceTextureLocked( - const sp& bufferProducer) const { - sp surfaceTextureBinder(IInterface::asBinder(bufferProducer)); - return mGraphicBufferProducerList.count(surfaceTextureBinder.get()) > 0; + const sp& /* bufferProducer */) const { + return false; } status_t SurfaceFlinger::getSupportedFrameTimestamps( @@ -3355,20 +3354,15 @@ bool SurfaceFlinger::latchBuffers() { } status_t SurfaceFlinger::addClientLayer(const sp& client, const sp& handle, - const sp& gbc, const sp& lbc, - const wp& parent, bool addToRoot, - uint32_t* outTransformHint) { + const sp& lbc, const wp& parent, + bool addToRoot, uint32_t* outTransformHint) { if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) { ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(), ISurfaceComposer::MAX_LAYERS); return NO_MEMORY; } - wp initialProducer; - if (gbc != nullptr) { - initialProducer = IInterface::asBinder(gbc); - } - setLayerCreatedState(handle, lbc, parent, initialProducer, addToRoot); + setLayerCreatedState(handle, lbc, parent, addToRoot); // Create a transaction includes the initial parent and producer. Vector states; @@ -3384,7 +3378,9 @@ status_t SurfaceFlinger::addClientLayer(const sp& client, const spattachLayer(handle, lbc); + if (client != nullptr) { + client->attachLayer(handle, lbc); + } return setTransactionState(FrameTimelineInfo{}, states, displays, 0 /* flags */, nullptr, InputWindowCommands{}, -1 /* desiredPresentTime */, @@ -3392,13 +3388,6 @@ status_t SurfaceFlinger::addClientLayer(const sp& client, const sp& binder) { - static_cast(schedule([=] { - Mutex::Autolock lock(mStateLock); - mGraphicBufferProducerList.erase(binder); - })); -} - uint32_t SurfaceFlinger::getTransactionFlags() const { return mTransactionFlags; } @@ -4281,17 +4270,14 @@ status_t SurfaceFlinger::mirrorLayer(const sp& client, const sp sp mirrorLayer; sp mirrorFrom; - std::string layerName = "MirrorRoot"; - { Mutex::Autolock _l(mStateLock); mirrorFrom = fromHandle(mirrorFromHandle).promote(); if (!mirrorFrom) { return NAME_NOT_FOUND; } - - status_t result = createContainerLayer(client, std::move(layerName), -1, -1, 0, - LayerMetadata(), outHandle, &mirrorLayer); + LayerCreationArgs args(this, client, "MirrorRoot", 0, LayerMetadata()); + status_t result = createContainerLayer(args, outHandle, &mirrorLayer); if (result != NO_ERROR) { return result; } @@ -4300,22 +4286,13 @@ status_t SurfaceFlinger::mirrorLayer(const sp& client, const sp } *outLayerId = mirrorLayer->sequence; - return addClientLayer(client, *outHandle, nullptr, mirrorLayer, nullptr, false, - nullptr /* outTransformHint */); + return addClientLayer(client, *outHandle, mirrorLayer /* layer */, nullptr /* parent */, + false /* addAsRoot */, nullptr /* outTransformHint */); } -status_t SurfaceFlinger::createLayer(const String8& name, const sp& client, uint32_t w, - uint32_t h, PixelFormat format, uint32_t flags, - LayerMetadata metadata, sp* handle, - sp* gbp, +status_t SurfaceFlinger::createLayer(LayerCreationArgs& args, sp* outHandle, const sp& parentHandle, int32_t* outLayerId, const sp& parentLayer, uint32_t* outTransformHint) { - if (int32_t(w|h) < 0) { - ALOGE("createLayer() failed, w or h is negative (w=%d, h=%d)", - int(w), int(h)); - return BAD_VALUE; - } - ALOG_ASSERT(parentLayer == nullptr || parentHandle == nullptr, "Expected only one of parentLayer or parentHandle to be non-null. " "Programmer error?"); @@ -4324,40 +4301,22 @@ status_t SurfaceFlinger::createLayer(const String8& name, const sp& clie sp layer; - std::string layerName{name.string()}; - - switch (flags & ISurfaceComposerClient::eFXSurfaceMask) { + switch (args.flags & ISurfaceComposerClient::eFXSurfaceMask) { case ISurfaceComposerClient::eFXSurfaceBufferQueue: case ISurfaceComposerClient::eFXSurfaceBufferState: { - result = createBufferStateLayer(client, std::move(layerName), w, h, flags, - std::move(metadata), handle, &layer); + result = createBufferStateLayer(args, outHandle, &layer); std::atomic* pendingBufferCounter = layer->getPendingBufferCounter(); if (pendingBufferCounter) { std::string counterName = layer->getPendingBufferCounterName(); - mBufferCountTracker.add((*handle)->localBinder(), counterName, + mBufferCountTracker.add((*outHandle)->localBinder(), counterName, pendingBufferCounter); } } break; case ISurfaceComposerClient::eFXSurfaceEffect: - // check if buffer size is set for color layer. - if (w > 0 || h > 0) { - ALOGE("createLayer() failed, w or h cannot be set for color layer (w=%d, h=%d)", - int(w), int(h)); - return BAD_VALUE; - } - - result = createEffectLayer(client, std::move(layerName), w, h, flags, - std::move(metadata), handle, &layer); + result = createEffectLayer(args, outHandle, &layer); break; case ISurfaceComposerClient::eFXSurfaceContainer: - // check if buffer size is set for container layer. - if (w > 0 || h > 0) { - ALOGE("createLayer() failed, w or h cannot be set for container layer (w=%d, h=%d)", - int(w), int(h)); - return BAD_VALUE; - } - result = createContainerLayer(client, std::move(layerName), w, h, flags, - std::move(metadata), handle, &layer); + result = createContainerLayer(args, outHandle, &layer); break; default: result = BAD_VALUE; @@ -4377,7 +4336,7 @@ status_t SurfaceFlinger::createLayer(const String8& name, const sp& clie if (parentLayer != nullptr) { addToRoot = false; } - result = addClientLayer(client, *handle, *gbp, layer, parent, addToRoot, outTransformHint); + result = addClientLayer(args.client, *outHandle, layer, parent, addToRoot, outTransformHint); if (result != NO_ERROR) { return result; } @@ -4387,9 +4346,7 @@ status_t SurfaceFlinger::createLayer(const String8& name, const sp& clie return result; } -status_t SurfaceFlinger::createBufferQueueLayer(const sp& client, std::string name, - uint32_t w, uint32_t h, uint32_t flags, - LayerMetadata metadata, PixelFormat& format, +status_t SurfaceFlinger::createBufferQueueLayer(LayerCreationArgs& args, PixelFormat& format, sp* handle, sp* gbp, sp* outLayer) { @@ -4405,7 +4362,6 @@ status_t SurfaceFlinger::createBufferQueueLayer(const sp& client, std::s } sp layer; - LayerCreationArgs args(this, client, std::move(name), w, h, flags, std::move(metadata)); args.textureName = getNewTexture(); { // Grab the SF state lock during this since it's the only safe way to access @@ -4415,7 +4371,7 @@ status_t SurfaceFlinger::createBufferQueueLayer(const sp& client, std::s layer = getFactory().createBufferQueueLayer(args); } - status_t err = layer->setDefaultBufferProperties(w, h, format); + status_t err = layer->setDefaultBufferProperties(0, 0, format); if (err == NO_ERROR) { *handle = layer->getHandle(); *gbp = layer->getProducer(); @@ -4426,34 +4382,24 @@ status_t SurfaceFlinger::createBufferQueueLayer(const sp& client, std::s return err; } -status_t SurfaceFlinger::createBufferStateLayer(const sp& client, std::string name, - uint32_t w, uint32_t h, uint32_t flags, - LayerMetadata metadata, sp* handle, +status_t SurfaceFlinger::createBufferStateLayer(LayerCreationArgs& args, sp* handle, sp* outLayer) { - LayerCreationArgs args(this, client, std::move(name), w, h, flags, std::move(metadata)); args.textureName = getNewTexture(); - sp layer = getFactory().createBufferStateLayer(args); - *handle = layer->getHandle(); - *outLayer = layer; - + *outLayer = getFactory().createBufferStateLayer(args); + *handle = (*outLayer)->getHandle(); return NO_ERROR; } -status_t SurfaceFlinger::createEffectLayer(const sp& client, std::string name, uint32_t w, - uint32_t h, uint32_t flags, LayerMetadata metadata, - sp* handle, sp* outLayer) { - *outLayer = getFactory().createEffectLayer( - {this, client, std::move(name), w, h, flags, std::move(metadata)}); +status_t SurfaceFlinger::createEffectLayer(LayerCreationArgs& args, sp* handle, + sp* outLayer) { + *outLayer = getFactory().createEffectLayer(args); *handle = (*outLayer)->getHandle(); return NO_ERROR; } -status_t SurfaceFlinger::createContainerLayer(const sp& client, std::string name, - uint32_t w, uint32_t h, uint32_t flags, - LayerMetadata metadata, sp* handle, +status_t SurfaceFlinger::createContainerLayer(LayerCreationArgs& args, sp* handle, sp* outLayer) { - *outLayer = getFactory().createContainerLayer( - {this, client, std::move(name), w, h, flags, std::move(metadata)}); + *outLayer = getFactory().createContainerLayer(args); *handle = (*outLayer)->getHandle(); return NO_ERROR; } @@ -5039,8 +4985,6 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, std::string& result) co */ colorizer.bold(result); StringAppendF(&result, "Visible layers (count = %zu)\n", mNumLayers.load()); - StringAppendF(&result, "GraphicBufferProducers: %zu, max %zu\n", - mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize); colorizer.reset(result); { @@ -6802,11 +6746,10 @@ void TransactionState::traverseStatesWithBuffers( } void SurfaceFlinger::setLayerCreatedState(const sp& handle, const wp& layer, - const wp parent, const wp& producer, - bool addToRoot) { + const wp parent, bool addToRoot) { Mutex::Autolock lock(mCreatedLayersLock); mCreatedLayers[handle->localBinder()] = - std::make_unique(layer, parent, producer, addToRoot); + std::make_unique(layer, parent, addToRoot); } auto SurfaceFlinger::getLayerCreatedState(const sp& handle) { @@ -6867,19 +6810,6 @@ sp SurfaceFlinger::handleLayerCreatedLocked(const sp& handle) { layer->updateTransformHint(mActiveDisplayTransformHint); - if (state->initialProducer != nullptr) { - mGraphicBufferProducerList.insert(state->initialProducer); - LOG_ALWAYS_FATAL_IF(mGraphicBufferProducerList.size() > mMaxGraphicBufferProducerListSize, - "Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", - mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize, - mNumLayers.load()); - if (mGraphicBufferProducerList.size() > mGraphicBufferProducerListSizeLogThreshold) { - ALOGW("Suspected IGBP leak: %zu IGBPs (%zu max), %zu Layers", - mGraphicBufferProducerList.size(), mMaxGraphicBufferProducerListSize, - mNumLayers.load()); - } - } - mInterceptor->saveSurfaceCreation(layer); return layer; } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index bf628dc309..b432f246c0 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -763,29 +763,23 @@ private: /* * Layer management */ - status_t createLayer(const String8& name, const sp& client, uint32_t w, uint32_t h, - PixelFormat format, uint32_t flags, LayerMetadata metadata, - sp* handle, sp* gbp, + status_t createLayer(LayerCreationArgs& args, sp* outHandle, const sp& parentHandle, int32_t* outLayerId, const sp& parentLayer = nullptr, uint32_t* outTransformHint = nullptr); - status_t createBufferQueueLayer(const sp& client, std::string name, uint32_t w, - uint32_t h, uint32_t flags, LayerMetadata metadata, - PixelFormat& format, sp* outHandle, - sp* outGbp, sp* outLayer); + status_t createBufferQueueLayer(LayerCreationArgs& args, PixelFormat& format, + sp* outHandle, sp* outGbp, + sp* outLayer); - status_t createBufferStateLayer(const sp& client, std::string name, uint32_t w, - uint32_t h, uint32_t flags, LayerMetadata metadata, - sp* outHandle, sp* outLayer); + status_t createBufferStateLayer(LayerCreationArgs& args, sp* outHandle, + sp* outLayer); - status_t createEffectLayer(const sp& client, std::string name, uint32_t w, uint32_t h, - uint32_t flags, LayerMetadata metadata, sp* outHandle, + status_t createEffectLayer(LayerCreationArgs& args, sp* outHandle, sp* outLayer); - status_t createContainerLayer(const sp& client, std::string name, uint32_t w, - uint32_t h, uint32_t flags, LayerMetadata metadata, - sp* outHandle, sp* outLayer); + status_t createContainerLayer(LayerCreationArgs& args, sp* outHandle, + sp* outLayer); status_t mirrorLayer(const sp& client, const sp& mirrorFromHandle, sp* outHandle, int32_t* outLayerId); @@ -798,8 +792,7 @@ private: // add a layer to SurfaceFlinger status_t addClientLayer(const sp& client, const sp& handle, - const sp& gbc, const sp& lbc, - const wp& parentLayer, bool addToRoot, + const sp& lbc, const wp& parentLayer, bool addToRoot, uint32_t* outTransformHint); // Traverse through all the layers and compute and cache its bounds. @@ -1115,16 +1108,12 @@ private: float mGlobalSaturationFactor = 1.0f; mat4 mClientColorMatrix; - // Can't be unordered_set because wp<> isn't hashable - std::set> mGraphicBufferProducerList; size_t mMaxGraphicBufferProducerListSize = ISurfaceComposer::MAX_LAYERS; // If there are more GraphicBufferProducers tracked by SurfaceFlinger than // this threshold, then begin logging. size_t mGraphicBufferProducerListSizeLogThreshold = static_cast(0.95 * static_cast(MAX_LAYERS)); - void removeGraphicBufferProducerAsync(const wp&); - // protected by mStateLock (but we could use another lock) bool mLayersRemoved = false; bool mLayersAdded = false; @@ -1339,19 +1328,12 @@ private: GUARDED_BY(mStateLock); mutable Mutex mCreatedLayersLock; struct LayerCreatedState { - LayerCreatedState(const wp& layer, const wp parent, - const wp& producer, bool addToRoot) - : layer(layer), - initialParent(parent), - initialProducer(producer), - addToRoot(addToRoot) {} + LayerCreatedState(const wp& layer, const wp parent, bool addToRoot) + : layer(layer), initialParent(parent), addToRoot(addToRoot) {} wp layer; // Indicates the initial parent of the created layer, only used for creating layer in // SurfaceFlinger. If nullptr, it may add the created layer into the current root layers. wp initialParent; - // Indicates the initial graphic buffer producer of the created layer, only used for - // creating layer in SurfaceFlinger. - wp initialProducer; // Indicates whether the layer getting created should be added at root if there's no parent // and has permission ACCESS_SURFACE_FLINGER. If set to false and no parent, the layer will // be added offscreen. @@ -1362,7 +1344,7 @@ private: // thread. std::unordered_map> mCreatedLayers; void setLayerCreatedState(const sp& handle, const wp& layer, - const wp parent, const wp& producer, bool addToRoot); + const wp parent, bool addToRoot); auto getLayerCreatedState(const sp& handle); sp handleLayerCreatedLocked(const sp& handle) REQUIRES(mStateLock); diff --git a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp index 2082c42001..28e8b8c78b 100644 --- a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp +++ b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp @@ -754,9 +754,7 @@ void SurfaceInterceptorTest::assertAllUpdatesFound(const Trace& trace) { } bool SurfaceInterceptorTest::surfaceCreationFound(const Increment& increment, bool foundSurface) { - bool isMatch(increment.surface_creation().name() == getUniqueName(LAYER_NAME, increment) && - increment.surface_creation().w() == SIZE_UPDATE && - increment.surface_creation().h() == SIZE_UPDATE); + bool isMatch(increment.surface_creation().name() == getUniqueName(LAYER_NAME, increment)); if (isMatch && !foundSurface) { foundSurface = true; } else if (isMatch && foundSurface) { diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 40ef6e702d..52d8c35ede 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -535,6 +535,7 @@ struct BaseLayerProperties { static void setupLatchedBuffer(CompositionTest* test, sp layer) { // TODO: Eliminate the complexity of actually creating a buffer + layer->setSizeForTest(LayerProperties::WIDTH, LayerProperties::HEIGHT); status_t err = layer->setDefaultBufferProperties(LayerProperties::WIDTH, LayerProperties::HEIGHT, LayerProperties::FORMAT); @@ -901,7 +902,6 @@ struct EffectLayerVariant : public BaseLayerVariant { FlingerLayerType layer = Base::template createLayerWithFactory(test, [test]() { return new EffectLayer( LayerCreationArgs(test->mFlinger.flinger(), sp(), "test-layer", - LayerProperties::WIDTH, LayerProperties::HEIGHT, LayerProperties::LAYER_FLAGS, LayerMetadata())); }); @@ -940,7 +940,6 @@ struct BufferLayerVariant : public BaseLayerVariant { FlingerLayerType layer = Base::template createLayerWithFactory(test, [test]() { LayerCreationArgs args(test->mFlinger.flinger(), sp(), "test-layer", - LayerProperties::WIDTH, LayerProperties::HEIGHT, LayerProperties::LAYER_FLAGS, LayerMetadata()); args.textureName = test->mFlinger.mutableTexturePool().back(); return new BufferQueueLayer(args); @@ -952,7 +951,6 @@ struct BufferLayerVariant : public BaseLayerVariant { } static void cleanupInjectedLayers(CompositionTest* test) { - EXPECT_CALL(*test->mMessageQueue, postMessage(_)).Times(1); Base::cleanupInjectedLayers(test); } @@ -990,7 +988,6 @@ struct ContainerLayerVariant : public BaseLayerVariant { static FlingerLayerType createLayer(CompositionTest* test) { LayerCreationArgs args(test->mFlinger.flinger(), sp(), "test-container-layer", - LayerProperties::WIDTH, LayerProperties::HEIGHT, LayerProperties::LAYER_FLAGS, LayerMetadata()); FlingerLayerType layer = new ContainerLayer(args); Base::template initLayerDrawingStateAndComputeBounds(test, layer); diff --git a/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp b/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp index 010c675574..cd2fc7426e 100644 --- a/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp +++ b/services/surfaceflinger/tests/unittests/FpsReporterTest.cpp @@ -114,8 +114,7 @@ FpsReporterTest::~FpsReporterTest() { sp FpsReporterTest::createBufferStateLayer(LayerMetadata metadata = {}) { sp client; - LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", WIDTH, HEIGHT, - LAYER_FLAGS, metadata); + LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", LAYER_FLAGS, metadata); return new BufferStateLayer(args); } diff --git a/services/surfaceflinger/tests/unittests/GameModeTest.cpp b/services/surfaceflinger/tests/unittests/GameModeTest.cpp index 3fa1a2c2f5..d6459429fb 100644 --- a/services/surfaceflinger/tests/unittests/GameModeTest.cpp +++ b/services/surfaceflinger/tests/unittests/GameModeTest.cpp @@ -53,7 +53,7 @@ public: sp createBufferStateLayer() { sp client; - LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", 100, 100, 0, + LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", 0, LayerMetadata()); return new BufferStateLayer(args); } diff --git a/services/surfaceflinger/tests/unittests/RefreshRateSelectionTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateSelectionTest.cpp index e388a6f40f..1e6e3361b2 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateSelectionTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateSelectionTest.cpp @@ -91,15 +91,14 @@ RefreshRateSelectionTest::~RefreshRateSelectionTest() { sp RefreshRateSelectionTest::createBufferStateLayer() { sp client; - LayerCreationArgs args(mFlinger.flinger(), client, "buffer-queue-layer", WIDTH, HEIGHT, - LAYER_FLAGS, LayerMetadata()); + LayerCreationArgs args(mFlinger.flinger(), client, "buffer-queue-layer", LAYER_FLAGS, + LayerMetadata()); return new BufferStateLayer(args); } sp RefreshRateSelectionTest::createEffectLayer() { sp client; - LayerCreationArgs args(mFlinger.flinger(), client, "color-layer", WIDTH, HEIGHT, LAYER_FLAGS, - LayerMetadata()); + LayerCreationArgs args(mFlinger.flinger(), client, "color-layer", LAYER_FLAGS, LayerMetadata()); return new EffectLayer(args); } diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp index d0211780ab..360f9c684f 100644 --- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp +++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp @@ -70,8 +70,8 @@ public: std::string name() override { return "BufferStateLayer"; } sp createLayer(TestableSurfaceFlinger& flinger) override { sp client; - LayerCreationArgs args(flinger.flinger(), client, "buffer-state-layer", WIDTH, HEIGHT, - LAYER_FLAGS, LayerMetadata()); + LayerCreationArgs args(flinger.flinger(), client, "buffer-state-layer", LAYER_FLAGS, + LayerMetadata()); return new BufferStateLayer(args); } }; @@ -81,7 +81,7 @@ public: std::string name() override { return "EffectLayer"; } sp createLayer(TestableSurfaceFlinger& flinger) override { sp client; - LayerCreationArgs args(flinger.flinger(), client, "color-layer", WIDTH, HEIGHT, LAYER_FLAGS, + LayerCreationArgs args(flinger.flinger(), client, "color-layer", LAYER_FLAGS, LayerMetadata()); return new EffectLayer(args); } diff --git a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp index bd6a7805ec..deeb785bb9 100644 --- a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp @@ -57,7 +57,7 @@ public: sp createBufferStateLayer() { sp client; - LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", 100, 100, 0, + LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", 0, LayerMetadata()); return new BufferStateLayer(args); } diff --git a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp index bf69704210..704340deac 100644 --- a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp @@ -57,7 +57,7 @@ public: sp createBufferStateLayer() { sp client; - LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", 100, 100, 0, + LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", 0, LayerMetadata()); return new BufferStateLayer(args); } diff --git a/services/surfaceflinger/tests/unittests/TunnelModeEnabledReporterTest.cpp b/services/surfaceflinger/tests/unittests/TunnelModeEnabledReporterTest.cpp index e4f74694f7..ade4fbb850 100644 --- a/services/surfaceflinger/tests/unittests/TunnelModeEnabledReporterTest.cpp +++ b/services/surfaceflinger/tests/unittests/TunnelModeEnabledReporterTest.cpp @@ -100,8 +100,7 @@ TunnelModeEnabledReporterTest::~TunnelModeEnabledReporterTest() { sp TunnelModeEnabledReporterTest::createBufferStateLayer( LayerMetadata metadata = {}) { sp client; - LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", WIDTH, HEIGHT, - LAYER_FLAGS, metadata); + LayerCreationArgs args(mFlinger.flinger(), client, "buffer-state-layer", LAYER_FLAGS, metadata); return new BufferStateLayer(args); } diff --git a/services/surfaceflinger/tests/unittests/mock/MockLayer.h b/services/surfaceflinger/tests/unittests/mock/MockLayer.h index ba2e4db0fa..8b48e1c16d 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockLayer.h +++ b/services/surfaceflinger/tests/unittests/mock/MockLayer.h @@ -25,7 +25,7 @@ namespace android::mock { class MockLayer : public Layer { public: MockLayer(SurfaceFlinger* flinger, std::string name) - : Layer(LayerCreationArgs(flinger, nullptr, std::move(name), 800, 600, 0, {})) {} + : Layer(LayerCreationArgs(flinger, nullptr, std::move(name), 0, {})) {} explicit MockLayer(SurfaceFlinger* flinger) : MockLayer(flinger, "TestLayer") {} MOCK_CONST_METHOD0(getType, const char*()); -- cgit v1.2.3-59-g8ed1b From 84125ac04b253a98e4e74940aff14097878eee64 Mon Sep 17 00:00:00 2001 From: Vishnu Nair Date: Thu, 2 Dec 2021 08:47:48 -0800 Subject: SF: Add mirror layers to transaction trace Test: atest TransactionTracingTest Bug: 200284593 Change-Id: I40fbf806f68c6b0bad29fb4568ac5ae1a547cc3b --- services/surfaceflinger/Client.cpp | 3 +- services/surfaceflinger/SurfaceFlinger.cpp | 26 ++++++--- services/surfaceflinger/SurfaceFlinger.h | 6 +- .../Tracing/TransactionProtoParser.cpp | 7 ++- .../Tracing/TransactionProtoParser.h | 8 +-- .../surfaceflinger/Tracing/TransactionTracing.cpp | 24 +++++--- .../surfaceflinger/Tracing/TransactionTracing.h | 2 + .../surfaceflinger/layerproto/transactions.proto | 1 + .../tests/unittests/TransactionTracingTest.cpp | 65 ++++++++++++++++++++++ 9 files changed, 117 insertions(+), 25 deletions(-) (limited to 'services/surfaceflinger/Client.cpp') diff --git a/services/surfaceflinger/Client.cpp b/services/surfaceflinger/Client.cpp index 0a8ebec9f3..6d7b732b36 100644 --- a/services/surfaceflinger/Client.cpp +++ b/services/surfaceflinger/Client.cpp @@ -98,7 +98,8 @@ status_t Client::createWithSurfaceParent(const String8& /* name */, uint32_t /* status_t Client::mirrorSurface(const sp& mirrorFromHandle, sp* outHandle, int32_t* outLayerId) { - return mFlinger->mirrorLayer(this, mirrorFromHandle, outHandle, outLayerId); + LayerCreationArgs args(mFlinger.get(), this, "MirrorRoot", 0 /* flags */, LayerMetadata()); + return mFlinger->mirrorLayer(args, mirrorFromHandle, outHandle, outLayerId); } status_t Client::clearLayerFrameStats(const sp& handle) const { diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 21f3872ef7..2b7280da70 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -4328,8 +4328,9 @@ uint32_t SurfaceFlinger::addInputWindowCommands(const InputWindowCommands& input return hasChanges ? eTraversalNeeded : 0; } -status_t SurfaceFlinger::mirrorLayer(const sp& client, const sp& mirrorFromHandle, - sp* outHandle, int32_t* outLayerId) { +status_t SurfaceFlinger::mirrorLayer(const LayerCreationArgs& args, + const sp& mirrorFromHandle, sp* outHandle, + int32_t* outLayerId) { if (!mirrorFromHandle) { return NAME_NOT_FOUND; } @@ -4342,7 +4343,6 @@ status_t SurfaceFlinger::mirrorLayer(const sp& client, const sp if (!mirrorFrom) { return NAME_NOT_FOUND; } - LayerCreationArgs args(this, client, "MirrorRoot", 0, LayerMetadata()); status_t result = createContainerLayer(args, outHandle, &mirrorLayer); if (result != NO_ERROR) { return result; @@ -4352,7 +4352,11 @@ status_t SurfaceFlinger::mirrorLayer(const sp& client, const sp } *outLayerId = mirrorLayer->sequence; - return addClientLayer(client, *outHandle, mirrorLayer /* layer */, nullptr /* parent */, + if (mTransactionTracingEnabled) { + mTransactionTracing.onMirrorLayerAdded((*outHandle)->localBinder(), mirrorLayer->sequence, + args.name, mirrorFrom->sequence); + } + return addClientLayer(args.client, *outHandle, mirrorLayer /* layer */, nullptr /* parent */, false /* addAsRoot */, nullptr /* outTransformHint */); } @@ -4414,8 +4418,10 @@ status_t SurfaceFlinger::createLayer(LayerCreationArgs& args, sp* outHa if (parentSp != nullptr) { parentId = parentSp->getSequence(); } - mTransactionTracing.onLayerAdded((*outHandle)->localBinder(), layer->sequence, args.name, - args.flags, parentId); + if (mTransactionTracingEnabled) { + mTransactionTracing.onLayerAdded((*outHandle)->localBinder(), layer->sequence, args.name, + args.flags, parentId); + } setTransactionFlags(eTransactionNeeded); *outLayerId = layer->sequence; @@ -4466,14 +4472,14 @@ status_t SurfaceFlinger::createBufferStateLayer(LayerCreationArgs& args, sp* handle, +status_t SurfaceFlinger::createEffectLayer(const LayerCreationArgs& args, sp* handle, sp* outLayer) { *outLayer = getFactory().createEffectLayer(args); *handle = (*outLayer)->getHandle(); return NO_ERROR; } -status_t SurfaceFlinger::createContainerLayer(LayerCreationArgs& args, sp* handle, +status_t SurfaceFlinger::createContainerLayer(const LayerCreationArgs& args, sp* handle, sp* outLayer) { *outLayer = getFactory().createContainerLayer(args); *handle = (*outLayer)->getHandle(); @@ -6652,7 +6658,9 @@ void SurfaceFlinger::onLayerDestroyed(Layer* layer) { if (!layer->isRemovedFromCurrentState()) { mScheduler->deregisterLayer(layer); } - mTransactionTracing.onLayerRemoved(layer->getSequence()); + if (mTransactionTracingEnabled) { + mTransactionTracing.onLayerRemoved(layer->getSequence()); + } } void SurfaceFlinger::onLayerUpdate() { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 9794639070..5a15d60218 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -769,13 +769,13 @@ private: status_t createBufferStateLayer(LayerCreationArgs& args, sp* outHandle, sp* outLayer); - status_t createEffectLayer(LayerCreationArgs& args, sp* outHandle, + status_t createEffectLayer(const LayerCreationArgs& args, sp* outHandle, sp* outLayer); - status_t createContainerLayer(LayerCreationArgs& args, sp* outHandle, + status_t createContainerLayer(const LayerCreationArgs& args, sp* outHandle, sp* outLayer); - status_t mirrorLayer(const sp& client, const sp& mirrorFromHandle, + status_t mirrorLayer(const LayerCreationArgs& args, const sp& mirrorFromHandle, sp* outHandle, int32_t* outLayerId); // called when all clients have released all their references to diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp index 783b36e66c..d12b253d62 100644 --- a/services/surfaceflinger/Tracing/TransactionProtoParser.cpp +++ b/services/surfaceflinger/Tracing/TransactionProtoParser.cpp @@ -278,6 +278,7 @@ proto::LayerCreationArgs TransactionProtoParser::toProto(const TracingLayerCreat proto.set_name(args.name); proto.set_flags(args.flags); proto.set_parent_id(args.parentId); + proto.set_mirror_from_id(args.mirrorFromId); return proto; } @@ -312,6 +313,7 @@ void TransactionProtoParser::fromProto(const proto::LayerCreationArgs& proto, outArgs.name = proto.name(); outArgs.flags = proto.flags(); outArgs.parentId = proto.parent_id(); + outArgs.mirrorFromId = proto.mirror_from_id(); } void TransactionProtoParser::fromProto(const proto::LayerState& proto, @@ -320,6 +322,7 @@ void TransactionProtoParser::fromProto(const proto::LayerState& proto, fromProto(proto, getLayerHandle, static_cast(outState)); if (proto.what() & layer_state_t::eReparent) { outState.parentId = proto.parent_id(); + outState.args.parentId = outState.parentId; } if (proto.what() & layer_state_t::eRelativeLayerChanged) { outState.relativeParentId = proto.relative_parent_id(); @@ -508,7 +511,9 @@ DisplayState TransactionProtoParser::fromProto(const proto::DisplayState& proto, DisplayIdToHandleFn getDisplayHandle) { DisplayState display; display.what = proto.what(); - display.token = getDisplayHandle(proto.id()); + if (getDisplayHandle != nullptr) { + display.token = getDisplayHandle(proto.id()); + } if (display.what & DisplayState::eLayerStackChanged) { display.layerStack.id = proto.layer_stack(); diff --git a/services/surfaceflinger/Tracing/TransactionProtoParser.h b/services/surfaceflinger/Tracing/TransactionProtoParser.h index 16e9b5e729..b64c7821b8 100644 --- a/services/surfaceflinger/Tracing/TransactionProtoParser.h +++ b/services/surfaceflinger/Tracing/TransactionProtoParser.h @@ -25,8 +25,9 @@ namespace android::surfaceflinger { struct TracingLayerCreationArgs { int32_t layerId; std::string name; - uint32_t flags; - int32_t parentId; + uint32_t flags = 0; + int32_t parentId = -1; + int32_t mirrorFromId = -1; }; struct TracingLayerState : layer_state_t { @@ -37,8 +38,7 @@ struct TracingLayerState : layer_state_t { int32_t parentId; int32_t relativeParentId; int32_t inputCropId; - std::string name; - uint32_t layerCreationFlags; + TracingLayerCreationArgs args; }; class TransactionProtoParser { diff --git a/services/surfaceflinger/Tracing/TransactionTracing.cpp b/services/surfaceflinger/Tracing/TransactionTracing.cpp index cf488c26f9..6dd43ca3a8 100644 --- a/services/surfaceflinger/Tracing/TransactionTracing.cpp +++ b/services/surfaceflinger/Tracing/TransactionTracing.cpp @@ -227,7 +227,17 @@ void TransactionTracing::flush(int64_t vsyncId) { void TransactionTracing::onLayerAdded(BBinder* layerHandle, int layerId, const std::string& name, uint32_t flags, int parentId) { std::scoped_lock lock(mTraceLock); - TracingLayerCreationArgs args{layerId, name, flags, parentId}; + TracingLayerCreationArgs args{layerId, name, flags, parentId, -1 /* mirrorFromId */}; + mLayerHandles[layerHandle] = layerId; + proto::LayerCreationArgs protoArgs = TransactionProtoParser::toProto(args); + proto::LayerCreationArgs protoArgsCopy = protoArgs; + mCreatedLayers.push_back(protoArgs); +} + +void TransactionTracing::onMirrorLayerAdded(BBinder* layerHandle, int layerId, + const std::string& name, int mirrorFromId) { + std::scoped_lock lock(mTraceLock); + TracingLayerCreationArgs args{layerId, name, 0 /* flags */, -1 /* parentId */, mirrorFromId}; mLayerHandles[layerHandle] = layerId; mCreatedLayers.emplace_back(TransactionProtoParser::toProto(args)); } @@ -270,9 +280,7 @@ void TransactionTracing::updateStartingStateLocked( for (const proto::LayerCreationArgs& addedLayer : removedEntry.added_layers()) { TracingLayerState& startingState = mStartingStates[addedLayer.layer_id()]; startingState.layerId = addedLayer.layer_id(); - startingState.name = addedLayer.name(); - startingState.layerCreationFlags = addedLayer.flags(); - startingState.parentId = addedLayer.parent_id(); + TransactionProtoParser::fromProto(addedLayer, startingState.args); } // Merge layer states to starting transaction state. @@ -305,11 +313,13 @@ void TransactionTracing::addStartingStateToProtoLocked(proto::TransactionTraceFi proto::TransactionTraceEntry* entryProto = proto.add_entry(); entryProto->set_elapsed_realtime_nanos(mStartingTimestamp); entryProto->set_vsync_id(0); + if (mStartingStates.size() == 0) { + return; + } + entryProto->mutable_added_layers()->Reserve(static_cast(mStartingStates.size())); for (auto& [layerId, state] : mStartingStates) { - TracingLayerCreationArgs args{layerId, state.name, state.layerCreationFlags, - state.parentId}; - entryProto->mutable_added_layers()->Add(TransactionProtoParser::toProto(args)); + entryProto->mutable_added_layers()->Add(TransactionProtoParser::toProto(state.args)); } proto::TransactionState transactionProto = TransactionProtoParser::toProto(mStartingStates); diff --git a/services/surfaceflinger/Tracing/TransactionTracing.h b/services/surfaceflinger/Tracing/TransactionTracing.h index 546ac7afb0..814f8579ce 100644 --- a/services/surfaceflinger/Tracing/TransactionTracing.h +++ b/services/surfaceflinger/Tracing/TransactionTracing.h @@ -64,6 +64,8 @@ public: void setBufferSize(size_t bufferSizeInBytes); void onLayerAdded(BBinder* layerHandle, int layerId, const std::string& name, uint32_t flags, int parentId); + void onMirrorLayerAdded(BBinder* layerHandle, int layerId, const std::string& name, + int mirrorFromId); void onLayerRemoved(int layerId); void dump(std::string&) const; static constexpr auto CONTINUOUS_TRACING_BUFFER_SIZE = 512 * 1024; diff --git a/services/surfaceflinger/layerproto/transactions.proto b/services/surfaceflinger/layerproto/transactions.proto index 10222cc283..e31b502efc 100644 --- a/services/surfaceflinger/layerproto/transactions.proto +++ b/services/surfaceflinger/layerproto/transactions.proto @@ -53,6 +53,7 @@ message LayerCreationArgs { string name = 2; uint32 flags = 3; int32 parent_id = 4; + int32 mirror_from_id = 5; } message TransactionState { diff --git a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp index 4e49c18059..71c7bd935f 100644 --- a/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionTracingTest.cpp @@ -293,4 +293,69 @@ TEST_F(TransactionTracingLayerHandlingTest, startingStateSurvivesBufferFlush) { EXPECT_EQ(proto.entry(0).transactions(0).layer_changes(0).layer_id(), mParentLayerId); } +class TransactionTracingMirrorLayerTest : public TransactionTracingTest { +protected: + void SetUp() override { + TransactionTracingTest::SetUp(); + mTracing->enable(); + // add layers + mTracing->setBufferSize(SMALL_BUFFER_SIZE); + const sp fakeLayerHandle = new BBinder(); + mTracing->onLayerAdded(fakeLayerHandle->localBinder(), mLayerId, "Test Layer", + 123 /* flags */, -1 /* parentId */); + const sp fakeMirrorLayerHandle = new BBinder(); + mTracing->onMirrorLayerAdded(fakeMirrorLayerHandle->localBinder(), mMirrorLayerId, "Mirror", + mLayerId); + + // add some layer transaction + { + TransactionState transaction; + transaction.id = 50; + ComposerState layerState; + layerState.state.surface = fakeLayerHandle; + layerState.state.what = layer_state_t::eLayerChanged; + layerState.state.z = 42; + transaction.states.add(layerState); + ComposerState mirrorState; + mirrorState.state.surface = fakeMirrorLayerHandle; + mirrorState.state.what = layer_state_t::eLayerChanged; + mirrorState.state.z = 43; + transaction.states.add(mirrorState); + mTracing->addQueuedTransaction(transaction); + + std::vector transactions; + transactions.emplace_back(transaction); + mTracing->addCommittedTransactions(transactions, ++mVsyncId); + flush(mVsyncId); + } + } + + void TearDown() override { + mTracing->disable(); + verifyDisabledTracingState(); + TransactionTracingTest::TearDown(); + } + + int mLayerId = 5; + int mMirrorLayerId = 55; + int64_t mVsyncId = 0; + int64_t VSYNC_ID_FIRST_LAYER_CHANGE; + int64_t VSYNC_ID_SECOND_LAYER_CHANGE; + int64_t VSYNC_ID_CHILD_LAYER_REMOVED; +}; + +TEST_F(TransactionTracingMirrorLayerTest, canAddMirrorLayers) { + proto::TransactionTraceFile proto = writeToProto(); + // We don't have any starting states since no layer was removed from. + EXPECT_EQ(proto.entry().size(), 2); + EXPECT_EQ(proto.entry(0).transactions().size(), 0); + EXPECT_EQ(proto.entry(0).added_layers().size(), 0); + + // Verify the mirror layer was added + EXPECT_EQ(proto.entry(1).transactions().size(), 1); + EXPECT_EQ(proto.entry(1).added_layers().size(), 2); + EXPECT_EQ(proto.entry(1).added_layers(1).layer_id(), mMirrorLayerId); + EXPECT_EQ(proto.entry(1).transactions(0).layer_changes().size(), 2); + EXPECT_EQ(proto.entry(1).transactions(0).layer_changes(1).z(), 43); +} } // namespace android -- cgit v1.2.3-59-g8ed1b