From 561d05c18f127e2e43e93c82a559c537fd25ec52 Mon Sep 17 00:00:00 2001 From: Galia Peycheva Date: Mon, 8 Mar 2021 15:14:05 +0100 Subject: 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 --- libs/renderengine/skia/filters/BlurFilter.cpp | 17 ++- .../surfaceflinger/tests/LayerTransactionTest.h | 15 ++- .../LayerTypeAndRenderTypeTransaction_test.cpp | 120 +++++++++++++-------- .../surfaceflinger/tests/utils/ScreenshotUtils.h | 9 ++ 4 files changed, 110 insertions(+), 51 deletions(-) diff --git a/libs/renderengine/skia/filters/BlurFilter.cpp b/libs/renderengine/skia/filters/BlurFilter.cpp index 5960e48f7b..ec710d97c3 100644 --- a/libs/renderengine/skia/filters/BlurFilter.cpp +++ b/libs/renderengine/skia/filters/BlurFilter.cpp @@ -36,13 +36,18 @@ BlurFilter::BlurFilter() { 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 @@ sk_sp BlurFilter::generate(GrRecordingContext* context, const uint32_t 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 tmpBlur(blurBuilder.makeImage(context, nullptr, scaledInfo, false)); @@ -108,6 +115,8 @@ sk_sp BlurFilter::generate(GrRecordingContext* context, const uint32_t 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 eba2c250a5..05729be34b 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 #include #include #include @@ -245,6 +246,18 @@ protected: sp 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 mDisplay; uint32_t mDisplayWidth; uint32_t mDisplayHeight; @@ -307,4 +320,4 @@ private: } // 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 782a364ea7..44e718921a 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 #include #include "TransactionTestHarnesses.h" @@ -301,47 +300,86 @@ TEST_P(LayerTypeAndRenderTypeTransactionTest, SetCornerRadiusChildCrop) { } } -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 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 leftLayer; - ASSERT_NO_FATAL_FAILURE(leftLayer = createLayer("left", size / 2, size)); - ASSERT_NO_FATAL_FAILURE(fillLayerColor(leftLayer, Color::RED, size / 2, size)); - + sp rightLayer; + sp greenLayer; sp 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, SetBackgroundBlurRadiusOnMultipleL } 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 left; sp right; sp blur; sp 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 2fefa45e55..7efd730983 100644 --- a/services/surfaceflinger/tests/utils/ScreenshotUtils.h +++ b/services/surfaceflinger/tests/utils/ScreenshotUtils.h @@ -159,6 +159,15 @@ public: } } + 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); } -- cgit v1.2.3-59-g8ed1b From e7f6ee7ea154b4b4d322966bd003386ec181aa65 Mon Sep 17 00:00:00 2001 From: Galia Peycheva Date: Mon, 8 Mar 2021 15:19:23 +0100 Subject: Fix bg blur not showing when layer alpha is 0 A layer should be considered visible when the layer has a background blur value. The blur is always rendered at full opacity, even if the layer alpha is 0. This CL makes an effect layer visible when the layer has background blur. Bug: 167166562 Test: atest EffectLayerTest#BlurEffectLayerIsVisible Change-Id: I8d2a0988b185b1caef4fc458dff7ec4e8976ae16 --- services/surfaceflinger/EffectLayer.cpp | 2 +- services/surfaceflinger/tests/EffectLayer_test.cpp | 66 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/services/surfaceflinger/EffectLayer.cpp b/services/surfaceflinger/EffectLayer.cpp index 44d4d756e0..caef33848e 100644 --- a/services/surfaceflinger/EffectLayer.cpp +++ b/services/surfaceflinger/EffectLayer.cpp @@ -77,7 +77,7 @@ std::vector EffectLayer::prepareClien } bool EffectLayer::isVisible() const { - return !isHiddenByPolicy() && getAlpha() > 0.0_hf && hasSomethingToDraw(); + return !isHiddenByPolicy() && (getAlpha() > 0.0_hf || hasBlur()) && hasSomethingToDraw(); } bool EffectLayer::setColor(const half3& color) { diff --git a/services/surfaceflinger/tests/EffectLayer_test.cpp b/services/surfaceflinger/tests/EffectLayer_test.cpp index fafb49efba..7a3c45dbba 100644 --- a/services/surfaceflinger/tests/EffectLayer_test.cpp +++ b/services/surfaceflinger/tests/EffectLayer_test.cpp @@ -111,6 +111,72 @@ TEST_F(EffectLayerTest, EffectLayerCanSetColor) { } } +TEST_F(EffectLayerTest, BlurEffectLayerIsVisible) { + if (!deviceSupportsBlurs()) GTEST_SKIP(); + if (!deviceUsesSkiaRenderEngine()) GTEST_SKIP(); + + const auto canvasSize = 256; + + sp leftLayer = createColorLayer("Left", Color::BLUE); + sp rightLayer = createColorLayer("Right", Color::GREEN); + sp blurLayer; + 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); + + asTransaction([&](Transaction& t) { + t.setLayer(leftLayer, mLayerZBase + 1); + t.reparent(leftLayer, mParentLayer); + t.setCrop_legacy(leftLayer, leftRect); + t.setLayer(rightLayer, mLayerZBase + 2); + t.reparent(rightLayer, mParentLayer); + t.setCrop_legacy(rightLayer, rightRect); + t.show(leftLayer); + t.show(rightLayer); + }); + + { + auto shot = screenshot(); + shot->expectColor(leftRect, Color::BLUE); + shot->expectColor(rightRect, Color::GREEN); + } + + ASSERT_NO_FATAL_FAILURE(blurLayer = createColorLayer("BackgroundBlur", Color::TRANSPARENT)); + + const auto blurRadius = canvasSize / 2; + asTransaction([&](Transaction& t) { + t.setLayer(blurLayer, mLayerZBase + 3); + t.reparent(blurLayer, mParentLayer); + t.setBackgroundBlurRadius(blurLayer, blurRadius); + t.setCrop_legacy(blurLayer, blurRect); + t.setFrame(blurLayer, blurRect); + t.setAlpha(blurLayer, 0.0f); + t.show(blurLayer); + }); + + { + auto shot = screenshot(); + + 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.g, previousColor.g); + ASSERT_LT(currentColor.b, previousColor.b); + ASSERT_EQ(0, currentColor.r); + } + shot->checkPixel(canvasSize - 1, y, 0, 255, 0); + } + } +} + } // namespace android // TODO(b/129481165): remove the #pragma below and fix conversion issues -- cgit v1.2.3-59-g8ed1b