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

Commit ec7e4d9a authored by Garfield Tan's avatar Garfield Tan Committed by Alec Mouri
Browse files

Skip creating external textures that exceeds size limit

Skia always accept an Android buffer and wraps it around with a texture
even if its size exceeds the limit GL exposes. Therefore let's skip
creating the texture in SurfaceFlinger and outputs an error log to
logcat.

I chose to do it in SurfaceFlinger rather than RenderEngine is because
the external texture mapping is designed to be asynchronous, so it'd be
better to keep that way. The limit is also exposed out of RenderEngine
so SurfaceFlinger can check it before creating external textures as
well.

Bug: 190399306
Bug: 204316511
Test: The test mentioned in the bug fails instead of crashing
SurfaceFlinger.
Test: atest SurfaceFlinger_test
Test: atest libsurfaceflinger_unittest

Change-Id: I52d253ed5a10f0e4ade372048721913405ed668a
(cherry picked from commit 9c9c1913)
Merged-In: I52d253ed5a10f0e4ade372048721913405ed668a
parent 26cd8a07
Loading
Loading
Loading
Loading
+2 −5
Original line number Original line Diff line number Diff line
@@ -515,13 +515,10 @@ void BufferQueueLayer::onFirstRef() {
}
}


status_t BufferQueueLayer::setDefaultBufferProperties(uint32_t w, uint32_t h, PixelFormat format) {
status_t BufferQueueLayer::setDefaultBufferProperties(uint32_t w, uint32_t h, PixelFormat format) {
    uint32_t const maxSurfaceDims =
          std::min(mFlinger->getMaxTextureSize(), mFlinger->getMaxViewportDims());

    // never allow a surface larger than what our underlying GL implementation
    // never allow a surface larger than what our underlying GL implementation
    // can handle.
    // can handle.
    if ((uint32_t(w) > maxSurfaceDims) || (uint32_t(h) > maxSurfaceDims)) {
    if (mFlinger->exceedsMaxRenderTargetSize(w, h)) {
        ALOGE("dimensions too large %u x %u", uint32_t(w), uint32_t(h));
        ALOGE("dimensions too large %" PRIu32 " x %" PRIu32, w, h);
        return BAD_VALUE;
        return BAD_VALUE;
    }
    }


+28 −14
Original line number Original line Diff line number Diff line
@@ -792,6 +792,8 @@ void SurfaceFlinger::init() {
                                    ? renderengine::RenderEngine::ContextPriority::REALTIME
                                    ? renderengine::RenderEngine::ContextPriority::REALTIME
                                    : renderengine::RenderEngine::ContextPriority::MEDIUM)
                                    : renderengine::RenderEngine::ContextPriority::MEDIUM)
                    .build()));
                    .build()));
    mMaxRenderTargetSize =
            std::min(getRenderEngine().getMaxTextureSize(), getRenderEngine().getMaxViewportDims());


    // Set SF main policy after initializing RenderEngine which has its own policy.
    // Set SF main policy after initializing RenderEngine which has its own policy.
    if (!SetTaskProfiles(0, {"SFMainPolicy"})) {
    if (!SetTaskProfiles(0, {"SFMainPolicy"})) {
@@ -879,14 +881,6 @@ void SurfaceFlinger::startBootAnim() {
    }
    }
}
}


size_t SurfaceFlinger::getMaxTextureSize() const {
    return getRenderEngine().getMaxTextureSize();
}

size_t SurfaceFlinger::getMaxViewportDims() const {
    return getRenderEngine().getMaxViewportDims();
}

// ----------------------------------------------------------------------------
// ----------------------------------------------------------------------------


