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

Commit c6ad8afa authored by chaviw's avatar chaviw
Browse files

Remove rotation and use flag useIdentityTransform for screenshots. 

There's a lot of confusing logic where 90 and 270 rotation values need
to be flipped to ensure the screenshot is taken the correct orientation.
There's also confusion what useIdentityTransform means, especially if a
non 0 rotation value is sent.

The cases screenshot cares about is the following:
1. Take screenshot in current display orientation
2. Take screenshot with 0 rotation so the caller can handle rotating the
screenshot themselves.

With these two cases in mind, remove the rotation value passed in for
screenshots. If useIdentityTransform is true, it will rotate the
screenshot so it's in the 0 orientation. If useIdentityTransform is
false, it will use the current display rotation.

This simplifies the logic in DisplayRenderArea since it only needs to
compute the rotation when useIdentityTransform is set. It also
simplifies the caller logic since they no longer have to find the
current display rotation to ensure the screenshot is taken in the
current rotation. The callers can just request the screenshot with
useIdentityTransform set to false.

Test: adb shell screencap
Test: Power + volume screenshot
Test: Screen rotation
Test: SurfaceFlinger_test
Test: libsurfaceflinger_unittest
Fixes: 135942984
Change-Id: I1da025c7340a11a719d4630da2469b281bddc6e9
parent 63dd5e25

libs/gui/LayerState.cpp

+2 −8
Original line number Diff line number Diff line
@@ -530,23 +530,17 @@ status_t DisplayCaptureArgs::write(Parcel& output) const { 
    status |= output.writeStrongBinder(displayToken) ?:

        output.writeUint32(width) ?:

        output.writeUint32(height) ?:

        output.writeBool(useIdentityTransform) ?:

        output.writeInt32(static_cast<int32_t>(rotation));

        output.writeBool(useIdentityTransform);

    return status;

}



status_t DisplayCaptureArgs::read(const Parcel& input) {

    status_t status = CaptureArgs::read(input);



    int32_t rotationInt = 0;



    status |= input.readStrongBinder(&displayToken) ?:

        input.readUint32(&width) ?:

        input.readUint32(&height) ?:

        input.readBool(&useIdentityTransform) ?:

        input.readInt32(&rotationInt);



    rotation = ui::toRotation(rotationInt);

        input.readBool(&useIdentityTransform);

    return status;

}

libs/gui/include/gui/LayerState.h

