Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c31985e5 authored by Derek Sollenberger's avatar Derek Sollenberger
Browse files

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
parent d01b8e9a

libs/renderengine/include/renderengine/LayerSettings.h

+16 −4
Original line number Original line Diff line number Diff line
@@ -107,6 +107,9 @@ struct PixelSource { 
 * material design guidelines.

 * material design guidelines.

 */

 */

struct ShadowSettings {

struct ShadowSettings {

    // Boundaries of the shadow.

    FloatRect boundaries = FloatRect();



    // Color to the ambient shadow. The alpha is premultiplied.

    // Color to the ambient shadow. The alpha is premultiplied.

    vec4 ambientColor = vec4();

    vec4 ambientColor = vec4();
@@ -150,6 +153,10 @@ struct LayerSettings { 
    // True if blending will be forced to be disabled.

    // True if blending will be forced to be disabled.

    bool disableBlending = false;

    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;

    ShadowSettings shadow;





    int backgroundBlurRadius = 0;

    int backgroundBlurRadius = 0;
@@ -189,9 +196,10 @@ static inline bool operator==(const PixelSource& lhs, const PixelSource& rhs) { 
}

}





static inline bool operator==(const ShadowSettings& lhs, const ShadowSettings& rhs) {

static inline bool operator==(const ShadowSettings& lhs, const ShadowSettings& rhs) {

    return lhs.ambientColor == rhs.ambientColor && lhs.spotColor == rhs.spotColor &&

    return lhs.boundaries == rhs.boundaries && lhs.ambientColor == rhs.ambientColor &&

            lhs.lightPos == rhs.lightPos && lhs.lightRadius == rhs.lightRadius &&

            lhs.spotColor == rhs.spotColor && lhs.lightPos == rhs.lightPos &&

            lhs.length == rhs.length && lhs.casterIsTranslucent == rhs.casterIsTranslucent;

            lhs.lightRadius == rhs.lightRadius && lhs.length == rhs.length &&

            lhs.casterIsTranslucent == rhs.casterIsTranslucent;

}

}





static inline bool operator==(const LayerSettings& lhs, const LayerSettings& rhs) {

static inline bool operator==(const LayerSettings& lhs, const LayerSettings& rhs) {
@@ -208,7 +216,8 @@ static inline bool operator==(const LayerSettings& lhs, const LayerSettings& rhs 
    return lhs.geometry == rhs.geometry && lhs.source == rhs.source && lhs.alpha == rhs.alpha &&

    return lhs.geometry == rhs.geometry && lhs.source == rhs.source && lhs.alpha == rhs.alpha &&

            lhs.sourceDataspace == rhs.sourceDataspace &&

            lhs.sourceDataspace == rhs.sourceDataspace &&

            lhs.colorTransform == rhs.colorTransform &&

            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.backgroundBlurRadius == rhs.backgroundBlurRadius &&

            lhs.blurRegionTransform == rhs.blurRegionTransform &&

            lhs.blurRegionTransform == rhs.blurRegionTransform &&

            lhs.stretchEffect == rhs.stretchEffect;

            lhs.stretchEffect == rhs.stretchEffect;
@@ -251,6 +260,8 @@ static inline void PrintTo(const PixelSource& settings, ::std::ostream* os) { 




static inline void PrintTo(const ShadowSettings& settings, ::std::ostream* os) {

static inline void PrintTo(const ShadowSettings& settings, ::std::ostream* os) {

    *os << "ShadowSettings {";

    *os << "ShadowSettings {";

    *os << "\n    .boundaries = ";

    PrintTo(settings.boundaries, os);

    *os << "\n    .ambientColor = " << settings.ambientColor;

    *os << "\n    .ambientColor = " << settings.ambientColor;

    *os << "\n    .spotColor = " << settings.spotColor;

    *os << "\n    .spotColor = " << settings.spotColor;

    *os << "\n    .lightPos = " << settings.lightPos;

    *os << "\n    .lightPos = " << settings.lightPos;
@@ -286,6 +297,7 @@ static inline void PrintTo(const LayerSettings& settings, ::std::ostream* os) { 
    PrintTo(settings.sourceDataspace, os);

    PrintTo(settings.sourceDataspace, os);

    *os << "\n    .colorTransform = " << settings.colorTransform;

    *os << "\n    .colorTransform = " << settings.colorTransform;

    *os << "\n    .disableBlending = " << settings.disableBlending;

    *os << "\n    .disableBlending = " << settings.disableBlending;

    *os << "\n    .skipContentDraw = " << settings.skipContentDraw;

    *os << "\n    .backgroundBlurRadius = " << settings.backgroundBlurRadius;

    *os << "\n    .backgroundBlurRadius = " << settings.backgroundBlurRadius;

    for (auto blurRegion : settings.blurRegions) {

    for (auto blurRegion : settings.blurRegions) {

        *os << "\n";

        *os << "\n";

libs/renderengine/skia/SkiaGLRenderEngine.cpp

+23 −14
Original line number Original line Diff line number Diff line
@@ -846,7 +846,9 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
        // Layers have a local transform that should be applied to them

        // Layers have a local transform that should be applied to them

        canvas->concat(getSkM44(layer->geometry.positionTransform).asM33());

        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)) {

        if (mBlurFilter && layerHasBlur(layer)) {

            std::unordered_map<uint32_t, sk_sp<SkImage>> cachedBlurs;

            std::unordered_map<uint32_t, sk_sp<SkImage>> cachedBlurs;
@@ -895,14 +897,21 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
            }

            }

        }

        }





        // 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) {

        if (layer->shadow.length > 0) {

            // This would require a new parameter/flag to SkShadowUtils::DrawShadow

            // This would require a new parameter/flag to SkShadowUtils::DrawShadow

            LOG_ALWAYS_FATAL_IF(layer->disableBlending, "Cannot disableBlending with a shadow");

            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,

            // Technically, if bounds is a rect and roundRectClip is not empty,

            // it means that the bounds and roundedCornersCrop were different

            // it means that the bounds and roundedCornersCrop were different

            // enough that we should intersect them to find the proper shadow.

            // enough that we should intersect them to find the proper shadow.
@@ -910,9 +919,8 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
            // not match due to rounding errors. Draw the rounded version, which

            // not match due to rounding errors. Draw the rounded version, which

            // looks more like the intent.

            // looks more like the intent.

            const auto& rrect =

            const auto& rrect =

                    bounds.isRect() && !roundRectClip.isEmpty() ? roundRectClip : bounds;

                    shadowBounds.isRect() && !shadowClip.isEmpty() ? shadowClip : shadowBounds;

            drawShadow(canvas, rrect, layer->shadow);

            drawShadow(canvas, rrect, layer->shadow);

            continue;

        }

        }





        const bool requiresLinearEffect = layer->colorTransform != mat4() ||

        const bool requiresLinearEffect = layer->colorTransform != mat4() ||
@@ -922,8 +930,9 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
                 display.sdrWhitePointNits != display.maxLuminance);

                 display.sdrWhitePointNits != display.maxLuminance);





        // quick abort from drawing the remaining portion of the layer

        // quick abort from drawing the remaining portion of the layer

        if (layer->alpha == 0 && !requiresLinearEffect && !layer->disableBlending &&

        if (layer->skipContentDraw ||

            (!displayColorTransform || displayColorTransform->isAlphaUnchanged())) {

            (layer->alpha == 0 && !requiresLinearEffect && !layer->disableBlending &&

             (!displayColorTransform || displayColorTransform->isAlphaUnchanged()))) {

            continue;

            continue;

        }

        }
@@ -1093,11 +1102,11 @@ inline SkRect SkiaGLRenderEngine::getSkRect(const Rect& rect) { 
    return SkRect::MakeLTRB(rect.left, rect.top, rect.right, rect.bottom);

    return SkRect::MakeLTRB(rect.left, rect.top, rect.right, rect.bottom);

}

}





inline std::pair<SkRRect, SkRRect> SkiaGLRenderEngine::getBoundsAndClip(

inline std::pair<SkRRect, SkRRect> SkiaGLRenderEngine::getBoundsAndClip(const FloatRect& boundsRect,

        const LayerSettings* layer) {

                                                                        const FloatRect& cropRect,

    const auto bounds = getSkRect(layer->geometry.boundaries);

                                                                        const float cornerRadius) {

    const auto crop = getSkRect(layer->geometry.roundedCornersCrop);

    const SkRect bounds = getSkRect(boundsRect);

    const auto cornerRadius = layer->geometry.roundedCornersRadius;

    const SkRect crop = getSkRect(cropRect);





    SkRRect clip;

    SkRRect clip;

    if (cornerRadius > 0) {

    if (cornerRadius > 0) {

libs/renderengine/skia/SkiaGLRenderEngine.h

+2 −1
Original line number Original line Diff line number Diff line
@@ -88,7 +88,8 @@ private: 
                                                         int hwcFormat, Protection protection);

                                                         int hwcFormat, Protection protection);

    inline SkRect getSkRect(const FloatRect& layer);

    inline SkRect getSkRect(const FloatRect& layer);

    inline SkRect getSkRect(const Rect& 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 bool layerHasBlur(const LayerSettings* layer);

    inline SkColor getSkColor(const vec4& color);

    inline SkColor getSkColor(const vec4& color);

    inline SkM44 getSkM44(const mat4& matrix);

    inline SkM44 getSkM44(const mat4& matrix);

libs/renderengine/tests/RenderEngineTest.cpp

+1 −0
Original line number Original line Diff line number Diff line
@@ -1265,6 +1265,7 @@ void RenderEngineTest::drawShadowWithoutCaster(const FloatRect& castingBounds, 
    renderengine::LayerSettings shadowLayer;

    renderengine::LayerSettings shadowLayer;

    shadowLayer.sourceDataspace = ui::Dataspace::V0_SRGB_LINEAR;

    shadowLayer.sourceDataspace = ui::Dataspace::V0_SRGB_LINEAR;

    shadowLayer.geometry.boundaries = castingBounds;

    shadowLayer.geometry.boundaries = castingBounds;

    shadowLayer.skipContentDraw = true;

    shadowLayer.alpha = 1.0f;

    shadowLayer.alpha = 1.0f;

    ColorSourceVariant::fillColor(shadowLayer, 0, 0, 0, this);

    ColorSourceVariant::fillColor(shadowLayer, 0, 0, 0, this);

    shadowLayer.shadow = shadow;

    shadowLayer.shadow = shadow;

services/surfaceflinger/EffectLayer.cpp

+3 −7
Original line number Original line Diff line number Diff line
@@ -57,19 +57,15 @@ std::vector<compositionengine::LayerFE::LayerSettings> EffectLayer::prepareClien 
        return {};

        return {};

    }

    }





    std::optional<compositionengine::LayerFE::LayerSettings> shadowSettings =

    // set the shadow for the layer if needed

            prepareShadowClientComposition(*layerSettings, targetSettings.viewport,

    prepareShadowClientComposition(*layerSettings, targetSettings.viewport);

                                           targetSettings.dataspace);

    if (shadowSettings) {

        results.push_back(*shadowSettings);

    }





    // If fill bounds are occluded or the fill color is invalid skip the fill settings.

    // If fill bounds are occluded or the fill color is invalid skip the fill settings.

    if (targetSettings.realContentIsVisible && fillsColor()) {

    if (targetSettings.realContentIsVisible && fillsColor()) {

        // Set color for color fill settings.

        // Set color for color fill settings.

        layerSettings->source.solidColor = getColor().rgb;

        layerSettings->source.solidColor = getColor().rgb;

        results.push_back(*layerSettings);

        results.push_back(*layerSettings);

    } else if (hasBlur()) {

    } else if (hasBlur() || drawShadows()) {

        results.push_back(*layerSettings);

        results.push_back(*layerSettings);

    }

    }