bool SurfaceFlinger::authenticateSurfaceTexture(
bool SurfaceFlinger::authenticateSurfaceTexture(
@@ -4218,17 +4212,30 @@ uint32_t SurfaceFlinger::setClientStateLocked(
    }
    }
    bool bufferChanged = what & layer_state_t::eBufferChanged;
    bool bufferChanged = what & layer_state_t::eBufferChanged;
    bool cacheIdChanged = what & layer_state_t::eCachedBufferChanged;
    bool cacheIdChanged = what & layer_state_t::eCachedBufferChanged;
    bool bufferSizeExceedsLimit = false;
    std::shared_ptr<renderengine::ExternalTexture> buffer;
    std::shared_ptr<renderengine::ExternalTexture> buffer;
    if (bufferChanged && cacheIdChanged && s.buffer != nullptr) {
    if (bufferChanged && cacheIdChanged && s.buffer != nullptr) {
        bufferSizeExceedsLimit =
                exceedsMaxRenderTargetSize(s.buffer->getWidth(), s.buffer->getHeight());
        if (!bufferSizeExceedsLimit) {
            ClientCache::getInstance().add(s.cachedBuffer, s.buffer);
            ClientCache::getInstance().add(s.cachedBuffer, s.buffer);
            buffer = ClientCache::getInstance().get(s.cachedBuffer);
            buffer = ClientCache::getInstance().get(s.cachedBuffer);
        }
    } else if (cacheIdChanged) {
    } else if (cacheIdChanged) {
        buffer = ClientCache::getInstance().get(s.cachedBuffer);
        buffer = ClientCache::getInstance().get(s.cachedBuffer);
    } else if (bufferChanged && s.buffer != nullptr) {
    } else if (bufferChanged && s.buffer != nullptr) {
        bufferSizeExceedsLimit =
                exceedsMaxRenderTargetSize(s.buffer->getWidth(), s.buffer->getHeight());
        if (!bufferSizeExceedsLimit) {
            buffer = std::make_shared<
            buffer = std::make_shared<
                    renderengine::ExternalTexture>(s.buffer, getRenderEngine(),
                    renderengine::ExternalTexture>(s.buffer, getRenderEngine(),
                                                   renderengine::ExternalTexture::Usage::READABLE);
                                                   renderengine::ExternalTexture::Usage::READABLE);
        }
        }
    }
    ALOGE_IF(bufferSizeExceedsLimit,
             "Attempted to create an ExternalTexture for layer %s that exceeds render target size "
             "limit.",
             layer->getDebugName());
    if (buffer) {
    if (buffer) {
        const bool frameNumberChanged = what & layer_state_t::eFrameNumberChanged;
        const bool frameNumberChanged = what & layer_state_t::eFrameNumberChanged;
        const uint64_t frameNumber = frameNumberChanged
        const uint64_t frameNumber = frameNumberChanged
@@ -6198,6 +6205,13 @@ status_t SurfaceFlinger::captureScreenCommon(RenderAreaFuture renderAreaFuture,
                                             const sp<IScreenCaptureListener>& captureListener) {
                                             const sp<IScreenCaptureListener>& captureListener) {
    ATRACE_CALL();
    ATRACE_CALL();


    if (exceedsMaxRenderTargetSize(bufferSize.getWidth(), bufferSize.getHeight())) {
        ALOGE("Attempted to capture screen with size (%" PRId32 ", %" PRId32
              ") that exceeds render target size limit.",
              bufferSize.getWidth(), bufferSize.getHeight());
        return BAD_VALUE;
    }

    // Loop over all visible layers to see whether there's any protected layer. A protected layer is
    // Loop over all visible layers to see whether there's any protected layer. A protected layer is
    // typically a layer with DRM contents, or have the GRALLOC_USAGE_PROTECTED set on the buffer.
    // typically a layer with DRM contents, or have the GRALLOC_USAGE_PROTECTED set on the buffer.
    // A protected layer has no implication on whether it's secure, which is explicitly set by
    // A protected layer has no implication on whether it's secure, which is explicitly set by
+6 −2
Original line number Original line Diff line number Diff line
@@ -931,8 +931,9 @@ private:


    void readPersistentProperties();
    void readPersistentProperties();


    size_t getMaxTextureSize() const;
    bool exceedsMaxRenderTargetSize(uint32_t width, uint32_t height) const {
    size_t getMaxViewportDims() const;
        return width > mMaxRenderTargetSize || height > mMaxRenderTargetSize;
    }


    int getMaxAcquiredBufferCountForCurrentRefreshRate(uid_t uid) const;
    int getMaxAcquiredBufferCountForCurrentRefreshRate(uid_t uid) const;


@@ -1381,6 +1382,9 @@ private:


    SurfaceFlingerBE mBE;
    SurfaceFlingerBE mBE;
    std::unique_ptr<compositionengine::CompositionEngine> mCompositionEngine;
    std::unique_ptr<compositionengine::CompositionEngine> mCompositionEngine;
    // mMaxRenderTargetSize is only set once in init() so it doesn't need to be protected by
    // any mutex.
    size_t mMaxRenderTargetSize{1};


    const std::string mHwcServiceName;
    const std::string mHwcServiceName;


+17 −1
Original line number Original line Diff line number Diff line
@@ -515,7 +515,8 @@ TEST_F(ScreenCaptureTest, CaptureSize) {
}
}


TEST_F(ScreenCaptureTest, CaptureInvalidLayer) {
TEST_F(ScreenCaptureTest, CaptureInvalidLayer) {
    sp<SurfaceControl> redLayer = createLayer(String8("Red surface"), 60, 60, 0);
    sp<SurfaceControl> redLayer = createLayer(String8("Red surface"), 60, 60,
                                              ISurfaceComposerClient::eFXSurfaceBufferState);


    ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(redLayer, Color::RED, 60, 60));
    ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(redLayer, Color::RED, 60, 60));


@@ -532,6 +533,21 @@ TEST_F(ScreenCaptureTest, CaptureInvalidLayer) {
    ASSERT_EQ(NAME_NOT_FOUND, ScreenCapture::captureLayers(args, captureResults));
    ASSERT_EQ(NAME_NOT_FOUND, ScreenCapture::captureLayers(args, captureResults));
}
}


TEST_F(ScreenCaptureTest, CaptureTooLargeLayer) {
    sp<SurfaceControl> redLayer = createLayer(String8("Red surface"), 60, 60);
    ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(redLayer, Color::RED, 60, 60));

    Transaction().show(redLayer).setLayer(redLayer, INT32_MAX).apply(true);

    LayerCaptureArgs captureArgs;
    captureArgs.layerHandle = redLayer->getHandle();
    captureArgs.frameScaleX = INT32_MAX / 60;
    captureArgs.frameScaleY = INT32_MAX / 60;

    ScreenCaptureResults captureResults;
    ASSERT_EQ(BAD_VALUE, ScreenCapture::captureLayers(captureArgs, captureResults));
}

TEST_F(ScreenCaptureTest, CaptureSecureLayer) {
TEST_F(ScreenCaptureTest, CaptureSecureLayer) {
    sp<SurfaceControl> redLayer = createLayer(String8("Red surface"), 60, 60,
    sp<SurfaceControl> redLayer = createLayer(String8("Red surface"), 60, 60,
                                              ISurfaceComposerClient::eFXSurfaceBufferState);
                                              ISurfaceComposerClient::eFXSurfaceBufferState);
+2 −2
Original line number Original line Diff line number Diff line
@@ -108,6 +108,8 @@ public:


        mComposer = new Hwc2::mock::Composer();
        mComposer = new Hwc2::mock::Composer();
        mFlinger.setupComposer(std::unique_ptr<Hwc2::Composer>(mComposer));
        mFlinger.setupComposer(std::unique_ptr<Hwc2::Composer>(mComposer));

        mFlinger.mutableMaxRenderTargetSize() = 16384;
    }
    }


    ~CompositionTest() {
    ~CompositionTest() {
@@ -519,8 +521,6 @@ struct BaseLayerProperties {


    static void setupLatchedBuffer(CompositionTest* test, sp<BufferQueueLayer> layer) {
    static void setupLatchedBuffer(CompositionTest* test, sp<BufferQueueLayer> layer) {
        // TODO: Eliminate the complexity of actually creating a buffer
        // TODO: Eliminate the complexity of actually creating a buffer
        EXPECT_CALL(*test->mRenderEngine, getMaxTextureSize()).WillOnce(Return(16384));
        EXPECT_CALL(*test->mRenderEngine, getMaxViewportDims()).WillOnce(Return(16384));
        status_t err =
        status_t err =
                layer->setDefaultBufferProperties(LayerProperties::WIDTH, LayerProperties::HEIGHT,
                layer->setDefaultBufferProperties(LayerProperties::WIDTH, LayerProperties::HEIGHT,
                                                  LayerProperties::FORMAT);
                                                  LayerProperties::FORMAT);
Loading