blast: fix crop and display frame
BufferStateLayers can have an infinite width and height. Check for
this edge case when calculating the Layer's transform.
Test: atest CtsViewTestCases:android.view.cts.ASurfaceControlTest
Bug: 124128940
Change-Id: I01f53483f154076b24cb3cf588026e410b2f87f9
diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp
index 756ca42..39defb8 100644
--- a/services/surfaceflinger/BufferLayer.cpp
+++ b/services/surfaceflinger/BufferLayer.cpp
@@ -204,8 +204,16 @@
}
const Rect win{getBounds()};
- const float bufferWidth = getBufferSize(s).getWidth();
- const float bufferHeight = getBufferSize(s).getHeight();
+ float bufferWidth = getBufferSize(s).getWidth();
+ float bufferHeight = getBufferSize(s).getHeight();
+
+ // BufferStateLayers can have a "buffer size" of [0, 0, -1, -1] when no display frame has
+ // been set and there is no parent layer bounds. In that case, the scale is meaningless so
+ // ignore them.
+ if (!getBufferSize(s).isValid()) {
+ bufferWidth = float(win.right) - float(win.left);
+ bufferHeight = float(win.bottom) - float(win.top);
+ }
const float scaleHeight = (float(win.bottom) - float(win.top)) / bufferHeight;
const float scaleWidth = (float(win.right) - float(win.left)) / bufferWidth;
diff --git a/services/surfaceflinger/BufferLayer.h b/services/surfaceflinger/BufferLayer.h
index e9dbdea..bcc57bd 100644
--- a/services/surfaceflinger/BufferLayer.h
+++ b/services/surfaceflinger/BufferLayer.h
@@ -180,6 +180,8 @@
// main thread.
bool mBufferLatched{false}; // TODO: Use mActiveBuffer?
+ // BufferStateLayers can return Rect::INVALID_RECT if the layer does not have a display frame
+ // and its parent layer is not bounded
Rect getBufferSize(const State& s) const override;
std::shared_ptr<compositionengine::Layer> mCompositionLayer;
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index b2383ad..fa08b2a 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -156,9 +156,22 @@
}
bool BufferStateLayer::setCrop(const Rect& crop) {
- if (mCurrentState.crop == crop) return false;
+ Rect c = crop;
+ if (c.left < 0) {
+ c.left = 0;
+ }
+ if (c.top < 0) {
+ c.top = 0;
+ }
+ // If the width and/or height are < 0, make it [0, 0, -1, -1] so the equality comparision below
+ // treats all invalid rectangles the same.
+ if (!c.isValid()) {
+ c.makeInvalid();
+ }
+
+ if (mCurrentState.crop == c) return false;
mCurrentState.sequence++;
- mCurrentState.crop = crop;
+ mCurrentState.crop = c;
mCurrentState.modified = true;
setTransactionFlags(eTransactionNeeded);
return true;
@@ -327,10 +340,6 @@
}
}
- // if there is no parent layer, use the buffer's bounds as the buffer size
- if (s.buffer) {
- return s.buffer->getBounds();
- }
return Rect::INVALID_RECT;
}
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 4567916..c7fb459 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -285,6 +285,14 @@
return mEffectiveTransform;
}
+ // If the layer is a buffer state layer, the active width and height
+ // could be infinite. In that case, return the effective transform.
+ const uint32_t activeWidth = getActiveWidth(getDrawingState());
+ const uint32_t activeHeight = getActiveHeight(getDrawingState());
+ if (activeWidth >= UINT32_MAX && activeHeight >= UINT32_MAX) {
+ return mEffectiveTransform;
+ }
+
int bufferWidth;
int bufferHeight;
if ((mCurrentTransform & NATIVE_WINDOW_TRANSFORM_ROT_90) == 0) {
@@ -294,8 +302,9 @@
bufferHeight = mActiveBuffer->getWidth();
bufferWidth = mActiveBuffer->getHeight();
}
- float sx = getActiveWidth(getDrawingState()) / static_cast<float>(bufferWidth);
- float sy = getActiveHeight(getDrawingState()) / static_cast<float>(bufferHeight);
+ float sx = activeWidth / static_cast<float>(bufferWidth);
+ float sy = activeHeight / static_cast<float>(bufferHeight);
+
ui::Transform extraParentScaling;
extraParentScaling.set(sx, 0, 0, sy);
return mEffectiveTransform * extraParentScaling;
@@ -399,6 +408,15 @@
Rect activeCrop = computeInitialCrop(display);
Rect bufferSize = getBufferSize(s);
+ int32_t winWidth = bufferSize.getWidth();
+ int32_t winHeight = bufferSize.getHeight();
+
+ // The bufferSize for buffer state layers can be unbounded ([0, 0, -1, -1]) if display frame
+ // hasn't been set and the parent is an unbounded layer.
+ if (winWidth < 0 && winHeight < 0) {
+ return crop;
+ }
+
// Transform the window crop to match the buffer coordinate system,
// which means using the inverse of the current transform set on the
// SurfaceFlingerConsumer.
@@ -418,8 +436,6 @@
ui::Transform(invTransform)).getOrientation();
}
- int winWidth = bufferSize.getWidth();
- int winHeight = bufferSize.getHeight();
if (invTransform & NATIVE_WINDOW_TRANSFORM_ROT_90) {
// If the activeCrop has been rotate the ends are rotated but not
// the space itself so when transforming ends back we can't rely on
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 34cdff7..6c518bc 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -2013,8 +2013,8 @@
Transaction().setCrop(layer, crop).apply();
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
TEST_P(LayerRenderTypeTransactionTest, SetCropEmpty_BufferQueue) {
@@ -2044,13 +2044,13 @@
{
SCOPED_TRACE("empty rect");
Transaction().setCrop(layer, Rect(8, 8, 8, 8)).apply();
- getScreenCapture()->expectColor(Rect(0, 0, 32, 32), Color::RED);
+ getScreenCapture()->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
}
{
SCOPED_TRACE("negative rect");
Transaction().setCrop(layer, Rect(8, 8, 0, 0)).apply();
- getScreenCapture()->expectColor(Rect(0, 0, 32, 32), Color::RED);
+ getScreenCapture()->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
}
}
@@ -2067,37 +2067,36 @@
TEST_P(LayerRenderTypeTransactionTest, SetCropOutOfBounds_BufferState) {
sp<SurfaceControl> layer;
- ASSERT_NO_FATAL_FAILURE(layer = createLayer("test", mDisplayWidth, mDisplayHeight / 2,
- ISurfaceComposerClient::eFXSurfaceBufferState));
+ ASSERT_NO_FATAL_FAILURE(
+ layer = createLayer("test", 32, 64, ISurfaceComposerClient::eFXSurfaceBufferState));
sp<GraphicBuffer> buffer =
- new GraphicBuffer(mDisplayWidth, mDisplayHeight / 2, PIXEL_FORMAT_RGBA_8888, 1,
+ new GraphicBuffer(32, 64, PIXEL_FORMAT_RGBA_8888, 1,
BufferUsage::CPU_READ_OFTEN | BufferUsage::CPU_WRITE_OFTEN |
BufferUsage::COMPOSER_OVERLAY,
"test");
- fillGraphicBufferColor(buffer, Rect(0, 0, mDisplayWidth, mDisplayHeight / 4), Color::BLUE);
- fillGraphicBufferColor(buffer, Rect(0, mDisplayHeight / 4, mDisplayWidth, mDisplayHeight / 2),
- Color::RED);
+ fillGraphicBufferColor(buffer, Rect(0, 0, 32, 16), Color::BLUE);
+ fillGraphicBufferColor(buffer, Rect(0, 16, 32, 64), Color::RED);
+
+ Transaction().setFrame(layer, Rect(0, 0, 64, 64)).apply();
Transaction().setBuffer(layer, buffer).apply();
// Partially out of bounds in the negative (upper left) direction
- Transaction().setCrop(layer, Rect(-128, -128, mDisplayWidth, mDisplayHeight / 4)).apply();
+ Transaction().setCrop(layer, Rect(-128, -128, 32, 16)).apply();
{
SCOPED_TRACE("out of bounds, negative (upper left) direction");
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight / 2), Color::BLUE);
- shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight / 2), Color::BLACK);
+ shot->expectColor(Rect(0, 0, 64, 64), Color::BLUE);
+ shot->expectBorder(Rect(0, 0, 64, 64), Color::BLACK);
}
// Partially out of bounds in the positive (lower right) direction
- Transaction()
- .setCrop(layer, Rect(0, mDisplayHeight / 4, mDisplayWidth + 1, mDisplayHeight))
- .apply();
+ Transaction().setCrop(layer, Rect(0, 16, 128, 128)).apply();
{
SCOPED_TRACE("out of bounds, positive (lower right) direction");
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight / 2), Color::RED);
- shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight / 2), Color::BLACK);
+ shot->expectColor(Rect(0, 0, 64, 64), Color::RED);
+ shot->expectBorder(Rect(0, 0, 64, 64), Color::BLACK);
}
// Fully out of buffer space bounds
@@ -2105,9 +2104,9 @@
{
SCOPED_TRACE("Fully out of bounds");
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight / 4), Color::BLUE);
- shot->expectColor(Rect(0, mDisplayHeight / 4, mDisplayWidth, mDisplayHeight / 2),
- Color::RED);
+ shot->expectColor(Rect(0, 0, 64, 16), Color::BLUE);
+ shot->expectColor(Rect(0, 16, 64, 64), Color::RED);
+ shot->expectBorder(Rect(0, 0, 64, 64), Color::BLACK);
}
}
@@ -2284,8 +2283,8 @@
// A parentless layer will default to a frame with the same size as the buffer
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 10, 10), Color::RED);
- shot->expectBorder(Rect(0, 0, 10, 10), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
TEST_P(LayerRenderTypeTransactionTest, SetFrameDefaultBSParent_BufferState) {
@@ -2368,8 +2367,8 @@
ASSERT_NO_FATAL_FAILURE(fillBufferStateLayerColor(layer, Color::RED, 32, 32));
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
TEST_P(LayerRenderTypeTransactionTest, SetBufferMultipleBuffers_BufferState) {
@@ -2382,8 +2381,8 @@
{
SCOPED_TRACE("set buffer 1");
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
ASSERT_NO_FATAL_FAILURE(fillBufferStateLayerColor(layer, Color::BLUE, 32, 32));
@@ -2391,8 +2390,8 @@
{
SCOPED_TRACE("set buffer 2");
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::BLUE);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLUE);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
ASSERT_NO_FATAL_FAILURE(fillBufferStateLayerColor(layer, Color::RED, 32, 32));
@@ -2400,8 +2399,8 @@
{
SCOPED_TRACE("set buffer 3");
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
}
@@ -2483,8 +2482,8 @@
Color color = colors[idx % colors.size()];
auto shot = screenshot();
- shot->expectColor(Rect(0, 0, 32, 32), color);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), color);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
idx++;
}
@@ -2519,8 +2518,8 @@
Color color = colors[idx % colors.size()];
auto shot = screenshot();
- shot->expectColor(Rect(0, 0, 32, 32), color);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), color);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
idx++;
}
@@ -2555,8 +2554,8 @@
Color color = colors[idx % colors.size()];
auto shot = screenshot();
- shot->expectColor(Rect(0, 0, 32, 32), color);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), color);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
if (idx == 0) {
buffers[0].clear();
@@ -2654,8 +2653,8 @@
std::this_thread::sleep_for(200ms);
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
TEST_P(LayerRenderTypeTransactionTest, SetFenceNull_BufferState) {
@@ -2678,8 +2677,8 @@
.apply();
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
TEST_P(LayerRenderTypeTransactionTest, SetDataspaceBasic_BufferState) {
@@ -2700,8 +2699,8 @@
.apply();
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
TEST_P(LayerRenderTypeTransactionTest, SetHdrMetadataBasic_BufferState) {
@@ -2724,8 +2723,8 @@
.apply();
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
TEST_P(LayerRenderTypeTransactionTest, SetSurfaceDamageRegionBasic_BufferState) {
@@ -2748,8 +2747,8 @@
.apply();
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
TEST_P(LayerRenderTypeTransactionTest, SetApiBasic_BufferState) {
@@ -2770,8 +2769,8 @@
.apply();
auto shot = getScreenCapture();
- shot->expectColor(Rect(0, 0, 32, 32), Color::RED);
- shot->expectBorder(Rect(0, 0, 32, 32), Color::BLACK);
+ shot->expectColor(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::RED);
+ shot->expectBorder(Rect(0, 0, mDisplayWidth, mDisplayHeight), Color::BLACK);
}
TEST_F(LayerTransactionTest, SetSidebandStreamNull_BufferState) {