From 0ef7caa3ee99f1043e1d922d59e6d56341e501af Mon Sep 17 00:00:00 2001 From: chaviw Date: Tue, 5 Jan 2021 11:04:50 -0800 Subject: Convert IScreenCaptureListener to AIDL Test: Screen capture works Fixes: 166271443 Change-Id: Iecb991a0c8e0885adb87141ed85caa6dcc7c543f --- libs/gui/ScreenCaptureResults.cpp | 50 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 libs/gui/ScreenCaptureResults.cpp (limited to 'libs/gui/ScreenCaptureResults.cpp') diff --git a/libs/gui/ScreenCaptureResults.cpp b/libs/gui/ScreenCaptureResults.cpp new file mode 100644 index 0000000000..2b29487268 --- /dev/null +++ b/libs/gui/ScreenCaptureResults.cpp @@ -0,0 +1,50 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +namespace android::gui { + +status_t ScreenCaptureResults::writeToParcel(android::Parcel* parcel) const { + if (buffer != nullptr) { + SAFE_PARCEL(parcel->writeBool, true); + SAFE_PARCEL(parcel->write, *buffer); + } else { + SAFE_PARCEL(parcel->writeBool, false); + } + SAFE_PARCEL(parcel->writeBool, capturedSecureLayers); + SAFE_PARCEL(parcel->writeUint32, static_cast(capturedDataspace)); + SAFE_PARCEL(parcel->writeInt32, result); + return NO_ERROR; +} + +status_t ScreenCaptureResults::readFromParcel(const android::Parcel* parcel) { + bool hasGraphicBuffer; + SAFE_PARCEL(parcel->readBool, &hasGraphicBuffer); + if (hasGraphicBuffer) { + buffer = new GraphicBuffer(); + SAFE_PARCEL(parcel->read, *buffer); + } + + SAFE_PARCEL(parcel->readBool, &capturedSecureLayers); + uint32_t dataspace = 0; + SAFE_PARCEL(parcel->readUint32, &dataspace); + capturedDataspace = static_cast(dataspace); + SAFE_PARCEL(parcel->readInt32, &result); + return NO_ERROR; +} + +} // namespace android::gui -- cgit v1.2.3-59-g8ed1b From 99599940f68158aeef735d8763980d0dbd397b51 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Wed, 27 Jan 2021 20:27:23 -0800 Subject: 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 --- libs/gui/ScreenCaptureResults.cpp | 15 +++++++++++++++ .../aidl/android/gui/IScreenCaptureListener.aidl | 2 +- libs/gui/include/gui/ScreenCaptureResults.h | 2 ++ libs/gui/include/gui/SyncScreenCaptureListener.h | 6 ++++-- services/surfaceflinger/SurfaceFlinger.cpp | 21 ++++++++------------- services/surfaceflinger/SurfaceFlinger.h | 4 ++-- services/surfaceflinger/tests/LayerState_test.cpp | 2 ++ .../tests/unittests/CompositionTest.cpp | 7 +------ .../tests/unittests/TestableSurfaceFlinger.h | 4 ++-- 9 files changed, 37 insertions(+), 26 deletions(-) (limited to 'libs/gui/ScreenCaptureResults.cpp') diff --git a/libs/gui/ScreenCaptureResults.cpp b/libs/gui/ScreenCaptureResults.cpp index 2b29487268..f3849bcdcd 100644 --- a/libs/gui/ScreenCaptureResults.cpp +++ b/libs/gui/ScreenCaptureResults.cpp @@ -25,6 +25,14 @@ status_t ScreenCaptureResults::writeToParcel(android::Parcel* parcel) const { } 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(capturedDataspace)); SAFE_PARCEL(parcel->writeInt32, result); @@ -39,6 +47,13 @@ status_t ScreenCaptureResults::readFromParcel(const android::Parcel* parcel) { 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 f490118b8f..e0f66cd0a1 100644 --- a/libs/gui/aidl/android/gui/IScreenCaptureListener.aidl +++ b/libs/gui/aidl/android/gui/IScreenCaptureListener.aidl @@ -20,5 +20,5 @@ import android.gui.ScreenCaptureResults; /** @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 fdb4b6985e..0ccc6e8b8a 100644 --- a/libs/gui/include/gui/ScreenCaptureResults.h +++ b/libs/gui/include/gui/ScreenCaptureResults.h @@ -27,6 +27,7 @@ #include #include +#include #include namespace android::gui { @@ -39,6 +40,7 @@ public: status_t readFromParcel(const android::Parcel* parcel) override; sp buffer; + sp 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 8620b77c18..0784fbc058 100644 --- a/libs/gui/include/gui/SyncScreenCaptureListener.h +++ b/libs/gui/include/gui/SyncScreenCaptureListener.h @@ -26,14 +26,16 @@ using gui::ScreenCaptureResults; 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 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 4225e92c60..27df232472 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -5851,23 +5851,18 @@ status_t SurfaceFlinger::captureScreenCommon(RenderAreaFuture renderAreaFuture, 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::captureScreenCommon(RenderAreaFuture renderAreaFuture, status_t SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, TraverseLayersFunction traverseLayers, const sp& buffer, bool forSystem, - int* outSyncFd, bool regionSampling, + bool regionSampling, ScreenCaptureResults& captureResults) { ATRACE_CALL(); @@ -5985,14 +5980,14 @@ status_t SurfaceFlinger::renderScreenImplLocked(const RenderArea& renderArea, getRenderEngine().drawLayers(clientCompositionDisplay, clientCompositionLayerPointers, buffer, /*useFramebufferCache=*/false, std::move(bufferFence), &drawFence); - *outSyncFd = drawFence.release(); - - if (*outSyncFd >= 0) { - sp releaseFence = new Fence(dup(*outSyncFd)); + if (drawFence >= 0) { + sp 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 1deef6eb17..50d6099698 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -815,8 +815,8 @@ private: status_t captureScreenCommon(RenderAreaFuture, TraverseLayersFunction, sp&, bool regionSampling, const sp&); status_t renderScreenImplLocked(const RenderArea&, TraverseLayersFunction, - const sp&, bool forSystem, int* outSyncFd, - bool regionSampling, ScreenCaptureResults&); + const sp&, bool forSystem, bool regionSampling, + ScreenCaptureResults&); sp getDisplayByIdOrLayerStack(uint64_t displayOrLayerStack) REQUIRES(mStateLock); sp getDisplayByLayerStack(uint64_t layerStack) REQUIRES(mStateLock); diff --git a/services/surfaceflinger/tests/LayerState_test.cpp b/services/surfaceflinger/tests/LayerState_test.cpp index ecfed13adb..93d5f2f8ec 100644 --- a/services/surfaceflinger/tests/LayerState_test.cpp +++ b/services/surfaceflinger/tests/LayerState_test.cpp @@ -83,6 +83,7 @@ TEST(LayerStateTest, ParcellingLayerCaptureArgs) { 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 @@ TEST(LayerStateTest, ParcellingScreenCaptureResults) { 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 83e3ba414e..f2051d91c9 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -253,14 +253,9 @@ void CompositionTest::captureScreenComposition() { 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 fc284ff948..2701f472aa 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -349,11 +349,11 @@ public: auto renderScreenImplLocked(const RenderArea& renderArea, SurfaceFlinger::TraverseLayersFunction traverseLayers, - const sp& buffer, bool forSystem, int* outSyncFd, + const sp& 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, -- cgit v1.2.3-59-g8ed1b From 3b1f7bcf8fd0ca2458eb26ec31a1bddf4bf7fd43 Mon Sep 17 00:00:00 2001 From: Marin Shalamanov Date: Tue, 16 Mar 2021 15:51:53 +0100 Subject: Extract ParcelableUtils to avoid code duplication Currently the SAFE_PARCEL macros are copies on multiple places. This CL extracts them in a single ParcelableUtils. Additionally in SAFE_PARCEL also the stringified error is printed. Bug: 179116474 Test: presubmit Change-Id: Ie09c3a9753b5742be14fe3cdb0061d5c64465e66 --- libs/gui/FrameTimelineInfo.cpp | 1 + libs/gui/ISurfaceComposer.cpp | 1 + libs/gui/ITransactionCompletedListener.cpp | 3 ++- libs/gui/LayerState.cpp | 1 + libs/gui/ScreenCaptureResults.cpp | 2 ++ libs/gui/SurfaceComposerClient.cpp | 1 + libs/gui/SurfaceControl.cpp | 1 + libs/gui/include/gui/LayerState.h | 18 +------------- libs/gui/include/gui/ScreenCaptureResults.h | 9 ------- libs/gui/include/private/gui/ParcelUtils.h | 38 +++++++++++++++++++++++++++++ 10 files changed, 48 insertions(+), 27 deletions(-) create mode 100644 libs/gui/include/private/gui/ParcelUtils.h (limited to 'libs/gui/ScreenCaptureResults.cpp') diff --git a/libs/gui/FrameTimelineInfo.cpp b/libs/gui/FrameTimelineInfo.cpp index f40077403a..9231a570fc 100644 --- a/libs/gui/FrameTimelineInfo.cpp +++ b/libs/gui/FrameTimelineInfo.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp index 989abd9a15..f44f10a2ce 100644 --- a/libs/gui/ISurfaceComposer.cpp +++ b/libs/gui/ISurfaceComposer.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp index 0ded9361bf..2579f50f2a 100644 --- a/libs/gui/ITransactionCompletedListener.cpp +++ b/libs/gui/ITransactionCompletedListener.cpp @@ -17,9 +17,10 @@ #define LOG_TAG "ITransactionCompletedListener" //#define LOG_NDEBUG 0 -#include #include #include +#include +#include namespace android { diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 6bb8bf2d5b..04a878563e 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include diff --git a/libs/gui/ScreenCaptureResults.cpp b/libs/gui/ScreenCaptureResults.cpp index f3849bcdcd..e91f74f3d3 100644 --- a/libs/gui/ScreenCaptureResults.cpp +++ b/libs/gui/ScreenCaptureResults.cpp @@ -16,6 +16,8 @@ #include +#include + namespace android::gui { status_t ScreenCaptureResults::writeToParcel(android::Parcel* parcel) const { diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index f56578981b..18a0cbd1c0 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -39,6 +39,7 @@ #include #include #include +#include #include #include diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp index 1dcfe2e804..7e2f8f9d36 100644 --- a/libs/gui/SurfaceControl.cpp +++ b/libs/gui/SurfaceControl.cpp @@ -39,6 +39,7 @@ #include #include #include +#include namespace android { diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 183ec40fba..f7a66985b7 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -16,23 +16,7 @@ #ifndef ANDROID_SF_LAYER_STATE_H #define ANDROID_SF_LAYER_STATE_H -#define SAFE_PARCEL(FUNC, ...) \ - { \ - status_t error = FUNC(__VA_ARGS__); \ - if (error) { \ - ALOGE("ERROR(%d). Failed to call parcel %s(%s)", error, #FUNC, #__VA_ARGS__); \ - return error; \ - } \ - } - -#define SAFE_PARCEL_READ_SIZE(FUNC, COUNT, SIZE) \ - { \ - SAFE_PARCEL(FUNC, COUNT); \ - if (static_cast(*COUNT) > SIZE) { \ - ALOGE("ERROR(BAD_VALUE). %s was greater than dataSize", #COUNT); \ - return BAD_VALUE; \ - } \ - } + #include #include diff --git a/libs/gui/include/gui/ScreenCaptureResults.h b/libs/gui/include/gui/ScreenCaptureResults.h index 0ccc6e8b8a..99c35c1a3d 100644 --- a/libs/gui/include/gui/ScreenCaptureResults.h +++ b/libs/gui/include/gui/ScreenCaptureResults.h @@ -16,15 +16,6 @@ #pragma once -#define SAFE_PARCEL(FUNC, ...) \ - { \ - status_t error = FUNC(__VA_ARGS__); \ - if (error) { \ - ALOGE("ERROR(%d). Failed to call parcel %s(%s)", error, #FUNC, #__VA_ARGS__); \ - return error; \ - } \ - } - #include #include #include diff --git a/libs/gui/include/private/gui/ParcelUtils.h b/libs/gui/include/private/gui/ParcelUtils.h new file mode 100644 index 0000000000..1cdd07e36e --- /dev/null +++ b/libs/gui/include/private/gui/ParcelUtils.h @@ -0,0 +1,38 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#define SAFE_PARCEL(FUNC, ...) \ + { \ + status_t error = FUNC(__VA_ARGS__); \ + if (error) { \ + ALOGE("ERROR(%s, %d). Failed to call parcel %s(%s)", strerror(-error), error, #FUNC, \ + #__VA_ARGS__); \ + return error; \ + } \ + } + +#define SAFE_PARCEL_READ_SIZE(FUNC, COUNT, SIZE) \ + { \ + SAFE_PARCEL(FUNC, COUNT); \ + if (static_cast(*COUNT) > SIZE) { \ + ALOGE("ERROR(BAD_VALUE). %s was greater than dataSize", #COUNT); \ + return BAD_VALUE; \ + } \ + } \ No newline at end of file -- cgit v1.2.3-59-g8ed1b