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

Commit e0c7b82a authored by chaviw's avatar chaviw
Browse files

Place mirrored layers offscreen instead of at root. 

Mirrored Layers should never be placed at the root of the layer
hierarchy. Instead, they should be created offscreen and allow the
caller to place the layer where they want in the hierarchy. This
behavior was changed with the introduction of LayerCreatedState.

Send placeAtRoot flag when creating a layer to allow different create
callpoints to specify whether they want the layer created to be placed
at the root.

Test: Window Magnification no longer flickers.
Test: MirrorLayerTest
Fixes: 192536474
Change-Id: I08f47b1b1f19b7c655c3687cb4c1fa64844e2cc5
parent b9d529ba

services/surfaceflinger/SurfaceFlinger.cpp

+10 −10
Original line number Original line Diff line number Diff line
@@ -3382,7 +3382,7 @@ void SurfaceFlinger::invalidateHwcGeometry() { 
status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,

status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,

                                        const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc,

                                        const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc,

                                        const sp<IBinder>& parentHandle,

                                        const sp<IBinder>& parentHandle,

                                        const sp<Layer>& parentLayer, bool addToCurrentState,

                                        const sp<Layer>& parentLayer, bool addToRoot,

                                        uint32_t* outTransformHint) {

                                        uint32_t* outTransformHint) {

    if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) {

    if (mNumLayers >= ISurfaceComposer::MAX_LAYERS) {

        ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(),

        ALOGE("AddClientLayer failed, mNumLayers (%zu) >= MAX_LAYERS (%zu)", mNumLayers.load(),
@@ -3394,7 +3394,7 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, const sp<IBind 
    if (gbc != nullptr) {

    if (gbc != nullptr) {

        initialProducer = IInterface::asBinder(gbc);

        initialProducer = IInterface::asBinder(gbc);

    }

    }

    setLayerCreatedState(handle, lbc, parentHandle, parentLayer, initialProducer);

    setLayerCreatedState(handle, lbc, parentHandle, parentLayer, initialProducer, addToRoot);





    // Create a transaction includes the initial parent and producer.

    // Create a transaction includes the initial parent and producer.

    Vector<ComposerState> states;

    Vector<ComposerState> states;
@@ -3917,7 +3917,7 @@ uint32_t SurfaceFlinger::setClientStateLocked( 
    sp<Layer> layer = nullptr;

    sp<Layer> layer = nullptr;

    if (s.surface) {

    if (s.surface) {

        if (what & layer_state_t::eLayerCreated) {

        if (what & layer_state_t::eLayerCreated) {

            layer = handleLayerCreatedLocked(s.surface, privileged);

            layer = handleLayerCreatedLocked(s.surface);

            if (layer) {

            if (layer) {

                // put the created layer into mLayersByLocalBinderToken.

                // put the created layer into mLayersByLocalBinderToken.

                mLayersByLocalBinderToken.emplace(s.surface->localBinder(), layer);

                mLayersByLocalBinderToken.emplace(s.surface->localBinder(), layer);
@@ -4327,9 +4327,9 @@ status_t SurfaceFlinger::createLayer(const String8& name, const sp<Client>& clie 
        return result;

        return result;

    }

    }





    bool addToCurrentState = callingThreadHasUnscopedSurfaceFlingerAccess();

    bool addToRoot = callingThreadHasUnscopedSurfaceFlingerAccess();

    result = addClientLayer(client, *handle, *gbp, layer, parentHandle, parentLayer,

    result = addClientLayer(client, *handle, *gbp, layer, parentHandle, parentLayer, addToRoot,

                            addToCurrentState, outTransformHint);

                            outTransformHint);

    if (result != NO_ERROR) {

    if (result != NO_ERROR) {

        return result;

        return result;

    }

    }
@@ -6872,10 +6872,10 @@ void SurfaceFlinger::TransactionState::traverseStatesWithBuffers( 




void SurfaceFlinger::setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer,

void SurfaceFlinger::setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer,

                                          const wp<IBinder>& parent, const wp<Layer> parentLayer,

                                          const wp<IBinder>& parent, const wp<Layer> parentLayer,

                                          const wp<IBinder>& producer) {

                                          const wp<IBinder>& producer, bool addToRoot) {

    Mutex::Autolock lock(mCreatedLayersLock);

    Mutex::Autolock lock(mCreatedLayersLock);

    mCreatedLayers[handle->localBinder()] =

    mCreatedLayers[handle->localBinder()] =

            std::make_unique<LayerCreatedState>(layer, parent, parentLayer, producer);

            std::make_unique<LayerCreatedState>(layer, parent, parentLayer, producer, addToRoot);

}

}





auto SurfaceFlinger::getLayerCreatedState(const sp<IBinder>& handle) {

auto SurfaceFlinger::getLayerCreatedState(const sp<IBinder>& handle) {
@@ -6900,7 +6900,7 @@ auto SurfaceFlinger::getLayerCreatedState(const sp<IBinder>& handle) { 
    return state;

    return state;

}

}





sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle, bool privileged) {

sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle) {

    const auto& state = getLayerCreatedState(handle);

    const auto& state = getLayerCreatedState(handle);

    if (!state) {

    if (!state) {

        return nullptr;

        return nullptr;
@@ -6913,7 +6913,7 @@ sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle, bo 
    }

    }





    sp<Layer> parent;

    sp<Layer> parent;

    bool allowAddRoot = privileged;

    bool allowAddRoot = state->addToRoot;

    if (state->initialParent != nullptr) {

    if (state->initialParent != nullptr) {

        parent = fromHandleLocked(state->initialParent.promote()).promote();

        parent = fromHandleLocked(state->initialParent.promote()).promote();

        if (parent == nullptr) {

        if (parent == nullptr) {

services/surfaceflinger/SurfaceFlinger.h

+10 −6
Original line number Original line Diff line number Diff line
@@ -905,7 +905,7 @@ private: 
    status_t addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,

    status_t addClientLayer(const sp<Client>& client, const sp<IBinder>& handle,

                            const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc,

                            const sp<IGraphicBufferProducer>& gbc, const sp<Layer>& lbc,

                            const sp<IBinder>& parentHandle, const sp<Layer>& parentLayer,

                            const sp<IBinder>& parentHandle, const sp<Layer>& parentLayer,

                            bool addToCurrentState, uint32_t* outTransformHint);

                            bool addToRoot, uint32_t* outTransformHint);





    // Traverse through all the layers and compute and cache its bounds.

    // Traverse through all the layers and compute and cache its bounds.

    void computeLayerBounds();

    void computeLayerBounds();
@@ -1454,11 +1454,12 @@ private: 
    mutable Mutex mCreatedLayersLock;

    mutable Mutex mCreatedLayersLock;

    struct LayerCreatedState {

    struct LayerCreatedState {

        LayerCreatedState(const wp<Layer>& layer, const wp<IBinder>& parent,

        LayerCreatedState(const wp<Layer>& layer, const wp<IBinder>& parent,

                          const wp<Layer> parentLayer, const wp<IBinder>& producer)

                          const wp<Layer> parentLayer, const wp<IBinder>& producer, bool addToRoot)

              : layer(layer),

              : layer(layer),

                initialParent(parent),

                initialParent(parent),

                initialParentLayer(parentLayer),

                initialParentLayer(parentLayer),

                initialProducer(producer) {}

                initialProducer(producer),

                addToRoot(addToRoot) {}

        wp<Layer> layer;

        wp<Layer> layer;

        // Indicates the initial parent of the created layer, only used for creating layer in

        // Indicates the initial parent of the created layer, only used for creating layer in

        // SurfaceFlinger. If nullptr, it may add the created layer into the current root layers.

        // SurfaceFlinger. If nullptr, it may add the created layer into the current root layers.
@@ -1467,6 +1468,10 @@ private: 
        // Indicates the initial graphic buffer producer of the created layer, only used for

        // Indicates the initial graphic buffer producer of the created layer, only used for

        // creating layer in SurfaceFlinger.

        // creating layer in SurfaceFlinger.

        wp<IBinder> initialProducer;

        wp<IBinder> initialProducer;

        // Indicates whether the layer getting created should be added at root if there's no parent

        // and has permission ACCESS_SURFACE_FLINGER. If set to false and no parent, the layer will

        // be added offscreen.

        bool addToRoot;

    };

    };





    // A temporay pool that store the created layers and will be added to current state in main

    // A temporay pool that store the created layers and will be added to current state in main
@@ -1474,10 +1479,9 @@ private: 
    std::unordered_map<BBinder*, std::unique_ptr<LayerCreatedState>> mCreatedLayers;

    std::unordered_map<BBinder*, std::unique_ptr<LayerCreatedState>> mCreatedLayers;

    void setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer,

    void setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer,

                              const wp<IBinder>& parent, const wp<Layer> parentLayer,

                              const wp<IBinder>& parent, const wp<Layer> parentLayer,

                              const wp<IBinder>& producer);

                              const wp<IBinder>& producer, bool addToRoot);

    auto getLayerCreatedState(const sp<IBinder>& handle);

    auto getLayerCreatedState(const sp<IBinder>& handle);

    sp<Layer> handleLayerCreatedLocked(const sp<IBinder>& handle, bool privileged)

    sp<Layer> handleLayerCreatedLocked(const sp<IBinder>& handle) REQUIRES(mStateLock);

            REQUIRES(mStateLock);





    std::atomic<ui::Transform::RotationFlags> mDefaultDisplayTransformHint;

    std::atomic<ui::Transform::RotationFlags> mDefaultDisplayTransformHint;

services/surfaceflinger/tests/MirrorLayer_test.cpp

+46 −0
Original line number Original line Diff line number Diff line
@@ -18,7 +18,9 @@ 
#pragma clang diagnostic push

#pragma clang diagnostic push

#pragma clang diagnostic ignored "-Wconversion"

#pragma clang diagnostic ignored "-Wconversion"





#include <private/android_filesystem_config.h>

#include "LayerTransactionTest.h"

#include "LayerTransactionTest.h"

#include "utils/TransactionUtils.h"





namespace android {

namespace android {
@@ -227,6 +229,50 @@ TEST_F(MirrorLayerTest, MirrorBufferLayer) { 
    }

    }

}

}





// Test that the mirror layer is initially offscreen.

TEST_F(MirrorLayerTest, InitialMirrorState) {

    const auto display = SurfaceComposerClient::getInternalDisplayToken();

    ui::DisplayMode mode;

    SurfaceComposerClient::getActiveDisplayMode(display, &mode);

    const ui::Size& size = mode.resolution;



    sp<SurfaceControl> mirrorLayer = nullptr;

    {

        // Run as system to get the ACCESS_SURFACE_FLINGER permission when mirroring

        UIDFaker f(AID_SYSTEM);

        // Mirror mChildLayer

        mirrorLayer = mClient->mirrorSurface(mChildLayer.get());

        ASSERT_NE(mirrorLayer, nullptr);

    }



    // Show the mirror layer, but don't reparent to a layer on screen.

    Transaction()

            .setPosition(mirrorLayer, 500, 500)

            .show(mirrorLayer)

            .setLayer(mirrorLayer, INT32_MAX - 1)

            .apply();



    {

        SCOPED_TRACE("Offscreen Mirror");

        auto shot = screenshot();

        shot->expectColor(Rect(0, 0, size.getWidth(), 50), Color::RED);

        shot->expectColor(Rect(0, 0, 50, size.getHeight()), Color::RED);

        shot->expectColor(Rect(450, 0, size.getWidth(), size.getHeight()), Color::RED);

        shot->expectColor(Rect(0, 450, size.getWidth(), size.getHeight()), Color::RED);

        shot->expectColor(Rect(50, 50, 450, 450), Color::GREEN);

    }



    // Add mirrorLayer as child of mParentLayer so it's shown on the display

    Transaction().reparent(mirrorLayer, mParentLayer).apply();



    {

        SCOPED_TRACE("On Screen Mirror");

        auto shot = screenshot();

        // Child mirror

        shot->expectColor(Rect(550, 550, 950, 950), Color::GREEN);

    }

}



} // namespace android

} // namespace android





// TODO(b/129481165): remove the #pragma below and fix conversion issues

// TODO(b/129481165): remove the #pragma below and fix conversion issues