SurfaceFlinger: move sync_wait for screen capture to client
Free up time from the SF's main thread by moving the fence waiting
to the client.
Test: Observe systrace of region sample thread
Test: adb shell screencap
Test: Recents takes the screenshot
Test: Rotate device
Test: Volume + power down for screenshot
Bug: 178649983
Change-Id: I0a4991c013375b1f354e0728a06ca30a835b0422
diff --git a/libs/gui/ScreenCaptureResults.cpp b/libs/gui/ScreenCaptureResults.cpp
index 2b29487..f3849bc 100644
--- a/libs/gui/ScreenCaptureResults.cpp
+++ b/libs/gui/ScreenCaptureResults.cpp
@@ -25,6 +25,14 @@
} else {
SAFE_PARCEL(parcel->writeBool, false);
}
+
+ if (fence != Fence::NO_FENCE) {
+ SAFE_PARCEL(parcel->writeBool, true);
+ SAFE_PARCEL(parcel->write, *fence);
+ } else {
+ SAFE_PARCEL(parcel->writeBool, false);
+ }
+
SAFE_PARCEL(parcel->writeBool, capturedSecureLayers);
SAFE_PARCEL(parcel->writeUint32, static_cast<uint32_t>(capturedDataspace));
SAFE_PARCEL(parcel->writeInt32, result);
@@ -39,6 +47,13 @@
SAFE_PARCEL(parcel->read, *buffer);
}
+ bool hasFence;
+ SAFE_PARCEL(parcel->readBool, &hasFence);
+ if (hasFence) {
+ fence = new Fence();
+ SAFE_PARCEL(parcel->read, *fence);
+ }
+
SAFE_PARCEL(parcel->readBool, &capturedSecureLayers);
uint32_t dataspace = 0;
SAFE_PARCEL(parcel->readUint32, &dataspace);
diff --git a/libs/gui/aidl/android/gui/IScreenCaptureListener.aidl b/libs/gui/aidl/android/gui/IScreenCaptureListener.aidl
index f490118..e0f66cd 100644
--- a/libs/gui/aidl/android/gui/IScreenCaptureListener.aidl
+++ b/libs/gui/aidl/android/gui/IScreenCaptureListener.aidl
@@ -20,5 +20,5 @@
/** @hide */
oneway interface IScreenCaptureListener {
- void onScreenCaptureComplete(in ScreenCaptureResults captureResults);
+ void onScreenCaptureCompleted(in ScreenCaptureResults captureResults);
}
\ No newline at end of file
diff --git a/libs/gui/include/gui/ScreenCaptureResults.h b/libs/gui/include/gui/ScreenCaptureResults.h
index fdb4b69..0ccc6e8 100644
--- a/libs/gui/include/gui/ScreenCaptureResults.h
+++ b/libs/gui/include/gui/ScreenCaptureResults.h
@@ -27,6 +27,7 @@
#include <binder/Parcel.h>
#include <binder/Parcelable.h>
+#include <ui/Fence.h>
#include <ui/GraphicBuffer.h>
namespace android::gui {
@@ -39,6 +40,7 @@
status_t readFromParcel(const android::Parcel* parcel) override;
sp<GraphicBuffer> buffer;
+ sp<Fence> fence = Fence::NO_FENCE;
bool capturedSecureLayers{false};
ui::Dataspace capturedDataspace{ui::Dataspace::V0_SRGB};
status_t result = OK;
diff --git a/libs/gui/include/gui/SyncScreenCaptureListener.h b/libs/gui/include/gui/SyncScreenCaptureListener.h
index 8620b77..0784fbc 100644
--- a/libs/gui/include/gui/SyncScreenCaptureListener.h
+++ b/libs/gui/include/gui/SyncScreenCaptureListener.h
@@ -26,14 +26,16 @@
struct SyncScreenCaptureListener : gui::BnScreenCaptureListener {
public:
- binder::Status onScreenCaptureComplete(const ScreenCaptureResults& captureResults) override {
+ binder::Status onScreenCaptureCompleted(const ScreenCaptureResults& captureResults) override {
resultsPromise.set_value(captureResults);
return binder::Status::ok();
}
ScreenCaptureResults waitForResults() {
std::future<ScreenCaptureResults> resultsFuture = resultsPromise.get_future();
- return resultsFuture.get();
+ const auto screenCaptureResults = resultsFuture.get();
+ screenCaptureResults.fence->waitForever("");
+ return screenCaptureResults;
}
private:
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 4225e92..27df232 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -5851,23 +5851,18 @@
if (!renderArea) {
ALOGW("Skipping screen capture because of invalid render area.");
captureResults.result = NO_MEMORY;
- captureListener->onScreenCaptureComplete(captureResults);
+ captureListener->onScreenCaptureCompleted(captureResults);
return;
}
status_t result = NO_ERROR;
- int syncFd = -1;
renderArea->render([&] {
- result = renderScreenImplLocked(*renderArea, traverseLayers, buffer, forSystem, &syncFd,
+ result = renderScreenImplLocked(*renderArea, traverseLayers, buffer, forSystem,
regionSampling, captureResults);
});
- if (result == NO_ERROR) {
- sync_wait(syncFd, -1);
- close(syncFd);
- }
captureResults.result = result;
- captureListener->onScreenCaptureComplete(captureResults);
+ captureListener->onScreenCaptureCompleted(captureResults);
}));
return NO_ERROR;
@@ -5876,7 +5871,7 @@
status_t SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea,
TraverseLayersFunction traverseLayers,
const sp<GraphicBuffer>& buffer, bool forSystem,
- int* outSyncFd, bool regionSampling,
+ bool regionSampling,
ScreenCaptureResults& captureResults) {
ATRACE_CALL();
@@ -5985,14 +5980,14 @@
getRenderEngine().drawLayers(clientCompositionDisplay, clientCompositionLayerPointers, buffer,
/*useFramebufferCache=*/false, std::move(bufferFence), &drawFence);
- *outSyncFd = drawFence.release();
-
- if (*outSyncFd >= 0) {
- sp<Fence> releaseFence = new Fence(dup(*outSyncFd));
+ if (drawFence >= 0) {
+ sp<Fence> releaseFence = new Fence(dup(drawFence));
for (auto* layer : renderedLayers) {
layer->onLayerDisplayed(releaseFence);
}
}
+
+ captureResults.fence = new Fence(drawFence.release());
// Always switch back to unprotected context.
getRenderEngine().useProtectedContext(false);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 1deef6e..50d6099 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -815,8 +815,8 @@
status_t captureScreenCommon(RenderAreaFuture, TraverseLayersFunction, sp<GraphicBuffer>&,
bool regionSampling, const sp<IScreenCaptureListener>&);
status_t renderScreenImplLocked(const RenderArea&, TraverseLayersFunction,
- const sp<GraphicBuffer>&, bool forSystem, int* outSyncFd,
- bool regionSampling, ScreenCaptureResults&);
+ const sp<GraphicBuffer>&, bool forSystem, bool regionSampling,
+ ScreenCaptureResults&);
sp<DisplayDevice> getDisplayByIdOrLayerStack(uint64_t displayOrLayerStack) REQUIRES(mStateLock);
sp<DisplayDevice> getDisplayByLayerStack(uint64_t layerStack) REQUIRES(mStateLock);
diff --git a/services/surfaceflinger/tests/LayerState_test.cpp b/services/surfaceflinger/tests/LayerState_test.cpp
index ecfed13..93d5f2f 100644
--- a/services/surfaceflinger/tests/LayerState_test.cpp
+++ b/services/surfaceflinger/tests/LayerState_test.cpp
@@ -83,6 +83,7 @@
TEST(LayerStateTest, ParcellingScreenCaptureResults) {
ScreenCaptureResults results;
results.buffer = new GraphicBuffer(100, 200, PIXEL_FORMAT_RGBA_8888, 1, 0);
+ results.fence = new Fence(dup(fileno(tmpfile())));
results.capturedSecureLayers = true;
results.capturedDataspace = ui::Dataspace::DISPLAY_P3;
results.result = BAD_VALUE;
@@ -99,6 +100,7 @@
ASSERT_EQ(results.buffer->getWidth(), results2.buffer->getWidth());
ASSERT_EQ(results.buffer->getHeight(), results2.buffer->getHeight());
ASSERT_EQ(results.buffer->getPixelFormat(), results2.buffer->getPixelFormat());
+ ASSERT_EQ(results.fence->isValid(), results2.fence->isValid());
ASSERT_EQ(results.capturedSecureLayers, results2.capturedSecureLayers);
ASSERT_EQ(results.capturedDataspace, results2.capturedDataspace);
ASSERT_EQ(results.result, results2.result);
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 83e3ba4..f2051d9 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -253,14 +253,9 @@
mCaptureScreenBuffer = new GraphicBuffer(renderArea->getReqWidth(), renderArea->getReqHeight(),
HAL_PIXEL_FORMAT_RGBA_8888, 1, usage, "screenshot");
- int fd = -1;
status_t result =
mFlinger.renderScreenImplLocked(*renderArea, traverseLayers, mCaptureScreenBuffer.get(),
- forSystem, &fd, regionSampling);
- if (fd >= 0) {
- close(fd);
- }
-
+ forSystem, regionSampling);
EXPECT_EQ(NO_ERROR, result);
LayerCase::cleanup(this);
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index fc284ff..2701f47 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -349,11 +349,11 @@
auto renderScreenImplLocked(const RenderArea& renderArea,
SurfaceFlinger::TraverseLayersFunction traverseLayers,
- const sp<GraphicBuffer>& buffer, bool forSystem, int* outSyncFd,
+ const sp<GraphicBuffer>& buffer, bool forSystem,
bool regionSampling) {
ScreenCaptureResults captureResults;
return mFlinger->renderScreenImplLocked(renderArea, traverseLayers, buffer, forSystem,
- outSyncFd, regionSampling, captureResults);
+ regionSampling, captureResults);
}
auto traverseLayersInLayerStack(ui::LayerStack layerStack, int32_t uid,