summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--services/surfaceflinger/Layer.cpp41
-rw-r--r--services/surfaceflinger/Layer.h7
-rw-r--r--services/surfaceflinger/LayerVector.cpp21
-rw-r--r--services/surfaceflinger/LayerVector.h15
-rw-r--r--services/surfaceflinger/SurfaceFlinger.cpp14
-rw-r--r--services/surfaceflinger/SurfaceFlinger.h18
-rw-r--r--services/surfaceflinger/SurfaceFlinger_hwc1.cpp14
-rw-r--r--services/surfaceflinger/SurfaceInterceptor.cpp2
-rw-r--r--services/surfaceflinger/tests/Transaction_test.cpp31
9 files changed, 115 insertions, 48 deletions
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 406debf703..d33d3704e8 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -1033,7 +1033,7 @@ void Layer::onDraw(const sp<const DisplayDevice>& hw, const Region& clip,
// figure out if there is something below us
Region under;
bool finished = false;
- mFlinger->mDrawingState.layersSortedByZ.traverseInZOrder([&](Layer* layer) {
+ mFlinger->mDrawingState.traverseInZOrder([&](Layer* layer) {
if (finished || layer == static_cast<Layer const*>(this)) {
finished = true;
return;
@@ -2519,7 +2519,7 @@ bool Layer::reparentChildren(const sp<IBinder>& newParentHandle) {
}
bool Layer::detachChildren() {
- traverseInZOrder([this](Layer* child) {
+ traverseInZOrder(LayerVector::StateSet::Drawing, [this](Layer* child) {
if (child == this) {
return;
}
@@ -2553,20 +2553,26 @@ int32_t Layer::getZ() const {
return mDrawingState.z;
}
-LayerVector Layer::makeTraversalList() {
- if (mDrawingState.zOrderRelatives.size() == 0) {
- return mDrawingChildren;
+LayerVector Layer::makeTraversalList(LayerVector::StateSet stateSet) {
+ LOG_ALWAYS_FATAL_IF(stateSet == LayerVector::StateSet::Invalid,
+ "makeTraversalList received invalid stateSet");
+ const bool useDrawing = stateSet == LayerVector::StateSet::Drawing;
+ const LayerVector& children = useDrawing ? mDrawingChildren : mCurrentChildren;
+ const State& state = useDrawing ? mDrawingState : mCurrentState;
+
+ if (state.zOrderRelatives.size() == 0) {
+ return children;
}
LayerVector traverse;
- for (const wp<Layer>& weakRelative : mDrawingState.zOrderRelatives) {
+ for (const wp<Layer>& weakRelative : state.zOrderRelatives) {
sp<Layer> strongRelative = weakRelative.promote();
if (strongRelative != nullptr) {
traverse.add(strongRelative);
}
}
- for (const sp<Layer>& child : mDrawingChildren) {
+ for (const sp<Layer>& child : children) {
traverse.add(child);
}
@@ -2576,8 +2582,8 @@ LayerVector Layer::makeTraversalList() {
/**
* Negatively signed relatives are before 'this' in Z-order.
*/
-void Layer::traverseInZOrder(const std::function<void(Layer*)>& exec) {
- LayerVector list = makeTraversalList();
+void Layer::traverseInZOrder(LayerVector::StateSet stateSet, const LayerVector::Visitor& visitor) {
+ LayerVector list = makeTraversalList(stateSet);
size_t i = 0;
for (; i < list.size(); i++) {
@@ -2585,20 +2591,21 @@ void Layer::traverseInZOrder(const std::function<void(Layer*)>& exec) {
if (relative->getZ() >= 0) {
break;
}
- relative->traverseInZOrder(exec);
+ relative->traverseInZOrder(stateSet, visitor);
}
- exec(this);
+ visitor(this);
for (; i < list.size(); i++) {
const auto& relative = list[i];
- relative->traverseInZOrder(exec);
+ relative->traverseInZOrder(stateSet, visitor);
}
}
/**
* Positively signed relatives are before 'this' in reverse Z-order.
*/
-void Layer::traverseInReverseZOrder(const std::function<void(Layer*)>& exec) {
- LayerVector list = makeTraversalList();
+void Layer::traverseInReverseZOrder(LayerVector::StateSet stateSet,
+ const LayerVector::Visitor& visitor) {
+ LayerVector list = makeTraversalList(stateSet);
int32_t i = 0;
for (i = list.size()-1; i>=0; i--) {
@@ -2606,12 +2613,12 @@ void Layer::traverseInReverseZOrder(const std::function<void(Layer*)>& exec) {
if (relative->getZ() < 0) {
break;
}
- relative->traverseInReverseZOrder(exec);
+ relative->traverseInReverseZOrder(stateSet, visitor);
}
- exec(this);
+ visitor(this);
for (; i>=0; i--) {
const auto& relative = list[i];
- relative->traverseInReverseZOrder(exec);
+ relative->traverseInReverseZOrder(stateSet, visitor);
}
}
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index a5224ecb4b..9f45435327 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -476,8 +476,9 @@ public:
uint8_t getAlpha() const;
#endif
- void traverseInReverseZOrder(const std::function<void(Layer*)>& exec);
- void traverseInZOrder(const std::function<void(Layer*)>& exec);
+ void traverseInReverseZOrder(LayerVector::StateSet stateSet,
+ const LayerVector::Visitor& visitor);
+ void traverseInZOrder(LayerVector::StateSet stateSet, const LayerVector::Visitor& visitor);
void addChild(const sp<Layer>& layer);
// Returns index if removed, or negative value otherwise
@@ -557,7 +558,7 @@ private:
void setParent(const sp<Layer>& layer);
- LayerVector makeTraversalList();
+ LayerVector makeTraversalList(LayerVector::StateSet stateSet);
void addZOrderRelative(const wp<Layer>& relative);
void removeZOrderRelative(const wp<Layer>& relative);
diff --git a/services/surfaceflinger/LayerVector.cpp b/services/surfaceflinger/LayerVector.cpp
index 90e6395ea1..2233e78c2b 100644
--- a/services/surfaceflinger/LayerVector.cpp
+++ b/services/surfaceflinger/LayerVector.cpp
@@ -18,9 +18,14 @@
#include "Layer.h"
namespace android {
+
+LayerVector::LayerVector() = default;
+
LayerVector::LayerVector(const LayerVector& rhs) : SortedVector<sp<Layer>>(rhs) {
}
+LayerVector::~LayerVector() = default;
+
int LayerVector::do_compare(const void* lhs, const void* rhs) const
{
// sort layers per layer-stack, then by z-order and finally by sequence
@@ -40,23 +45,27 @@ int LayerVector::do_compare(const void* lhs, const void* rhs) const
return l->sequence - r->sequence;
}
-void LayerVector::traverseInZOrder(const std::function<void(Layer*)>& consume) const {
+void LayerVector::traverseInZOrder(StateSet stateSet, const Visitor& visitor) const {
for (size_t i = 0; i < size(); i++) {
const auto& layer = (*this)[i];
- if (layer->getDrawingState().zOrderRelativeOf != nullptr) {
+ auto& state = (stateSet == StateSet::Current) ? layer->getCurrentState()
+ : layer->getDrawingState();
+ if (state.zOrderRelativeOf != nullptr) {
continue;
}
- layer->traverseInZOrder(consume);
+ layer->traverseInZOrder(stateSet, visitor);
}
}
-void LayerVector::traverseInReverseZOrder(const std::function<void(Layer*)>& consume) const {
+void LayerVector::traverseInReverseZOrder(StateSet stateSet, const Visitor& visitor) const {
for (auto i = static_cast<int64_t>(size()) - 1; i >= 0; i--) {
const auto& layer = (*this)[i];
- if (layer->getDrawingState().zOrderRelativeOf != nullptr) {
+ auto& state = (stateSet == StateSet::Current) ? layer->getCurrentState()
+ : layer->getDrawingState();
+ if (state.zOrderRelativeOf != nullptr) {
continue;
}
- layer->traverseInReverseZOrder(consume);
+ layer->traverseInReverseZOrder(stateSet, visitor);
}
}
} // namespace android
diff --git a/services/surfaceflinger/LayerVector.h b/services/surfaceflinger/LayerVector.h
index 7dfa4a0350..a9adb4113a 100644
--- a/services/surfaceflinger/LayerVector.h
+++ b/services/surfaceflinger/LayerVector.h
@@ -32,13 +32,22 @@ class Layer;
*/
class LayerVector : public SortedVector<sp<Layer>> {
public:
- LayerVector() = default;
+ LayerVector();
LayerVector(const LayerVector& rhs);
+ ~LayerVector() override;
+
+ enum class StateSet {
+ Invalid,
+ Current,
+ Drawing,
+ };
+
// Sorts layer by layer-stack, Z order, and finally creation order (sequence).
int do_compare(const void* lhs, const void* rhs) const override;
- void traverseInReverseZOrder(const std::function<void(Layer*)>& consume) const;
- void traverseInZOrder(const std::function<void(Layer*)>& consume) const;
+ using Visitor = std::function<void(Layer*)>;
+ void traverseInReverseZOrder(StateSet stateSet, const Visitor& visitor) const;
+ void traverseInZOrder(StateSet stateSet, const Visitor& visitor) const;
};
}
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 9cd1214be6..bc4e6c5696 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4195,7 +4195,7 @@ void SurfaceFlinger::renderScreenImplLocked(
if (state.z < minLayerZ || state.z > maxLayerZ) {
continue;
}
- layer->traverseInZOrder([&](Layer* layer) {
+ layer->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) {
if (!layer->isVisible()) {
return;
}
@@ -4243,7 +4243,7 @@ status_t SurfaceFlinger::captureScreenImplLocked(
(state.z < minLayerZ || state.z > maxLayerZ)) {
continue;
}
- layer->traverseInZOrder([&](Layer *layer) {
+ layer->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer *layer) {
secureLayerIsVisible = secureLayerIsVisible || (layer->isVisible() &&
layer->isSecure());
});
@@ -4391,7 +4391,7 @@ void SurfaceFlinger::checkScreenshot(size_t w, size_t s, size_t h, void const* v
const Layer::State& state(layer->getDrawingState());
if (layer->getLayerStack() == hw->getLayerStack() && state.z >= minLayerZ &&
state.z <= maxLayerZ) {
- layer->traverseInZOrder([&](Layer* layer) {
+ layer->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) {
ALOGE("%c index=%zu, name=%s, layerStack=%d, z=%d, visible=%d, flags=%x, alpha=%.3f",
layer->isVisible() ? '+' : '-',
i, layer->getName().string(), layer->getLayerStack(), state.z,
@@ -4405,12 +4405,12 @@ void SurfaceFlinger::checkScreenshot(size_t w, size_t s, size_t h, void const* v
// ---------------------------------------------------------------------------
-void SurfaceFlinger::State::traverseInZOrder(const std::function<void(Layer*)>& consume) const {
- layersSortedByZ.traverseInZOrder(consume);
+void SurfaceFlinger::State::traverseInZOrder(const LayerVector::Visitor& visitor) const {
+ layersSortedByZ.traverseInZOrder(stateSet, visitor);
}
-void SurfaceFlinger::State::traverseInReverseZOrder(const std::function<void(Layer*)>& consume) const {
- layersSortedByZ.traverseInReverseZOrder(consume);
+void SurfaceFlinger::State::traverseInReverseZOrder(const LayerVector::Visitor& visitor) const {
+ layersSortedByZ.traverseInReverseZOrder(stateSet, visitor);
}
}; // namespace android
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index c01f70122a..1bc689dc79 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -238,11 +238,21 @@ private:
class State {
public:
+ explicit State(LayerVector::StateSet set) : stateSet(set) {}
+ State& operator=(const State& other) {
+ // We explicitly don't copy stateSet so that, e.g., mDrawingState
+ // always uses the Drawing StateSet.
+ layersSortedByZ = other.layersSortedByZ;
+ displays = other.displays;
+ return *this;
+ }
+
+ const LayerVector::StateSet stateSet = LayerVector::StateSet::Invalid;
LayerVector layersSortedByZ;
DefaultKeyedVector< wp<IBinder>, DisplayDeviceState> displays;
- void traverseInZOrder(const std::function<void(Layer*)>& consume) const;
- void traverseInReverseZOrder(const std::function<void(Layer*)>& consume) const;
+ void traverseInZOrder(const LayerVector::Visitor& visitor) const;
+ void traverseInReverseZOrder(const LayerVector::Visitor& visitor) const;
};
/* ------------------------------------------------------------------------
@@ -577,7 +587,7 @@ private:
// access must be protected by mStateLock
mutable Mutex mStateLock;
- State mCurrentState;
+ State mCurrentState{LayerVector::StateSet::Current};
volatile int32_t mTransactionFlags;
Condition mTransactionCV;
bool mTransactionPending;
@@ -617,7 +627,7 @@ private:
// Can only accessed from the main thread, these members
// don't need synchronization
- State mDrawingState;
+ State mDrawingState{LayerVector::StateSet::Drawing};
bool mVisibleRegionsDirty;
#ifndef USE_HWC2
bool mHwWorkListDirty;
diff --git a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
index 02923ae5bb..019c8e31ff 100644
--- a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
+++ b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
@@ -3808,7 +3808,7 @@ void SurfaceFlinger::renderScreenImplLocked(
if (state.z < minLayerZ || state.z > maxLayerZ) {
continue;
}
- layer->traverseInZOrder([&](Layer* layer) {
+ layer->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) {
if (!layer->isVisible()) {
return;
}
@@ -3858,7 +3858,7 @@ status_t SurfaceFlinger::captureScreenImplLocked(
(state.z < minLayerZ || state.z > maxLayerZ)) {
continue;
}
- layer->traverseInZOrder([&](Layer *layer) {
+ layer->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer *layer) {
secureLayerIsVisible = secureLayerIsVisible || (layer->isVisible() &&
layer->isSecure());
});
@@ -3997,7 +3997,7 @@ void SurfaceFlinger::checkScreenshot(size_t w, size_t s, size_t h, void const* v
const Layer::State& state(layer->getDrawingState());
if (layer->getLayerStack() == hw->getLayerStack() && state.z >= minLayerZ &&
state.z <= maxLayerZ) {
- layer->traverseInZOrder([&](Layer* layer) {
+ layer->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) {
ALOGE("%c index=%zu, name=%s, layerStack=%d, z=%d, visible=%d, flags=%x, alpha=%x",
layer->isVisible() ? '+' : '-',
i, layer->getName().string(), layer->getLayerStack(), state.z,
@@ -4011,12 +4011,12 @@ void SurfaceFlinger::checkScreenshot(size_t w, size_t s, size_t h, void const* v
// ---------------------------------------------------------------------------
-void SurfaceFlinger::State::traverseInZOrder(const std::function<void(Layer*)>& consume) const {
- layersSortedByZ.traverseInZOrder(consume);
+void SurfaceFlinger::State::traverseInZOrder(const LayerVector::Visitor& visitor) const {
+ layersSortedByZ.traverseInZOrder(stateSet, visitor);
}
-void SurfaceFlinger::State::traverseInReverseZOrder(const std::function<void(Layer*)>& consume) const {
- layersSortedByZ.traverseInReverseZOrder(consume);
+void SurfaceFlinger::State::traverseInReverseZOrder(const LayerVector::Visitor& visitor) const {
+ layersSortedByZ.traverseInReverseZOrder(stateSet, visitor);
}
}; // namespace android
diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp
index 026fe803c0..db489b2456 100644
--- a/services/surfaceflinger/SurfaceInterceptor.cpp
+++ b/services/surfaceflinger/SurfaceInterceptor.cpp
@@ -80,7 +80,7 @@ void SurfaceInterceptor::saveExistingDisplaysLocked(
void SurfaceInterceptor::saveExistingSurfacesLocked(const SortedVector<sp<Layer>>& layers) {
ATRACE_CALL();
for (const auto& l : layers) {
- l->traverseInZOrder([this](Layer* layer) {
+ l->traverseInZOrder(LayerVector::StateSet::Drawing, [this](Layer* layer) {
addSurfaceCreationLocked(createTraceIncrementLocked(), layer);
addInitialSurfaceStateLocked(createTraceIncrementLocked(), layer);
});
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 5474fda3b7..1b17e55b30 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -977,4 +977,35 @@ TEST_F(ChildLayerTest, ChildrenWithParentBufferTransform) {
}
}
+TEST_F(ChildLayerTest, Bug36858924) {
+ // Destroy the child layer
+ mChild.clear();
+
+ // Now recreate it as hidden
+ mChild = mComposerClient->createSurface(String8("Child surface"), 10, 10,
+ PIXEL_FORMAT_RGBA_8888, ISurfaceComposerClient::eHidden,
+ mFGSurfaceControl.get());
+
+ // Show the child layer in a deferred transaction
+ SurfaceComposerClient::openGlobalTransaction();
+ mChild->deferTransactionUntil(mFGSurfaceControl->getHandle(),
+ mFGSurfaceControl->getSurface()->getNextFrameNumber());
+ mChild->show();
+ SurfaceComposerClient::closeGlobalTransaction(true);
+
+ // 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(mFGSurfaceControl, 0, 255, 0);
+ ALOGI("Filling 2");
+ fillSurfaceRGBA8(mFGSurfaceControl, 0, 0, 255);
+ ALOGI("Filling 3");
+ fillSurfaceRGBA8(mFGSurfaceControl, 255, 0, 0);
+ ALOGI("Filling 4");
+ fillSurfaceRGBA8(mFGSurfaceControl, 0, 255, 0);
+}
+
}
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-12-08 22:57:43 +0000