+0 −1
Original line number Diff line number Diff line
@@ -330,7 +330,6 @@ struct DisplayCaptureArgs : CaptureArgs { 
    uint32_t width{0};

    uint32_t height{0};

    bool useIdentityTransform{false};

    ui::Rotation rotation{ui::ROTATION_0};



    status_t write(Parcel& output) const override;

    status_t read(const Parcel& input) override;

services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFE.h

+0 −6
Original line number Diff line number Diff line
@@ -80,10 +80,6 @@ public: 
        // The clip region, or visible region that is being rendered to

        const Region& clip;



        // If true, the layer should use an identity transform for its position

        // transform. Used only by the captureScreen API call.

        const bool useIdentityTransform;



        // If set to true, the layer should enable filtering when rendering.

        const bool needsFiltering;
@@ -148,7 +144,6 @@ using LayerFESet = std::unordered_set<sp<LayerFE>, LayerFESpHash>; 
static inline bool operator==(const LayerFE::ClientCompositionTargetSettings& lhs,

                              const LayerFE::ClientCompositionTargetSettings& rhs) {

    return lhs.clip.hasSameRects(rhs.clip) &&

            lhs.useIdentityTransform == rhs.useIdentityTransform &&

            lhs.needsFiltering == rhs.needsFiltering && lhs.isSecure == rhs.isSecure &&

            lhs.supportsProtectedContent == rhs.supportsProtectedContent &&

            lhs.clearRegion.hasSameRects(rhs.clearRegion) && lhs.viewport == rhs.viewport &&
@@ -170,7 +165,6 @@ static inline void PrintTo(const LayerFE::ClientCompositionTargetSettings& setti 
    *os << "ClientCompositionTargetSettings{";

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

    PrintTo(settings.clip, os);

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

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

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

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

services/surfaceflinger/CompositionEngine/src/Output.cpp

+0 −2
Original line number Diff line number Diff line
@@ -946,7 +946,6 @@ std::vector<LayerFE::LayerSettings> Output::generateClientCompositionRequests( 


    const auto& outputState = getState();

    const Region viewportRegion(outputState.viewport);

    const bool useIdentityTransform = false;

    bool firstLayer = true;

    // Used when a layer clears part of the buffer.

    Region stubRegion;
@@ -984,7 +983,6 @@ std::vector<LayerFE::LayerSettings> Output::generateClientCompositionRequests( 
        if (clientComposition || clearClientComposition) {

            compositionengine::LayerFE::ClientCompositionTargetSettings targetSettings{

                    clip,

                    useIdentityTransform,

                    layer->needsFiltering() || outputState.needsFiltering,

                    outputState.isSecure,

                    supportsProtectedContent,

services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp

+0 −21
Original line number Diff line number Diff line
@@ -3579,7 +3579,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, clearsHWCLayersIfOpaqu 


    compositionengine::LayerFE::ClientCompositionTargetSettings layer1TargetSettings{

            Region(kDisplayFrame),

            false,      /* identity transform */

            false,      /* needs filtering */

            false,      /* secure */

            false,      /* supports protected content */
@@ -3591,7 +3590,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, clearsHWCLayersIfOpaqu 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer2TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            false, /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3635,7 +3633,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 


    compositionengine::LayerFE::ClientCompositionTargetSettings layer0TargetSettings{

            Region(Rect(10, 10, 20, 20)),

            false, /* identity transform */

            false, /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3647,7 +3644,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer1TargetSettings{

            Region(Rect(0, 0, 30, 30)),

            false, /* identity transform */

            false, /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3659,7 +3655,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer2TargetSettings{

            Region(Rect(0, 0, 40, 201)),

            false, /* identity transform */

            false, /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3691,7 +3686,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 


    compositionengine::LayerFE::ClientCompositionTargetSettings layer0TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            true,  /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3703,7 +3697,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer1TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            false, /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3715,7 +3708,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer2TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            false, /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3747,7 +3739,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 


    compositionengine::LayerFE::ClientCompositionTargetSettings layer0TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            true,  /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3760,7 +3751,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer1TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            true,  /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3772,7 +3762,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer2TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            true,  /* needs filtering */

            false, /* secure */

            false, /* supports protected content */
@@ -3803,7 +3792,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 


    compositionengine::LayerFE::ClientCompositionTargetSettings layer0TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            false, /* needs filtering */

            true,  /* secure */

            false, /* supports protected content */
@@ -3815,7 +3803,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer1TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            false, /* needs filtering */

            true,  /* secure */

            false, /* supports protected content */
@@ -3827,7 +3814,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer2TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            false, /* needs filtering */

            true,  /* secure */

            false, /* supports protected content */
@@ -3856,7 +3842,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 


    compositionengine::LayerFE::ClientCompositionTargetSettings layer0TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            false, /* needs filtering */

            false, /* secure */

            true,  /* supports protected content */
@@ -3868,7 +3853,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer1TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            false, /* needs filtering */

            false, /* secure */

            true,  /* supports protected content */
@@ -3880,7 +3864,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    };

    compositionengine::LayerFE::ClientCompositionTargetSettings layer2TargetSettings{

            Region(kDisplayFrame),

            false, /* identity transform */

            false, /* needs filtering */

            false, /* secure */

            true,  /* supports protected content */
@@ -3972,7 +3955,6 @@ TEST_F(GenerateClientCompositionRequestsTest, handlesLandscapeModeSplitScreenReq 


    compositionengine::LayerFE::ClientCompositionTargetSettings leftLayerSettings{

            Region(Rect(0, 0, 1000, 1000)),

            false, /* identity transform */

            false, /* needs filtering */

            true,  /* secure */

            true,  /* supports protected content */
@@ -3990,7 +3972,6 @@ TEST_F(GenerateClientCompositionRequestsTest, handlesLandscapeModeSplitScreenReq 


    compositionengine::LayerFE::ClientCompositionTargetSettings rightLayerSettings{

            Region(Rect(1000, 0, 2000, 1000)),

            false, /* identity transform */

            false, /* needs filtering */

            true,  /* secure */

            true,  /* supports protected content */
@@ -4024,7 +4005,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    Region accumClearRegion(Rect(10, 11, 12, 13));

    compositionengine::LayerFE::ClientCompositionTargetSettings layer2Settings{

            Region(Rect(60, 40, 70, 80)).merge(Rect(40, 80, 70, 90)), /* visible region */

            false,                                                    /* identity transform */

            false,                                                    /* needs filtering */

            false,                                                    /* secure */

            false, /* supports protected content */
@@ -4070,7 +4050,6 @@ TEST_F(GenerateClientCompositionRequestsTest_ThreeLayers, 
    Region accumClearRegion(Rect(10, 11, 12, 13));

    compositionengine::LayerFE::ClientCompositionTargetSettings layer2Settings{

            Region(Rect(50, 40, 70, 80)).merge(Rect(40, 80, 70, 90)), /* visible region */

            false,                                                    /* identity transform */

            false,                                                    /* needs filtering */

            false,                                                    /* secure */

            false, /* supports protected content */