Don't fill boundary rectangle when drawing shadows.
SurfaceFlinger splits up EffectLayers containing shadows into multiple
parts:
1. A layer only containing the shadow metadata, and
2. Either a background blur or a solid color.
Since the default solid color is an opaque black, this causes shadows to
black out PIP windows.
A follow-up patch should change the default solid color to be a
transparent black instead so that nothing is drawn, or define a
canonical invalid color but we'd have to check callers to make sure
the GLES path doesn't break either.
Bug: 175824511
Bug: 175819287
Test: PIP window
Change-Id: I21336a7885299a127f3dad2a6b88c9dcab0b5980
diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
index 1f98a46..9c7a3bb 100644
--- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp
+++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
@@ -673,13 +673,17 @@
? getSkRect(layer->geometry.roundedCornersCrop)
: dest;
drawShadow(canvas, rect, layer->geometry.roundedCornersRadius, layer->shadow);
+ } else {
+ // Shadows are assumed to live only on their own layer - it's not valid
+ // to draw the boundary retangles when there is already a caster shadow
+ // TODO(b/175915334): consider relaxing this restriction to enable more flexible
+ // composition - using a well-defined invalid color is long-term less error-prone.
+ // Push the clipRRect onto the clip stack. Draw the image. Pop the clip.
+ if (layer->geometry.roundedCornersRadius > 0) {
+ canvas->clipRRect(getRoundedRect(layer), true);
+ }
+ canvas->drawRect(dest, paint);
}
-
- // Push the clipRRect onto the clip stack. Draw the image. Pop the clip.
- if (layer->geometry.roundedCornersRadius > 0) {
- canvas->clipRRect(getRoundedRect(layer), true);
- }
- canvas->drawRect(dest, paint);
canvas->restore();
}
canvas->restore();
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index d9dfd8c..d755431 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -305,6 +305,26 @@
backgroundColor.a);
}
+ void expectShadowColorWithoutCaster(const FloatRect& casterBounds,
+ const renderengine::ShadowSettings& shadow,
+ const ubyte4& backgroundColor) {
+ const float shadowInset = shadow.length * -1.0f;
+ const Rect casterRect(casterBounds);
+ const Rect shadowRect =
+ Rect(casterRect).inset(shadowInset, shadowInset, shadowInset, shadowInset);
+
+ const Region backgroundRegion =
+ Region(fullscreenRect()).subtractSelf(casterRect).subtractSelf(shadowRect);
+
+ expectAlpha(shadowRect, 255);
+ // (0, 0, 0) fill on the bounds of the layer should be ignored.
+ expectBufferColor(casterRect, 255, 255, 255, 255, 254);
+
+ // verify background
+ expectBufferColor(backgroundRegion, backgroundColor.r, backgroundColor.g, backgroundColor.b,
+ backgroundColor.a);
+ }
+
static renderengine::ShadowSettings getShadowSettings(const vec2& casterPos, float shadowLength,
bool casterIsTranslucent) {
renderengine::ShadowSettings shadow;
@@ -446,6 +466,10 @@
const renderengine::ShadowSettings& shadow, const ubyte4& casterColor,
const ubyte4& backgroundColor);
+ void drawShadowWithoutCaster(const FloatRect& castingBounds,
+ const renderengine::ShadowSettings& shadow,
+ const ubyte4& backgroundColor);
+
std::unique_ptr<renderengine::gl::GLESRenderEngine> mRE;
// Dumb hack to avoid NPE in the EGL driver: the GraphicBuffer needs to
@@ -1118,6 +1142,37 @@
invokeDraw(settings, layers, mBuffer);
}
+void RenderEngineTest::drawShadowWithoutCaster(const FloatRect& castingBounds,
+ const renderengine::ShadowSettings& shadow,
+ const ubyte4& backgroundColor) {
+ renderengine::DisplaySettings settings;
+ settings.outputDataspace = ui::Dataspace::V0_SRGB_LINEAR;
+ settings.physicalDisplay = fullscreenRect();
+ settings.clip = fullscreenRect();
+
+ std::vector<const renderengine::LayerSettings*> layers;
+
+ // add background layer
+ renderengine::LayerSettings bgLayer;
+ bgLayer.sourceDataspace = ui::Dataspace::V0_SRGB_LINEAR;
+ bgLayer.geometry.boundaries = fullscreenRect().toFloatRect();
+ ColorSourceVariant::fillColor(bgLayer, backgroundColor.r / 255.0f, backgroundColor.g / 255.0f,
+ backgroundColor.b / 255.0f, this);
+ bgLayer.alpha = backgroundColor.a / 255.0f;
+ layers.push_back(&bgLayer);
+
+ // add shadow layer
+ renderengine::LayerSettings shadowLayer;
+ shadowLayer.sourceDataspace = ui::Dataspace::V0_SRGB_LINEAR;
+ shadowLayer.geometry.boundaries = castingBounds;
+ shadowLayer.alpha = 1.0f;
+ ColorSourceVariant::fillColor(shadowLayer, 0, 0, 0, this);
+ shadowLayer.shadow = shadow;
+ layers.push_back(&shadowLayer);
+
+ invokeDraw(settings, layers, mBuffer);
+}
+
INSTANTIATE_TEST_SUITE_P(PerRenderEngineType, RenderEngineTest,
testing::Values(std::make_shared<GLESRenderEngineFactory>(),
std::make_shared<GLESCMRenderEngineFactory>(),
@@ -1608,6 +1663,22 @@
EXPECT_FALSE(mRE->isImageCachedForTesting(bufferId));
}
+TEST_P(RenderEngineTest, drawLayers_fillShadow_castsWithoutCasterLayer) {
+ const auto& renderEngineFactory = GetParam();
+ mRE = renderEngineFactory->createRenderEngine();
+
+ const ubyte4 backgroundColor(255, 255, 255, 255);
+ const float shadowLength = 5.0f;
+ Rect casterBounds(DEFAULT_DISPLAY_WIDTH / 3.0f, DEFAULT_DISPLAY_HEIGHT / 3.0f);
+ casterBounds.offsetBy(shadowLength + 1, shadowLength + 1);
+ renderengine::ShadowSettings settings =
+ getShadowSettings(vec2(casterBounds.left, casterBounds.top), shadowLength,
+ false /* casterIsTranslucent */);
+
+ drawShadowWithoutCaster(casterBounds.toFloatRect(), settings, backgroundColor);
+ expectShadowColorWithoutCaster(casterBounds.toFloatRect(), settings, backgroundColor);
+}
+
TEST_P(RenderEngineTest, drawLayers_fillShadow_casterLayerMinSize) {
const auto& renderEngineFactory = GetParam();
mRE = renderEngineFactory->createRenderEngine();
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 0e861ab..9237113 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -688,6 +688,7 @@
shadowLayer.source.buffer.fence = nullptr;
shadowLayer.frameNumber = 0;
shadowLayer.bufferId = 0;
+ shadowLayer.name = getName();
if (shadowLayer.shadow.ambientColor.a <= 0.f && shadowLayer.shadow.spotColor.a <= 0.f) {
return {};
@@ -723,6 +724,7 @@
// If layer is blacked out, force alpha to 1 so that we draw a black color layer.
layerSettings.alpha = blackout ? 1.0f : 0.0f;
+ layerSettings.name = getName();
}
std::vector<compositionengine::LayerFE::LayerSettings> Layer::prepareClientCompositionList(