Fix SurfaceControl#captureLayers when the layer is boundless
- Return an error if the client tries to screenshot a boundless layer without
specifying a crop. Otherwise the client will get a screenshot of 0x0.
- Use the crop in addition to the buffer size when determining the bounds of
the captured layer. This will enable us to capture container layers and color
layers that have a crop specified.
Fixes: 141326137
Test: atest SurfaceFlinger_test
Test: go/wm-smoke
Change-Id: Ibba4c01ad2d6739caee0d85b8d9c2d236fbf0ce0
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 7abcd0f..adf3075 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -699,6 +699,14 @@
Region debugGetVisibleRegionOnDefaultDisplay() const;
+ /**
+ * Returns the cropped buffer size or the layer crop if the layer has no buffer. Return
+ * INVALID_RECT if the layer has no buffer and no crop.
+ * A layer with an invalid buffer size and no crop is considered to be boundless. The layer
+ * bounds are constrained by its parent bounds.
+ */
+ Rect getCroppedBufferSize(const Layer::State& s) const;
+
protected:
// constant
sp<SurfaceFlinger> mFlinger;
@@ -907,13 +915,6 @@
const LayerVector::Visitor& visitor);
LayerVector makeChildrenTraversalList(LayerVector::StateSet stateSet,
const std::vector<Layer*>& layersInTree);
- /**
- * Returns the cropped buffer size or the layer crop if the layer has no buffer. Return
- * INVALID_RECT if the layer has no buffer and no crop.
- * A layer with an invalid buffer size and no crop is considered to be boundless. The layer
- * bounds are constrained by its parent bounds.
- */
- Rect getCroppedBufferSize(const Layer::State& s) const;
// Cached properties computed from drawing state
// Effective transform taking into account parent transforms and any parent scaling.
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index c555f79..8b04b65 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -5067,14 +5067,21 @@
return PERMISSION_DENIED;
}
+ Rect parentSourceBounds = parent->getCroppedBufferSize(parent->getCurrentState());
if (sourceCrop.width() <= 0) {
crop.left = 0;
- crop.right = parent->getBufferSize(parent->getCurrentState()).getWidth();
+ crop.right = parentSourceBounds.getWidth();
}
if (sourceCrop.height() <= 0) {
crop.top = 0;
- crop.bottom = parent->getBufferSize(parent->getCurrentState()).getHeight();
+ crop.bottom = parentSourceBounds.getHeight();
+ }
+
+ if (crop.isEmpty() || frameScale <= 0.0f) {
+ // Error out if the layer has no source bounds (i.e. they are boundless) and a source
+ // crop was not specified, or an invalid frame scale was provided.
+ return BAD_VALUE;
}
reqWidth = crop.width() * frameScale;
reqHeight = crop.height() * frameScale;
diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp
index b667a74..b1bb7fd 100644
--- a/services/surfaceflinger/tests/Credentials_test.cpp
+++ b/services/surfaceflinger/tests/Credentials_test.cpp
@@ -274,7 +274,7 @@
sp<GraphicBuffer> outBuffer;
return ScreenshotClient::captureLayers(mBGSurfaceControl->getHandle(),
ui::Dataspace::V0_SRGB, ui::PixelFormat::RGBA_8888,
- Rect(), FRAME_SCALE, &outBuffer);
+ Rect(0, 0, 1, 1), FRAME_SCALE, &outBuffer);
};
ASSERT_NO_FATAL_FAILURE(checkWithPrivileges<status_t>(condition, NO_ERROR, PERMISSION_DENIED));
}
diff --git a/services/surfaceflinger/tests/LayerUpdate_test.cpp b/services/surfaceflinger/tests/LayerUpdate_test.cpp
index 73f563d..0ad122b 100644
--- a/services/surfaceflinger/tests/LayerUpdate_test.cpp
+++ b/services/surfaceflinger/tests/LayerUpdate_test.cpp
@@ -1443,6 +1443,61 @@
mCapture->expectColor(Rect(0, 0, 9, 9), {100, 100, 100, 255});
}
+TEST_F(ScreenCaptureTest, CaptureBoundlessLayerWithSourceCrop) {
+ sp<SurfaceControl> child = createColorLayer("Child layer", Color::RED, mFGSurfaceControl.get());
+ SurfaceComposerClient::Transaction().show(child).apply(true);
+
+ sp<ISurfaceComposer> sf(ComposerService::getComposerService());
+ sp<GraphicBuffer> outBuffer;
+ Rect sourceCrop(0, 0, 10, 10);
+ ASSERT_EQ(NO_ERROR, sf->captureLayers(child->getHandle(), &outBuffer, sourceCrop));
+ ScreenCapture sc(outBuffer);
+
+ sc.expectColor(Rect(0, 0, 9, 9), Color::RED);
+}
+
+TEST_F(ScreenCaptureTest, CaptureBoundedLayerWithoutSourceCrop) {
+ sp<SurfaceControl> child = createColorLayer("Child layer", Color::RED, mFGSurfaceControl.get());
+ Rect layerCrop(0, 0, 10, 10);
+ SurfaceComposerClient::Transaction().setCrop_legacy(child, layerCrop).show(child).apply(true);
+
+ sp<ISurfaceComposer> sf(ComposerService::getComposerService());
+ sp<GraphicBuffer> outBuffer;
+ Rect sourceCrop = Rect();
+ ASSERT_EQ(NO_ERROR, sf->captureLayers(child->getHandle(), &outBuffer, sourceCrop));
+ ScreenCapture sc(outBuffer);
+
+ sc.expectColor(Rect(0, 0, 9, 9), Color::RED);
+}
+
+TEST_F(ScreenCaptureTest, CaptureBoundlessLayerWithoutSourceCropFails) {
+ sp<SurfaceControl> child = createColorLayer("Child layer", Color::RED, mFGSurfaceControl.get());
+ SurfaceComposerClient::Transaction().show(child).apply(true);
+
+ sp<ISurfaceComposer> sf(ComposerService::getComposerService());
+ sp<GraphicBuffer> outBuffer;
+ Rect sourceCrop = Rect();
+
+ ASSERT_EQ(BAD_VALUE, sf->captureLayers(child->getHandle(), &outBuffer, sourceCrop));
+}
+
+TEST_F(ScreenCaptureTest, CaptureBufferLayerWithoutBufferFails) {
+ sp<SurfaceControl> child = createSurface(mClient, "Child surface", 10, 10,
+ PIXEL_FORMAT_RGBA_8888, 0, mFGSurfaceControl.get());
+ SurfaceComposerClient::Transaction().show(child).apply(true);
+
+ sp<ISurfaceComposer> sf(ComposerService::getComposerService());
+ sp<GraphicBuffer> outBuffer;
+ Rect sourceCrop = Rect();
+ ASSERT_EQ(BAD_VALUE, sf->captureLayers(child->getHandle(), &outBuffer, sourceCrop));
+
+ TransactionUtils::fillSurfaceRGBA8(child, Color::RED);
+ SurfaceComposerClient::Transaction().apply(true);
+ ASSERT_EQ(NO_ERROR, sf->captureLayers(child->getHandle(), &outBuffer, sourceCrop));
+ ScreenCapture sc(outBuffer);
+ sc.expectColor(Rect(0, 0, 9, 9), Color::RED);
+}
+
// In the following tests we verify successful skipping of a parent layer,
// so we use the same verification logic and only change how we mutate
// the parent layer to verify that various properties are ignored.
diff --git a/services/surfaceflinger/tests/utils/TransactionUtils.h b/services/surfaceflinger/tests/utils/TransactionUtils.h
index 22df255..2c63da0 100644
--- a/services/surfaceflinger/tests/utils/TransactionUtils.h
+++ b/services/surfaceflinger/tests/utils/TransactionUtils.h
@@ -136,6 +136,11 @@
}
}
+ static void fillSurfaceRGBA8(const sp<SurfaceControl>& sc, const Color& color,
+ bool unlock = true) {
+ fillSurfaceRGBA8(sc, color.r, color.g, color.b, unlock);
+ }
+
// Fill an RGBA_8888 formatted surface with a single color.
static void fillSurfaceRGBA8(const sp<SurfaceControl>& sc, uint8_t r, uint8_t g, uint8_t b,
bool unlock = true) {