Merge "Revert "Revert "SF: Test coverage for OutputLayer::updateGeometryState"""
diff --git a/cmds/dumpstate/dumpstate.cpp b/cmds/dumpstate/dumpstate.cpp
index f94afda..8ba9526 100644
--- a/cmds/dumpstate/dumpstate.cpp
+++ b/cmds/dumpstate/dumpstate.cpp
@@ -94,7 +94,6 @@
using android::os::dumpstate::CommandOptions;
using android::os::dumpstate::DumpFileToFd;
using android::os::dumpstate::DumpstateSectionReporter;
-using android::os::dumpstate::GetPidByName;
using android::os::dumpstate::PropertiesHelper;
typedef Dumpstate::ConsentCallback::ConsentResult UserConsentResult;
@@ -421,108 +420,6 @@
closedir(d);
}
-
-
-// dump anrd's trace and add to the zip file.
-// 1. check if anrd is running on this device.
-// 2. send a SIGUSR1 to its pid which will dump anrd's trace.
-// 3. wait until the trace generation completes and add to the zip file.
-static bool dump_anrd_trace() {
- int pid;
- char buf[50], path[PATH_MAX];
- struct dirent *trace;
- struct stat st;
- DIR *trace_dir;
- int retry = 5;
- long max_ctime = 0, old_mtime;
- long long cur_size = 0;
- const char *trace_path = "/data/misc/anrd/";
-
- if (!ds.IsZipping()) {
- MYLOGE("Not dumping anrd trace because it's not a zipped bugreport\n");
- return false;
- }
-
- // find anrd's pid if it is running.
- pid = GetPidByName("/system/bin/anrd");
-
- if (pid > 0) {
- if (stat(trace_path, &st) == 0) {
- old_mtime = st.st_mtime;
- } else {
- MYLOGE("Failed to find: %s\n", trace_path);
- return false;
- }
-
- // send SIGUSR1 to the anrd to generate a trace.
- sprintf(buf, "%d", pid);
- if (RunCommand("ANRD_DUMP", {"kill", "-SIGUSR1", buf},
- CommandOptions::WithTimeout(1).Build())) {
- MYLOGE("anrd signal timed out. Please manually collect trace\n");
- return false;
- }
-
- while (retry-- > 0 && old_mtime == st.st_mtime) {
- sleep(1);
- stat(trace_path, &st);
- }
-
- if (retry < 0 && old_mtime == st.st_mtime) {
- MYLOGE("Failed to stat %s or trace creation timeout\n", trace_path);
- return false;
- }
-
- // identify the trace file by its creation time.
- if (!(trace_dir = opendir(trace_path))) {
- MYLOGE("Can't open trace file under %s\n", trace_path);
- }
- while ((trace = readdir(trace_dir))) {
- if (strcmp(trace->d_name, ".") == 0
- || strcmp(trace->d_name, "..") == 0) {
- continue;
- }
- sprintf(path, "%s%s", trace_path, trace->d_name);
- if (stat(path, &st) == 0) {
- if (st.st_ctime > max_ctime) {
- max_ctime = st.st_ctime;
- sprintf(buf, "%s", trace->d_name);
- }
- }
- }
- closedir(trace_dir);
-
- // Wait until the dump completes by checking the size of the trace.
- if (max_ctime > 0) {
- sprintf(path, "%s%s", trace_path, buf);
- while(true) {
- sleep(1);
- if (stat(path, &st) == 0) {
- if (st.st_size == cur_size) {
- break;
- } else if (st.st_size > cur_size) {
- cur_size = st.st_size;
- } else {
- return false;
- }
- } else {
- MYLOGE("Cant stat() %s anymore\n", path);
- return false;
- }
- }
- // Add to the zip file.
- if (!ds.AddZipEntry("anrd_trace.txt", path)) {
- MYLOGE("Unable to add anrd_trace file %s to zip file\n", path);
- } else {
- android::os::UnlinkAndLogOnError(path);
- return true;
- }
- } else {
- MYLOGE("Can't stats any trace file under %s\n", trace_path);
- }
- }
- return false;
-}
-
static bool skip_not_stat(const char *path) {
static const char stat[] = "/stat";
size_t len = strlen(path);
@@ -1380,7 +1277,7 @@
/* Dump Bluetooth HCI logs */
ds.AddDir("/data/misc/bluetooth/logs", true);
- if (!ds.do_early_screenshot_) {
+ if (ds.options_->do_fb && !ds.do_early_screenshot_) {
MYLOGI("taking late screenshot\n");
ds.TakeScreenshot();
}
@@ -1538,9 +1435,6 @@
* with the caller.
*/
static Dumpstate::RunStatus DumpstateDefault() {
- // Try to dump anrd trace if the daemon is running.
- dump_anrd_trace();
-
// Invoking the following dumpsys calls before DumpTraces() to try and
// keep the system stats as close to its initial state as possible.
RUN_SLOW_FUNCTION_WITH_CONSENT_CHECK(RunDumpsysCritical);
@@ -2625,13 +2519,8 @@
}
if (options_->do_fb && do_early_screenshot_) {
- if (screenshot_path_.empty()) {
- // should not have happened
- MYLOGE("INTERNAL ERROR: skipping early screenshot because path was not set\n");
- } else {
- MYLOGI("taking early screenshot\n");
- TakeScreenshot();
- }
+ MYLOGI("taking early screenshot\n");
+ TakeScreenshot();
}
if (options_->do_zip_file && zip_file != nullptr) {
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 3ca6ef5..cae1b46 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -158,18 +158,26 @@
}
}
-void Layer::onRemovedFromCurrentState() {
- mRemovedFromCurrentState = true;
+void Layer::removeRelativeZ(const std::vector<Layer*>& layersInTree) {
+ if (mCurrentState.zOrderRelativeOf == nullptr) {
+ return;
+ }
- // the layer is removed from SF mCurrentState to mLayersPendingRemoval
- if (mCurrentState.zOrderRelativeOf != nullptr) {
- sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();
- if (strongRelative != nullptr) {
- strongRelative->removeZOrderRelative(this);
- mFlinger->setTransactionFlags(eTraversalNeeded);
- }
+ sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();
+ if (strongRelative == nullptr) {
+ setZOrderRelativeOf(nullptr);
+ return;
+ }
+
+ if (!std::binary_search(layersInTree.begin(), layersInTree.end(), strongRelative.get())) {
+ strongRelative->removeZOrderRelative(this);
+ mFlinger->setTransactionFlags(eTraversalNeeded);
setZOrderRelativeOf(nullptr);
}
+}
+
+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
@@ -186,13 +194,18 @@
mLocalSyncPoints.clear();
}
- for (const auto& child : mCurrentChildren) {
- child->onRemovedFromCurrentState();
- }
-
mFlinger->markLayerPendingRemovalLocked(this);
}
+void Layer::onRemovedFromCurrentState() {
+ auto layersInTree = getLayersInTree(LayerVector::StateSet::Current);
+ std::sort(layersInTree.begin(), layersInTree.end());
+ for (const auto& layer : layersInTree) {
+ layer->removeFromCurrentState();
+ layer->removeRelativeZ(layersInTree);
+ }
+}
+
void Layer::addToCurrentState() {
mRemovedFromCurrentState = false;
@@ -415,7 +428,7 @@
auto& parentState = parent->getDrawingState();
const int parentType = parentState.metadata.getInt32(METADATA_WINDOW_TYPE, 0);
const int parentAppId = parentState.metadata.getInt32(METADATA_OWNER_UID, 0);
- if (parentType >= 0 || parentAppId >= 0) {
+ if (parentType > 0 && parentAppId > 0) {
type = parentType;
appId = parentAppId;
}
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index bbbc5b0..5c55111 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -571,6 +571,17 @@
virtual bool isBufferLatched() const { return false; }
/*
+ * Remove relative z for the layer if its relative parent is not part of the
+ * provided layer tree.
+ */
+ void removeRelativeZ(const std::vector<Layer*>& layersInTree);
+
+ /*
+ * Remove from current state and mark for removal.
+ */
+ void removeFromCurrentState();
+
+ /*
* called with the state lock from a binder thread when the layer is
* removed from the current list to the pending removal list
*/
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 50fabe3..87c1bf9 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1530,10 +1530,25 @@
void SurfaceFlinger::setPrimaryVsyncEnabled(bool enabled) {
ATRACE_CALL();
- Mutex::Autolock lock(mStateLock);
+
+ // Enable / Disable HWVsync from the main thread to avoid race conditions with
+ // display power state.
+ postMessageAsync(new LambdaMessage(
+ [=]() NO_THREAD_SAFETY_ANALYSIS { setPrimaryVsyncEnabledInternal(enabled); }));
+}
+
+void SurfaceFlinger::setPrimaryVsyncEnabledInternal(bool enabled) {
+ ATRACE_CALL();
+
if (const auto displayId = getInternalDisplayIdLocked()) {
- getHwComposer().setVsyncEnabled(*displayId,
- enabled ? HWC2::Vsync::Enable : HWC2::Vsync::Disable);
+ sp<DisplayDevice> display = getDefaultDisplayDeviceLocked();
+ if (display && display->isPoweredOn()) {
+ getHwComposer().setVsyncEnabled(*displayId,
+ enabled ? HWC2::Vsync::Enable : HWC2::Vsync::Disable);
+ } else {
+ // Cache the latest vsync state and apply it when screen is on again
+ mEnableHWVsyncScreenOn = enabled;
+ }
}
}
@@ -4431,6 +4446,11 @@
// Turn on the display
getHwComposer().setPowerMode(*displayId, mode);
if (display->isPrimary() && mode != HWC_POWER_MODE_DOZE_SUSPEND) {
+ if (mEnableHWVsyncScreenOn) {
+ setPrimaryVsyncEnabledInternal(mEnableHWVsyncScreenOn);
+ mEnableHWVsyncScreenOn = false;
+ }
+
mScheduler->onScreenAcquired(mAppConnectionHandle);
mScheduler->resyncToHardwareVsync(true, getVsyncPeriod());
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 74fefa7..46fc4d2 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -293,6 +293,9 @@
// TODO: this should be made accessible only to EventThread
void setPrimaryVsyncEnabled(bool enabled);
+ // main thread function to enable/disable h/w composer event
+ void setPrimaryVsyncEnabledInternal(bool enabled);
+
// called on the main thread by MessageQueue when an internal message
// is received
// TODO: this should be made accessible only to MessageQueue
@@ -1168,6 +1171,9 @@
// The Layer pointer is removed from the set when the destructor is called so there shouldn't
// be any issues with a raw pointer referencing an invalid object.
std::unordered_set<Layer*> mOffscreenLayers;
+
+ // Flag to indicate whether to re-enable HWVsync when screen is on
+ bool mEnableHWVsyncScreenOn = false;
};
} // namespace android
diff --git a/services/surfaceflinger/tests/SurfaceFlinger_test.filter b/services/surfaceflinger/tests/SurfaceFlinger_test.filter
index 5ebae1e..6b4634a 100644
--- a/services/surfaceflinger/tests/SurfaceFlinger_test.filter
+++ b/services/surfaceflinger/tests/SurfaceFlinger_test.filter
@@ -1,5 +1,5 @@
{
"presubmit": {
- "filter": "CredentialsTest.*:SurfaceFlingerStress.*:SurfaceInterceptorTest.*:LayerTransactionTest.*:LayerTypeTransactionTest.*:LayerUpdateTest.*:GeometryLatchingTest.*:CropLatchingTest.*:ChildLayerTest.*:ScreenCaptureTest.*:ScreenCaptureChildOnlyTest.*:DereferenceSurfaceControlTest.*:BoundlessLayerTest.*:MultiDisplayLayerBoundsTest.*:InvalidHandleTest.*:VirtualDisplayTest.*"
+ "filter": "CredentialsTest.*:SurfaceFlingerStress.*:SurfaceInterceptorTest.*:LayerTransactionTest.*:LayerTypeTransactionTest.*:LayerUpdateTest.*:GeometryLatchingTest.*:CropLatchingTest.*:ChildLayerTest.*:ScreenCaptureTest.*:ScreenCaptureChildOnlyTest.*:DereferenceSurfaceControlTest.*:BoundlessLayerTest.*:MultiDisplayLayerBoundsTest.*:InvalidHandleTest.*:VirtualDisplayTest.*:RelativeZTest.*"
}
}
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 62db5ec..f83b3ea 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -385,6 +385,18 @@
return createLayer(mClient, name, width, height, flags, parent);
}
+ sp<SurfaceControl> createColorLayer(const char* name, const Color& color,
+ SurfaceControl* parent = nullptr) {
+ auto colorLayer = createSurface(mClient, name, 0 /* buffer width */, 0 /* buffer height */,
+ PIXEL_FORMAT_RGBA_8888,
+ ISurfaceComposerClient::eFXSurfaceColor, parent);
+ asTransaction([&](Transaction& t) {
+ t.setColor(colorLayer, half3{color.r / 255.0f, color.g / 255.0f, color.b / 255.0f});
+ t.setAlpha(colorLayer, color.a / 255.0f);
+ });
+ return colorLayer;
+ }
+
ANativeWindow_Buffer getBufferQueueLayerBuffer(const sp<SurfaceControl>& layer) {
// wait for previous transactions (such as setSize) to complete
Transaction().apply(true);
@@ -5875,4 +5887,126 @@
allowedConfigs.end());
}
+class RelativeZTest : public LayerTransactionTest {
+protected:
+ virtual void SetUp() {
+ LayerTransactionTest::SetUp();
+ ASSERT_EQ(NO_ERROR, mClient->initCheck());
+
+ const auto display = SurfaceComposerClient::getInternalDisplayToken();
+ ASSERT_FALSE(display == nullptr);
+
+ // Back layer
+ mBackgroundLayer = createColorLayer("Background layer", Color::RED);
+
+ // Front layer
+ mForegroundLayer = createColorLayer("Foreground layer", Color::GREEN);
+
+ asTransaction([&](Transaction& t) {
+ t.setDisplayLayerStack(display, 0);
+ t.setLayer(mBackgroundLayer, INT32_MAX - 2).show(mBackgroundLayer);
+ t.setLayer(mForegroundLayer, INT32_MAX - 1).show(mForegroundLayer);
+ });
+ }
+
+ virtual void TearDown() {
+ LayerTransactionTest::TearDown();
+ mBackgroundLayer = 0;
+ mForegroundLayer = 0;
+ }
+
+ sp<SurfaceControl> mBackgroundLayer;
+ sp<SurfaceControl> mForegroundLayer;
+};
+
+// When a layer is reparented offscreen, remove relative z order if the relative parent
+// is still onscreen so that the layer is not drawn.
+TEST_F(RelativeZTest, LayerRemoved) {
+ std::unique_ptr<ScreenCapture> sc;
+
+ // Background layer (RED)
+ // Child layer (WHITE) (relative to foregroud layer)
+ // Foregroud layer (GREEN)
+ sp<SurfaceControl> childLayer =
+ createColorLayer("Child layer", Color::BLUE, mBackgroundLayer.get());
+
+ Transaction{}
+ .setRelativeLayer(childLayer, mForegroundLayer->getHandle(), 1)
+ .show(childLayer)
+ .apply();
+
+ {
+ // The childLayer should be in front of the FG control.
+ ScreenCapture::captureScreen(&sc);
+ sc->checkPixel(1, 1, Color::BLUE.r, Color::BLUE.g, Color::BLUE.b);
+ }
+
+ // Background layer (RED)
+ // Foregroud layer (GREEN)
+ Transaction{}.reparent(childLayer, nullptr).apply();
+
+ // Background layer (RED)
+ // Child layer (WHITE)
+ // Foregroud layer (GREEN)
+ Transaction{}.reparent(childLayer, mBackgroundLayer->getHandle()).apply();
+
+ {
+ // The relative z info for child layer should be reset, leaving FG control on top.
+ ScreenCapture::captureScreen(&sc);
+ sc->checkPixel(1, 1, Color::GREEN.r, Color::GREEN.g, Color::GREEN.b);
+ }
+}
+
+// When a layer is reparented offscreen, preseve relative z order if the relative parent
+// is also offscreen. Regression test b/132613412
+TEST_F(RelativeZTest, LayerRemovedOffscreenRelativeParent) {
+ std::unique_ptr<ScreenCapture> sc;
+
+ // Background layer (RED)
+ // Foregroud layer (GREEN)
+ // child level 1 (WHITE)
+ // child level 2a (BLUE)
+ // child level 3 (GREEN) (relative to child level 2b)
+ // child level 2b (BLACK)
+ sp<SurfaceControl> childLevel1 =
+ createColorLayer("child level 1", Color::WHITE, mForegroundLayer.get());
+ sp<SurfaceControl> childLevel2a =
+ createColorLayer("child level 2a", Color::BLUE, childLevel1.get());
+ sp<SurfaceControl> childLevel2b =
+ createColorLayer("child level 2b", Color::BLACK, childLevel1.get());
+ sp<SurfaceControl> childLevel3 =
+ createColorLayer("child level 3", Color::GREEN, childLevel2a.get());
+
+ Transaction{}
+ .setRelativeLayer(childLevel3, childLevel2b->getHandle(), 1)
+ .show(childLevel2a)
+ .show(childLevel2b)
+ .show(childLevel3)
+ .apply();
+
+ {
+ // The childLevel3 should be in front of childLevel2b.
+ ScreenCapture::captureScreen(&sc);
+ sc->checkPixel(1, 1, Color::GREEN.r, Color::GREEN.g, Color::GREEN.b);
+ }
+
+ // Background layer (RED)
+ // Foregroud layer (GREEN)
+ Transaction{}.reparent(childLevel1, nullptr).apply();
+
+ // Background layer (RED)
+ // Foregroud layer (GREEN)
+ // child level 1 (WHITE)
+ // child level 2 back (BLUE)
+ // child level 3 (GREEN) (relative to child level 2b)
+ // child level 2 front (BLACK)
+ Transaction{}.reparent(childLevel1, mForegroundLayer->getHandle()).apply();
+
+ {
+ // Nothing should change at this point since relative z info was preserved.
+ ScreenCapture::captureScreen(&sc);
+ sc->checkPixel(1, 1, Color::GREEN.r, Color::GREEN.g, Color::GREEN.b);
+ }
+}
+
} // namespace android