diff options
23 files changed, 14 insertions, 858 deletions
diff --git a/cmds/surfacereplayer/proto/src/trace.proto b/cmds/surfacereplayer/proto/src/trace.proto index 06afefd3c8..3798ba73a4 100644 --- a/cmds/surfacereplayer/proto/src/trace.proto +++ b/cmds/surfacereplayer/proto/src/trace.proto @@ -46,7 +46,6 @@ message SurfaceChange { HiddenFlagChange hidden_flag = 12; OpaqueFlagChange opaque_flag = 13; SecureFlagChange secure_flag = 14; - DeferredTransactionChange deferred_transaction = 15; CornerRadiusChange corner_radius = 16; ReparentChange reparent = 17; RelativeParentChange relative_parent = 18; @@ -113,11 +112,6 @@ message SecureFlagChange { required bool secure_flag = 1; } -message DeferredTransactionChange { - required int32 layer_id = 1; - required uint64 frame_number = 2; -} - message DisplayChange { required int32 id = 1; diff --git a/cmds/surfacereplayer/replayer/Replayer.cpp b/cmds/surfacereplayer/replayer/Replayer.cpp index a6d9a3feb6..cfd42fec30 100644 --- a/cmds/surfacereplayer/replayer/Replayer.cpp +++ b/cmds/surfacereplayer/replayer/Replayer.cpp @@ -402,11 +402,6 @@ status_t Replayer::doSurfaceTransaction( case SurfaceChange::SurfaceChangeCase::kSecureFlag: setSecureFlag(transaction, change.id(), change.secure_flag()); break; - case SurfaceChange::SurfaceChangeCase::kDeferredTransaction: - waitUntilDeferredTransactionLayerExists(change.deferred_transaction(), lock); - setDeferredTransaction(transaction, change.id(), - change.deferred_transaction()); - break; case SurfaceChange::SurfaceChangeCase::kReparent: setReparentChange(transaction, change.id(), change.reparent()); break; @@ -560,19 +555,6 @@ void Replayer::setSecureFlag(SurfaceComposerClient::Transaction& t, t.setFlags(mLayers[id], flag, layer_state_t::eLayerSecure); } -void Replayer::setDeferredTransaction(SurfaceComposerClient::Transaction& t, - layer_id id, const DeferredTransactionChange& dtc) { - ALOGV("Layer %d: Setting Deferred Transaction -- layer_id=%d, " - "frame_number=%llu", - id, dtc.layer_id(), dtc.frame_number()); - if (mLayers.count(dtc.layer_id()) == 0 || mLayers[dtc.layer_id()] == nullptr) { - ALOGE("Layer %d not found in Deferred Transaction", dtc.layer_id()); - return; - } - - t.deferTransactionUntil_legacy(mLayers[id], mLayers[dtc.layer_id()], dtc.frame_number()); -} - void Replayer::setDisplaySurface(SurfaceComposerClient::Transaction& t, display_id id, const DispSurfaceChange& /*dsc*/) { sp<IGraphicBufferProducer> outProducer; @@ -676,13 +658,6 @@ void Replayer::waitUntilTimestamp(int64_t timestamp) { std::this_thread::sleep_for(std::chrono::nanoseconds(timestamp - mCurrentTime)); } -void Replayer::waitUntilDeferredTransactionLayerExists( - const DeferredTransactionChange& dtc, std::unique_lock<std::mutex>& lock) { - if (mLayers.count(dtc.layer_id()) == 0 || mLayers[dtc.layer_id()] == nullptr) { - mLayerCond.wait(lock, [&] { return (mLayers[dtc.layer_id()] != nullptr); }); - } -} - status_t Replayer::loadSurfaceComposerClient() { mComposerClient = new SurfaceComposerClient; return mComposerClient->initCheck(); diff --git a/cmds/surfacereplayer/replayer/Replayer.h b/cmds/surfacereplayer/replayer/Replayer.h index 252db2bfbb..d62522a497 100644 --- a/cmds/surfacereplayer/replayer/Replayer.h +++ b/cmds/surfacereplayer/replayer/Replayer.h @@ -110,8 +110,6 @@ class Replayer { layer_id id, const OpaqueFlagChange& ofc); void setSecureFlag(SurfaceComposerClient::Transaction& t, layer_id id, const SecureFlagChange& sfc); - void setDeferredTransaction(SurfaceComposerClient::Transaction& t, - layer_id id, const DeferredTransactionChange& dtc); void setReparentChange(SurfaceComposerClient::Transaction& t, layer_id id, const ReparentChange& c); void setRelativeParentChange(SurfaceComposerClient::Transaction& t, @@ -131,8 +129,6 @@ class Replayer { display_id id, const ProjectionChange& pc); void waitUntilTimestamp(int64_t timestamp); - void waitUntilDeferredTransactionLayerExists( - const DeferredTransactionChange& dtc, std::unique_lock<std::mutex>& lock); status_t loadSurfaceComposerClient(); Trace mTrace; diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 809438534c..55ed7fe298 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -45,7 +45,6 @@ layer_state_t::layer_state_t() reserved(0), cornerRadius(0.0f), backgroundBlurRadius(0), - barrierFrameNumber(0), transform(0), transformToDisplayInverse(false), crop(Rect::INVALID_RECT), @@ -87,9 +86,7 @@ status_t layer_state_t::write(Parcel& output) const SAFE_PARCEL(output.writeUint32, mask); SAFE_PARCEL(matrix.write, output); SAFE_PARCEL(output.write, crop); - SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, barrierSurfaceControl_legacy); SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, reparentSurfaceControl); - SAFE_PARCEL(output.writeUint64, barrierFrameNumber); SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, relativeLayerSurfaceControl); SAFE_PARCEL(SurfaceControl::writeNullableToParcel, output, parentSurfaceControlForChild); SAFE_PARCEL(output.writeFloat, color.r); @@ -193,9 +190,7 @@ status_t layer_state_t::read(const Parcel& input) SAFE_PARCEL(matrix.read, input); SAFE_PARCEL(input.read, crop); - SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &barrierSurfaceControl_legacy); SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &reparentSurfaceControl); - SAFE_PARCEL(input.readUint64, &barrierFrameNumber); SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &relativeLayerSurfaceControl); SAFE_PARCEL(SurfaceControl::readNullableFromParcel, input, &parentSurfaceControlForChild); @@ -425,11 +420,6 @@ void layer_state_t::merge(const layer_state_t& other) { what |= eBlurRegionsChanged; blurRegions = other.blurRegions; } - if (other.what & eDeferTransaction_legacy) { - what |= eDeferTransaction_legacy; - barrierSurfaceControl_legacy = other.barrierSurfaceControl_legacy; - barrierFrameNumber = other.barrierFrameNumber; - } if (other.what & eRelativeLayerChanged) { what |= eRelativeLayerChanged; what &= ~eLayerChanged; diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 9ce094aa77..e6baba6e1d 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -1140,23 +1140,6 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setBlurR return *this; } -SurfaceComposerClient::Transaction& -SurfaceComposerClient::Transaction::deferTransactionUntil_legacy( - const sp<SurfaceControl>& sc, const sp<SurfaceControl>& barrierSurfaceControl, - uint64_t frameNumber) { - layer_state_t* s = getLayerState(sc); - if (!s) { - mStatus = BAD_INDEX; - return *this; - } - s->what |= layer_state_t::eDeferTransaction_legacy; - s->barrierSurfaceControl_legacy = barrierSurfaceControl; - s->barrierFrameNumber = frameNumber; - - registerSurfaceControlForCallback(sc); - return *this; -} - SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::reparent( const sp<SurfaceControl>& sc, const sp<SurfaceControl>& newParent) { layer_state_t* s = getLayerState(sc); diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 41a022f90e..d2d1e5b91c 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -82,8 +82,6 @@ struct layer_state_t { eTransparentRegionChanged = 0x00000020, eFlagsChanged = 0x00000040, eLayerStackChanged = 0x00000080, - /* was eCropChanged_legacy, now available 0x00000100, */ - eDeferTransaction_legacy = 0x00000200, eReleaseBufferListenerChanged = 0x00000400, eShadowRadiusChanged = 0x00000800, /* was eDetachChildren, now available 0x00002000, */ @@ -152,9 +150,7 @@ struct layer_state_t { matrix22_t matrix; float cornerRadius; uint32_t backgroundBlurRadius; - sp<SurfaceControl> barrierSurfaceControl_legacy; sp<SurfaceControl> reparentSurfaceControl; - uint64_t barrierFrameNumber; sp<SurfaceControl> relativeLayerSurfaceControl; diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 1590b10a00..fe6a30aa42 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -454,13 +454,7 @@ public: const std::vector<BlurRegion>& regions); Transaction& setLayerStack(const sp<SurfaceControl>& sc, uint32_t layerStack); Transaction& setMetadata(const sp<SurfaceControl>& sc, uint32_t key, const Parcel& p); - // Defers applying any changes made in this transaction until the Layer - // identified by handle reaches the given frameNumber. If the Layer identified - // by handle is removed, then we will apply this transaction regardless of - // what frame number has been reached. - Transaction& deferTransactionUntil_legacy(const sp<SurfaceControl>& sc, - const sp<SurfaceControl>& barrierSurfaceControl, - uint64_t frameNumber); + /// Reparents the current layer to the new parent handle. The new parent must not be null. Transaction& reparent(const sp<SurfaceControl>& sc, const sp<SurfaceControl>& newParent); diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp index be9bce0795..12f63dbbf1 100644 --- a/services/surfaceflinger/BufferLayer.cpp +++ b/services/surfaceflinger/BufferLayer.cpp @@ -507,11 +507,6 @@ bool BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime, BufferInfo oldBufferInfo = mBufferInfo; - if (!allTransactionsSignaled(expectedPresentTime)) { - mFlinger->setTransactionFlags(eTraversalNeeded); - return false; - } - status_t err = updateTexImage(recomputeVisibleRegions, latchTime, expectedPresentTime); if (err != NO_ERROR) { return false; @@ -556,53 +551,9 @@ bool BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime, recomputeVisibleRegions = true; } - // Remove any sync points corresponding to the buffer which was just - // latched - { - Mutex::Autolock lock(mLocalSyncPointMutex); - auto point = mLocalSyncPoints.begin(); - while (point != mLocalSyncPoints.end()) { - if (!(*point)->frameIsAvailable() || !(*point)->transactionIsApplied()) { - // This sync point must have been added since we started - // latching. Don't drop it yet. - ++point; - continue; - } - - if ((*point)->getFrameNumber() <= mCurrentFrameNumber) { - std::stringstream ss; - ss << "Dropping sync point " << (*point)->getFrameNumber(); - ATRACE_NAME(ss.str().c_str()); - point = mLocalSyncPoints.erase(point); - } else { - ++point; - } - } - } - return true; } -// transaction -void BufferLayer::notifyAvailableFrames(nsecs_t expectedPresentTime) { - const auto headFrameNumber = getHeadFrameNumber(expectedPresentTime); - const bool headFenceSignaled = fenceHasSignaled(); - const bool presentTimeIsCurrent = framePresentTimeIsCurrent(expectedPresentTime); - Mutex::Autolock lock(mLocalSyncPointMutex); - for (auto& point : mLocalSyncPoints) { - if (headFrameNumber >= point->getFrameNumber() && headFenceSignaled && - presentTimeIsCurrent) { - point->setFrameAvailable(); - sp<Layer> requestedSyncLayer = point->getRequestedSyncLayer(); - if (requestedSyncLayer) { - // Need to update the transaction flag to ensure the layer's pending transaction - // gets applied. - requestedSyncLayer->setTransactionFlags(eTransactionNeeded); - } - } - } -} - bool BufferLayer::hasReadyFrame() const { return hasFrameUpdate() || getSidebandStreamChanged() || getAutoRefresh(); } @@ -616,33 +567,6 @@ bool BufferLayer::isProtected() const { return (buffer != 0) && (buffer->getUsage() & GRALLOC_USAGE_PROTECTED); } -// h/w composer set-up -bool BufferLayer::allTransactionsSignaled(nsecs_t expectedPresentTime) { - const auto headFrameNumber = getHeadFrameNumber(expectedPresentTime); - bool matchingFramesFound = false; - bool allTransactionsApplied = true; - Mutex::Autolock lock(mLocalSyncPointMutex); - - for (auto& point : mLocalSyncPoints) { - if (point->getFrameNumber() > headFrameNumber) { - break; - } - matchingFramesFound = true; - - if (!point->frameIsAvailable()) { - // We haven't notified the remote layer that the frame for - // this point is available yet. Notify it now, and then - // abort this attempt to latch. - point->setFrameAvailable(); - allTransactionsApplied = false; - break; - } - - allTransactionsApplied = allTransactionsApplied && point->transactionIsApplied(); - } - return !matchingFramesFound || allTransactionsApplied; -} - // As documented in libhardware header, formats in the range // 0x100 - 0x1FF are specific to the HAL implementation, and // are known to have no alpha channel diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h index b8d3f12322..0a5235aa08 100644 --- a/services/surfaceflinger/BufferLayer.h +++ b/services/surfaceflinger/BufferLayer.h @@ -90,8 +90,6 @@ public: bool isBufferLatched() const override { return mRefreshPending; } - void notifyAvailableFrames(nsecs_t expectedPresentTime) override; - bool hasReadyFrame() const override; // Returns the current scaling mode @@ -153,11 +151,6 @@ protected: bool onPreComposition(nsecs_t) override; void preparePerFrameCompositionState() override; - // Check all of the local sync points to ensure that all transactions - // which need to have been applied prior to the frame which is about to - // be latched have signaled - bool allTransactionsSignaled(nsecs_t expectedPresentTime); - static bool getOpacityForFormat(uint32_t format); // from graphics API diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index ed826a0100..fa9cecf787 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -259,25 +259,6 @@ bool BufferStateLayer::willPresentCurrentTransaction() const { (mCurrentState.buffer != nullptr || mCurrentState.bgColorLayer != nullptr))); } -/* TODO: vhau uncomment once deferred transaction migration complete in - * WindowManager -void BufferStateLayer::pushPendingState() { - if (!mCurrentState.modified) { - return; - } - mPendingStates.push_back(mCurrentState); - ATRACE_INT(mTransactionName.c_str(), mPendingStates.size()); -} -*/ - -bool BufferStateLayer::applyPendingStates(Layer::State* stateToCommit) { - mCurrentStateModified = mCurrentState.modified; - bool stateUpdateAvailable = Layer::applyPendingStates(stateToCommit); - mCurrentStateModified = stateUpdateAvailable && mCurrentStateModified; - mCurrentState.modified = false; - return stateUpdateAvailable; -} - Rect BufferStateLayer::getCrop(const Layer::State& s) const { return s.crop; } diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h index 8ce3e1f55b..24e0ad290f 100644 --- a/services/surfaceflinger/BufferStateLayer.h +++ b/services/surfaceflinger/BufferStateLayer.h @@ -50,10 +50,6 @@ public: uint32_t doTransactionResize(uint32_t flags, Layer::State* /*stateToCommit*/) override { return flags; } - /*TODO:vhau return to using BufferStateLayer override once WM - * has removed deferred transactions! - void pushPendingState() override;*/ - bool applyPendingStates(Layer::State* stateToCommit) override; uint32_t getActiveWidth(const Layer::State& s) const override { return s.width; } uint32_t getActiveHeight(const Layer::State& s) const override { return s.height; } @@ -87,10 +83,6 @@ public: // Override to ignore legacy layer state properties that are not used by BufferStateLayer bool setSize(uint32_t /*w*/, uint32_t /*h*/) override { return false; } bool setTransparentRegionHint(const Region& transparent) override; - void deferTransactionUntil_legacy(const sp<IBinder>& /*barrierHandle*/, - uint64_t /*frameNumber*/) override {} - void deferTransactionUntil_legacy(const sp<Layer>& /*barrierLayer*/, - uint64_t /*frameNumber*/) override {} Rect getBufferSize(const State& s) const override; FloatRect computeSourceBounds(const FloatRect& parentBounds) const override; @@ -165,7 +157,6 @@ private: uint64_t mPreviousBufferId = 0; uint64_t mPreviousReleasedFrameNumber = 0; - mutable bool mCurrentStateModified = false; bool mReleasePreviousBuffer = false; // Stores the last set acquire fence signal time used to populate the callback handle's acquire diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 6038658ee6..ebf845cb95 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -202,19 +202,6 @@ LayerCreationArgs::LayerCreationArgs(SurfaceFlinger* flinger, sp<Client> client, */ void Layer::onLayerDisplayed(const sp<Fence>& /*releaseFence*/) {} -void Layer::removeRemoteSyncPoints() { - for (auto& point : mRemoteSyncPoints) { - point->setTransactionApplied(); - } - mRemoteSyncPoints.clear(); - - { - for (State pendingState : mPendingStates) { - pendingState.barrierLayer_legacy = nullptr; - } - } -} - void Layer::removeRelativeZ(const std::vector<Layer*>& layersInTree) { if (mCurrentState.zOrderRelativeOf == nullptr) { return; @@ -236,21 +223,6 @@ void Layer::removeRelativeZ(const std::vector<Layer*>& layersInTree) { void Layer::removeFromCurrentState() { mRemovedFromCurrentState = true; - // Since we are no longer reachable from CurrentState SurfaceFlinger - // will no longer invoke doTransaction for us, and so we will - // never finish applying transactions. We signal the sync point - // now so that another layer will not become indefinitely - // blocked. - removeRemoteSyncPoints(); - - { - Mutex::Autolock syncLock(mLocalSyncPointMutex); - for (auto& point : mLocalSyncPoints) { - point->setFrameAvailable(); - } - mLocalSyncPoints.clear(); - } - mFlinger->markLayerPendingRemovalLocked(this); } @@ -775,21 +747,6 @@ Hwc2::IComposerClient::Composition Layer::getCompositionType(const DisplayDevice } } -bool Layer::addSyncPoint(const std::shared_ptr<SyncPoint>& point) { - if (point->getFrameNumber() <= mCurrentFrameNumber) { - // Don't bother with a SyncPoint, since we've already latched the - // relevant frame - return false; - } - if (isRemovedFromCurrentState()) { - return false; - } - - Mutex::Autolock lock(mLocalSyncPointMutex); - mLocalSyncPoints.push_back(point); - return true; -} - // ---------------------------------------------------------------------------- // local state // ---------------------------------------------------------------------------- @@ -808,132 +765,6 @@ bool Layer::isSecure() const { // transaction // ---------------------------------------------------------------------------- -void Layer::pushPendingState() { - if (!mCurrentState.modified) { - return; - } - ATRACE_CALL(); - - // If this transaction is waiting on the receipt of a frame, generate a sync - // point and send it to the remote layer. - // We don't allow installing sync points after we are removed from the current state - // as we won't be able to signal our end. - if (mCurrentState.barrierLayer_legacy != nullptr && !isRemovedFromCurrentState()) { - sp<Layer> barrierLayer = mCurrentState.barrierLayer_legacy.promote(); - if (barrierLayer == nullptr) { - ALOGE("[%s] Unable to promote barrier Layer.", getDebugName()); - // If we can't promote the layer we are intended to wait on, - // then it is expired or otherwise invalid. Allow this transaction - // to be applied as per normal (no synchronization). - mCurrentState.barrierLayer_legacy = nullptr; - } else { - auto syncPoint = std::make_shared<SyncPoint>(mCurrentState.barrierFrameNumber, this, - barrierLayer); - if (barrierLayer->addSyncPoint(syncPoint)) { - std::stringstream ss; - ss << "Adding sync point " << mCurrentState.barrierFrameNumber; - ATRACE_NAME(ss.str().c_str()); - mRemoteSyncPoints.push_back(std::move(syncPoint)); - } else { - // We already missed the frame we're supposed to synchronize - // on, so go ahead and apply the state update - mCurrentState.barrierLayer_legacy = nullptr; - } - } - - // Wake us up to check if the frame has been received - setTransactionFlags(eTransactionNeeded); - mFlinger->setTransactionFlags(eTraversalNeeded); - } - if (mCurrentState.bufferlessSurfaceFramesTX.size() >= State::kStateSurfaceFramesThreshold) { - // Ideally, the currentState would only contain one SurfaceFrame per transaction (assuming - // each Tx uses a different token). We don't expect the current state to hold a huge amount - // of SurfaceFrames. However, in the event it happens, this debug statement will leave a - // trail that can help in debugging. - ALOGW("Bufferless SurfaceFrames size on current state of layer %s is %" PRIu32 "", - mName.c_str(), static_cast<uint32_t>(mCurrentState.bufferlessSurfaceFramesTX.size())); - } - mPendingStates.push_back(mCurrentState); - // Since the current state along with the SurfaceFrames has been pushed into the pendingState, - // we no longer need to retain them. If multiple states are pushed and applied together, we have - // a merging logic to address the SurfaceFrames at mergeSurfaceFrames(). - mCurrentState.bufferlessSurfaceFramesTX.clear(); - ATRACE_INT(mTransactionName.c_str(), mPendingStates.size()); -} - -void Layer::mergeSurfaceFrames(State& source, State& target) { - // No need to merge BufferSurfaceFrame as the target's surfaceFrame, if it exists, will be used - // directly. Dropping of source's SurfaceFrame is taken care of at setBuffer(). - target.bufferlessSurfaceFramesTX.merge(source.bufferlessSurfaceFramesTX); - source.bufferlessSurfaceFramesTX.clear(); -} - -void Layer::popPendingState(State* stateToCommit) { - ATRACE_CALL(); - - mergeSurfaceFrames(*stateToCommit, mPendingStates[0]); - *stateToCommit = mPendingStates[0]; - mPendingStates.pop_front(); - ATRACE_INT(mTransactionName.c_str(), mPendingStates.size()); -} - -bool Layer::applyPendingStates(State* stateToCommit) { - bool stateUpdateAvailable = false; - while (!mPendingStates.empty()) { - if (mPendingStates[0].barrierLayer_legacy != nullptr) { - if (mRemoteSyncPoints.empty()) { - // If we don't have a sync point for this, apply it anyway. It - // will be visually wrong, but it should keep us from getting - // into too much trouble. - ALOGV("[%s] No local sync point found", getDebugName()); - popPendingState(stateToCommit); - stateUpdateAvailable = true; - continue; - } - - if (mRemoteSyncPoints.front()->getFrameNumber() != - mPendingStates[0].barrierFrameNumber) { - ALOGE("[%s] Unexpected sync point frame number found", getDebugName()); - - // Signal our end of the sync point and then dispose of it - mRemoteSyncPoints.front()->setTransactionApplied(); - mRemoteSyncPoints.pop_front(); - continue; - } - - if (mRemoteSyncPoints.front()->frameIsAvailable()) { - ATRACE_NAME("frameIsAvailable"); - // Apply the state update - popPendingState(stateToCommit); - stateUpdateAvailable = true; - - // Signal our end of the sync point and then dispose of it - mRemoteSyncPoints.front()->setTransactionApplied(); - mRemoteSyncPoints.pop_front(); - } else { - ATRACE_NAME("!frameIsAvailable"); - mRemoteSyncPoints.front()->checkTimeoutAndLog(); - break; - } - } else { - popPendingState(stateToCommit); - stateUpdateAvailable = true; - } - } - - // If we still have pending updates, we need to ensure SurfaceFlinger - // will keep calling doTransaction, and so we force a traversal. - // However, our pending states won't clear until a frame is available, - // and so there is no need to specifically trigger a wakeup. - if (!mPendingStates.empty()) { - setTransactionFlags(eTransactionNeeded); - mFlinger->setTraversalNeeded(); - } - - mCurrentState.modified = false; - return stateUpdateAvailable; -} - uint32_t Layer::doTransactionResize(uint32_t flags, State* stateToCommit) { const State& s(getDrawingState()); @@ -1014,15 +845,14 @@ uint32_t Layer::doTransaction(uint32_t flags) { mChildrenChanged = false; } - pushPendingState(); - State c = getCurrentState(); - if (!applyPendingStates(&c)) { - return flags; - } + // TODO: This is unfortunate. + mCurrentStateModified = mCurrentState.modified; + mCurrentState.modified = false; - flags = doTransactionResize(flags, &c); + flags = doTransactionResize(flags, &mCurrentState); const State& s(getDrawingState()); + State& c(getCurrentState()); if (getActiveGeometry(c) != getActiveGeometry(s)) { // invalidate and recompute the visible regions if needed @@ -1054,7 +884,6 @@ uint32_t Layer::doTransaction(uint32_t flags) { // Commit the transaction commitTransaction(c); - mPendingStatesSnapshot = mPendingStates; mCurrentState.callbackHandles = {}; return flags; @@ -1662,25 +1491,6 @@ Layer::FrameRate Layer::getFrameRateForLayerTree() const { return frameRate; } -void Layer::deferTransactionUntil_legacy(const sp<Layer>& barrierLayer, uint64_t frameNumber) { - ATRACE_CALL(); - - mCurrentState.barrierLayer_legacy = barrierLayer; - mCurrentState.barrierFrameNumber = frameNumber; - // We don't set eTransactionNeeded, because just receiving a deferral - // request without any other state updates shouldn't actually induce a delay - mCurrentState.modified = true; - pushPendingState(); - mCurrentState.barrierLayer_legacy = nullptr; - mCurrentState.barrierFrameNumber = 0; - mCurrentState.modified = false; -} - -void Layer::deferTransactionUntil_legacy(const sp<IBinder>& barrierHandle, uint64_t frameNumber) { - sp<Handle> handle = static_cast<Handle*>(barrierHandle.get()); - deferTransactionUntil_legacy(handle->owner.promote(), frameNumber); -} - // ---------------------------------------------------------------------------- // pageflip handling... // ---------------------------------------------------------------------------- @@ -2362,14 +2172,6 @@ void Layer::writeToProtoDrawingState(LayerProto* layerInfo, uint32_t traceFlags, const ui::Transform transform = getTransform(); if (traceFlags & SurfaceTracing::TRACE_CRITICAL) { - for (const auto& pendingState : mPendingStatesSnapshot) { - auto barrierLayer = pendingState.barrierLayer_legacy.promote(); - if (barrierLayer != nullptr) { - BarrierLayerProto* barrierLayerProto = layerInfo->add_barrier_layer(); - barrierLayerProto->set_id(barrierLayer->sequence); - barrierLayerProto->set_frame_number(pendingState.barrierFrameNumber); - } - } auto buffer = getBuffer(); if (buffer != nullptr) { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 5528a8190f..304eb36b9d 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -166,11 +166,6 @@ public: Rect crop; Rect requestedCrop; - // If set, defers this state update until the identified Layer - // receives a frame with the given frameNumber - wp<Layer> barrierLayer_legacy; - uint64_t barrierFrameNumber; - // the transparentRegion hint is a bit special, it's latched only // when we receive a buffer -- this is because it's "content" // dependent. @@ -398,9 +393,6 @@ public: virtual bool setFlags(uint32_t flags, uint32_t mask); virtual bool setLayerStack(uint32_t layerStack); virtual uint32_t getLayerStack() const; - virtual void deferTransactionUntil_legacy(const sp<IBinder>& barrierHandle, - uint64_t frameNumber); - virtual void deferTransactionUntil_legacy(const sp<Layer>& barrierLayer, uint64_t frameNumber); virtual bool setMetadata(const LayerMetadata& data); virtual void setChildrenDrawingParent(const sp<Layer>&); virtual bool reparent(const sp<IBinder>& newParentHandle); @@ -578,14 +570,6 @@ public: virtual int32_t getQueuedFrameCount() const { return 0; } - virtual void pushPendingState(); - - /* - * Merges the BufferlessSurfaceFrames from source with the target. If the same token exists in - * both source and target, target's SurfaceFrame will be retained. - */ - void mergeSurfaceFrames(State& source, State& target); - /** * Returns active buffer size in the correct orientation. Buffer size is determined by undoing * any buffer transformations. If the layer has no buffer then return INVALID_RECT. @@ -615,7 +599,6 @@ public: // ignored. virtual RoundedCornerState getRoundedCornerState() const; - virtual void notifyAvailableFrames(nsecs_t /*expectedPresentTime*/) {} virtual PixelFormat getPixelFormat() const { return PIXEL_FORMAT_NONE; } /** * Return whether this layer needs an input info. For most layer types @@ -903,65 +886,6 @@ public: virtual std::string getPendingBufferCounterName() { return ""; } protected: - class SyncPoint { - public: - explicit SyncPoint(uint64_t frameNumber, wp<Layer> requestedSyncLayer, - wp<Layer> barrierLayer_legacy) - : mFrameNumber(frameNumber), - mFrameIsAvailable(false), - mTransactionIsApplied(false), - mRequestedSyncLayer(requestedSyncLayer), - mBarrierLayer_legacy(barrierLayer_legacy) {} - uint64_t getFrameNumber() const { return mFrameNumber; } - - bool frameIsAvailable() const { return mFrameIsAvailable; } - - void setFrameAvailable() { mFrameIsAvailable = true; } - - bool transactionIsApplied() const { return mTransactionIsApplied; } - - void setTransactionApplied() { mTransactionIsApplied = true; } - - sp<Layer> getRequestedSyncLayer() { return mRequestedSyncLayer.promote(); } - - sp<Layer> getBarrierLayer() const { return mBarrierLayer_legacy.promote(); } - - bool isTimeout() const { - using namespace std::chrono_literals; - static constexpr std::chrono::nanoseconds TIMEOUT_THRESHOLD = 1s; - - return std::chrono::steady_clock::now() - mCreateTimeStamp > TIMEOUT_THRESHOLD; - } - - void checkTimeoutAndLog() { - using namespace std::chrono_literals; - static constexpr std::chrono::nanoseconds LOG_PERIOD = 1s; - - if (!frameIsAvailable() && isTimeout()) { - const auto now = std::chrono::steady_clock::now(); - if (now - mLastLogTime > LOG_PERIOD) { - mLastLogTime = now; - sp<Layer> requestedSyncLayer = getRequestedSyncLayer(); - sp<Layer> barrierLayer = getBarrierLayer(); - ALOGW("[%s] sync point %" PRIu64 " wait timeout %lld for %s", - requestedSyncLayer ? requestedSyncLayer->getDebugName() : "Removed", - mFrameNumber, (now - mCreateTimeStamp).count(), - barrierLayer ? barrierLayer->getDebugName() : "Removed"); - } - } - } - - private: - const uint64_t mFrameNumber; - std::atomic<bool> mFrameIsAvailable; - std::atomic<bool> mTransactionIsApplied; - wp<Layer> mRequestedSyncLayer; - wp<Layer> mBarrierLayer_legacy; - const std::chrono::time_point<std::chrono::steady_clock> mCreateTimeStamp = - std::chrono::steady_clock::now(); - std::chrono::time_point<std::chrono::steady_clock> mLastLogTime; - }; - friend class impl::SurfaceInterceptor; // For unit tests @@ -980,7 +904,6 @@ protected: ui::Dataspace outputDataspace); virtual void preparePerFrameCompositionState(); virtual void commitTransaction(State& stateToCommit); - virtual bool applyPendingStates(State* stateToCommit); virtual uint32_t doTransactionResize(uint32_t flags, Layer::State* stateToCommit); virtual void onSurfaceFrameCreated(const std::shared_ptr<frametimeline::SurfaceFrame>&) {} @@ -1026,21 +949,6 @@ protected: virtual ui::Transform getInputTransform() const; virtual Rect getInputBounds() const; - // SyncPoints which will be signaled when the correct frame is at the head - // of the queue and dropped after the frame has been latched. Protected by - // mLocalSyncPointMutex. - Mutex mLocalSyncPointMutex; - std::list<std::shared_ptr<SyncPoint>> mLocalSyncPoints; - - // SyncPoints which will be signaled and then dropped when the transaction - // is applied - std::list<std::shared_ptr<SyncPoint>> mRemoteSyncPoints; - - // Returns false if the relevant frame has already been latched - bool addSyncPoint(const std::shared_ptr<SyncPoint>& point); - - void popPendingState(State* stateToCommit); - // constant sp<SurfaceFlinger> mFlinger; @@ -1050,14 +958,10 @@ protected: // These are only accessed by the main thread or the tracing thread. State mDrawingState; - // Store a copy of the pending state so that the drawing thread can access the - // states without a lock. - std::deque<State> mPendingStatesSnapshot; // these are protected by an external lock (mStateLock) State mCurrentState; std::atomic<uint32_t> mTransactionFlags{0}; - std::deque<State> mPendingStates; // Timestamp history for UIAutomation. Thread safe. FrameTracker mFrameTracker; @@ -1119,6 +1023,8 @@ protected: // Used in buffer stuffing analysis in FrameTimeline. nsecs_t mLastLatchTime = 0; + mutable bool mCurrentStateModified = false; + private: virtual void setTransformHint(ui::Transform::RotationFlags) {} @@ -1143,7 +1049,6 @@ private: void updateTreeHasFrameRateVote(); void setZOrderRelativeOf(const wp<Layer>& relativeOf); - void removeRemoteSyncPoints(); // Find the root of the cloned hierarchy, this means the first non cloned parent. // This will return null if first non cloned parent is not found. diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 02579c6bde..274d2833b2 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2815,13 +2815,6 @@ void SurfaceFlinger::processDisplayChangesLocked() { } void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) { - const nsecs_t expectedPresentTime = mExpectedPresentTime.load(); - - // Notify all layers of available frames - mCurrentState.traverse([expectedPresentTime](Layer* layer) { - layer->notifyAvailableFrames(expectedPresentTime); - }); - /* * Traversal of the children * (perform the transaction for each of them if needed) @@ -3833,12 +3826,6 @@ uint32_t SurfaceFlinger::setClientStateLocked( const uint64_t what = s.what; - // If we are deferring transaction, make sure to push the pending state, as otherwise the - // pending state will also be deferred. - if (what & layer_state_t::eDeferTransaction_legacy) { - layer->pushPendingState(); - } - // Only set by BLAST adapter layers if (what & layer_state_t::eProducerDisconnect) { layer->onDisconnect(); @@ -3973,12 +3960,6 @@ uint32_t SurfaceFlinger::setClientStateLocked( flags |= eTransactionNeeded | eTraversalNeeded | eTransformHintUpdateNeeded; } } - if (what & layer_state_t::eDeferTransaction_legacy) { - layer->deferTransactionUntil_legacy(s.barrierSurfaceControl_legacy->getHandle(), - s.barrierFrameNumber); - // We don't trigger a traversal here because if no other state is - // changed, we don't want this to cause any more work - } if (what & layer_state_t::eTransformChanged) { if (layer->setTransform(s.transform)) flags |= eTraversalNeeded; } diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp index b49562a0a5..113f463c39 100644 --- a/services/surfaceflinger/SurfaceInterceptor.cpp +++ b/services/surfaceflinger/SurfaceInterceptor.cpp @@ -141,11 +141,6 @@ void SurfaceInterceptor::addInitialSurfaceStateLocked(Increment* increment, addCornerRadiusLocked(transaction, layerId, layer->mCurrentState.cornerRadius); addBackgroundBlurRadiusLocked(transaction, layerId, layer->mCurrentState.backgroundBlurRadius); addBlurRegionsLocked(transaction, layerId, layer->mCurrentState.blurRegions); - if (layer->mCurrentState.barrierLayer_legacy != nullptr) { - addDeferTransactionLocked(transaction, layerId, - layer->mCurrentState.barrierLayer_legacy.promote(), - layer->mCurrentState.barrierFrameNumber); - } addFlagsLocked(transaction, layerId, layer->mCurrentState.flags, layer_state_t::eLayerHidden | layer_state_t::eLayerOpaque | layer_state_t::eLayerSecure); @@ -380,20 +375,6 @@ void SurfaceInterceptor::addBlurRegionsLocked(Transaction* transaction, int32_t } } -void SurfaceInterceptor::addDeferTransactionLocked(Transaction* transaction, int32_t layerId, - const sp<const Layer>& layer, uint64_t frameNumber) -{ - SurfaceChange* change(createSurfaceChangeLocked(transaction, layerId)); - if (layer == nullptr) { - ALOGE("An existing layer could not be retrieved with the handle" - " for the deferred transaction"); - return; - } - DeferredTransactionChange* deferTransaction(change->mutable_deferred_transaction()); - deferTransaction->set_layer_id(getLayerId(layer)); - deferTransaction->set_frame_number(frameNumber); -} - void SurfaceInterceptor::addReparentLocked(Transaction* transaction, int32_t layerId, int32_t parentId) { SurfaceChange* change(createSurfaceChangeLocked(transaction, layerId)); @@ -464,15 +445,6 @@ void SurfaceInterceptor::addSurfaceChangesLocked(Transaction* transaction, if (state.what & layer_state_t::eBlurRegionsChanged) { addBlurRegionsLocked(transaction, layerId, state.blurRegions); } - if (state.what & layer_state_t::eDeferTransaction_legacy) { - sp<Layer> otherLayer = nullptr; - if (state.barrierSurfaceControl_legacy != nullptr) { - otherLayer = static_cast<Layer::Handle*>( - state.barrierSurfaceControl_legacy->getHandle().get()) - ->owner.promote(); - } - addDeferTransactionLocked(transaction, layerId, otherLayer, state.barrierFrameNumber); - } if (state.what & layer_state_t::eReparent) { auto parentHandle = (state.parentSurfaceControlForChild) ? state.parentSurfaceControlForChild->getHandle() diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h index d2cbf40426..30aca8340e 100644 --- a/services/surfaceflinger/SurfaceInterceptor.h +++ b/services/surfaceflinger/SurfaceInterceptor.h @@ -167,8 +167,6 @@ private: int32_t backgroundBlurRadius); void addBlurRegionsLocked(Transaction* transaction, int32_t layerId, const std::vector<BlurRegion>& effectRegions); - void addDeferTransactionLocked(Transaction* transaction, int32_t layerId, - const sp<const Layer>& layer, uint64_t frameNumber); void addSurfaceChangesLocked(Transaction* transaction, const layer_state_t& state); void addTransactionLocked(Increment* increment, const Vector<ComposerState>& stateUpdates, const DefaultKeyedVector<wp<IBinder>, DisplayDeviceState>& displays, diff --git a/services/surfaceflinger/tests/LayerUpdate_test.cpp b/services/surfaceflinger/tests/LayerUpdate_test.cpp index 39d9206e1a..adb5d58e8c 100644 --- a/services/surfaceflinger/tests/LayerUpdate_test.cpp +++ b/services/surfaceflinger/tests/LayerUpdate_test.cpp @@ -160,61 +160,6 @@ protected: } }; -TEST_F(LayerUpdateTest, DeferredTransactionTest) { - std::unique_ptr<ScreenCapture> sc; - { - SCOPED_TRACE("before anything"); - ScreenCapture::captureScreen(&sc); - sc->expectBGColor(32, 32); - sc->expectFGColor(96, 96); - sc->expectBGColor(160, 160); - } - - // set up two deferred transactions on different frames - asTransaction([&](Transaction& t) { - t.setAlpha(mFGSurfaceControl, 0.75); - t.deferTransactionUntil_legacy(mFGSurfaceControl, mSyncSurfaceControl, - mSyncSurfaceControl->getSurface()->getNextFrameNumber()); - }); - - asTransaction([&](Transaction& t) { - t.setPosition(mFGSurfaceControl, 128, 128); - t.deferTransactionUntil_legacy(mFGSurfaceControl, mSyncSurfaceControl, - mSyncSurfaceControl->getSurface()->getNextFrameNumber() + 1); - }); - - { - SCOPED_TRACE("before any trigger"); - ScreenCapture::captureScreen(&sc); - sc->expectBGColor(32, 32); - sc->expectFGColor(96, 96); - sc->expectBGColor(160, 160); - } - - // should trigger the first deferred transaction, but not the second one - TransactionUtils::fillSurfaceRGBA8(mSyncSurfaceControl, 31, 31, 31); - { - SCOPED_TRACE("after first trigger"); - ScreenCapture::captureScreen(&sc); - sc->expectBGColor(32, 32); - sc->checkPixel(96, 96, 162, 63, 96); - sc->expectBGColor(160, 160); - } - - // should show up immediately since it's not deferred - asTransaction([&](Transaction& t) { t.setAlpha(mFGSurfaceControl, 1.0); }); - - // trigger the second deferred transaction - TransactionUtils::fillSurfaceRGBA8(mSyncSurfaceControl, 31, 31, 31); - { - SCOPED_TRACE("after second trigger"); - ScreenCapture::captureScreen(&sc); - sc->expectBGColor(32, 32); - sc->expectBGColor(96, 96); - sc->expectFGColor(160, 160); - } -} - TEST_F(LayerUpdateTest, LayerWithNoBuffersResizesImmediately) { std::unique_ptr<ScreenCapture> sc; @@ -702,36 +647,6 @@ TEST_F(ChildLayerTest, ChildrenWithParentBufferTransformAndScale) { } } -TEST_F(ChildLayerTest, Bug36858924) { - // Destroy the child layer - mChild.clear(); - - // Now recreate it as hidden - mChild = createSurface(mClient, "Child surface", 10, 10, PIXEL_FORMAT_RGBA_8888, - ISurfaceComposerClient::eHidden, mFGSurfaceControl.get()); - - // Show the child layer in a deferred transaction - asTransaction([&](Transaction& t) { - t.deferTransactionUntil_legacy(mChild, mFGSurfaceControl, - mFGSurfaceControl->getSurface()->getNextFrameNumber()); - t.show(mChild); - }); - - // Render the foreground surface a few times - // - // Prior to the bugfix for b/36858924, this would usually hang while trying to fill the third - // frame because SurfaceFlinger would never process the deferred transaction and would therefore - // never acquire/release the first buffer - ALOGI("Filling 1"); - TransactionUtils::fillSurfaceRGBA8(mFGSurfaceControl, 0, 255, 0); - ALOGI("Filling 2"); - TransactionUtils::fillSurfaceRGBA8(mFGSurfaceControl, 0, 0, 255); - ALOGI("Filling 3"); - TransactionUtils::fillSurfaceRGBA8(mFGSurfaceControl, 255, 0, 0); - ALOGI("Filling 4"); - TransactionUtils::fillSurfaceRGBA8(mFGSurfaceControl, 0, 255, 0); -} - TEST_F(ChildLayerTest, Reparent) { asTransaction([&](Transaction& t) { t.show(mChild); diff --git a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp index 09bd775872..af23e2a9cc 100644 --- a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp +++ b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp @@ -44,7 +44,6 @@ constexpr uint32_t BUFFER_UPDATES = 18; constexpr uint32_t LAYER_UPDATE = INT_MAX - 2; constexpr uint32_t SIZE_UPDATE = 134; constexpr uint32_t STACK_UPDATE = 1; -constexpr uint64_t DEFERRED_UPDATE = 0; constexpr int32_t RELATIVE_Z = 42; constexpr float ALPHA_UPDATE = 0.29f; constexpr float CORNER_RADIUS_UPDATE = 0.2f; @@ -191,7 +190,6 @@ public: bool hiddenFlagUpdateFound(const SurfaceChange& change, bool foundHiddenFlag); bool opaqueFlagUpdateFound(const SurfaceChange& change, bool foundOpaqueFlag); bool secureFlagUpdateFound(const SurfaceChange& change, bool foundSecureFlag); - bool deferredTransactionUpdateFound(const SurfaceChange& change, bool foundDeferred); bool reparentUpdateFound(const SurfaceChange& change, bool found); bool relativeParentUpdateFound(const SurfaceChange& change, bool found); bool shadowRadiusUpdateFound(const SurfaceChange& change, bool found); @@ -227,7 +225,6 @@ public: void hiddenFlagUpdate(Transaction&); void opaqueFlagUpdate(Transaction&); void secureFlagUpdate(Transaction&); - void deferredTransactionUpdate(Transaction&); void reparentUpdate(Transaction&); void relativeParentUpdate(Transaction&); void shadowRadiusUpdate(Transaction&); @@ -396,10 +393,6 @@ void SurfaceInterceptorTest::secureFlagUpdate(Transaction& t) { t.setFlags(mBGSurfaceControl, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure); } -void SurfaceInterceptorTest::deferredTransactionUpdate(Transaction& t) { - t.deferTransactionUntil_legacy(mBGSurfaceControl, mBGSurfaceControl, DEFERRED_UPDATE); -} - void SurfaceInterceptorTest::reparentUpdate(Transaction& t) { t.reparent(mBGSurfaceControl, mFGSurfaceControl); } @@ -437,7 +430,6 @@ void SurfaceInterceptorTest::runAllUpdates() { runInTransaction(&SurfaceInterceptorTest::hiddenFlagUpdate); runInTransaction(&SurfaceInterceptorTest::opaqueFlagUpdate); runInTransaction(&SurfaceInterceptorTest::secureFlagUpdate); - runInTransaction(&SurfaceInterceptorTest::deferredTransactionUpdate); runInTransaction(&SurfaceInterceptorTest::reparentUpdate); runInTransaction(&SurfaceInterceptorTest::relativeParentUpdate); runInTransaction(&SurfaceInterceptorTest::shadowRadiusUpdate); @@ -621,18 +613,6 @@ bool SurfaceInterceptorTest::secureFlagUpdateFound(const SurfaceChange& change, return foundSecureFlag; } -bool SurfaceInterceptorTest::deferredTransactionUpdateFound(const SurfaceChange& change, - bool foundDeferred) { - bool hasId(change.deferred_transaction().layer_id() == mBGLayerId); - bool hasFrameNumber(change.deferred_transaction().frame_number() == DEFERRED_UPDATE); - if (hasId && hasFrameNumber && !foundDeferred) { - foundDeferred = true; - } else if (hasId && hasFrameNumber && foundDeferred) { - [] () { FAIL(); }(); - } - return foundDeferred; -} - bool SurfaceInterceptorTest::reparentUpdateFound(const SurfaceChange& change, bool found) { bool hasId(change.reparent().parent_id() == mFGLayerId); if (hasId && !found) { @@ -715,9 +695,6 @@ bool SurfaceInterceptorTest::surfaceUpdateFound(const Trace& trace, case SurfaceChange::SurfaceChangeCase::kSecureFlag: foundUpdate = secureFlagUpdateFound(change, foundUpdate); break; - case SurfaceChange::SurfaceChangeCase::kDeferredTransaction: - foundUpdate = deferredTransactionUpdateFound(change, foundUpdate); - break; case SurfaceChange::SurfaceChangeCase::kReparent: foundUpdate = reparentUpdateFound(change, foundUpdate); break; @@ -749,7 +726,6 @@ void SurfaceInterceptorTest::assertAllUpdatesFound(const Trace& trace) { ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kHiddenFlag)); ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kOpaqueFlag)); ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kSecureFlag)); - ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kDeferredTransaction)); ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kReparent)); ASSERT_TRUE(surfaceUpdateFound(trace, SurfaceChange::SurfaceChangeCase::kRelativeParent)); } @@ -906,11 +882,6 @@ TEST_F(SurfaceInterceptorTest, InterceptSecureFlagUpdateWorks) { SurfaceChange::SurfaceChangeCase::kSecureFlag); } -TEST_F(SurfaceInterceptorTest, InterceptDeferredTransactionUpdateWorks) { - captureTest(&SurfaceInterceptorTest::deferredTransactionUpdate, - SurfaceChange::SurfaceChangeCase::kDeferredTransaction); -} - TEST_F(SurfaceInterceptorTest, InterceptReparentUpdateWorks) { captureTest(&SurfaceInterceptorTest::reparentUpdate, SurfaceChange::SurfaceChangeCase::kReparent); diff --git a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp index c081f9b642..eb31e2eca1 100644 --- a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp +++ b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp @@ -91,7 +91,6 @@ constexpr static TestColor RED = {195, 63, 63, 255}; constexpr static TestColor LIGHT_RED = {255, 177, 177, 255}; constexpr static TestColor GREEN = {63, 195, 63, 255}; constexpr static TestColor BLUE = {63, 63, 195, 255}; -constexpr static TestColor DARK_GRAY = {63, 63, 63, 255}; constexpr static TestColor LIGHT_GRAY = {200, 200, 200, 255}; // Fill an RGBA_8888 formatted surface with a single color. @@ -1469,77 +1468,6 @@ protected: } } - void Test_DeferredTransaction() { - // Synchronization surface - constexpr static int SYNC_LAYER = 2; - auto syncSurfaceControl = mComposerClient->createSurface(String8("Sync Test Surface"), 1, 1, - PIXEL_FORMAT_RGBA_8888, 0); - ASSERT_TRUE(syncSurfaceControl != nullptr); - ASSERT_TRUE(syncSurfaceControl->isValid()); - - fillSurfaceRGBA8(syncSurfaceControl, DARK_GRAY); - - { - TransactionScope ts(*sFakeComposer); - ts.setLayer(syncSurfaceControl, INT32_MAX - 1); - ts.setPosition(syncSurfaceControl, mDisplayWidth - 2, mDisplayHeight - 2); - ts.show(syncSurfaceControl); - } - auto referenceFrame = mBaseFrame; - referenceFrame.push_back(makeSimpleRect(mDisplayWidth - 2, mDisplayHeight - 2, - mDisplayWidth - 1, mDisplayHeight - 1)); - referenceFrame[SYNC_LAYER].mSwapCount = 1; - EXPECT_EQ(2, sFakeComposer->getFrameCount()); - EXPECT_TRUE(framesAreSame(referenceFrame, sFakeComposer->getLatestFrame())); - - // set up two deferred transactions on different frames - these should not yield composited - // frames - { - TransactionScope ts(*sFakeComposer); - ts.setAlpha(mFGSurfaceControl, 0.75); - ts.deferTransactionUntil_legacy(mFGSurfaceControl, syncSurfaceControl, - syncSurfaceControl->getSurface()->getNextFrameNumber()); - } - EXPECT_TRUE(framesAreSame(referenceFrame, sFakeComposer->getLatestFrame())); - - { - TransactionScope ts(*sFakeComposer); - ts.setPosition(mFGSurfaceControl, 128, 128); - ts.deferTransactionUntil_legacy(mFGSurfaceControl, syncSurfaceControl, - syncSurfaceControl->getSurface()->getNextFrameNumber() + - 1); - } - EXPECT_EQ(4, sFakeComposer->getFrameCount()); - EXPECT_TRUE(framesAreSame(referenceFrame, sFakeComposer->getLatestFrame())); - - // should trigger the first deferred transaction, but not the second one - fillSurfaceRGBA8(syncSurfaceControl, DARK_GRAY); - sFakeComposer->runVSyncAndWait(); - EXPECT_EQ(5, sFakeComposer->getFrameCount()); - - referenceFrame[FG_LAYER].mPlaneAlpha = 0.75f; - referenceFrame[SYNC_LAYER].mSwapCount++; - EXPECT_TRUE(framesAreSame(referenceFrame, sFakeComposer->getLatestFrame())); - - // should show up immediately since it's not deferred - { - TransactionScope ts(*sFakeComposer); - ts.setAlpha(mFGSurfaceControl, 1.0); - } - referenceFrame[FG_LAYER].mPlaneAlpha = 1.f; - EXPECT_EQ(6, sFakeComposer->getFrameCount()); - EXPECT_TRUE(framesAreSame(referenceFrame, sFakeComposer->getLatestFrame())); - - // trigger the second deferred transaction - fillSurfaceRGBA8(syncSurfaceControl, DARK_GRAY); - sFakeComposer->runVSyncAndWait(); - // TODO: Compute from layer size? - referenceFrame[FG_LAYER].mDisplayFrame = hwc_rect_t{128, 128, 128 + 64, 128 + 64}; - referenceFrame[SYNC_LAYER].mSwapCount++; - EXPECT_EQ(7, sFakeComposer->getFrameCount()); - EXPECT_TRUE(framesAreSame(referenceFrame, sFakeComposer->getLatestFrame())); - } - void Test_SetRelativeLayer() { constexpr int RELATIVE_LAYER = 2; auto relativeSurfaceControl = mComposerClient->createSurface(String8("Test Surface"), 64, @@ -1625,10 +1553,6 @@ TEST_F(TransactionTest_2_1, DISABLED_LayerSetMatrix) { Test_LayerSetMatrix(); } -TEST_F(TransactionTest_2_1, DISABLED_DeferredTransaction) { - Test_DeferredTransaction(); -} - TEST_F(TransactionTest_2_1, DISABLED_SetRelativeLayer) { Test_SetRelativeLayer(); } @@ -1799,44 +1723,6 @@ protected: EXPECT_TRUE(framesAreSame(referenceFrame, Base::sFakeComposer->getLatestFrame())); } - void Test_Bug36858924() { - // Destroy the child layer - mChild.clear(); - - // Now recreate it as hidden - mChild = Base::mComposerClient->createSurface(String8("Child surface"), 10, 10, - PIXEL_FORMAT_RGBA_8888, - ISurfaceComposerClient::eHidden, - Base::mFGSurfaceControl->getHandle()); - - // Show the child layer in a deferred transaction - { - TransactionScope ts(*Base::sFakeComposer); - ts.deferTransactionUntil_legacy(mChild, Base::mFGSurfaceControl, - Base::mFGSurfaceControl->getSurface() - ->getNextFrameNumber()); - ts.show(mChild); - } - - // Render the foreground surface a few times - // - // Prior to the bugfix for b/36858924, this would usually hang while trying to fill the - // third frame because SurfaceFlinger would never process the deferred transaction and would - // therefore never acquire/release the first buffer - ALOGI("Filling 1"); - fillSurfaceRGBA8(Base::mFGSurfaceControl, GREEN); - Base::sFakeComposer->runVSyncAndWait(); - ALOGI("Filling 2"); - fillSurfaceRGBA8(Base::mFGSurfaceControl, BLUE); - Base::sFakeComposer->runVSyncAndWait(); - ALOGI("Filling 3"); - fillSurfaceRGBA8(Base::mFGSurfaceControl, RED); - Base::sFakeComposer->runVSyncAndWait(); - ALOGI("Filling 4"); - fillSurfaceRGBA8(Base::mFGSurfaceControl, GREEN); - Base::sFakeComposer->runVSyncAndWait(); - } - sp<SurfaceControl> mChild; }; @@ -1867,10 +1753,6 @@ TEST_F(ChildLayerTest_2_1, DISABLED_ChildrenWithParentBufferTransform) { Test_ChildrenWithParentBufferTransform(); } -TEST_F(ChildLayerTest_2_1, DISABLED_Bug36858924) { - Test_Bug36858924(); -} - template <typename FakeComposerService> class ChildColorLayerTest : public ChildLayerTest<FakeComposerService> { using Base = ChildLayerTest<FakeComposerService>; diff --git a/services/surfaceflinger/tests/unittests/RefreshRateSelectionTest.cpp b/services/surfaceflinger/tests/unittests/RefreshRateSelectionTest.cpp index abecd4b45b..9c6ad06e1d 100644 --- a/services/surfaceflinger/tests/unittests/RefreshRateSelectionTest.cpp +++ b/services/surfaceflinger/tests/unittests/RefreshRateSelectionTest.cpp @@ -118,11 +118,8 @@ void RefreshRateSelectionTest::setParent(Layer* child, Layer* parent) { } void RefreshRateSelectionTest::commitTransaction(Layer* layer) { - layer->pushPendingState(); auto c = layer->getCurrentState(); - if (layer->applyPendingStates(&c)) { - layer->commitTransaction(c); - } + layer->commitTransaction(c); } void RefreshRateSelectionTest::setupScheduler() { diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp index 0bb7e31194..c088ddc971 100644 --- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp +++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp @@ -153,11 +153,8 @@ void SetFrameRateTest::removeChild(sp<Layer> layer, sp<Layer> child) { void SetFrameRateTest::commitTransaction() { for (auto layer : mLayers) { - layer->pushPendingState(); auto c = layer->getCurrentState(); - if (layer->applyPendingStates(&c)) { - layer->commitTransaction(c); - } + layer->commitTransaction(c); } } diff --git a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp index b5ef0a1334..ea1ce47b70 100644 --- a/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionFrameTracerTest.cpp @@ -60,11 +60,8 @@ public: } void commitTransaction(Layer* layer) { - layer->pushPendingState(); auto c = layer->getCurrentState(); - if (layer->applyPendingStates(&c)) { - layer->commitTransaction(c); - } + layer->commitTransaction(c); } void setupScheduler() { @@ -151,4 +148,4 @@ TEST_F(TransactionFrameTracerTest, BLASTTransactionSendsFrameTracerEvents) { BLASTTransactionSendsFrameTracerEvents(); } -} // namespace android
\ No newline at end of file +} // namespace android diff --git a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp index c75538f476..09a1c2af75 100644 --- a/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp @@ -60,13 +60,8 @@ public: } void commitTransaction(Layer* layer) { - layer->pushPendingState(); - // After pushing the state, the currentState should not store any BufferlessSurfaceFrames - EXPECT_EQ(0u, layer->mCurrentState.bufferlessSurfaceFramesTX.size()); auto c = layer->getCurrentState(); - if (layer->applyPendingStates(&c)) { - layer->commitTransaction(c); - } + layer->commitTransaction(c); } void setupScheduler() { @@ -283,69 +278,6 @@ public: EXPECT_EQ(PresentState::Presented, bufferSurfaceFrameTX->getPresentState()); } - void MergePendingStates_BufferlessSurfaceFramesWithoutOverlappingToken() { - sp<BufferStateLayer> layer = createBufferStateLayer(); - layer->setFrameTimelineVsyncForBufferlessTransaction({/*vsyncId*/ 1, /*inputEventId*/ 0}, - 10); - EXPECT_EQ(1u, layer->mCurrentState.bufferlessSurfaceFramesTX.size()); - ASSERT_EQ(nullptr, layer->mCurrentState.bufferSurfaceFrameTX); - const auto bufferlessSurfaceFrame1 = - layer->mCurrentState.bufferlessSurfaceFramesTX.at(/*token*/ 1); - - layer->pushPendingState(); - EXPECT_EQ(0u, layer->mCurrentState.bufferlessSurfaceFramesTX.size()); - - layer->setFrameTimelineVsyncForBufferlessTransaction({/*vsyncId*/ 2, /*inputEventId*/ 0}, - 12); - EXPECT_EQ(1u, layer->mCurrentState.bufferlessSurfaceFramesTX.size()); - ASSERT_EQ(nullptr, layer->mCurrentState.bufferSurfaceFrameTX); - const auto bufferlessSurfaceFrame2 = - layer->mCurrentState.bufferlessSurfaceFramesTX.at(/*token*/ 2); - - commitTransaction(layer.get()); - - EXPECT_EQ(1, bufferlessSurfaceFrame1->getToken()); - EXPECT_EQ(false, bufferlessSurfaceFrame1->getIsBuffer()); - EXPECT_EQ(PresentState::Presented, bufferlessSurfaceFrame1->getPresentState()); - EXPECT_EQ(10, bufferlessSurfaceFrame1->getActuals().endTime); - - EXPECT_EQ(2, bufferlessSurfaceFrame2->getToken()); - EXPECT_EQ(false, bufferlessSurfaceFrame2->getIsBuffer()); - EXPECT_EQ(PresentState::Presented, bufferlessSurfaceFrame2->getPresentState()); - EXPECT_EQ(12, bufferlessSurfaceFrame2->getActuals().endTime); - } - - void MergePendingStates_BufferlessSurfaceFramesWithOverlappingToken() { - sp<BufferStateLayer> layer = createBufferStateLayer(); - layer->setFrameTimelineVsyncForBufferlessTransaction({/*vsyncId*/ 1, /*inputEventId*/ 0}, - 10); - EXPECT_EQ(1u, layer->mCurrentState.bufferlessSurfaceFramesTX.size()); - ASSERT_EQ(nullptr, layer->mCurrentState.bufferSurfaceFrameTX); - const auto bufferlessSurfaceFrame1 = - layer->mCurrentState.bufferlessSurfaceFramesTX.at(/*token*/ 1); - - layer->pushPendingState(); - EXPECT_EQ(0u, layer->mCurrentState.bufferlessSurfaceFramesTX.size()); - - layer->setFrameTimelineVsyncForBufferlessTransaction({/*vsyncId*/ 1, /*inputEventId*/ 0}, - 12); - EXPECT_EQ(1u, layer->mCurrentState.bufferlessSurfaceFramesTX.size()); - ASSERT_EQ(nullptr, layer->mCurrentState.bufferSurfaceFrameTX); - const auto bufferlessSurfaceFrame2 = - layer->mCurrentState.bufferlessSurfaceFramesTX.at(/*token*/ 1); - - commitTransaction(layer.get()); - - EXPECT_EQ(1, bufferlessSurfaceFrame1->getToken()); - EXPECT_EQ(false, bufferlessSurfaceFrame1->getIsBuffer()); - EXPECT_EQ(PresentState::Unknown, bufferlessSurfaceFrame1->getPresentState()); - - EXPECT_EQ(1, bufferlessSurfaceFrame2->getToken()); - EXPECT_EQ(false, bufferlessSurfaceFrame2->getIsBuffer()); - EXPECT_EQ(PresentState::Presented, bufferlessSurfaceFrame2->getPresentState()); - EXPECT_EQ(12, bufferlessSurfaceFrame2->getActuals().endTime); - } - void PendingSurfaceFramesRemovedAfterClassification() { sp<BufferStateLayer> layer = createBufferStateLayer(); @@ -529,16 +461,6 @@ TEST_F(TransactionSurfaceFrameTest, MultipleSurfaceFramesPresentedTogether) { MultipleSurfaceFramesPresentedTogether(); } -TEST_F(TransactionSurfaceFrameTest, - MergePendingStates_BufferlessSurfaceFramesWithoutOverlappingToken) { - MergePendingStates_BufferlessSurfaceFramesWithoutOverlappingToken(); -} - -TEST_F(TransactionSurfaceFrameTest, - MergePendingStates_BufferlessSurfaceFramesWithOverlappingToken) { - MergePendingStates_BufferlessSurfaceFramesWithOverlappingToken(); -} - TEST_F(TransactionSurfaceFrameTest, PendingSurfaceFramesRemovedAfterClassification) { PendingSurfaceFramesRemovedAfterClassification(); } @@ -552,4 +474,4 @@ TEST_F(TransactionSurfaceFrameTest, MultipleCommitsBeforeLatch) { MultipleCommitsBeforeLatch(); } -} // namespace android
\ No newline at end of file +} // namespace android |