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

Commit 5a6d857f authored by Alec Mouri's avatar Alec Mouri
Browse files

Fix filtering for screenshotting custom displays. 

Previously, filtering was computed for screenshots by checking if
filtering was required for the DisplayDevice itself. That's correct for
rendering custom display sizes since we need to upsample to the
physical panel, but it's wrong for screenshotting to an output buffer
where the logical transform does not involve a scale. Instead, we want
to compare the source crop with the destination buffer size when
determining whether we need to apply linear filtering.

As part of this, some of the rendering math is now simpler. Rather than
setting DisplaySettings::physicalDisplay and DisplaySettings::clip to be
the same and explicitly computing the transform matrix in RenderEngine,
we instead set DisplaySettings::clip to be the logical viewport. Then
the global transform is implicitly applied as part of mapping the output
in glViewport. This is because if the logical display is smaller than
the screen size, with the previous behavior applying the global transform
will scale up the layer stack to the physical display size. But if we
want pixel-accurate screenshots we will need to downsample back down to
the logical display size, which may cause errors. The math simplification
will avoid that scenario entirely.

This also means that screenshot code needs to be adjusted - some callers
pass in a cropping rectangle that assumes the display screenshot is
captured in portrait orientation. Other callers pass in a cropping
rectangle relative to a particular layer's coordinate space. For each of
those paths, the cropping rectangle must be transformed to the logical
display space.

This has also discovered a bug in setting the background fill of the
screenshot, which has now been fixed.

Bug: 129101431
Test: adb shell screepcap
Test: Modify wm size and density, and run
SurfaceViewTests#testMovingWhiteSurfaceView
Test: Capture screenshots through vysor
Test: Capture screenshots on Pixel 3XL with notch hide
Test: Physically capture screenshots
Test: Display rotations
Test: SurfaceFlinger_test
Test: librenderengine_test
Test: libsurfaceflinger_unittest
Test: libcompositionengine_test

Change-Id: I7f4e98d4c5aa53a995fd7da70078a2e5ea43b14f
parent dd543193

libs/renderengine/gl/GLESRenderEngine.cpp

