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

Commit bd17b3bf authored by Alec Mouri's avatar Alec Mouri
Browse files

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

libs/renderengine/skia/SkiaGLRenderEngine.cpp

+10 −6
Original line number Diff line number Diff line
@@ -673,13 +673,17 @@ status_t SkiaGLRenderEngine::drawLayers(const DisplaySettings& display, 
                    ? 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);

        }

        canvas->restore();

    }

    canvas->restore();

libs/renderengine/tests/RenderEngineTest.cpp

+71 −0
Original line number Diff line number Diff line
@@ -305,6 +305,26 @@ public: 
                          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 @@ public: 
                    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 @@ void RenderEngineTest::drawShadow(const renderengine::LayerSettings& castingLaye 
    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 @@ TEST_P(RenderEngineTest, cacheExternalBuffer_cachesImages) { 
    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();

services/surfaceflinger/Layer.cpp

+2 −0
Original line number Diff line number Diff line
@@ -688,6 +688,7 @@ std::optional<compositionengine::LayerFE::LayerSettings> Layer::prepareShadowCli 
    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 @@ void Layer::prepareClearClientComposition(LayerFE::LayerSettings& layerSettings, 


    // 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(