Fix blur sampling outside bounds of layer
The image that the blur shader uses is the screenshot of the whole
screen, translated to the start of the blurRect. However, nothing stops
the shader from sampling outside of the right/bottom edges of the
blurRect. As a result, the blurring takes samples from outside of the
layer bounds.
This CL restricts the blur shader to sample the blurRect edge color when
it tries to sample outside the blurRect.
Bug: 167166562
Test: atest SurfaceFlinger_test -- --test-arg
com.android.tradefed.testtype.GTest:native-test-flag:"--gtest_filter=Layer*Tests/Layer*Test.SetBackgroundBlurRadiusSimple*"
Change-Id: I5f8f7c4a87cbcd3b0cb080720e554119d8d07f69
diff --git a/libs/renderengine/skia/filters/BlurFilter.cpp b/libs/renderengine/skia/filters/BlurFilter.cpp
index 5960e48..ec710d9 100644
--- a/libs/renderengine/skia/filters/BlurFilter.cpp
+++ b/libs/renderengine/skia/filters/BlurFilter.cpp
@@ -36,13 +36,18 @@
SkString blurString(R"(
in shader input;
uniform float2 in_blurOffset;
+ uniform float2 in_maxSizeXY;
half4 main(float2 xy) {
half4 c = sample(input, xy);
- c += sample(input, xy + float2( in_blurOffset.x, in_blurOffset.y));
- c += sample(input, xy + float2( in_blurOffset.x, -in_blurOffset.y));
- c += sample(input, xy + float2(-in_blurOffset.x, in_blurOffset.y));
- c += sample(input, xy + float2(-in_blurOffset.x, -in_blurOffset.y));
+ c += sample(input, float2( clamp( in_blurOffset.x + xy.x, 0, in_maxSizeXY.x),
+ clamp(in_blurOffset.y + xy.y, 0, in_maxSizeXY.y)));
+ c += sample(input, float2( clamp( in_blurOffset.x + xy.x, 0, in_maxSizeXY.x),
+ clamp(-in_blurOffset.y + xy.y, 0, in_maxSizeXY.y)));
+ c += sample(input, float2( clamp( -in_blurOffset.x + xy.x, 0, in_maxSizeXY.x),
+ clamp(in_blurOffset.y + xy.y, 0, in_maxSizeXY.y)));
+ c += sample(input, float2( clamp( -in_blurOffset.x + xy.x, 0, in_maxSizeXY.x),
+ clamp(-in_blurOffset.y + xy.y, 0, in_maxSizeXY.y)));
return half4(c.rgb * 0.2, 1.0);
}
@@ -99,6 +104,8 @@
blurBuilder.child("input") =
input->makeShader(SkTileMode::kClamp, SkTileMode::kClamp, linear, blurMatrix);
blurBuilder.uniform("in_blurOffset") = SkV2{stepX * kInputScale, stepY * kInputScale};
+ blurBuilder.uniform("in_maxSizeXY") =
+ SkV2{blurRect.width() * kInputScale - 1, blurRect.height() * kInputScale - 1};
sk_sp<SkImage> tmpBlur(blurBuilder.makeImage(context, nullptr, scaledInfo, false));
@@ -108,6 +115,8 @@
blurBuilder.child("input") =
tmpBlur->makeShader(SkTileMode::kClamp, SkTileMode::kClamp, linear);
blurBuilder.uniform("in_blurOffset") = SkV2{stepX * stepScale, stepY * stepScale};
+ blurBuilder.uniform("in_maxSizeXY") =
+ SkV2{blurRect.width() * kInputScale - 1, blurRect.height() * kInputScale - 1};
tmpBlur = blurBuilder.makeImage(context, nullptr, scaledInfo, false);
}
diff --git a/services/surfaceflinger/tests/LayerTransactionTest.h b/services/surfaceflinger/tests/LayerTransactionTest.h
index eba2c25..05729be 100644
--- a/services/surfaceflinger/tests/LayerTransactionTest.h
+++ b/services/surfaceflinger/tests/LayerTransactionTest.h
@@ -21,6 +21,7 @@
#pragma clang diagnostic ignored "-Wconversion"
#pragma clang diagnostic ignored "-Wextra"
+#include <cutils/properties.h>
#include <gtest/gtest.h>
#include <gui/ISurfaceComposer.h>
#include <gui/SurfaceComposerClient.h>
@@ -245,6 +246,18 @@
sp<SurfaceComposerClient> mClient;
+ bool deviceSupportsBlurs() {
+ char value[PROPERTY_VALUE_MAX];
+ property_get("ro.surface_flinger.supports_background_blur", value, "0");
+ return atoi(value);
+ }
+
+ bool deviceUsesSkiaRenderEngine() {
+ char value[PROPERTY_VALUE_MAX];
+ property_get("debug.renderengine.backend", value, "default");
+ return strstr(value, "skia") != nullptr;
+ }
+
sp<IBinder> mDisplay;
uint32_t mDisplayWidth;
uint32_t mDisplayHeight;
@@ -307,4 +320,4 @@
} // namespace android
// TODO(b/129481165): remove the #pragma below and fix conversion issues
-#pragma clang diagnostic pop // ignored "-Wconversion -Wextra"
\ No newline at end of file
+#pragma clang diagnostic pop // ignored "-Wconversion -Wextra"
diff --git a/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp b/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp
index 782a364..44e7189 100644
--- a/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp
+++ b/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp
@@ -18,7 +18,6 @@
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wconversion"
-#include <cutils/properties.h>
#include <gui/BufferItemConsumer.h>
#include "TransactionTestHarnesses.h"
@@ -301,47 +300,86 @@
}
}
-TEST_P(LayerTypeAndRenderTypeTransactionTest, SetBackgroundBlurRadius) {
- char value[PROPERTY_VALUE_MAX];
- property_get("ro.surface_flinger.supports_background_blur", value, "0");
- if (!atoi(value)) {
- // This device doesn't support blurs, no-op.
- return;
- }
+TEST_P(LayerTypeAndRenderTypeTransactionTest, SetBackgroundBlurRadiusSimple) {
+ if (!deviceSupportsBlurs()) GTEST_SKIP();
+ if (!deviceUsesSkiaRenderEngine()) GTEST_SKIP();
- auto size = 256;
- auto center = size / 2;
- auto blurRadius = 50;
-
- sp<SurfaceControl> backgroundLayer;
- ASSERT_NO_FATAL_FAILURE(backgroundLayer = createLayer("background", size, size));
- ASSERT_NO_FATAL_FAILURE(fillLayerColor(backgroundLayer, Color::GREEN, size, size));
+ const auto canvasSize = 256;
sp<SurfaceControl> leftLayer;
- ASSERT_NO_FATAL_FAILURE(leftLayer = createLayer("left", size / 2, size));
- ASSERT_NO_FATAL_FAILURE(fillLayerColor(leftLayer, Color::RED, size / 2, size));
-
+ sp<SurfaceControl> rightLayer;
+ sp<SurfaceControl> greenLayer;
sp<SurfaceControl> blurLayer;
- ASSERT_NO_FATAL_FAILURE(blurLayer = createLayer("blur", size, size));
- ASSERT_NO_FATAL_FAILURE(fillLayerColor(blurLayer, Color::TRANSPARENT, size, size));
+ const auto leftRect = Rect(0, 0, canvasSize / 2, canvasSize);
+ const auto rightRect = Rect(canvasSize / 2, 0, canvasSize, canvasSize);
+ const auto blurRect = Rect(0, 0, canvasSize, canvasSize);
- Transaction().setBackgroundBlurRadius(blurLayer, blurRadius).apply();
+ ASSERT_NO_FATAL_FAILURE(leftLayer =
+ createLayer("Left", leftRect.getWidth(), leftRect.getHeight()));
+ ASSERT_NO_FATAL_FAILURE(
+ fillLayerColor(leftLayer, Color::BLUE, leftRect.getWidth(), leftRect.getHeight()));
+ ASSERT_NO_FATAL_FAILURE(greenLayer = createLayer("Green", canvasSize * 2, canvasSize * 2));
+ ASSERT_NO_FATAL_FAILURE(
+ fillLayerColor(greenLayer, Color::GREEN, canvasSize * 2, canvasSize * 2));
+ ASSERT_NO_FATAL_FAILURE(
+ rightLayer = createLayer("Right", rightRect.getWidth(), rightRect.getHeight()));
+ ASSERT_NO_FATAL_FAILURE(
+ fillLayerColor(rightLayer, Color::RED, rightRect.getWidth(), rightRect.getHeight()));
- auto shot = getScreenCapture();
- // Edges are mixed
- shot->expectColor(Rect(center - 1, center - 5, center, center + 5), Color{150, 150, 0, 255},
- 50 /* tolerance */);
- shot->expectColor(Rect(center, center - 5, center + 1, center + 5), Color{150, 150, 0, 255},
- 50 /* tolerance */);
+ Transaction()
+ .setLayer(greenLayer, mLayerZBase)
+ .setFrame(leftLayer, {0, 0, canvasSize * 2, canvasSize * 2})
+ .setLayer(leftLayer, mLayerZBase + 1)
+ .setFrame(leftLayer, leftRect)
+ .setLayer(rightLayer, mLayerZBase + 2)
+ .setPosition(rightLayer, rightRect.left, rightRect.top)
+ .setFrame(rightLayer, rightRect)
+ .apply();
+
+ {
+ auto shot = getScreenCapture();
+ shot->expectColor(leftRect, Color::BLUE);
+ shot->expectColor(rightRect, Color::RED);
+ }
+
+ ASSERT_NO_FATAL_FAILURE(blurLayer = createColorLayer("BackgroundBlur", Color::TRANSPARENT));
+
+ const auto blurRadius = canvasSize / 2;
+ Transaction()
+ .setLayer(blurLayer, mLayerZBase + 3)
+ .setBackgroundBlurRadius(blurLayer, blurRadius)
+ .setCrop_legacy(blurLayer, blurRect)
+ .setFrame(blurLayer, blurRect)
+ .setSize(blurLayer, blurRect.getWidth(), blurRect.getHeight())
+ .setAlpha(blurLayer, 0.0f)
+ .apply();
+
+ {
+ auto shot = getScreenCapture();
+
+ const auto stepSize = 1;
+ const auto blurAreaOffset = blurRadius * 0.7f;
+ const auto blurAreaStartX = canvasSize / 2 - blurRadius + blurAreaOffset;
+ const auto blurAreaEndX = canvasSize / 2 + blurRadius - blurAreaOffset;
+ Color previousColor;
+ Color currentColor;
+ for (int y = 0; y < canvasSize; y++) {
+ shot->checkPixel(0, y, /* r = */ 0, /* g = */ 0, /* b = */ 255);
+ previousColor = shot->getPixelColor(0, y);
+ for (int x = blurAreaStartX; x < blurAreaEndX; x += stepSize) {
+ currentColor = shot->getPixelColor(x, y);
+ ASSERT_GT(currentColor.r, previousColor.r);
+ ASSERT_LT(currentColor.b, previousColor.b);
+ ASSERT_EQ(0, currentColor.g);
+ }
+ shot->checkPixel(canvasSize - 1, y, 255, 0, 0);
+ }
+ }
}
TEST_P(LayerTypeAndRenderTypeTransactionTest, SetBackgroundBlurRadiusOnMultipleLayers) {
- char value[PROPERTY_VALUE_MAX];
- property_get("ro.surface_flinger.supports_background_blur", value, "0");
- if (!atoi(value)) {
- // This device doesn't support blurs, no-op.
- return;
- }
+ if (!deviceSupportsBlurs()) GTEST_SKIP();
+ if (!deviceUsesSkiaRenderEngine()) GTEST_SKIP();
auto size = 256;
auto center = size / 2;
@@ -378,25 +416,15 @@
}
TEST_P(LayerTypeAndRenderTypeTransactionTest, SetBackgroundBlurAffectedByParentAlpha) {
- char value[PROPERTY_VALUE_MAX];
- property_get("ro.surface_flinger.supports_background_blur", value, "0");
- if (!atoi(value)) {
- // This device doesn't support blurs, no-op.
- return;
- }
-
- property_get("debug.renderengine.backend", value, "");
- if (strcmp(value, "skiagl") != 0) {
- // This device isn't using Skia render engine, no-op.
- return;
- }
+ if (!deviceSupportsBlurs()) GTEST_SKIP();
+ if (!deviceUsesSkiaRenderEngine()) GTEST_SKIP();
sp<SurfaceControl> left;
sp<SurfaceControl> right;
sp<SurfaceControl> blur;
sp<SurfaceControl> blurParent;
- const auto size = 32;
+ const auto size = 256;
ASSERT_NO_FATAL_FAILURE(left = createLayer("Left", size, size));
ASSERT_NO_FATAL_FAILURE(fillLayerColor(left, Color::BLUE, size, size));
ASSERT_NO_FATAL_FAILURE(right = createLayer("Right", size, size));
diff --git a/services/surfaceflinger/tests/utils/ScreenshotUtils.h b/services/surfaceflinger/tests/utils/ScreenshotUtils.h
index 2fefa45..7efd730 100644
--- a/services/surfaceflinger/tests/utils/ScreenshotUtils.h
+++ b/services/surfaceflinger/tests/utils/ScreenshotUtils.h
@@ -159,6 +159,15 @@
}
}
+ Color getPixelColor(uint32_t x, uint32_t y) {
+ if (!mOutBuffer || mOutBuffer->getPixelFormat() != HAL_PIXEL_FORMAT_RGBA_8888) {
+ return {0, 0, 0, 0};
+ }
+
+ const uint8_t* pixel = mPixels + (4 * (y * mOutBuffer->getStride() + x));
+ return {pixel[0], pixel[1], pixel[2], pixel[3]};
+ }
+
void expectFGColor(uint32_t x, uint32_t y) { checkPixel(x, y, 195, 63, 63); }
void expectBGColor(uint32_t x, uint32_t y) { checkPixel(x, y, 63, 63, 195); }