+3 −2
Original line number Diff line number Diff line
@@ -795,6 +795,7 @@ void GLESRenderEngine::handleRoundedCorners(const DisplaySettings& display, 


    // Firstly, we need to convert the coordination from layer native coordination space to

    // device coordination space.

    // TODO(143929254): Verify that this transformation is correct

    const auto transformMatrix = display.globalTransform * layer.geometry.positionTransform;

    const vec4 leftTopCoordinate(bounds.left, bounds.top, 1.0, 1.0);

    const vec4 rightBottomCoordinate(bounds.right, bounds.bottom, 1.0, 1.0);
@@ -1015,8 +1016,8 @@ status_t GLESRenderEngine::drawLayers(const DisplaySettings& display, 
    setOutputDataSpace(display.outputDataspace);

    setDisplayMaxLuminance(display.maxLuminance);



    mat4 projectionMatrix = mState.projectionMatrix * display.globalTransform;

    mState.projectionMatrix = projectionMatrix;

    const mat4 projectionMatrix =

            ui::Transform(display.orientation).asMatrix4() * mState.projectionMatrix;

    if (!display.clearRegion.isEmpty()) {

        glDisable(GL_BLEND);

        fillRegionWithColor(display.clearRegion, 0.0, 0.0, 0.0, 1.0);

libs/renderengine/include/renderengine/DisplaySettings.h

+11 −1
Original line number Diff line number Diff line
@@ -41,6 +41,13 @@ struct DisplaySettings { 
    Rect clip = Rect::INVALID_RECT;



    // Global transform to apply to all layers.

    // The global transform is assumed to automatically apply when projecting

    // the clip rectangle onto the physical display; however, this should be

    // explicitly provided to perform CPU-side optimizations such as computing

    // scissor rectangles for rounded corners which require transformation to

    // the phsical display space.

    //

    // This transform is also assumed to include the orientation flag below.

    mat4 globalTransform = mat4();



    // Maximum luminance pulled from the display's HDR capabilities.
@@ -60,7 +67,10 @@ struct DisplaySettings { 
    // rendered layers.

    Region clearRegion = Region::INVALID_REGION;



    // The orientation of the physical display.

    // An additional orientation flag to be applied after clipping the output.

    // By way of example, this may be used for supporting fullscreen screenshot

    // capture of a device in landscape while the buffer is in portrait

    // orientation.

    uint32_t orientation = ui::Transform::ROT_0;

};

libs/renderengine/tests/RenderEngineTest.cpp

+8 −11
Original line number Diff line number Diff line
@@ -298,7 +298,7 @@ struct RenderEngineTest : public ::testing::Test { 
    void fillBufferPhysicalOffset();



    template <typename SourceVariant>

    void fillBufferCheckers(mat4 transform);

    void fillBufferCheckers(uint32_t rotation);



    template <typename SourceVariant>

    void fillBufferCheckersRotate0();
@@ -509,12 +509,12 @@ void RenderEngineTest::fillBufferPhysicalOffset() { 
}



template <typename SourceVariant>

void RenderEngineTest::fillBufferCheckers(mat4 transform) {

void RenderEngineTest::fillBufferCheckers(uint32_t orientationFlag) {

    renderengine::DisplaySettings settings;

    settings.physicalDisplay = fullscreenRect();

    // Here logical space is 2x2

    settings.clip = Rect(2, 2);

    settings.globalTransform = transform;

    settings.orientation = orientationFlag;



    std::vector<const renderengine::LayerSettings*> layers;
@@ -545,7 +545,7 @@ void RenderEngineTest::fillBufferCheckers(mat4 transform) { 


template <typename SourceVariant>

void RenderEngineTest::fillBufferCheckersRotate0() {

    fillBufferCheckers<SourceVariant>(mat4());

    fillBufferCheckers<SourceVariant>(ui::Transform::ROT_0);

    expectBufferColor(Rect(0, 0, DEFAULT_DISPLAY_WIDTH / 2, DEFAULT_DISPLAY_HEIGHT / 2), 255, 0, 0,

                      255);

    expectBufferColor(Rect(DEFAULT_DISPLAY_WIDTH / 2, 0, DEFAULT_DISPLAY_WIDTH,
@@ -561,8 +561,7 @@ void RenderEngineTest::fillBufferCheckersRotate0() { 


template <typename SourceVariant>

void RenderEngineTest::fillBufferCheckersRotate90() {

    mat4 matrix = mat4(0, 1, 0, 0, -1, 0, 0, 0, 0, 0, 1, 0, 2, 0, 0, 1);

    fillBufferCheckers<SourceVariant>(matrix);

    fillBufferCheckers<SourceVariant>(ui::Transform::ROT_90);

    expectBufferColor(Rect(0, 0, DEFAULT_DISPLAY_WIDTH / 2, DEFAULT_DISPLAY_HEIGHT / 2), 0, 255, 0,

                      255);

    expectBufferColor(Rect(DEFAULT_DISPLAY_WIDTH / 2, 0, DEFAULT_DISPLAY_WIDTH,
@@ -578,8 +577,7 @@ void RenderEngineTest::fillBufferCheckersRotate90() { 


template <typename SourceVariant>

void RenderEngineTest::fillBufferCheckersRotate180() {

    mat4 matrix = mat4(-1, 0, 0, 0, 0, -1, 0, 0, 0, 0, 1, 0, 2, 2, 0, 1);

    fillBufferCheckers<SourceVariant>(matrix);

    fillBufferCheckers<SourceVariant>(ui::Transform::ROT_180);

    expectBufferColor(Rect(0, 0, DEFAULT_DISPLAY_WIDTH / 2, DEFAULT_DISPLAY_HEIGHT / 2), 0, 0, 0,

                      0);

    expectBufferColor(Rect(DEFAULT_DISPLAY_WIDTH / 2, 0, DEFAULT_DISPLAY_WIDTH,
@@ -595,8 +593,7 @@ void RenderEngineTest::fillBufferCheckersRotate180() { 


template <typename SourceVariant>

void RenderEngineTest::fillBufferCheckersRotate270() {

    mat4 matrix = mat4(0, -1, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 2, 0, 1);

    fillBufferCheckers<SourceVariant>(matrix);

    fillBufferCheckers<SourceVariant>(ui::Transform::ROT_270);

    expectBufferColor(Rect(0, 0, DEFAULT_DISPLAY_WIDTH / 2, DEFAULT_DISPLAY_HEIGHT / 2), 0, 0, 255,

                      255);

    expectBufferColor(Rect(DEFAULT_DISPLAY_WIDTH / 2, 0, DEFAULT_DISPLAY_WIDTH,
@@ -928,7 +925,7 @@ void RenderEngineTest::clearLeftRegion() { 
    // Here logical space is 4x4

    settings.clip = Rect(4, 4);

    settings.globalTransform = mat4::scale(vec4(2, 4, 0, 1));

    settings.clearRegion = Region(Rect(1, 1));

    settings.clearRegion = Region(Rect(2, 4));

    std::vector<const renderengine::LayerSettings*> layers;

    // dummy layer, without bounds should not render anything

    renderengine::LayerSettings layer;

services/surfaceflinger/BufferLayer.cpp

+33 −0
Original line number Diff line number Diff line
@@ -618,6 +618,39 @@ bool BufferLayer::needsFiltering(const sp<const DisplayDevice>& displayDevice) c 
            sourceCrop.getWidth() != displayFrame.getWidth();

}



bool BufferLayer::needsFilteringForScreenshots(const sp<const DisplayDevice>& displayDevice,

                                               const ui::Transform& inverseParentTransform) const {

    // If we are not capturing based on the state of a known display device,

    // just return false.

    if (displayDevice == nullptr) {

        return false;

    }



    const auto outputLayer = findOutputLayerForDisplay(displayDevice);

    if (outputLayer == nullptr) {

        return false;

    }



    // We need filtering if the sourceCrop rectangle size does not match the

    // viewport rectangle size (not a 1:1 render)

    const auto& compositionState = outputLayer->getState();

    const ui::Transform& displayTransform = displayDevice->getTransform();

    const ui::Transform inverseTransform = inverseParentTransform * displayTransform.inverse();

    // Undo the transformation of the displayFrame so that we're back into

    // layer-stack space.

    const Rect frame = inverseTransform.transform(compositionState.displayFrame);

    const FloatRect sourceCrop = compositionState.sourceCrop;



    int32_t frameHeight = frame.getHeight();

    int32_t frameWidth = frame.getWidth();

    // If the display transform had a rotational component then undo the

    // rotation so that the orientation matches the source crop.

    if (displayTransform.getOrientation() & ui::Transform::ROT_90) {

        std::swap(frameHeight, frameWidth);

    }

    return sourceCrop.getHeight() != frameHeight || sourceCrop.getWidth() != frameWidth;

}



uint64_t BufferLayer::getHeadFrameNumber(nsecs_t expectedPresentTime) const {

    if (hasFrameUpdate()) {

        return getFrameNumber(expectedPresentTime);

services/surfaceflinger/BufferLayer.h

+2 −0
Original line number Diff line number Diff line
@@ -208,6 +208,8 @@ protected: 
private:

    // Returns true if this layer requires filtering

    bool needsFiltering(const sp<const DisplayDevice>& displayDevice) const override;

    bool needsFilteringForScreenshots(const sp<const DisplayDevice>& displayDevice,

                                      const ui::Transform& inverseParentTransform) const override;



    // BufferStateLayers can return Rect::INVALID_RECT if the layer does not have a display frame

    // and its parent layer is not bounded