diff options
| -rw-r--r-- | services/surfaceflinger/CompositionEngine/src/Output.cpp | 3 | ||||
| -rw-r--r-- | services/surfaceflinger/RefreshRateOverlay.cpp | 51 | ||||
| -rw-r--r-- | services/surfaceflinger/RefreshRateOverlay.h | 16 | ||||
| -rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 9 |
4 files changed, 58 insertions, 21 deletions
diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index c3385a8a8b..f360504070 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1218,7 +1218,8 @@ std::optional<base::unique_fd> Output::composeSurfaces( ATRACE_NAME("ClientCompositionCacheHit"); outputCompositionState.reusedClientComposition = true; setExpensiveRenderingExpected(false); - return base::unique_fd(); + // b/239944175 pass the fence associated with the buffer. + return base::unique_fd(std::move(fd)); } ATRACE_NAME("ClientCompositionCacheMiss"); mClientCompositionRequestCache->add(tex->getBuffer()->getId(), clientCompositionDisplay, diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index d4435c2818..a9180d4483 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -16,9 +16,10 @@ #include <algorithm> -#include "RefreshRateOverlay.h" +#include "BackgroundExecutor.h" #include "Client.h" #include "Layer.h" +#include "RefreshRateOverlay.h" #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wconversion" @@ -56,6 +57,14 @@ SurfaceComposerClient::Transaction createTransaction(const sp<SurfaceControl>& s } // namespace +SurfaceControlHolder::~SurfaceControlHolder() { + // Hand the sp<SurfaceControl> to the helper thread to release the last + // reference. This makes sure that the SurfaceControl is destructed without + // SurfaceFlinger::mStateLock held. + BackgroundExecutor::getInstance().sendCallbacks( + {[sc = std::move(mSurfaceControl)]() mutable { sc.clear(); }}); +} + void RefreshRateOverlay::SevenSegmentDrawer::drawSegment(Segment segment, int left, SkColor color, SkCanvas& canvas) { const SkRect rect = [&]() { @@ -210,21 +219,27 @@ auto RefreshRateOverlay::SevenSegmentDrawer::draw(int number, SkColor color, return buffers; } +std::unique_ptr<SurfaceControlHolder> createSurfaceControlHolder() { + sp<SurfaceControl> surfaceControl = + SurfaceComposerClient::getDefault() + ->createSurface(String8("RefreshRateOverlay"), kBufferWidth, kBufferHeight, + PIXEL_FORMAT_RGBA_8888, + ISurfaceComposerClient::eFXSurfaceBufferState); + return std::make_unique<SurfaceControlHolder>(std::move(surfaceControl)); +} + RefreshRateOverlay::RefreshRateOverlay(FpsRange fpsRange, bool showSpinner) : mFpsRange(fpsRange), mShowSpinner(showSpinner), - mSurfaceControl(SurfaceComposerClient::getDefault() - ->createSurface(String8("RefreshRateOverlay"), kBufferWidth, - kBufferHeight, PIXEL_FORMAT_RGBA_8888, - ISurfaceComposerClient::eFXSurfaceBufferState)) { + mSurfaceControl(createSurfaceControlHolder()) { if (!mSurfaceControl) { ALOGE("%s: Failed to create buffer state layer", __func__); return; } - createTransaction(mSurfaceControl) - .setLayer(mSurfaceControl, INT32_MAX - 2) - .setTrustedOverlay(mSurfaceControl, true) + createTransaction(mSurfaceControl->get()) + .setLayer(mSurfaceControl->get(), INT32_MAX - 2) + .setTrustedOverlay(mSurfaceControl->get(), true) .apply(); } @@ -233,7 +248,7 @@ auto RefreshRateOverlay::getOrCreateBuffers(Fps fps) -> const Buffers& { if (!mSurfaceControl) return kNoBuffers; const auto transformHint = - static_cast<ui::Transform::RotationFlags>(mSurfaceControl->getTransformHint()); + static_cast<ui::Transform::RotationFlags>(mSurfaceControl->get()->getTransformHint()); // Tell SurfaceFlinger about the pre-rotation on the buffer. const auto transform = [&] { @@ -247,7 +262,9 @@ auto RefreshRateOverlay::getOrCreateBuffers(Fps fps) -> const Buffers& { } }(); - createTransaction(mSurfaceControl).setTransform(mSurfaceControl, transform).apply(); + createTransaction(mSurfaceControl->get()) + .setTransform(mSurfaceControl->get(), transform) + .apply(); BufferCache::const_iterator it = mBufferCache.find({fps.getIntValue(), transformHint}); if (it == mBufferCache.end()) { @@ -289,21 +306,21 @@ void RefreshRateOverlay::setViewport(ui::Size viewport) { Rect frame((3 * width) >> 4, height >> 5); frame.offsetBy(width >> 5, height >> 4); - createTransaction(mSurfaceControl) - .setMatrix(mSurfaceControl, frame.getWidth() / static_cast<float>(kBufferWidth), 0, 0, - frame.getHeight() / static_cast<float>(kBufferHeight)) - .setPosition(mSurfaceControl, frame.left, frame.top) + createTransaction(mSurfaceControl->get()) + .setMatrix(mSurfaceControl->get(), frame.getWidth() / static_cast<float>(kBufferWidth), + 0, 0, frame.getHeight() / static_cast<float>(kBufferHeight)) + .setPosition(mSurfaceControl->get(), frame.left, frame.top) .apply(); } void RefreshRateOverlay::setLayerStack(ui::LayerStack stack) { - createTransaction(mSurfaceControl).setLayerStack(mSurfaceControl, stack).apply(); + createTransaction(mSurfaceControl->get()).setLayerStack(mSurfaceControl->get(), stack).apply(); } void RefreshRateOverlay::changeRefreshRate(Fps fps) { mCurrentFps = fps; const auto buffer = getOrCreateBuffers(fps)[mFrame]; - createTransaction(mSurfaceControl).setBuffer(mSurfaceControl, buffer).apply(); + createTransaction(mSurfaceControl->get()).setBuffer(mSurfaceControl->get(), buffer).apply(); } void RefreshRateOverlay::animate() { @@ -312,7 +329,7 @@ void RefreshRateOverlay::animate() { const auto& buffers = getOrCreateBuffers(*mCurrentFps); mFrame = (mFrame + 1) % buffers.size(); const auto buffer = buffers[mFrame]; - createTransaction(mSurfaceControl).setBuffer(mSurfaceControl, buffer).apply(); + createTransaction(mSurfaceControl->get()).setBuffer(mSurfaceControl->get(), buffer).apply(); } } // namespace android diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h index a465a36747..a2966e654e 100644 --- a/services/surfaceflinger/RefreshRateOverlay.h +++ b/services/surfaceflinger/RefreshRateOverlay.h @@ -33,6 +33,20 @@ namespace android { class GraphicBuffer; class SurfaceControl; +class SurfaceFlinger; + +// Helper class to delete the SurfaceControl on a helper thread as +// SurfaceControl assumes its destruction happens without SurfaceFlinger::mStateLock held. +class SurfaceControlHolder { +public: + explicit SurfaceControlHolder(sp<SurfaceControl> sc) : mSurfaceControl(std::move(sc)){}; + ~SurfaceControlHolder(); + + const sp<SurfaceControl>& get() const { return mSurfaceControl; } + +private: + sp<SurfaceControl> mSurfaceControl; +}; class RefreshRateOverlay { public: @@ -75,7 +89,7 @@ private: const FpsRange mFpsRange; // For color interpolation. const bool mShowSpinner; - const sp<SurfaceControl> mSurfaceControl; + const std::unique_ptr<SurfaceControlHolder> mSurfaceControl; }; } // namespace android diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index e9fbf6ea7b..ce973e4eca 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -6631,8 +6631,13 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenCommon( 1 /* layerCount */, usage, "screenshot"); const status_t bufferStatus = buffer->initCheck(); - LOG_ALWAYS_FATAL_IF(bufferStatus != OK, "captureScreenCommon: Buffer failed to allocate: %d", - bufferStatus); + if (bufferStatus != OK) { + // Animations may end up being really janky, but don't crash here. + // Otherwise an irreponsible process may cause an SF crash by allocating + // too much. + ALOGE("%s: Buffer failed to allocate: %d", __func__, bufferStatus); + return ftl::yield<FenceResult>(base::unexpected(bufferStatus)).share(); + } const std::shared_ptr<renderengine::ExternalTexture> texture = std::make_shared< renderengine::impl::ExternalTexture>(buffer, getRenderEngine(), renderengine::impl::ExternalTexture::Usage:: |