Remove unnecessary drawing commands when drawing shadows.
When blurring behind a shadow, this will ensure that it
is only blurred once. It does so by updating
Layer::prepareShadowClientComposition to add ShadowSettings
to the input layer, rather than adding an extra layer.
Collapsing the shadow layer into the same layer as the caster will
result in undefined behavior in the old GLESRenderEngine.
Bug: 188050128
Bug: 175915334
Test: atest SurfaceFlinger_test and atest librenderengine_test
Change-Id: I5b74ef9659b519fcc6b89dfd873c35fc579fddd7
diff --git a/libs/renderengine/include/renderengine/LayerSettings.h b/libs/renderengine/include/renderengine/LayerSettings.h
index 63ed1ba..715f3f8 100644
--- a/libs/renderengine/include/renderengine/LayerSettings.h
+++ b/libs/renderengine/include/renderengine/LayerSettings.h
@@ -107,6 +107,9 @@
* material design guidelines.
*/
struct ShadowSettings {
+ // Boundaries of the shadow.
+ FloatRect boundaries = FloatRect();
+
// Color to the ambient shadow. The alpha is premultiplied.
vec4 ambientColor = vec4();
@@ -150,6 +153,10 @@
// True if blending will be forced to be disabled.
bool disableBlending = false;
+ // If true, then this layer casts a shadow and/or blurs behind it, but it does
+ // not otherwise draw any of the layer's other contents.
+ bool skipContentDraw = false;
+
ShadowSettings shadow;
int backgroundBlurRadius = 0;
@@ -189,9 +196,10 @@
}
static inline bool operator==(const ShadowSettings& lhs, const ShadowSettings& rhs) {
- return lhs.ambientColor == rhs.ambientColor && lhs.spotColor == rhs.spotColor &&
- lhs.lightPos == rhs.lightPos && lhs.lightRadius == rhs.lightRadius &&
- lhs.length == rhs.length && lhs.casterIsTranslucent == rhs.casterIsTranslucent;
+ return lhs.boundaries == rhs.boundaries && lhs.ambientColor == rhs.ambientColor &&
+ lhs.spotColor == rhs.spotColor && lhs.lightPos == rhs.lightPos &&
+ lhs.lightRadius == rhs.lightRadius && lhs.length == rhs.length &&
+ lhs.casterIsTranslucent == rhs.casterIsTranslucent;
}
static inline bool operator==(const LayerSettings& lhs, const LayerSettings& rhs) {
@@ -208,7 +216,8 @@
return lhs.geometry == rhs.geometry && lhs.source == rhs.source && lhs.alpha == rhs.alpha &&
lhs.sourceDataspace == rhs.sourceDataspace &&
lhs.colorTransform == rhs.colorTransform &&
- lhs.disableBlending == rhs.disableBlending && lhs.shadow == rhs.shadow &&
+ lhs.disableBlending == rhs.disableBlending &&
+ lhs.skipContentDraw == rhs.skipContentDraw && lhs.shadow == rhs.shadow &&
lhs.backgroundBlurRadius == rhs.backgroundBlurRadius &&
lhs.blurRegionTransform == rhs.blurRegionTransform &&
lhs.stretchEffect == rhs.stretchEffect;
@@ -251,6 +260,8 @@
static inline void PrintTo(const ShadowSettings& settings, ::std::ostream* os) {
*os << "ShadowSettings {";
+ *os << "\n .boundaries = ";
+ PrintTo(settings.boundaries, os);
*os << "\n .ambientColor = " << settings.ambientColor;
*os << "\n .spotColor = " << settings.spotColor;
*os << "\n .lightPos = " << settings.lightPos;
@@ -286,6 +297,7 @@
PrintTo(settings.sourceDataspace, os);
*os << "\n .colorTransform = " << settings.colorTransform;
*os << "\n .disableBlending = " << settings.disableBlending;
+ *os << "\n .skipContentDraw = " << settings.skipContentDraw;
*os << "\n .backgroundBlurRadius = " << settings.backgroundBlurRadius;
for (auto blurRegion : settings.blurRegions) {
*os << "\n";
diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.cpp b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
index 6cc3d37..6d749ed 100644
--- a/libs/renderengine/skia/SkiaGLRenderEngine.cpp
+++ b/libs/renderengine/skia/SkiaGLRenderEngine.cpp
@@ -846,7 +846,9 @@
// Layers have a local transform that should be applied to them
canvas->concat(getSkM44(layer->geometry.positionTransform).asM33());
- const auto [bounds, roundRectClip] = getBoundsAndClip(layer);
+ const auto [bounds, roundRectClip] =
+ getBoundsAndClip(layer->geometry.boundaries, layer->geometry.roundedCornersCrop,
+ layer->geometry.roundedCornersRadius);
if (mBlurFilter && layerHasBlur(layer)) {
std::unordered_map<uint32_t, sk_sp<SkImage>> cachedBlurs;
@@ -895,14 +897,21 @@
}
}
- // Shadows are assumed to live only on their own layer - it's not valid
- // to draw the boundary rectangles 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.
if (layer->shadow.length > 0) {
// This would require a new parameter/flag to SkShadowUtils::DrawShadow
LOG_ALWAYS_FATAL_IF(layer->disableBlending, "Cannot disableBlending with a shadow");
+ SkRRect shadowBounds, shadowClip;
+ if (layer->geometry.boundaries == layer->shadow.boundaries) {
+ shadowBounds = bounds;
+ shadowClip = roundRectClip;
+ } else {
+ std::tie(shadowBounds, shadowClip) =
+ getBoundsAndClip(layer->shadow.boundaries,
+ layer->geometry.roundedCornersCrop,
+ layer->geometry.roundedCornersRadius);
+ }
+
// Technically, if bounds is a rect and roundRectClip is not empty,
// it means that the bounds and roundedCornersCrop were different
// enough that we should intersect them to find the proper shadow.
@@ -910,9 +919,8 @@
// not match due to rounding errors. Draw the rounded version, which
// looks more like the intent.
const auto& rrect =
- bounds.isRect() && !roundRectClip.isEmpty() ? roundRectClip : bounds;
+ shadowBounds.isRect() && !shadowClip.isEmpty() ? shadowClip : shadowBounds;
drawShadow(canvas, rrect, layer->shadow);
- continue;
}
const bool requiresLinearEffect = layer->colorTransform != mat4() ||
@@ -922,8 +930,9 @@
display.sdrWhitePointNits != display.maxLuminance);
// quick abort from drawing the remaining portion of the layer
- if (layer->alpha == 0 && !requiresLinearEffect && !layer->disableBlending &&
- (!displayColorTransform || displayColorTransform->isAlphaUnchanged())) {
+ if (layer->skipContentDraw ||
+ (layer->alpha == 0 && !requiresLinearEffect && !layer->disableBlending &&
+ (!displayColorTransform || displayColorTransform->isAlphaUnchanged()))) {
continue;
}
@@ -1093,11 +1102,11 @@
return SkRect::MakeLTRB(rect.left, rect.top, rect.right, rect.bottom);
}
-inline std::pair<SkRRect, SkRRect> SkiaGLRenderEngine::getBoundsAndClip(
- const LayerSettings* layer) {
- const auto bounds = getSkRect(layer->geometry.boundaries);
- const auto crop = getSkRect(layer->geometry.roundedCornersCrop);
- const auto cornerRadius = layer->geometry.roundedCornersRadius;
+inline std::pair<SkRRect, SkRRect> SkiaGLRenderEngine::getBoundsAndClip(const FloatRect& boundsRect,
+ const FloatRect& cropRect,
+ const float cornerRadius) {
+ const SkRect bounds = getSkRect(boundsRect);
+ const SkRect crop = getSkRect(cropRect);
SkRRect clip;
if (cornerRadius > 0) {
diff --git a/libs/renderengine/skia/SkiaGLRenderEngine.h b/libs/renderengine/skia/SkiaGLRenderEngine.h
index 1f528b7..4265c08 100644
--- a/libs/renderengine/skia/SkiaGLRenderEngine.h
+++ b/libs/renderengine/skia/SkiaGLRenderEngine.h
@@ -88,7 +88,8 @@
int hwcFormat, Protection protection);
inline SkRect getSkRect(const FloatRect& layer);
inline SkRect getSkRect(const Rect& layer);
- inline std::pair<SkRRect, SkRRect> getBoundsAndClip(const LayerSettings* layer);
+ inline std::pair<SkRRect, SkRRect> getBoundsAndClip(const FloatRect& bounds,
+ const FloatRect& crop, float cornerRadius);
inline bool layerHasBlur(const LayerSettings* layer);
inline SkColor getSkColor(const vec4& color);
inline SkM44 getSkM44(const mat4& matrix);
diff --git a/libs/renderengine/tests/RenderEngineTest.cpp b/libs/renderengine/tests/RenderEngineTest.cpp
index 72e32ed..1ca7a16 100644
--- a/libs/renderengine/tests/RenderEngineTest.cpp
+++ b/libs/renderengine/tests/RenderEngineTest.cpp
@@ -1265,6 +1265,7 @@
renderengine::LayerSettings shadowLayer;
shadowLayer.sourceDataspace = ui::Dataspace::V0_SRGB_LINEAR;
shadowLayer.geometry.boundaries = castingBounds;
+ shadowLayer.skipContentDraw = true;
shadowLayer.alpha = 1.0f;
ColorSourceVariant::fillColor(shadowLayer, 0, 0, 0, this);
shadowLayer.shadow = shadow;
diff --git a/services/surfaceflinger/EffectLayer.cpp b/services/surfaceflinger/EffectLayer.cpp
index caef338..fd18c3b 100644
--- a/services/surfaceflinger/EffectLayer.cpp
+++ b/services/surfaceflinger/EffectLayer.cpp
@@ -57,19 +57,15 @@
return {};
}
- std::optional<compositionengine::LayerFE::LayerSettings> shadowSettings =
- prepareShadowClientComposition(*layerSettings, targetSettings.viewport,
- targetSettings.dataspace);
- if (shadowSettings) {
- results.push_back(*shadowSettings);
- }
+ // set the shadow for the layer if needed
+ prepareShadowClientComposition(*layerSettings, targetSettings.viewport);
// If fill bounds are occluded or the fill color is invalid skip the fill settings.
if (targetSettings.realContentIsVisible && fillsColor()) {
// Set color for color fill settings.
layerSettings->source.solidColor = getColor().rgb;
results.push_back(*layerSettings);
- } else if (hasBlur()) {
+ } else if (hasBlur() || drawShadows()) {
results.push_back(*layerSettings);
}
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 8b7dc4d..a7c8704 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -573,6 +573,9 @@
layerSettings.geometry.boundaries = bounds;
layerSettings.geometry.positionTransform = getTransform().asMatrix4();
+ // skip drawing content if the targetSettings indicate the content will be occluded
+ layerSettings.skipContentDraw = !targetSettings.realContentIsVisible;
+
if (hasColorTransform()) {
layerSettings.colorTransform = getColorTransform();
}
@@ -595,42 +598,6 @@
return layerSettings;
}
-std::optional<compositionengine::LayerFE::LayerSettings> Layer::prepareShadowClientComposition(
- const LayerFE::LayerSettings& casterLayerSettings, const Rect& layerStackRect,
- ui::Dataspace outputDataspace) {
- renderengine::ShadowSettings shadow = getShadowSettings(layerStackRect);
- if (shadow.length <= 0.f) {
- return {};
- }
-
- const float casterAlpha = casterLayerSettings.alpha;
- const bool casterIsOpaque = ((casterLayerSettings.source.buffer.buffer != nullptr) &&
- casterLayerSettings.source.buffer.isOpaque);
-
- compositionengine::LayerFE::LayerSettings shadowLayer = casterLayerSettings;
-
- shadowLayer.shadow = shadow;
- shadowLayer.geometry.boundaries = mBounds; // ignore transparent region
-
- // If the casting layer is translucent, we need to fill in the shadow underneath the layer.
- // Otherwise the generated shadow will only be shown around the casting layer.
- shadowLayer.shadow.casterIsTranslucent = !casterIsOpaque || (casterAlpha < 1.0f);
- shadowLayer.shadow.ambientColor *= casterAlpha;
- shadowLayer.shadow.spotColor *= casterAlpha;
- shadowLayer.sourceDataspace = outputDataspace;
- shadowLayer.source.buffer.buffer = nullptr;
- 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 {};
- }
-
- return shadowLayer;
-}
-
void Layer::prepareClearClientComposition(LayerFE::LayerSettings& layerSettings,
bool blackout) const {
layerSettings.source.buffer.buffer = nullptr;
@@ -644,6 +611,9 @@
layerSettings.name = getName();
}
+// TODO(b/188891810): This method now only ever returns 0 or 1 layers so we should return
+// std::optional instead of a vector. Additionally, we should consider removing
+// this method entirely in favor of calling prepareClientComposition directly.
std::vector<compositionengine::LayerFE::LayerSettings> Layer::prepareClientCompositionList(
compositionengine::LayerFE::ClientCompositionTargetSettings& targetSettings) {
std::optional<compositionengine::LayerFE::LayerSettings> layerSettings =
@@ -659,21 +629,10 @@
return {*layerSettings};
}
- std::optional<compositionengine::LayerFE::LayerSettings> shadowSettings =
- prepareShadowClientComposition(*layerSettings, targetSettings.viewport,
- targetSettings.dataspace);
- // There are no shadows to render.
- if (!shadowSettings) {
- return {*layerSettings};
- }
+ // set the shadow for the layer if needed
+ prepareShadowClientComposition(*layerSettings, targetSettings.viewport);
- // If the layer casts a shadow but the content casting the shadow is occluded, skip
- // composing the non-shadow content and only draw the shadows.
- if (targetSettings.realContentIsVisible) {
- return {*shadowSettings, *layerSettings};
- }
-
- return {*shadowSettings};
+ return {*layerSettings};
}
Hwc2::IComposerClient::Composition Layer::getCompositionType(const DisplayDevice& display) const {
@@ -2061,16 +2020,37 @@
: RoundedCornerState();
}
-renderengine::ShadowSettings Layer::getShadowSettings(const Rect& layerStackRect) const {
+void Layer::prepareShadowClientComposition(LayerFE::LayerSettings& caster,
+ const Rect& layerStackRect) {
renderengine::ShadowSettings state = mFlinger->mDrawingState.globalShadowSettings;
+ // Note: this preserves existing behavior of shadowing the entire layer and not cropping it if
+ // transparent regions are present. This may not be necessary since shadows are only cast by
+ // SurfaceFlinger's EffectLayers, which do not typically use transparent regions.
+ state.boundaries = mBounds;
+
// Shift the spot light x-position to the middle of the display and then
// offset it by casting layer's screen pos.
state.lightPos.x = (layerStackRect.width() / 2.f) - mScreenBounds.left;
state.lightPos.y -= mScreenBounds.top;
state.length = mEffectiveShadowRadius;
- return state;
+
+ if (state.length > 0.f) {
+ const float casterAlpha = caster.alpha;
+ const bool casterIsOpaque =
+ ((caster.source.buffer.buffer != nullptr) && caster.source.buffer.isOpaque);
+
+ // If the casting layer is translucent, we need to fill in the shadow underneath the layer.
+ // Otherwise the generated shadow will only be shown around the casting layer.
+ state.casterIsTranslucent = !casterIsOpaque || (casterAlpha < 1.0f);
+ state.ambientColor *= casterAlpha;
+ state.spotColor *= casterAlpha;
+
+ if (state.ambientColor.a > 0.f && state.spotColor.a > 0.f) {
+ caster.shadow = state;
+ }
+ }
}
void Layer::commitChildList() {
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 8139d8a..66d7018 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -764,8 +764,6 @@
// is ready to acquire a buffer.
ui::Transform::RotationFlags getFixedTransformHint() const;
- renderengine::ShadowSettings getShadowSettings(const Rect& layerStackRect) const;
-
/**
* Traverse this layer and it's hierarchy of children directly. Unlike traverseInZOrder
* which will not emit children who have relativeZOrder to another layer, this method
@@ -895,9 +893,6 @@
virtual void setInitialValuesForClone(const sp<Layer>& clonedFrom);
virtual std::optional<compositionengine::LayerFE::LayerSettings> prepareClientComposition(
compositionengine::LayerFE::ClientCompositionTargetSettings&);
- virtual std::optional<compositionengine::LayerFE::LayerSettings> prepareShadowClientComposition(
- const LayerFE::LayerSettings&, const Rect& layerStackRect,
- ui::Dataspace outputDataspace);
virtual void preparePerFrameCompositionState();
virtual void commitTransaction(State& stateToCommit);
virtual uint32_t doTransactionResize(uint32_t flags, Layer::State* stateToCommit);
@@ -923,6 +918,7 @@
// Modifies the passed in layer settings to clear the contents. If the blackout flag is set,
// the settings clears the content with a solid black fill.
void prepareClearClientComposition(LayerFE::LayerSettings&, bool blackout) const;
+ void prepareShadowClientComposition(LayerFE::LayerSettings& caster, const Rect& layerStackRect);
void prepareBasicGeometryCompositionState();
void prepareGeometryCompositionState();