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

Commit 854ce1c4 authored by Vishnu Nair's avatar Vishnu Nair
Browse files

[sf] Set visible regions change flag when layer starts drawing 

If a layer state changes so it has a buffer or an effect like
color then, update the flags to indicate that visiblity of a
snapshot has changed. This will mark the visible regions
flag in CE args and force CE to rebuild its layer stack.
Without this flag, there was a scenario where CE
would get a null snapshot and crash.

Also crash early with traces if we ever pass a null snapshot to
CE.

Test: presubmit
Bug: 238781169, 295069311, 294889236
Change-Id: I355502fbe7f1be38c46a5ed25233b6b07c6b4eeb
parent 606d9d07

libs/gui/include/gui/LayerState.h

+1 −0
Original line number Diff line number Diff line
@@ -265,6 +265,7 @@ struct layer_state_t { 
    // Changes affecting child states.

    static constexpr uint64_t AFFECTS_CHILDREN = layer_state_t::GEOMETRY_CHANGES |

            layer_state_t::HIERARCHY_CHANGES | layer_state_t::eAlphaChanged |

            layer_state_t::eBackgroundBlurRadiusChanged | layer_state_t::eBlurRegionsChanged |

            layer_state_t::eColorTransformChanged | layer_state_t::eCornerRadiusChanged |

            layer_state_t::eFlagsChanged | layer_state_t::eTrustedOverlayChanged |

            layer_state_t::eFrameRateChanged | layer_state_t::eFrameRateSelectionPriority |

services/surfaceflinger/FrontEnd/RequestedLayerState.cpp

+11 −6
Original line number Diff line number Diff line
@@ -150,7 +150,7 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta 


    const bool hadSideStream = sidebandStream != nullptr;

    const layer_state_t& clientState = resolvedComposerState.state;

    const bool hadBlur = hasBlur();

    const bool hadSomethingToDraw = hasSomethingToDraw();

    uint64_t clientChanges = what | layer_state_t::diff(clientState);

    layer_state_t::merge(clientState);

    what = clientChanges;
@@ -228,12 +228,11 @@ void RequestedLayerState::merge(const ResolvedComposerState& resolvedComposerSta 
                    RequestedLayerState::Changes::VisibleRegion;

        }

    }

    if (what & (layer_state_t::eBackgroundBlurRadiusChanged | layer_state_t::eBlurRegionsChanged)) {

        if (hadBlur != hasBlur()) {



    if (hadSomethingToDraw != hasSomethingToDraw()) {

        changes |= RequestedLayerState::Changes::Visibility |

                RequestedLayerState::Changes::VisibleRegion;

    }

    }

    if (clientChanges & layer_state_t::HIERARCHY_CHANGES)

        changes |= RequestedLayerState::Changes::Hierarchy;

    if (clientChanges & layer_state_t::CONTENT_CHANGES)
@@ -579,6 +578,12 @@ bool RequestedLayerState::isProtected() const { 
    return externalTexture && externalTexture->getUsage() & GRALLOC_USAGE_PROTECTED;

}



bool RequestedLayerState::hasSomethingToDraw() const {

    return externalTexture != nullptr || sidebandStream != nullptr || shadowRadius > 0.f ||

            backgroundBlurRadius > 0 || blurRegions.size() > 0 ||

            (color.r >= 0.0_hf && color.g >= 0.0_hf && color.b >= 0.0_hf);

}



void RequestedLayerState::clearChanges() {

    what = 0;

    changes.clear();

services/surfaceflinger/FrontEnd/RequestedLayerState.h

+1 −0
Original line number Diff line number Diff line
@@ -87,6 +87,7 @@ struct RequestedLayerState : layer_state_t { 
    bool backpressureEnabled() const;

    bool isSimpleBufferUpdate(const layer_state_t&) const;

    bool isProtected() const;

    bool hasSomethingToDraw() const;



    // Layer serial number.  This gives layers an explicit ordering, so we

    // have a stable sort order when their layer stack and Z-order are

services/surfaceflinger/SurfaceFlinger.cpp

+18 −0
Original line number Diff line number Diff line
@@ -134,6 +134,7 @@ 
#include "FrontEnd/LayerCreationArgs.h"

#include "FrontEnd/LayerHandle.h"

#include "FrontEnd/LayerLifecycleManager.h"

#include "FrontEnd/LayerLog.h"

#include "FrontEnd/LayerSnapshot.h"

#include "HdrLayerInfoReporter.h"

#include "Layer.h"
@@ -2620,6 +2621,23 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( 
    constexpr bool kCursorOnly = false;

    const auto layers = moveSnapshotsToCompositionArgs(refreshArgs, kCursorOnly);



    if (mLayerLifecycleManagerEnabled && !refreshArgs.updatingGeometryThisFrame) {

        for (const auto& [token, display] : FTL_FAKE_GUARD(mStateLock, mDisplays)) {

            auto compositionDisplay = display->getCompositionDisplay();

            if (!compositionDisplay->getState().isEnabled) continue;

            for (auto outputLayer : compositionDisplay->getOutputLayersOrderedByZ()) {

                LLOG_ALWAYS_FATAL_WITH_TRACE_IF(outputLayer->getLayerFE().getCompositionState() ==

                                                        nullptr,

                                                "Output layer %s for display %s %" PRIu64

                                                " has a null "

                                                "snapshot.",

                                                outputLayer->getLayerFE().getDebugName(),

                                                compositionDisplay->getName().c_str(),

                                                compositionDisplay->getId().value);

            }

        }

    }



    mCompositionEngine->present(refreshArgs);

    moveSnapshotsFromCompositionArgs(refreshArgs, layers);

services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp

+48 −0
Original line number Diff line number Diff line
@@ -389,4 +389,52 @@ TEST_F(LayerLifecycleManagerTest, onParentDestroyDestroysBackgroundLayer) { 
    listener->expectLayersDestroyed({1, bgLayerId});

}



TEST_F(LayerLifecycleManagerTest, blurSetsVisibilityChangeFlag) {

    // clear default color on layer so we start with a layer that does not draw anything.

    setColor(1, {-1.f, -1.f, -1.f});

    mLifecycleManager.commitChanges();



    // layer has something to draw

    setBackgroundBlurRadius(1, 2);

    EXPECT_TRUE(

            mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));

    mLifecycleManager.commitChanges();



    // layer still has something to draw, so visibility shouldn't change

    setBackgroundBlurRadius(1, 3);

    EXPECT_FALSE(

            mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));

    mLifecycleManager.commitChanges();



    // layer has nothing to draw

    setBackgroundBlurRadius(1, 0);

    EXPECT_TRUE(

            mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));

    mLifecycleManager.commitChanges();

}



TEST_F(LayerLifecycleManagerTest, colorSetsVisibilityChangeFlag) {

    // clear default color on layer so we start with a layer that does not draw anything.

    setColor(1, {-1.f, -1.f, -1.f});

    mLifecycleManager.commitChanges();



    // layer has something to draw

    setColor(1, {2.f, 3.f, 4.f});

    EXPECT_TRUE(

            mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));

    mLifecycleManager.commitChanges();



    // layer still has something to draw, so visibility shouldn't change

    setColor(1, {0.f, 0.f, 0.f});

    EXPECT_FALSE(

            mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));

    mLifecycleManager.commitChanges();



    // layer has nothing to draw

    setColor(1, {-1.f, -1.f, -1.f});

    EXPECT_TRUE(

            mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Visibility));

    mLifecycleManager.commitChanges();

}



} // namespace android::surfaceflinger::frontend