From 97cb066a0049c7ed8268ab436df0cc3cc843dabb Mon Sep 17 00:00:00 2001 From: Adithya Srinivasan Date: Thu, 14 Jan 2021 23:49:46 +0000 Subject: Fix Transaction tracking for FrameTimeline The current setup only supports one SurfaceFrame per DrawingState for Transactions. This becomes problematic if more than one Transaction is submitted for the same vsync, on the same layer. On top of this, the Blast transactions can have a buffer that could result in a buffer drop. This change adds the support to hold multiple SurfaceFrames in the Layer's State. It also adds a bufferSurfaceFrame that's intended only for Blast Transactions with a Buffer. All other Transactions are tracked in the bufferlessSurfaceFrames. Additionally, this change also adds a lastLatchTime. It is needed for classifying BufferStuffing properly. Bug: 176106798 Test: open any app from the launcher and at the same time check dumpsys Change-Id: Id3b8369ca206f8b89be3041e5fc018f1f1be1d23 --- services/surfaceflinger/RefreshRateOverlay.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'services/surfaceflinger/RefreshRateOverlay.cpp') diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 9230e72f45..49ffc81359 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -238,7 +238,7 @@ void RefreshRateOverlay::changeRefreshRate(const Fps& fps) { auto buffer = getOrCreateBuffers(*mCurrentFps)[mFrame]; mLayer->setBuffer(buffer, Fence::NO_FENCE, 0, 0, true, {}, mLayer->getHeadFrameNumber(-1 /* expectedPresentTime */), - std::nullopt /* dequeueTime */); + std::nullopt /* dequeueTime */, FrameTimelineInfo{}); mFlinger.mTransactionFlags.fetch_or(eTransactionMask); } @@ -251,7 +251,7 @@ void RefreshRateOverlay::onInvalidate() { auto buffer = buffers[mFrame]; mLayer->setBuffer(buffer, Fence::NO_FENCE, 0, 0, true, {}, mLayer->getHeadFrameNumber(-1 /* expectedPresentTime */), - std::nullopt /* dequeueTime */); + std::nullopt /* dequeueTime */, FrameTimelineInfo{}); mFlinger.mTransactionFlags.fetch_or(eTransactionMask); } -- cgit v1.2.3-59-g8ed1b From 756005bf3505ac83c048651aa9c15406bbd2d692 Mon Sep 17 00:00:00 2001 From: rnlee Date: Thu, 27 May 2021 10:46:36 -0700 Subject: Use skia to draw pre-rotated RefreshRateOverlay. Bug: 166793515 Test: Manual with adb shell dumpsys SurfaceFlinger | grep -A 30 "HWC layers" Change-Id: I6aad5730794fdb2ca6e0575e650402939bbd24cc --- services/surfaceflinger/RefreshRateOverlay.cpp | 169 ++++++++++++--------- services/surfaceflinger/RefreshRateOverlay.h | 24 +-- services/surfaceflinger/tests/unittests/Android.bp | 5 +- .../mock/DisplayHardware/MockComposer.cpp | 1 + 4 files changed, 112 insertions(+), 87 deletions(-) (limited to 'services/surfaceflinger/RefreshRateOverlay.cpp') diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index a9fd16cb75..075d0eb1dc 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -25,6 +25,10 @@ #include "Client.h" #include "Layer.h" +#include +#include +#include +#include #include #undef LOG_TAG @@ -32,85 +36,64 @@ namespace android { -void RefreshRateOverlay::SevenSegmentDrawer::drawRect(const Rect& r, const half4& color, - const sp& buffer, - uint8_t* pixels) { - for (int32_t j = r.top; j < r.bottom; j++) { - if (j >= buffer->getHeight()) { - break; - } - - for (int32_t i = r.left; i < r.right; i++) { - if (i >= buffer->getWidth()) { - break; - } - - uint8_t* iter = pixels + 4 * (i + (buffer->getStride() * j)); - iter[0] = uint8_t(color.r * 255); - iter[1] = uint8_t(color.g * 255); - iter[2] = uint8_t(color.b * 255); - iter[3] = uint8_t(color.a * 255); - } - } -} - -void RefreshRateOverlay::SevenSegmentDrawer::drawSegment(Segment segment, int left, - const half4& color, - const sp& buffer, - uint8_t* pixels) { - const Rect rect = [&]() { +void RefreshRateOverlay::SevenSegmentDrawer::drawSegment(Segment segment, int left, SkColor& color, + SkCanvas& canvas) { + const SkRect rect = [&]() { switch (segment) { case Segment::Upper: - return Rect(left, 0, left + DIGIT_WIDTH, DIGIT_SPACE); + return SkRect::MakeLTRB(left, 0, left + DIGIT_WIDTH, DIGIT_SPACE); case Segment::UpperLeft: - return Rect(left, 0, left + DIGIT_SPACE, DIGIT_HEIGHT / 2); + return SkRect::MakeLTRB(left, 0, left + DIGIT_SPACE, DIGIT_HEIGHT / 2); case Segment::UpperRight: - return Rect(left + DIGIT_WIDTH - DIGIT_SPACE, 0, left + DIGIT_WIDTH, - DIGIT_HEIGHT / 2); + return SkRect::MakeLTRB(left + DIGIT_WIDTH - DIGIT_SPACE, 0, left + DIGIT_WIDTH, + DIGIT_HEIGHT / 2); case Segment::Middle: - return Rect(left, DIGIT_HEIGHT / 2 - DIGIT_SPACE / 2, left + DIGIT_WIDTH, - DIGIT_HEIGHT / 2 + DIGIT_SPACE / 2); + return SkRect::MakeLTRB(left, DIGIT_HEIGHT / 2 - DIGIT_SPACE / 2, + left + DIGIT_WIDTH, DIGIT_HEIGHT / 2 + DIGIT_SPACE / 2); case Segment::LowerLeft: - return Rect(left, DIGIT_HEIGHT / 2, left + DIGIT_SPACE, DIGIT_HEIGHT); + return SkRect::MakeLTRB(left, DIGIT_HEIGHT / 2, left + DIGIT_SPACE, DIGIT_HEIGHT); case Segment::LowerRight: - return Rect(left + DIGIT_WIDTH - DIGIT_SPACE, DIGIT_HEIGHT / 2, left + DIGIT_WIDTH, - DIGIT_HEIGHT); - case Segment::Buttom: - return Rect(left, DIGIT_HEIGHT - DIGIT_SPACE, left + DIGIT_WIDTH, DIGIT_HEIGHT); + return SkRect::MakeLTRB(left + DIGIT_WIDTH - DIGIT_SPACE, DIGIT_HEIGHT / 2, + left + DIGIT_WIDTH, DIGIT_HEIGHT); + case Segment::Bottom: + return SkRect::MakeLTRB(left, DIGIT_HEIGHT - DIGIT_SPACE, left + DIGIT_WIDTH, + DIGIT_HEIGHT); } }(); - drawRect(rect, color, buffer, pixels); + SkPaint paint; + paint.setColor(color); + paint.setBlendMode(SkBlendMode::kSrc); + canvas.drawRect(rect, paint); } -void RefreshRateOverlay::SevenSegmentDrawer::drawDigit(int digit, int left, const half4& color, - const sp& buffer, - uint8_t* pixels) { +void RefreshRateOverlay::SevenSegmentDrawer::drawDigit(int digit, int left, SkColor& color, + SkCanvas& canvas) { if (digit < 0 || digit > 9) return; if (digit == 0 || digit == 2 || digit == 3 || digit == 5 || digit == 6 || digit == 7 || digit == 8 || digit == 9) - drawSegment(Segment::Upper, left, color, buffer, pixels); + drawSegment(Segment::Upper, left, color, canvas); if (digit == 0 || digit == 4 || digit == 5 || digit == 6 || digit == 8 || digit == 9) - drawSegment(Segment::UpperLeft, left, color, buffer, pixels); + drawSegment(Segment::UpperLeft, left, color, canvas); if (digit == 0 || digit == 1 || digit == 2 || digit == 3 || digit == 4 || digit == 7 || digit == 8 || digit == 9) - drawSegment(Segment::UpperRight, left, color, buffer, pixels); + drawSegment(Segment::UpperRight, left, color, canvas); if (digit == 2 || digit == 3 || digit == 4 || digit == 5 || digit == 6 || digit == 8 || digit == 9) - drawSegment(Segment::Middle, left, color, buffer, pixels); + drawSegment(Segment::Middle, left, color, canvas); if (digit == 0 || digit == 2 || digit == 6 || digit == 8) - drawSegment(Segment::LowerLeft, left, color, buffer, pixels); + drawSegment(Segment::LowerLeft, left, color, canvas); if (digit == 0 || digit == 1 || digit == 3 || digit == 4 || digit == 5 || digit == 6 || digit == 7 || digit == 8 || digit == 9) - drawSegment(Segment::LowerRight, left, color, buffer, pixels); + drawSegment(Segment::LowerRight, left, color, canvas); if (digit == 0 || digit == 2 || digit == 3 || digit == 5 || digit == 6 || digit == 8 || digit == 9) - drawSegment(Segment::Buttom, left, color, buffer, pixels); + drawSegment(Segment::Bottom, left, color, canvas); } -std::vector> RefreshRateOverlay::SevenSegmentDrawer::drawNumber( - int number, const half4& color, bool showSpinner) { +std::vector> RefreshRateOverlay::SevenSegmentDrawer::draw( + int number, SkColor& color, ui::Transform::RotationFlags rotation, bool showSpinner) { if (number < 0 || number > 1000) return {}; const auto hundreds = number / 100; @@ -120,55 +103,76 @@ std::vector> RefreshRateOverlay::SevenSegmentDrawer::drawNumbe std::vector> buffers; const auto loopCount = showSpinner ? 6 : 1; for (int i = 0; i < loopCount; i++) { + // Pre-rotate the buffer before it reaches SurfaceFlinger. + SkMatrix canvasTransform = SkMatrix(); + auto [bufferWidth, bufferHeight] = [&] { + switch (rotation) { + case ui::Transform::ROT_90: + canvasTransform.setTranslate(BUFFER_HEIGHT, 0); + canvasTransform.preRotate(90); + return std::make_tuple(BUFFER_HEIGHT, BUFFER_WIDTH); + case ui::Transform::ROT_270: + canvasTransform.setRotate(270, BUFFER_WIDTH / 2.0, BUFFER_WIDTH / 2.0); + return std::make_tuple(BUFFER_HEIGHT, BUFFER_WIDTH); + default: + return std::make_tuple(BUFFER_WIDTH, BUFFER_HEIGHT); + } + }(); sp buffer = - new GraphicBuffer(BUFFER_WIDTH, BUFFER_HEIGHT, HAL_PIXEL_FORMAT_RGBA_8888, 1, + new GraphicBuffer(bufferWidth, bufferHeight, HAL_PIXEL_FORMAT_RGBA_8888, 1, GRALLOC_USAGE_SW_WRITE_RARELY | GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_TEXTURE, "RefreshRateOverlayBuffer"); const status_t bufferStatus = buffer->initCheck(); LOG_ALWAYS_FATAL_IF(bufferStatus != OK, "RefreshRateOverlay: Buffer failed to allocate: %d", bufferStatus); - uint8_t* pixels; - buffer->lock(GRALLOC_USAGE_SW_WRITE_RARELY, reinterpret_cast(&pixels)); - // Clear buffer content - drawRect(Rect(BUFFER_WIDTH, BUFFER_HEIGHT), half4(0), buffer, pixels); + + sk_sp surface = SkSurface::MakeRasterN32Premul(bufferWidth, bufferHeight); + SkCanvas* canvas = surface->getCanvas(); + canvas->setMatrix(canvasTransform); + int left = 0; if (hundreds != 0) { - drawDigit(hundreds, left, color, buffer, pixels); + drawDigit(hundreds, left, color, *canvas); } left += DIGIT_WIDTH + DIGIT_SPACE; if (tens != 0) { - drawDigit(tens, left, color, buffer, pixels); + drawDigit(tens, left, color, *canvas); } left += DIGIT_WIDTH + DIGIT_SPACE; - drawDigit(ones, left, color, buffer, pixels); + drawDigit(ones, left, color, *canvas); left += DIGIT_WIDTH + DIGIT_SPACE; if (showSpinner) { switch (i) { case 0: - drawSegment(Segment::Upper, left, color, buffer, pixels); + drawSegment(Segment::Upper, left, color, *canvas); break; case 1: - drawSegment(Segment::UpperRight, left, color, buffer, pixels); + drawSegment(Segment::UpperRight, left, color, *canvas); break; case 2: - drawSegment(Segment::LowerRight, left, color, buffer, pixels); + drawSegment(Segment::LowerRight, left, color, *canvas); break; case 3: - drawSegment(Segment::Buttom, left, color, buffer, pixels); + drawSegment(Segment::Bottom, left, color, *canvas); break; case 4: - drawSegment(Segment::LowerLeft, left, color, buffer, pixels); + drawSegment(Segment::LowerLeft, left, color, *canvas); break; case 5: - drawSegment(Segment::UpperLeft, left, color, buffer, pixels); + drawSegment(Segment::UpperLeft, left, color, *canvas); break; } } + void* pixels = nullptr; + buffer->lock(GRALLOC_USAGE_SW_WRITE_RARELY, reinterpret_cast(&pixels)); + const SkImageInfo& imageInfo = surface->imageInfo(); + size_t dstRowBytes = buffer->getStride() * imageInfo.bytesPerPixel(); + canvas->readPixels(imageInfo, pixels, dstRowBytes, 0, 0); buffer->unlock(); buffers.emplace_back(buffer); } @@ -210,7 +214,22 @@ bool RefreshRateOverlay::createLayer() { const std::vector>& RefreshRateOverlay::getOrCreateBuffers(uint32_t fps) { - if (mBufferCache.find(fps) == mBufferCache.end()) { + ui::Transform::RotationFlags transformHint = mLayer->getTransformHint(); + // Tell SurfaceFlinger about the pre-rotation on the buffer. + const auto transform = [&] { + switch (transformHint) { + case ui::Transform::ROT_90: + return ui::Transform::ROT_270; + case ui::Transform::ROT_270: + return ui::Transform::ROT_90; + default: + return ui::Transform::ROT_0; + } + }(); + mLayer->setTransform(transform); + + if (mBufferCache.find(transformHint) == mBufferCache.end() || + mBufferCache.at(transformHint).find(fps) == mBufferCache.at(transformHint).end()) { // Ensure the range is > 0, so we don't divide by 0. const auto rangeLength = std::max(1u, mHighFps - mLowFps); // Clip values outside the range [mLowFps, mHighFps]. The current fps may be outside @@ -218,12 +237,14 @@ RefreshRateOverlay::getOrCreateBuffers(uint32_t fps) { fps = std::max(fps, mLowFps); fps = std::min(fps, mHighFps); const auto fpsScale = static_cast(fps - mLowFps) / rangeLength; - half4 color; - color.r = HIGH_FPS_COLOR.r * fpsScale + LOW_FPS_COLOR.r * (1 - fpsScale); - color.g = HIGH_FPS_COLOR.g * fpsScale + LOW_FPS_COLOR.g * (1 - fpsScale); - color.b = HIGH_FPS_COLOR.b * fpsScale + LOW_FPS_COLOR.b * (1 - fpsScale); - color.a = ALPHA; - auto buffers = SevenSegmentDrawer::drawNumber(fps, color, mShowSpinner); + SkColor4f colorBase = SkColor4f::FromColor(HIGH_FPS_COLOR) * fpsScale; + SkColor4f lowFpsColor = SkColor4f::FromColor(LOW_FPS_COLOR) * (1 - fpsScale); + colorBase.fR = colorBase.fR + lowFpsColor.fR; + colorBase.fG = colorBase.fG + lowFpsColor.fG; + colorBase.fB = colorBase.fB + lowFpsColor.fB; + colorBase.fA = ALPHA; + SkColor color = colorBase.toSkColor(); + auto buffers = SevenSegmentDrawer::draw(fps, color, transformHint, mShowSpinner); std::vector> textures; std::transform(buffers.begin(), buffers.end(), std::back_inserter(textures), [&](const auto& buffer) -> std::shared_ptr { @@ -233,10 +254,10 @@ RefreshRateOverlay::getOrCreateBuffers(uint32_t fps) { renderengine::ExternalTexture:: Usage::READABLE); }); - mBufferCache.emplace(fps, textures); + mBufferCache[transformHint].emplace(fps, textures); } - return mBufferCache[fps]; + return mBufferCache[transformHint][fps]; } void RefreshRateOverlay::setViewport(ui::Size viewport) { diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h index aa8329c46a..cb8b2271da 100644 --- a/services/surfaceflinger/RefreshRateOverlay.h +++ b/services/surfaceflinger/RefreshRateOverlay.h @@ -22,6 +22,8 @@ #include #include +#include +#include #include #include "Fps.h" @@ -47,20 +49,16 @@ public: private: class SevenSegmentDrawer { public: - static std::vector> drawNumber(int number, const half4& color, - bool showSpinner); + static std::vector> draw(int number, SkColor& color, + ui::Transform::RotationFlags, bool showSpinner); static uint32_t getHeight() { return BUFFER_HEIGHT; } static uint32_t getWidth() { return BUFFER_WIDTH; } private: - enum class Segment { Upper, UpperLeft, UpperRight, Middle, LowerLeft, LowerRight, Buttom }; + enum class Segment { Upper, UpperLeft, UpperRight, Middle, LowerLeft, LowerRight, Bottom }; - static void drawRect(const Rect& r, const half4& color, const sp& buffer, - uint8_t* pixels); - static void drawSegment(Segment segment, int left, const half4& color, - const sp& buffer, uint8_t* pixels); - static void drawDigit(int digit, int left, const half4& color, - const sp& buffer, uint8_t* pixels); + static void drawSegment(Segment segment, int left, SkColor& color, SkCanvas& canvas); + static void drawDigit(int digit, int left, SkColor& color, SkCanvas& canvas); static constexpr uint32_t DIGIT_HEIGHT = 100; static constexpr uint32_t DIGIT_WIDTH = 64; @@ -80,13 +78,15 @@ private: sp mIBinder; sp mGbp; - std::unordered_map>> + std::unordered_map< + ui::Transform::RotationFlags, + std::unordered_map>>> mBufferCache; std::optional mCurrentFps; int mFrame = 0; static constexpr float ALPHA = 0.8f; - const half3 LOW_FPS_COLOR = half3(1.0f, 0.0f, 0.0f); - const half3 HIGH_FPS_COLOR = half3(0.0f, 1.0f, 0.0f); + const SkColor LOW_FPS_COLOR = SK_ColorRED; + const SkColor HIGH_FPS_COLOR = SK_ColorGREEN; const bool mShowSpinner; diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index 7512fd3dfe..69c99a8aeb 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -23,7 +23,10 @@ package { cc_test { name: "libsurfaceflinger_unittest", - defaults: ["surfaceflinger_defaults"], + defaults: [ + "skia_renderengine_deps", + "surfaceflinger_defaults", + ], test_suites: ["device-tests"], sanitize: { // Using the address sanitizer not only helps uncover issues in the code diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.cpp b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.cpp index 7de187207e..20d41e6072 100644 --- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.cpp +++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.cpp @@ -18,6 +18,7 @@ #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wconversion" +#undef LOG_TAG #define LOG_TAG "MockComposer" #include "mock/DisplayHardware/MockComposer.h" -- cgit v1.2.3-59-g8ed1b From 29fa146d8d745cee950a1ed82ddb500fc6d6c771 Mon Sep 17 00:00:00 2001 From: Dominik Laskowski Date: Tue, 27 Apr 2021 15:51:50 -0700 Subject: SF: Consolidate layer-to-output filtering Add ui::LayerFilter for less repetitive CE plumbing. Make ui::LayerStack a type, and (unlike the alias) use it everywhere. Remove redundant state in CE's DisplayCreationArgs. Bug: 182939859 Test: Display cutout is excluded in screenshots. Test: libcompositionengine_test Test: libsurfaceflinger_unittest Test: SurfaceFlinger_test Test: libgui_test Change-Id: Ib854d354af7aef7168001c34297e875b71d53622 --- cmds/surfacereplayer/replayer/Replayer.cpp | 4 +- libs/gui/LayerState.cpp | 18 +-- libs/gui/SurfaceComposerClient.cpp | 4 +- libs/gui/include/gui/LayerState.h | 17 ++- libs/gui/include/gui/SurfaceComposerClient.h | 4 +- libs/gui/tests/BLASTBufferQueue_test.cpp | 12 +- libs/ui/include/ui/DisplayState.h | 7 +- libs/ui/include/ui/LayerStack.h | 78 +++++++++++ .../compositionengine/DisplayCreationArgs.h | 16 --- .../compositionengine/LayerFECompositionState.h | 7 +- .../include/compositionengine/Output.h | 21 +-- .../include/compositionengine/impl/Display.h | 3 +- .../include/compositionengine/impl/DumpHelpers.h | 14 +- .../include/compositionengine/impl/Output.h | 8 +- .../impl/OutputCompositionState.h | 8 +- .../include/compositionengine/mock/Output.h | 7 +- .../CompositionEngine/src/Display.cpp | 11 +- .../CompositionEngine/src/DumpHelpers.cpp | 16 ++- .../CompositionEngine/src/Output.cpp | 25 ++-- .../src/OutputCompositionState.cpp | 4 +- .../CompositionEngine/tests/DisplayTest.cpp | 59 +++----- .../CompositionEngine/tests/OutputTest.cpp | 149 ++++++++++----------- services/surfaceflinger/DisplayDevice.cpp | 4 +- services/surfaceflinger/DisplayDevice.h | 6 +- services/surfaceflinger/Layer.cpp | 71 +++++----- services/surfaceflinger/Layer.h | 33 +++-- services/surfaceflinger/LayerVector.cpp | 4 +- services/surfaceflinger/RefreshRateOverlay.cpp | 2 +- services/surfaceflinger/RefreshRateOverlay.h | 3 +- services/surfaceflinger/SurfaceFlinger.cpp | 98 ++++++-------- services/surfaceflinger/SurfaceInterceptor.cpp | 12 +- services/surfaceflinger/SurfaceInterceptor.h | 5 +- services/surfaceflinger/tests/Credentials_test.cpp | 2 +- services/surfaceflinger/tests/EffectLayer_test.cpp | 2 +- services/surfaceflinger/tests/IPC_test.cpp | 6 +- .../surfaceflinger/tests/LayerTransactionTest.h | 4 +- .../LayerTypeAndRenderTypeTransaction_test.cpp | 3 +- services/surfaceflinger/tests/LayerUpdate_test.cpp | 2 +- services/surfaceflinger/tests/MirrorLayer_test.cpp | 2 +- .../tests/MultiDisplayLayerBounds_test.cpp | 13 +- services/surfaceflinger/tests/RelativeZ_test.cpp | 2 +- .../surfaceflinger/tests/ScreenCapture_test.cpp | 8 +- .../tests/SurfaceInterceptor_test.cpp | 4 +- .../tests/TransactionTestHarnesses.h | 2 +- .../tests/WindowInfosListener_test.cpp | 4 +- .../tests/fakehwc/SFFakeHwc_test.cpp | 22 +-- .../tests/unittests/CompositionTest.cpp | 8 +- .../tests/unittests/DisplayTransactionTest.cpp | 8 +- .../unittests/DisplayTransactionTestHelpers.h | 8 +- .../SurfaceFlinger_HandleTransactionLockedTest.cpp | 4 +- .../SurfaceFlinger_OnInitializeDisplaysTest.cpp | 12 +- .../SurfaceFlinger_SetDisplayStateTest.cpp | 16 ++- 52 files changed, 414 insertions(+), 448 deletions(-) create mode 100644 libs/ui/include/ui/LayerStack.h (limited to 'services/surfaceflinger/RefreshRateOverlay.cpp') diff --git a/cmds/surfacereplayer/replayer/Replayer.cpp b/cmds/surfacereplayer/replayer/Replayer.cpp index cfd42fec30..3f7c7d6a7b 100644 --- a/cmds/surfacereplayer/replayer/Replayer.cpp +++ b/cmds/surfacereplayer/replayer/Replayer.cpp @@ -528,7 +528,7 @@ void Replayer::setTransparentRegionHint(SurfaceComposerClient::Transaction& t, void Replayer::setLayerStack(SurfaceComposerClient::Transaction& t, layer_id id, const LayerStackChange& lsc) { ALOGV("Layer %d: Setting LayerStack -- layer_stack=%d", id, lsc.layer_stack()); - t.setLayerStack(mLayers[id], lsc.layer_stack()); + t.setLayerStack(mLayers[id], ui::LayerStack::fromValue(lsc.layer_stack())); } void Replayer::setHiddenFlag(SurfaceComposerClient::Transaction& t, @@ -566,7 +566,7 @@ void Replayer::setDisplaySurface(SurfaceComposerClient::Transaction& t, void Replayer::setDisplayLayerStack(SurfaceComposerClient::Transaction& t, display_id id, const LayerStackChange& lsc) { - t.setDisplayLayerStack(mDisplays[id], lsc.layer_stack()); + t.setDisplayLayerStack(mDisplays[id], ui::LayerStack::fromValue(lsc.layer_stack())); } void Replayer::setDisplaySize(SurfaceComposerClient::Transaction& t, diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 23895c0c9f..95a2f602b9 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -41,7 +41,6 @@ layer_state_t::layer_state_t() z(0), w(0), h(0), - layerStack(0), alpha(0), flags(0), mask(0), @@ -86,7 +85,7 @@ status_t layer_state_t::write(Parcel& output) const SAFE_PARCEL(output.writeInt32, z); SAFE_PARCEL(output.writeUint32, w); SAFE_PARCEL(output.writeUint32, h); - SAFE_PARCEL(output.writeUint32, layerStack); + SAFE_PARCEL(output.writeUint32, layerStack.id); SAFE_PARCEL(output.writeFloat, alpha); SAFE_PARCEL(output.writeUint32, flags); SAFE_PARCEL(output.writeUint32, mask); @@ -187,7 +186,7 @@ status_t layer_state_t::read(const Parcel& input) SAFE_PARCEL(input.readInt32, &z); SAFE_PARCEL(input.readUint32, &w); SAFE_PARCEL(input.readUint32, &h); - SAFE_PARCEL(input.readUint32, &layerStack); + SAFE_PARCEL(input.readUint32, &layerStack.id); SAFE_PARCEL(input.readFloat, &alpha); SAFE_PARCEL(input.readUint32, &flags); @@ -314,21 +313,14 @@ status_t ComposerState::read(const Parcel& input) { return state.read(input); } -DisplayState::DisplayState() - : what(0), - layerStack(0), - flags(0), - layerStackSpaceRect(Rect::EMPTY_RECT), - orientedDisplaySpaceRect(Rect::EMPTY_RECT), - width(0), - height(0) {} +DisplayState::DisplayState() = default; status_t DisplayState::write(Parcel& output) const { SAFE_PARCEL(output.writeStrongBinder, token); SAFE_PARCEL(output.writeStrongBinder, IInterface::asBinder(surface)); SAFE_PARCEL(output.writeUint32, what); - SAFE_PARCEL(output.writeUint32, layerStack); SAFE_PARCEL(output.writeUint32, flags); + SAFE_PARCEL(output.writeUint32, layerStack.id); SAFE_PARCEL(output.writeUint32, toRotationInt(orientation)); SAFE_PARCEL(output.write, layerStackSpaceRect); SAFE_PARCEL(output.write, orientedDisplaySpaceRect); @@ -344,8 +336,8 @@ status_t DisplayState::read(const Parcel& input) { surface = interface_cast(tmpBinder); SAFE_PARCEL(input.readUint32, &what); - SAFE_PARCEL(input.readUint32, &layerStack); SAFE_PARCEL(input.readUint32, &flags); + SAFE_PARCEL(input.readUint32, &layerStack.id); uint32_t tmpUint = 0; SAFE_PARCEL(input.readUint32, &tmpUint); orientation = ui::toRotation(tmpUint); diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index f620f5ae07..c3d632fa01 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -1087,7 +1087,7 @@ SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setAlpha } SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setLayerStack( - const sp& sc, uint32_t layerStack) { + const sp& sc, ui::LayerStack layerStack) { layer_state_t* s = getLayerState(sc); if (!s) { mStatus = BAD_INDEX; @@ -1751,7 +1751,7 @@ status_t SurfaceComposerClient::Transaction::setDisplaySurface(const sp } void SurfaceComposerClient::Transaction::setDisplayLayerStack(const sp& token, - uint32_t layerStack) { + ui::LayerStack layerStack) { DisplayState& s(getDisplayState(token)); s.layerStack = layerStack; s.what |= DisplayState::eLayerStackChanged; diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index b27d9cf2bc..427f2a5ce0 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -35,6 +35,7 @@ #include #include #include +#include #include #include #include @@ -143,7 +144,7 @@ struct layer_state_t { int32_t z; uint32_t w; uint32_t h; - uint32_t layerStack; + ui::LayerStack layerStack = ui::DEFAULT_LAYER_STACK; float alpha; uint32_t flags; uint32_t mask; @@ -262,11 +263,12 @@ struct DisplayState { DisplayState(); void merge(const DisplayState& other); - uint32_t what; + uint32_t what = 0; + uint32_t flags = 0; sp token; sp surface; - uint32_t layerStack; - uint32_t flags; + + ui::LayerStack layerStack = ui::DEFAULT_LAYER_STACK; // These states define how layers are projected onto the physical display. // @@ -280,10 +282,11 @@ struct DisplayState { // will be additionally rotated by 90 degrees around the origin clockwise and translated by (W, // 0). ui::Rotation orientation = ui::ROTATION_0; - Rect layerStackSpaceRect; - Rect orientedDisplaySpaceRect; + Rect layerStackSpaceRect = Rect::EMPTY_RECT; + Rect orientedDisplaySpaceRect = Rect::EMPTY_RECT; - uint32_t width, height; + uint32_t width = 0; + uint32_t height = 0; status_t write(Parcel& output) const; status_t read(const Parcel& input); diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 869cef6cdc..ec68ca9798 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -456,7 +456,7 @@ public: int backgroundBlurRadius); Transaction& setBlurRegions(const sp& sc, const std::vector& regions); - Transaction& setLayerStack(const sp& sc, uint32_t layerStack); + Transaction& setLayerStack(const sp&, ui::LayerStack); Transaction& setMetadata(const sp& sc, uint32_t key, const Parcel& p); /// Reparents the current layer to the new parent handle. The new parent must not be null. @@ -566,7 +566,7 @@ public: status_t setDisplaySurface(const sp& token, const sp& bufferProducer); - void setDisplayLayerStack(const sp& token, uint32_t layerStack); + void setDisplayLayerStack(const sp& token, ui::LayerStack); void setDisplayFlags(const sp& token, uint32_t flags); diff --git a/libs/gui/tests/BLASTBufferQueue_test.cpp b/libs/gui/tests/BLASTBufferQueue_test.cpp index 9082d275a2..26d902d849 100644 --- a/libs/gui/tests/BLASTBufferQueue_test.cpp +++ b/libs/gui/tests/BLASTBufferQueue_test.cpp @@ -128,7 +128,7 @@ protected: mDisplayToken = mClient->getInternalDisplayToken(); ASSERT_NE(nullptr, mDisplayToken.get()); Transaction t; - t.setDisplayLayerStack(mDisplayToken, 0); + t.setDisplayLayerStack(mDisplayToken, ui::DEFAULT_LAYER_STACK); t.apply(); t.clear(); @@ -142,7 +142,7 @@ protected: mDisplayHeight, PIXEL_FORMAT_RGBA_8888, ISurfaceComposerClient::eFXSurfaceBufferState, /*parent*/ nullptr); - t.setLayerStack(mSurfaceControl, 0) + t.setLayerStack(mSurfaceControl, ui::DEFAULT_LAYER_STACK) .setLayer(mSurfaceControl, std::numeric_limits::max()) .show(mSurfaceControl) .setDataspace(mSurfaceControl, ui::Dataspace::V0_SRGB) @@ -490,7 +490,7 @@ TEST_F(BLASTBufferQueueTest, SetCrop_ScalingModeScaleCrop) { ISurfaceComposerClient::eFXSurfaceEffect); ASSERT_NE(nullptr, bg.get()); Transaction t; - t.setLayerStack(bg, 0) + t.setLayerStack(bg, ui::DEFAULT_LAYER_STACK) .setCrop(bg, Rect(0, 0, mDisplayWidth, mDisplayHeight)) .setColor(bg, half3{0, 0, 0}) .setLayer(bg, 0) @@ -545,7 +545,7 @@ TEST_F(BLASTBufferQueueTest, ScaleCroppedBufferToBufferSize) { ISurfaceComposerClient::eFXSurfaceEffect); ASSERT_NE(nullptr, bg.get()); Transaction t; - t.setLayerStack(bg, 0) + t.setLayerStack(bg, ui::DEFAULT_LAYER_STACK) .setCrop(bg, Rect(0, 0, mDisplayWidth, mDisplayHeight)) .setColor(bg, half3{0, 0, 0}) .setLayer(bg, 0) @@ -612,7 +612,7 @@ TEST_F(BLASTBufferQueueTest, ScaleCroppedBufferToWindowSize) { ISurfaceComposerClient::eFXSurfaceEffect); ASSERT_NE(nullptr, bg.get()); Transaction t; - t.setLayerStack(bg, 0) + t.setLayerStack(bg, ui::DEFAULT_LAYER_STACK) .setCrop(bg, Rect(0, 0, mDisplayWidth, mDisplayHeight)) .setColor(bg, half3{0, 0, 0}) .setLayer(bg, 0) @@ -733,7 +733,7 @@ TEST_F(BLASTBufferQueueTest, OutOfOrderTransactionTest) { ISurfaceComposerClient::eFXSurfaceBufferState); ASSERT_NE(nullptr, bgSurface.get()); Transaction t; - t.setLayerStack(bgSurface, 0) + t.setLayerStack(bgSurface, ui::DEFAULT_LAYER_STACK) .show(bgSurface) .setDataspace(bgSurface, ui::Dataspace::V0_SRGB) .setLayer(bgSurface, std::numeric_limits::max() - 1) diff --git a/libs/ui/include/ui/DisplayState.h b/libs/ui/include/ui/DisplayState.h index 70a0d50611..98ee35652a 100644 --- a/libs/ui/include/ui/DisplayState.h +++ b/libs/ui/include/ui/DisplayState.h @@ -16,21 +16,18 @@ #pragma once +#include #include #include -#include #include namespace android::ui { -using LayerStack = uint32_t; -constexpr LayerStack NO_LAYER_STACK = static_cast(-1); - // Transactional state of physical or virtual display. Note that libgui defines // android::DisplayState as a superset of android::ui::DisplayState. struct DisplayState { - LayerStack layerStack = NO_LAYER_STACK; + LayerStack layerStack; Rotation orientation = ROTATION_0; Size layerStackSpaceRect; }; diff --git a/libs/ui/include/ui/LayerStack.h b/libs/ui/include/ui/LayerStack.h new file mode 100644 index 0000000000..d6ffeb7fad --- /dev/null +++ b/libs/ui/include/ui/LayerStack.h @@ -0,0 +1,78 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include + +#include +#include +#include + +namespace android::ui { + +// A LayerStack identifies a Z-ordered group of layers. A layer can only be associated to a single +// LayerStack, but a LayerStack can be associated to multiple displays, mirroring the same content. +struct LayerStack { + uint32_t id = UINT32_MAX; + + template + static constexpr LayerStack fromValue(T v) { + if (ftl::cast_safety(v) == ftl::CastSafety::kSafe) { + return {static_cast(v)}; + } + + ALOGW("Invalid layer stack %s", ftl::to_string(v).c_str()); + return {}; + } +}; + +constexpr LayerStack INVALID_LAYER_STACK; +constexpr LayerStack DEFAULT_LAYER_STACK{0u}; + +inline bool operator==(LayerStack lhs, LayerStack rhs) { + return lhs.id == rhs.id; +} + +inline bool operator!=(LayerStack lhs, LayerStack rhs) { + return !(lhs == rhs); +} + +inline bool operator>(LayerStack lhs, LayerStack rhs) { + return lhs.id > rhs.id; +} + +// A LayerFilter determines if a layer is included for output to a display. +struct LayerFilter { + LayerStack layerStack; + + // True if the layer is only output to internal displays, i.e. excluded from screenshots, screen + // recordings, and mirroring to virtual or external displays. Used for display cutout overlays. + bool toInternalDisplay = false; + + // Returns true if the input filter can be output to this filter. + bool includes(LayerFilter other) const { + // The layer stacks must match. + if (other.layerStack == INVALID_LAYER_STACK || other.layerStack != layerStack) { + return false; + } + + // The output must be to an internal display if the input filter has that constraint. + return !other.toInternalDisplay || toInternalDisplay; + } +}; + +} // namespace android::ui diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/DisplayCreationArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/DisplayCreationArgs.h index 526e7daa80..98c4af487e 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/DisplayCreationArgs.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/DisplayCreationArgs.h @@ -36,18 +36,12 @@ class CompositionEngine; struct DisplayCreationArgs { DisplayId id; - // Unset for virtual displays - std::optional connectionType; - // Size of the display in pixels ui::Size pixels = ui::kInvalidSize; // True if this display should be considered secure bool isSecure = false; - // Gives the initial layer stack id to be used for the display - uint32_t layerStackId = ~0u; - // Optional pointer to the power advisor interface, if one is needed for // this display. Hwc2::PowerAdvisor* powerAdvisor = nullptr; @@ -69,11 +63,6 @@ public: return *this; } - DisplayCreationArgsBuilder& setConnectionType(ui::DisplayConnectionType connectionType) { - mArgs.connectionType = connectionType; - return *this; - } - DisplayCreationArgsBuilder& setPixels(ui::Size pixels) { mArgs.pixels = pixels; return *this; @@ -84,11 +73,6 @@ public: return *this; } - DisplayCreationArgsBuilder& setLayerStackId(uint32_t layerStackId) { - mArgs.layerStackId = layerStackId; - return *this; - } - DisplayCreationArgsBuilder& setPowerAdvisor(Hwc2::PowerAdvisor* powerAdvisor) { mArgs.powerAdvisor = powerAdvisor; return *this; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h index a45be8a7a2..ecb4e6d79d 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/LayerFECompositionState.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -93,11 +94,9 @@ struct LayerFECompositionState { /* * Visibility state */ - // the layer stack this layer belongs to - std::optional layerStackId; - // If true, this layer should be only visible on the internal display - bool internalOnly{false}; + // The filter that determines which outputs include this layer + ui::LayerFilter outputFilter; // If false, this layer should not be considered visible bool isVisible{true}; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h index 15a86af037..28baef8726 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/Output.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -180,9 +181,8 @@ public: // output. virtual ui::Transform::RotationFlags getTransformHint() const = 0; - // Sets the layer stack filtering settings for this output. See - // belongsInOutput for full details. - virtual void setLayerStackFilter(uint32_t layerStackId, bool isInternal) = 0; + // Sets the filter for this output. See Output::includesLayer. + virtual void setLayerFilter(ui::LayerFilter) = 0; // Sets the output color mode virtual void setColorProfile(const ColorProfile&) = 0; @@ -224,17 +224,10 @@ public: // If repaintEverything is true, this will be the full display bounds. virtual Region getDirtyRegion(bool repaintEverything) const = 0; - // Tests whether a given layerStackId belongs in this output. - // A layer belongs to the output if its layerStackId matches the of the output layerStackId, - // unless the layer should display on the primary output only and this is not the primary output - - // A layer belongs to the output if its layerStackId matches. Additionally - // if the layer should only show in the internal (primary) display only and - // this output allows that. - virtual bool belongsInOutput(std::optional layerStackId, bool internalOnly) const = 0; - - // Determines if a layer belongs to the output. - virtual bool belongsInOutput(const sp&) const = 0; + // Returns whether the output includes a layer, based on their respective filters. + // See Output::setLayerFilter. + virtual bool includesLayer(ui::LayerFilter) const = 0; + virtual bool includesLayer(const sp&) const = 0; // Returns a pointer to the output layer corresponding to the given layer on // this output, or nullptr if the layer does not have one diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h index bb540ea7ee..b407267560 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Display.h @@ -83,9 +83,8 @@ public: std::unique_ptr createOutputLayer(const sp&) const; private: - bool mIsVirtual = false; - bool mIsDisconnected = false; DisplayId mId; + bool mIsDisconnected = false; Hwc2::PowerAdvisor* mPowerAdvisor = nullptr; }; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/DumpHelpers.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/DumpHelpers.h index 6b9597b2d5..7521324756 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/DumpHelpers.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/DumpHelpers.h @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -52,13 +53,14 @@ void dumpVal(std::string& out, const char* name, const std::string& valueName, E dumpVal(out, name, valueName, static_cast>(value)); } -void dumpVal(std::string& out, const char* name, const FloatRect& rect); -void dumpVal(std::string& out, const char* name, const Rect& rect); -void dumpVal(std::string& out, const char* name, const Region& region); -void dumpVal(std::string& out, const char* name, const ui::Transform&); -void dumpVal(std::string& out, const char* name, const ui::Size&); +void dumpVal(std::string& out, const char* name, ui::LayerFilter); +void dumpVal(std::string& out, const char* name, ui::Size); -void dumpVal(std::string& out, const char* name, const mat4& tr); +void dumpVal(std::string& out, const char* name, const FloatRect&); +void dumpVal(std::string& out, const char* name, const Rect&); +void dumpVal(std::string& out, const char* name, const Region&); +void dumpVal(std::string& out, const char* name, const ui::Transform&); +void dumpVal(std::string& out, const char* name, const mat4&); void dumpVal(std::string& out, const char* name, const StretchEffect&); } // namespace android::compositionengine::impl diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h index 14f2163f52..cb9426c73d 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/Output.h @@ -23,6 +23,7 @@ #include #include #include + #include #include #include @@ -45,7 +46,7 @@ public: void setProjection(ui::Rotation orientation, const Rect& layerStackSpaceRect, const Rect& orientedDisplaySpaceRect) override; void setDisplaySize(const ui::Size&) override; - void setLayerStackFilter(uint32_t layerStackId, bool isInternal) override; + void setLayerFilter(ui::LayerFilter) override; ui::Transform::RotationFlags getTransformHint() const override; void setColorTransform(const compositionengine::CompositionRefreshArgs&) override; @@ -65,8 +66,9 @@ public: void setRenderSurface(std::unique_ptr) override; Region getDirtyRegion(bool repaintEverything) const override; - bool belongsInOutput(std::optional, bool) const override; - bool belongsInOutput(const sp&) const override; + + bool includesLayer(ui::LayerFilter) const override; + bool includesLayer(const sp&) const override; compositionengine::OutputLayer* getOutputLayerForLayer(const sp&) const override; diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h index f34cb94079..44f754f936 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/impl/OutputCompositionState.h @@ -32,6 +32,7 @@ #pragma clang diagnostic pop // ignored "-Wconversion -Wextra" #include +#include #include #include #include @@ -59,11 +60,8 @@ struct OutputCompositionState { // If true, the current frame reused the buffer from a previous client composition bool reusedClientComposition{false}; - // If true, this output displays layers that are internal-only - bool layerStackInternal{false}; - - // The layer stack to display on this display - uint32_t layerStackId{~0u}; + // The conditions for including a layer on this output + ui::LayerFilter layerFilter; // The common space for all layers in the layer stack. layerStackSpace.content is the Rect // which gets projected on the display. The orientation of this space is always ROTATION_0. diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h index 216019fd1d..58627da828 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/mock/Output.h @@ -40,9 +40,12 @@ public: MOCK_METHOD1(setLayerCachingTexturePoolEnabled, void(bool)); MOCK_METHOD3(setProjection, void(ui::Rotation, const Rect&, const Rect&)); MOCK_METHOD1(setDisplaySize, void(const ui::Size&)); - MOCK_METHOD2(setLayerStackFilter, void(uint32_t, bool)); MOCK_CONST_METHOD0(getTransformHint, ui::Transform::RotationFlags()); + MOCK_METHOD(void, setLayerFilter, (ui::LayerFilter)); + MOCK_METHOD(bool, includesLayer, (ui::LayerFilter), (const)); + MOCK_METHOD(bool, includesLayer, (const sp&), (const)); + MOCK_METHOD1(setColorTransform, void(const compositionengine::CompositionRefreshArgs&)); MOCK_METHOD1(setColorProfile, void(const ColorProfile&)); MOCK_METHOD2(setDisplayBrightness, void(float, float)); @@ -63,8 +66,6 @@ public: MOCK_METHOD0(editState, OutputCompositionState&()); MOCK_CONST_METHOD1(getDirtyRegion, Region(bool)); - MOCK_CONST_METHOD2(belongsInOutput, bool(std::optional, bool)); - MOCK_CONST_METHOD1(belongsInOutput, bool(const sp&)); MOCK_CONST_METHOD1(getOutputLayerForLayer, compositionengine::OutputLayer*(const sp&)); diff --git a/services/surfaceflinger/CompositionEngine/src/Display.cpp b/services/surfaceflinger/CompositionEngine/src/Display.cpp index 2f2c686805..2cc5faef33 100644 --- a/services/surfaceflinger/CompositionEngine/src/Display.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Display.cpp @@ -51,12 +51,9 @@ Display::~Display() = default; void Display::setConfiguration(const compositionengine::DisplayCreationArgs& args) { mId = args.id; - mIsVirtual = !args.connectionType; mPowerAdvisor = args.powerAdvisor; editState().isSecure = args.isSecure; editState().displaySpace.bounds = Rect(args.pixels); - setLayerStackFilter(args.layerStackId, - args.connectionType == ui::DisplayConnectionType::Internal); setName(args.name); } @@ -73,7 +70,7 @@ bool Display::isSecure() const { } bool Display::isVirtual() const { - return mIsVirtual; + return VirtualDisplayId::tryCast(mId).has_value(); } std::optional Display::getDisplayId() const { @@ -117,8 +114,8 @@ void Display::setColorProfile(const ColorProfile& colorProfile) { return; } - if (mIsVirtual) { - ALOGW("%s: Invalid operation on virtual display", __FUNCTION__); + if (isVirtual()) { + ALOGW("%s: Invalid operation on virtual display", __func__); return; } @@ -136,7 +133,7 @@ void Display::dump(std::string& out) const { StringAppendF(&out, " Composition Display State: [\"%s\"]", getName().c_str()); out.append("\n "); - dumpVal(out, "isVirtual", mIsVirtual); + dumpVal(out, "isVirtual", isVirtual()); dumpVal(out, "DisplayId", to_string(mId)); out.append("\n"); diff --git a/services/surfaceflinger/CompositionEngine/src/DumpHelpers.cpp b/services/surfaceflinger/CompositionEngine/src/DumpHelpers.cpp index 5565396922..01c368de88 100644 --- a/services/surfaceflinger/CompositionEngine/src/DumpHelpers.cpp +++ b/services/surfaceflinger/CompositionEngine/src/DumpHelpers.cpp @@ -63,6 +63,18 @@ void dumpVal(std::string& out, const char* name, const std::string& valueName, i dumpVal(out, name, valueName.c_str(), value); } +void dumpVal(std::string& out, const char* name, ui::LayerFilter filter) { + out.append(name); + out.append("={"); + dumpVal(out, "layerStack", filter.layerStack.id); + dumpVal(out, "toInternalDisplay", filter.toInternalDisplay); + out.push_back('}'); +} + +void dumpVal(std::string& out, const char* name, ui::Size size) { + StringAppendF(&out, "%s=[%d %d] ", name, size.width, size.height); +} + void dumpVal(std::string& out, const char* name, const FloatRect& rect) { StringAppendF(&out, "%s=[%f %f %f %f] ", name, rect.left, rect.top, rect.right, rect.bottom); } @@ -80,10 +92,6 @@ void dumpVal(std::string& out, const char* name, const ui::Transform& transform) out.append(" "); } -void dumpVal(std::string& out, const char* name, const ui::Size& size) { - StringAppendF(&out, "%s=[%d %d] ", name, size.width, size.height); -} - void dumpVal(std::string& out, const char* name, const mat4& tr) { StringAppendF(&out, "%s=[" diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 6ac488be29..5e40802e93 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -227,11 +227,8 @@ ui::Transform::RotationFlags Output::getTransformHint() const { return static_cast(getState().transform.getOrientation()); } -void Output::setLayerStackFilter(uint32_t layerStackId, bool isInternal) { - auto& outputState = editState(); - outputState.layerStackId = layerStackId; - outputState.layerStackInternal = isInternal; - +void Output::setLayerFilter(ui::LayerFilter filter) { + editState().layerFilter = filter; dirtyEntireOutput(); } @@ -380,17 +377,13 @@ Region Output::getDirtyRegion(bool repaintEverything) const { return dirty; } -bool Output::belongsInOutput(std::optional layerStackId, bool internalOnly) const { - // The layerStackId's must match, and also the layer must not be internal - // only when not on an internal output. - const auto& outputState = getState(); - return layerStackId && (*layerStackId == outputState.layerStackId) && - (!internalOnly || outputState.layerStackInternal); +bool Output::includesLayer(ui::LayerFilter filter) const { + return getState().layerFilter.includes(filter); } -bool Output::belongsInOutput(const sp& layerFE) const { +bool Output::includesLayer(const sp& layerFE) const { const auto* layerFEState = layerFE->getCompositionState(); - return layerFEState && belongsInOutput(layerFEState->layerStackId, layerFEState->internalOnly); + return layerFEState && includesLayer(layerFEState->outputFilter); } std::unique_ptr Output::createOutputLayer( @@ -496,8 +489,8 @@ void Output::ensureOutputLayerIfVisible(sp& layerFE, layerFE->prepareCompositionState(compositionengine::LayerFE::StateSubset::BasicGeometry); } - // Only consider the layers on the given layer stack - if (!belongsInOutput(layerFE)) { + // Only consider the layers on this output + if (!includesLayer(layerFE)) { return; } @@ -1122,7 +1115,7 @@ std::optional Output::composeSurfaces( // bounds its framebuffer cache but Skia RenderEngine has no current policy. The best fix is // probably to encapsulate the output buffer into a structure that dispatches resource cleanup // over to RenderEngine, in which case this flag can be removed from the drawLayers interface. - const bool useFramebufferCache = outputState.layerStackInternal; + const bool useFramebufferCache = outputState.layerFilter.toInternalDisplay; status_t status = renderEngine.drawLayers(clientCompositionDisplay, clientCompositionLayerPointers, tex, useFramebufferCache, std::move(fd), &readyFence); diff --git a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp index ee30ad8583..acc92162ac 100644 --- a/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp +++ b/services/surfaceflinger/CompositionEngine/src/OutputCompositionState.cpp @@ -28,9 +28,7 @@ void OutputCompositionState::dump(std::string& out) const { dumpVal(out, "usesDeviceComposition", usesDeviceComposition); dumpVal(out, "flipClientTarget", flipClientTarget); dumpVal(out, "reusedClientComposition", reusedClientComposition); - - dumpVal(out, "layerStack", layerStackId); - dumpVal(out, "layerStackInternal", layerStackInternal); + dumpVal(out, "layerFilter", layerFilter); out.append("\n "); diff --git a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp index 72b16e0741..543dde1ca0 100644 --- a/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/DisplayTest.cpp @@ -61,7 +61,6 @@ constexpr HalVirtualDisplayId HAL_VIRTUAL_DISPLAY_ID{456u}; constexpr GpuVirtualDisplayId GPU_VIRTUAL_DISPLAY_ID{789u}; constexpr ui::Size DEFAULT_RESOLUTION{1920, 1080}; -constexpr uint32_t DEFAULT_LAYER_STACK = 42; struct Layer { Layer() { @@ -161,13 +160,11 @@ struct DisplayTestCommon : public testing::Test { EXPECT_CALL(mRenderEngine, isProtected()).WillRepeatedly(Return(false)); } - DisplayCreationArgs getDisplayCreationArgsForPhysicalHWCDisplay() { + DisplayCreationArgs getDisplayCreationArgsForPhysicalDisplay() { return DisplayCreationArgsBuilder() .setId(DEFAULT_DISPLAY_ID) - .setConnectionType(ui::DisplayConnectionType::Internal) .setPixels(DEFAULT_RESOLUTION) .setIsSecure(true) - .setLayerStackId(DEFAULT_LAYER_STACK) .setPowerAdvisor(&mPowerAdvisor) .build(); } @@ -177,7 +174,6 @@ struct DisplayTestCommon : public testing::Test { .setId(GPU_VIRTUAL_DISPLAY_ID) .setPixels(DEFAULT_RESOLUTION) .setIsSecure(false) - .setLayerStackId(DEFAULT_LAYER_STACK) .setPowerAdvisor(&mPowerAdvisor) .build(); } @@ -193,14 +189,13 @@ struct PartialMockDisplayTestCommon : public DisplayTestCommon { using Display = DisplayTestCommon::PartialMockDisplay; std::shared_ptr mDisplay = createPartialMockDisplay(mCompositionEngine, - getDisplayCreationArgsForPhysicalHWCDisplay()); + getDisplayCreationArgsForPhysicalDisplay()); }; struct FullDisplayImplTestCommon : public DisplayTestCommon { using Display = DisplayTestCommon::FullImplDisplay; std::shared_ptr mDisplay = - createDisplay(mCompositionEngine, - getDisplayCreationArgsForPhysicalHWCDisplay()); + createDisplay(mCompositionEngine, getDisplayCreationArgsForPhysicalDisplay()); }; struct DisplayWithLayersTestCommon : public FullDisplayImplTestCommon { @@ -218,8 +213,7 @@ struct DisplayWithLayersTestCommon : public FullDisplayImplTestCommon { LayerNoHWC2Layer mLayer3; StrictMock hwc2LayerUnknown; std::shared_ptr mDisplay = - createDisplay(mCompositionEngine, - getDisplayCreationArgsForPhysicalHWCDisplay()); + createDisplay(mCompositionEngine, getDisplayCreationArgsForPhysicalDisplay()); }; /* @@ -232,7 +226,7 @@ struct DisplayCreationTest : public DisplayTestCommon { TEST_F(DisplayCreationTest, createPhysicalInternalDisplay) { auto display = - impl::createDisplay(mCompositionEngine, getDisplayCreationArgsForPhysicalHWCDisplay()); + impl::createDisplay(mCompositionEngine, getDisplayCreationArgsForPhysicalDisplay()); EXPECT_TRUE(display->isSecure()); EXPECT_FALSE(display->isVirtual()); EXPECT_EQ(DEFAULT_DISPLAY_ID, display->getId()); @@ -252,13 +246,11 @@ TEST_F(DisplayCreationTest, createGpuVirtualDisplay) { using DisplaySetConfigurationTest = PartialMockDisplayTestCommon; -TEST_F(DisplaySetConfigurationTest, configuresInternalSecurePhysicalDisplay) { +TEST_F(DisplaySetConfigurationTest, configuresPhysicalDisplay) { mDisplay->setConfiguration(DisplayCreationArgsBuilder() .setId(DEFAULT_DISPLAY_ID) - .setConnectionType(ui::DisplayConnectionType::Internal) .setPixels(DEFAULT_RESOLUTION) .setIsSecure(true) - .setLayerStackId(DEFAULT_LAYER_STACK) .setPowerAdvisor(&mPowerAdvisor) .setName(getDisplayNameFromCurrentTest()) .build()); @@ -266,28 +258,11 @@ TEST_F(DisplaySetConfigurationTest, configuresInternalSecurePhysicalDisplay) { EXPECT_EQ(DEFAULT_DISPLAY_ID, mDisplay->getId()); EXPECT_TRUE(mDisplay->isSecure()); EXPECT_FALSE(mDisplay->isVirtual()); - EXPECT_EQ(DEFAULT_LAYER_STACK, mDisplay->getState().layerStackId); - EXPECT_TRUE(mDisplay->getState().layerStackInternal); EXPECT_FALSE(mDisplay->isValid()); -} -TEST_F(DisplaySetConfigurationTest, configuresExternalInsecurePhysicalDisplay) { - mDisplay->setConfiguration(DisplayCreationArgsBuilder() - .setId(DEFAULT_DISPLAY_ID) - .setConnectionType(ui::DisplayConnectionType::External) - .setPixels(DEFAULT_RESOLUTION) - .setIsSecure(false) - .setLayerStackId(DEFAULT_LAYER_STACK) - .setPowerAdvisor(&mPowerAdvisor) - .setName(getDisplayNameFromCurrentTest()) - .build()); - - EXPECT_EQ(DEFAULT_DISPLAY_ID, mDisplay->getId()); - EXPECT_FALSE(mDisplay->isSecure()); - EXPECT_FALSE(mDisplay->isVirtual()); - EXPECT_EQ(DEFAULT_LAYER_STACK, mDisplay->getState().layerStackId); - EXPECT_FALSE(mDisplay->getState().layerStackInternal); - EXPECT_FALSE(mDisplay->isValid()); + const auto& filter = mDisplay->getState().layerFilter; + EXPECT_EQ(ui::INVALID_LAYER_STACK, filter.layerStack); + EXPECT_FALSE(filter.toInternalDisplay); } TEST_F(DisplaySetConfigurationTest, configuresHalVirtualDisplay) { @@ -295,7 +270,6 @@ TEST_F(DisplaySetConfigurationTest, configuresHalVirtualDisplay) { .setId(HAL_VIRTUAL_DISPLAY_ID) .setPixels(DEFAULT_RESOLUTION) .setIsSecure(false) - .setLayerStackId(DEFAULT_LAYER_STACK) .setPowerAdvisor(&mPowerAdvisor) .setName(getDisplayNameFromCurrentTest()) .build()); @@ -303,9 +277,11 @@ TEST_F(DisplaySetConfigurationTest, configuresHalVirtualDisplay) { EXPECT_EQ(HAL_VIRTUAL_DISPLAY_ID, mDisplay->getId()); EXPECT_FALSE(mDisplay->isSecure()); EXPECT_TRUE(mDisplay->isVirtual()); - EXPECT_EQ(DEFAULT_LAYER_STACK, mDisplay->getState().layerStackId); - EXPECT_FALSE(mDisplay->getState().layerStackInternal); EXPECT_FALSE(mDisplay->isValid()); + + const auto& filter = mDisplay->getState().layerFilter; + EXPECT_EQ(ui::INVALID_LAYER_STACK, filter.layerStack); + EXPECT_FALSE(filter.toInternalDisplay); } TEST_F(DisplaySetConfigurationTest, configuresGpuVirtualDisplay) { @@ -313,7 +289,6 @@ TEST_F(DisplaySetConfigurationTest, configuresGpuVirtualDisplay) { .setId(GPU_VIRTUAL_DISPLAY_ID) .setPixels(DEFAULT_RESOLUTION) .setIsSecure(false) - .setLayerStackId(DEFAULT_LAYER_STACK) .setPowerAdvisor(&mPowerAdvisor) .setName(getDisplayNameFromCurrentTest()) .build()); @@ -321,9 +296,11 @@ TEST_F(DisplaySetConfigurationTest, configuresGpuVirtualDisplay) { EXPECT_EQ(GPU_VIRTUAL_DISPLAY_ID, mDisplay->getId()); EXPECT_FALSE(mDisplay->isSecure()); EXPECT_TRUE(mDisplay->isVirtual()); - EXPECT_EQ(DEFAULT_LAYER_STACK, mDisplay->getState().layerStackId); - EXPECT_FALSE(mDisplay->getState().layerStackInternal); EXPECT_FALSE(mDisplay->isValid()); + + const auto& filter = mDisplay->getState().layerFilter; + EXPECT_EQ(ui::INVALID_LAYER_STACK, filter.layerStack); + EXPECT_FALSE(filter.toInternalDisplay); } /* @@ -998,10 +975,8 @@ struct DisplayFunctionalTest : public testing::Test { Display>(mCompositionEngine, DisplayCreationArgsBuilder() .setId(DEFAULT_DISPLAY_ID) - .setConnectionType(ui::DisplayConnectionType::Internal) .setPixels(DEFAULT_RESOLUTION) .setIsSecure(true) - .setLayerStackId(DEFAULT_LAYER_STACK) .setPowerAdvisor(&mPowerAdvisor) .build()); diff --git a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp index c3185e9e7e..e7fad594a6 100644 --- a/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp +++ b/services/surfaceflinger/CompositionEngine/tests/OutputTest.cpp @@ -403,17 +403,18 @@ TEST_F(OutputTest, setDisplaySpaceSizeUpdatesOutputStateAndDirtiesEntireOutput) } /* - * Output::setLayerStackFilter() + * Output::setLayerFilter() */ -TEST_F(OutputTest, setLayerStackFilterSetsFilterAndDirtiesEntireOutput) { - const uint32_t layerStack = 123u; - mOutput->setLayerStackFilter(layerStack, true); +TEST_F(OutputTest, setLayerFilterSetsFilterAndDirtiesEntireOutput) { + constexpr ui::LayerFilter kFilter{ui::LayerStack{123u}, true}; + mOutput->setLayerFilter(kFilter); - EXPECT_TRUE(mOutput->getState().layerStackInternal); - EXPECT_EQ(layerStack, mOutput->getState().layerStackId); + const auto& state = mOutput->getState(); + EXPECT_EQ(kFilter.layerStack, state.layerFilter.layerStack); + EXPECT_TRUE(state.layerFilter.toInternalDisplay); - EXPECT_THAT(mOutput->getState().dirtyRegion, RegionEq(Region(kDefaultDisplaySize))); + EXPECT_THAT(state.dirtyRegion, RegionEq(Region(kDefaultDisplaySize))); } /* @@ -593,100 +594,90 @@ TEST_F(OutputTest, getDirtyRegionWithRepaintEverythingFalse) { } /* - * Output::belongsInOutput() + * Output::includesLayer() */ -TEST_F(OutputTest, belongsInOutputFiltersAsExpected) { - const uint32_t layerStack1 = 123u; - const uint32_t layerStack2 = 456u; +TEST_F(OutputTest, layerFiltering) { + const ui::LayerStack layerStack1{123u}; + const ui::LayerStack layerStack2{456u}; - // If the output accepts layerStack1 and internal-only layers.... - mOutput->setLayerStackFilter(layerStack1, true); + // If the output is associated to layerStack1 and to an internal display... + mOutput->setLayerFilter({layerStack1, true}); - // A layer with no layerStack does not belong to it, internal-only or not. - EXPECT_FALSE(mOutput->belongsInOutput(std::nullopt, false)); - EXPECT_FALSE(mOutput->belongsInOutput(std::nullopt, true)); + // It excludes layers with no layer stack, internal-only or not. + EXPECT_FALSE(mOutput->includesLayer({ui::INVALID_LAYER_STACK, false})); + EXPECT_FALSE(mOutput->includesLayer({ui::INVALID_LAYER_STACK, true})); - // Any layer with layerStack1 belongs to it, internal-only or not. - EXPECT_TRUE(mOutput->belongsInOutput(layerStack1, false)); - EXPECT_TRUE(mOutput->belongsInOutput(layerStack1, true)); - EXPECT_FALSE(mOutput->belongsInOutput(layerStack2, true)); - EXPECT_FALSE(mOutput->belongsInOutput(layerStack2, false)); + // It includes layers on layerStack1, internal-only or not. + EXPECT_TRUE(mOutput->includesLayer({layerStack1, false})); + EXPECT_TRUE(mOutput->includesLayer({layerStack1, true})); + EXPECT_FALSE(mOutput->includesLayer({layerStack2, true})); + EXPECT_FALSE(mOutput->includesLayer({layerStack2, false})); - // If the output accepts layerStack21 but not internal-only layers... - mOutput->setLayerStackFilter(layerStack1, false); + // If the output is associated to layerStack1 but not to an internal display... + mOutput->setLayerFilter({layerStack1, false}); - // Only non-internal layers with layerStack1 belong to it. - EXPECT_TRUE(mOutput->belongsInOutput(layerStack1, false)); - EXPECT_FALSE(mOutput->belongsInOutput(layerStack1, true)); - EXPECT_FALSE(mOutput->belongsInOutput(layerStack2, true)); - EXPECT_FALSE(mOutput->belongsInOutput(layerStack2, false)); + // It includes layers on layerStack1, unless they are internal-only. + EXPECT_TRUE(mOutput->includesLayer({layerStack1, false})); + EXPECT_FALSE(mOutput->includesLayer({layerStack1, true})); + EXPECT_FALSE(mOutput->includesLayer({layerStack2, true})); + EXPECT_FALSE(mOutput->includesLayer({layerStack2, false})); } -TEST_F(OutputTest, belongsInOutputHandlesLayerWithNoCompositionState) { +TEST_F(OutputTest, layerFilteringWithoutCompositionState) { NonInjectedLayer layer; sp layerFE(layer.layerFE); - // If the layer has no composition state, it does not belong to any output. + // Layers without composition state are excluded. EXPECT_CALL(*layer.layerFE, getCompositionState).WillOnce(Return(nullptr)); - EXPECT_FALSE(mOutput->belongsInOutput(layerFE)); + EXPECT_FALSE(mOutput->includesLayer(layerFE)); } -TEST_F(OutputTest, belongsInOutputFiltersLayersAsExpected) { +TEST_F(OutputTest, layerFilteringWithCompositionState) { NonInjectedLayer layer; sp layerFE(layer.layerFE); - const uint32_t layerStack1 = 123u; - const uint32_t layerStack2 = 456u; + const ui::LayerStack layerStack1{123u}; + const ui::LayerStack layerStack2{456u}; - // If the output accepts layerStack1 and internal-only layers.... - mOutput->setLayerStackFilter(layerStack1, true); + // If the output is associated to layerStack1 and to an internal display... + mOutput->setLayerFilter({layerStack1, true}); - // A layer with no layerStack does not belong to it, internal-only or not. - layer.layerFEState.layerStackId = std::nullopt; - layer.layerFEState.internalOnly = false; - EXPECT_FALSE(mOutput->belongsInOutput(layerFE)); + // It excludes layers with no layer stack, internal-only or not. + layer.layerFEState.outputFilter = {ui::INVALID_LAYER_STACK, false}; + EXPECT_FALSE(mOutput->includesLayer(layerFE)); - layer.layerFEState.layerStackId = std::nullopt; - layer.layerFEState.internalOnly = true; - EXPECT_FALSE(mOutput->belongsInOutput(layerFE)); + layer.layerFEState.outputFilter = {ui::INVALID_LAYER_STACK, true}; + EXPECT_FALSE(mOutput->includesLayer(layerFE)); - // Any layer with layerStack1 belongs to it, internal-only or not. - layer.layerFEState.layerStackId = layerStack1; - layer.layerFEState.internalOnly = false; - EXPECT_TRUE(mOutput->belongsInOutput(layerFE)); + // It includes layers on layerStack1, internal-only or not. + layer.layerFEState.outputFilter = {layerStack1, false}; + EXPECT_TRUE(mOutput->includesLayer(layerFE)); - layer.layerFEState.layerStackId = layerStack1; - layer.layerFEState.internalOnly = true; - EXPECT_TRUE(mOutput->belongsInOutput(layerFE)); + layer.layerFEState.outputFilter = {layerStack1, true}; + EXPECT_TRUE(mOutput->includesLayer(layerFE)); - layer.layerFEState.layerStackId = layerStack2; - layer.layerFEState.internalOnly = true; - EXPECT_FALSE(mOutput->belongsInOutput(layerFE)); + layer.layerFEState.outputFilter = {layerStack2, true}; + EXPECT_FALSE(mOutput->includesLayer(layerFE)); - layer.layerFEState.layerStackId = layerStack2; - layer.layerFEState.internalOnly = false; - EXPECT_FALSE(mOutput->belongsInOutput(layerFE)); + layer.layerFEState.outputFilter = {layerStack2, false}; + EXPECT_FALSE(mOutput->includesLayer(layerFE)); - // If the output accepts layerStack1 but not internal-only layers... - mOutput->setLayerStackFilter(layerStack1, false); + // If the output is associated to layerStack1 but not to an internal display... + mOutput->setLayerFilter({layerStack1, false}); - // Only non-internal layers with layerStack1 belong to it. - layer.layerFEState.layerStackId = layerStack1; - layer.layerFEState.internalOnly = false; - EXPECT_TRUE(mOutput->belongsInOutput(layerFE)); + // It includes layers on layerStack1, unless they are internal-only. + layer.layerFEState.outputFilter = {layerStack1, false}; + EXPECT_TRUE(mOutput->includesLayer(layerFE)); - layer.layerFEState.layerStackId = layerStack1; - layer.layerFEState.internalOnly = true; - EXPECT_FALSE(mOutput->belongsInOutput(layerFE)); + layer.layerFEState.outputFilter = {layerStack1, true}; + EXPECT_FALSE(mOutput->includesLayer(layerFE)); - layer.layerFEState.layerStackId = layerStack2; - layer.layerFEState.internalOnly = true; - EXPECT_FALSE(mOutput->belongsInOutput(layerFE)); + layer.layerFEState.outputFilter = {layerStack2, true}; + EXPECT_FALSE(mOutput->includesLayer(layerFE)); - layer.layerFEState.layerStackId = layerStack2; - layer.layerFEState.internalOnly = false; - EXPECT_FALSE(mOutput->belongsInOutput(layerFE)); + layer.layerFEState.outputFilter = {layerStack2, false}; + EXPECT_FALSE(mOutput->includesLayer(layerFE)); } /* @@ -1268,14 +1259,15 @@ struct OutputEnsureOutputLayerIfVisibleTest : public testing::Test { struct OutputPartialMock : public OutputPartialMockBase { // Sets up the helper functions called by the function under test to use // mock implementations. - MOCK_CONST_METHOD1(belongsInOutput, bool(const sp&)); + MOCK_METHOD(bool, includesLayer, (const sp&), + (const, override)); MOCK_CONST_METHOD1(getOutputLayerOrderedByZByIndex, OutputLayer*(size_t)); MOCK_METHOD2(ensureOutputLayer, compositionengine::OutputLayer*(std::optional, const sp&)); }; OutputEnsureOutputLayerIfVisibleTest() { - EXPECT_CALL(mOutput, belongsInOutput(sp(mLayer.layerFE))) + EXPECT_CALL(mOutput, includesLayer(sp(mLayer.layerFE))) .WillRepeatedly(Return(true)); EXPECT_CALL(mOutput, getOutputLayerCount()).WillRepeatedly(Return(1u)); EXPECT_CALL(mOutput, getOutputLayerOrderedByZByIndex(0u)) @@ -1326,8 +1318,8 @@ const Region OutputEnsureOutputLayerIfVisibleTest::kLowerHalfBoundsNoRotation = const Region OutputEnsureOutputLayerIfVisibleTest::kFullBounds90Rotation = Region(Rect(0, 0, 200, 100)); -TEST_F(OutputEnsureOutputLayerIfVisibleTest, performsGeomLatchBeforeCheckingIfLayerBelongs) { - EXPECT_CALL(mOutput, belongsInOutput(sp(mLayer.layerFE))).WillOnce(Return(false)); +TEST_F(OutputEnsureOutputLayerIfVisibleTest, performsGeomLatchBeforeCheckingIfLayerIncluded) { + EXPECT_CALL(mOutput, includesLayer(sp(mLayer.layerFE))).WillOnce(Return(false)); EXPECT_CALL(*mLayer.layerFE, prepareCompositionState(compositionengine::LayerFE::StateSubset::BasicGeometry)); @@ -1337,8 +1329,8 @@ TEST_F(OutputEnsureOutputLayerIfVisibleTest, performsGeomLatchBeforeCheckingIfLa } TEST_F(OutputEnsureOutputLayerIfVisibleTest, - skipsLatchIfAlreadyLatchedBeforeCheckingIfLayerBelongs) { - EXPECT_CALL(mOutput, belongsInOutput(sp(mLayer.layerFE))).WillOnce(Return(false)); + skipsLatchIfAlreadyLatchedBeforeCheckingIfLayerIncluded) { + EXPECT_CALL(mOutput, includesLayer(sp(mLayer.layerFE))).WillOnce(Return(false)); ensureOutputLayerIfVisible(); } @@ -3188,8 +3180,7 @@ TEST_F(OutputComposeSurfacesTest, r1.geometry.boundaries = FloatRect{1, 2, 3, 4}; r2.geometry.boundaries = FloatRect{5, 6, 7, 8}; - const constexpr uint32_t kInternalLayerStack = 1234; - mOutput.setLayerStackFilter(kInternalLayerStack, true); + mOutput.setLayerFilter({ui::LayerStack{1234u}, true}); EXPECT_CALL(mOutput, getSkipColorTransform()).WillRepeatedly(Return(false)); EXPECT_CALL(*mDisplayColorProfile, hasWideColorGamut()).WillRepeatedly(Return(true)); diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 4445eea604..802a17d981 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -232,7 +232,7 @@ ui::Dataspace DisplayDevice::getCompositionDataSpace() const { } void DisplayDevice::setLayerStack(ui::LayerStack stack) { - mCompositionDisplay->setLayerStackFilter(stack, isInternal()); + mCompositionDisplay->setLayerFilter({stack, isInternal()}); if (mRefreshRateOverlay) { mRefreshRateOverlay->setLayerStack(stack); } @@ -349,7 +349,7 @@ bool DisplayDevice::needsFiltering() const { } ui::LayerStack DisplayDevice::getLayerStack() const { - return mCompositionDisplay->getState().layerStackId; + return mCompositionDisplay->getState().layerFilter.layerStack; } ui::Transform::RotationFlags DisplayDevice::getTransformHint() const { diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 7db87d9123..43a6bd540a 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -86,9 +86,7 @@ public: bool isVirtual() const { return !mConnectionType; } bool isPrimary() const { return mIsPrimary; } - bool isInternal() const { - return !isVirtual() && mConnectionType == ui::DisplayConnectionType::Internal; - } + bool isInternal() const { return mConnectionType == ui::DisplayConnectionType::Internal; } // isSecure indicates whether this display can be trusted to display // secure surfaces. @@ -311,7 +309,7 @@ struct DisplayDeviceState { int32_t sequenceId = sNextSequenceId++; std::optional physical; sp surface; - ui::LayerStack layerStack = ui::NO_LAYER_STACK; + ui::LayerStack layerStack; uint32_t flags = 0; Rect layerStackSpaceRect; Rect orientedDisplaySpaceRect; diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 28c8213fc9..51568547b4 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -107,7 +107,7 @@ Layer::Layer(const LayerCreationArgs& args) mDrawingState.requestedCrop = mDrawingState.crop; mDrawingState.z = 0; mDrawingState.color.a = 1.0f; - mDrawingState.layerStack = 0; + mDrawingState.layerStack = ui::DEFAULT_LAYER_STACK; mDrawingState.sequence = 0; mDrawingState.requested_legacy = mDrawingState.active_legacy; mDrawingState.width = UINT32_MAX; @@ -391,7 +391,6 @@ void Layer::setupRoundedCornersCropCoordinates(Rect win, void Layer::prepareBasicGeometryCompositionState() { const auto& drawingState{getDrawingState()}; - const uint32_t layerStack = getLayerStack(); const auto alpha = static_cast(getAlpha()); const bool opaque = isOpaque(drawingState); const bool usesRoundedCorners = getRoundedCornerState().radius != 0.f; @@ -403,9 +402,7 @@ void Layer::prepareBasicGeometryCompositionState() { } auto* compositionState = editCompositionState(); - compositionState->layerStackId = - (layerStack != ~0u) ? std::make_optional(layerStack) : std::nullopt; - compositionState->internalOnly = getPrimaryDisplayOnly(); + compositionState->outputFilter = getOutputFilter(); compositionState->isVisible = isVisible(); compositionState->isOpaque = opaque && !usesRoundedCorners && alpha == 1.f; compositionState->shadowRadius = mEffectiveShadowRadius; @@ -1005,7 +1002,7 @@ bool Layer::setMetadata(const LayerMetadata& data) { return true; } -bool Layer::setLayerStack(uint32_t layerStack) { +bool Layer::setLayerStack(ui::LayerStack layerStack) { if (mDrawingState.layerStack == layerStack) return false; mDrawingState.sequence++; mDrawingState.layerStack = layerStack; @@ -1052,12 +1049,11 @@ bool Layer::isLayerFocusedBasedOnPriority(int32_t priority) { return priority == PRIORITY_FOCUSED_WITH_MODE || priority == PRIORITY_FOCUSED_WITHOUT_MODE; }; -uint32_t Layer::getLayerStack() const { - auto p = mDrawingParent.promote(); - if (p == nullptr) { - return getDrawingState().layerStack; +ui::LayerStack Layer::getLayerStack() const { + if (const auto parent = mDrawingParent.promote()) { + return parent->getLayerStack(); } - return p->getLayerStack(); + return getDrawingState().layerStack; } bool Layer::setShadowRadius(float shadowRadius) { @@ -1371,7 +1367,7 @@ LayerDebugInfo Layer::getLayerDebugInfo(const DisplayDevice* display) const { info.mVisibleRegion = getVisibleRegion(display); info.mSurfaceDamageRegion = surfaceDamageRegion; - info.mLayerStack = getLayerStack(); + info.mLayerStack = getLayerStack().id; info.mX = ds.transform.tx(); info.mY = ds.transform.ty(); info.mZ = ds.z; @@ -2087,7 +2083,7 @@ void Layer::writeToProtoCommonState(LayerProto* layerInfo, LayerVector::StateSet LayerProtoHelper::writeToProto(state.activeTransparentRegion_legacy, [&]() { return layerInfo->mutable_transparent_region(); }); - layerInfo->set_layer_stack(getLayerStack()); + layerInfo->set_layer_stack(getLayerStack().id); layerInfo->set_z(state.z); LayerProtoHelper::writePositionToProto(requestedTransform.tx(), requestedTransform.ty(), @@ -2134,7 +2130,7 @@ void Layer::writeToProtoCommonState(LayerProto* layerInfo, LayerVector::StateSet if (traceFlags & SurfaceTracing::TRACE_INPUT) { WindowInfo info; if (useDrawing) { - info = fillInputInfo({nullptr}); + info = fillInputInfo(nullptr); } else { info = state.inputInfo; } @@ -2163,7 +2159,7 @@ Rect Layer::getInputBounds() const { return getCroppedBufferSize(getDrawingState()); } -void Layer::fillInputFrameInfo(WindowInfo& info, const ui::Transform& toPhysicalDisplay) { +void Layer::fillInputFrameInfo(WindowInfo& info, const ui::Transform& displayTransform) { // Transform layer size to screen space and inset it by surface insets. // If this is a portal window, set the touchableRegion to the layerBounds. Rect layerBounds = info.portalToDisplayId == ADISPLAY_ID_NONE @@ -2183,13 +2179,13 @@ void Layer::fillInputFrameInfo(WindowInfo& info, const ui::Transform& toPhysical return; } - ui::Transform layerToDisplay = getInputTransform(); + const ui::Transform layerTransform = getInputTransform(); // Transform that takes window coordinates to unrotated display coordinates - ui::Transform t = toPhysicalDisplay * layerToDisplay; + ui::Transform t = displayTransform * layerTransform; int32_t xSurfaceInset = info.surfaceInset; int32_t ySurfaceInset = info.surfaceInset; // Bring screenBounds into unrotated space - Rect screenBounds = toPhysicalDisplay.transform(Rect{mScreenBounds}); + Rect screenBounds = displayTransform.transform(Rect{mScreenBounds}); const float xScale = t.getScaleX(); const float yScale = t.getScaleY(); @@ -2268,33 +2264,33 @@ void Layer::fillTouchOcclusionMode(WindowInfo& info) { } } -WindowInfo Layer::fillInputInfo(const sp& display) { +WindowInfo Layer::fillInputInfo(const DisplayDevice* display) { if (!hasInputInfo()) { mDrawingState.inputInfo.name = getName(); mDrawingState.inputInfo.ownerUid = mOwnerUid; mDrawingState.inputInfo.ownerPid = mOwnerPid; mDrawingState.inputInfo.inputFeatures = WindowInfo::Feature::NO_INPUT_CHANNEL; mDrawingState.inputInfo.flags = WindowInfo::Flag::NOT_TOUCH_MODAL; - mDrawingState.inputInfo.displayId = getLayerStack(); + mDrawingState.inputInfo.displayId = getLayerStack().id; } WindowInfo info = mDrawingState.inputInfo; info.id = sequence; - info.displayId = getLayerStack(); + info.displayId = getLayerStack().id; - // Transform that goes from "logical(rotated)" display to physical/unrotated display. - // This is for when inputflinger operates in physical display-space. - ui::Transform toPhysicalDisplay; + // Transform that maps from LayerStack space to display space, e.g. rotated to unrotated. + // Used when InputFlinger operates in display space. + ui::Transform displayTransform; if (display) { - toPhysicalDisplay = display->getTransform(); + displayTransform = display->getTransform(); // getOrientation() without masking can contain more-significant bits (eg. ROT_INVALID). - static constexpr uint32_t ALL_ROTATIONS_MASK = + constexpr uint32_t kAllRotationsMask = ui::Transform::ROT_90 | ui::Transform::ROT_180 | ui::Transform::ROT_270; - info.displayOrientation = toPhysicalDisplay.getOrientation() & ALL_ROTATIONS_MASK; + info.displayOrientation = displayTransform.getOrientation() & kAllRotationsMask; info.displayWidth = display->getWidth(); info.displayHeight = display->getHeight(); } - fillInputFrameInfo(info, toPhysicalDisplay); + fillInputFrameInfo(info, displayTransform); // For compatibility reasons we let layers which can receive input // receive input before they have actually submitted a buffer. Because @@ -2310,15 +2306,11 @@ WindowInfo Layer::fillInputInfo(const sp& display) { auto cropLayer = mDrawingState.touchableRegionCrop.promote(); if (info.replaceTouchableRegionWithCrop) { - if (cropLayer == nullptr) { - info.touchableRegion = Region(toPhysicalDisplay.transform(Rect{mScreenBounds})); - } else { - info.touchableRegion = - Region(toPhysicalDisplay.transform(Rect{cropLayer->mScreenBounds})); - } + const Rect bounds(cropLayer ? cropLayer->mScreenBounds : mScreenBounds); + info.touchableRegion = Region(displayTransform.transform(bounds)); } else if (cropLayer != nullptr) { info.touchableRegion = info.touchableRegion.intersect( - toPhysicalDisplay.transform(Rect{cropLayer->mScreenBounds})); + displayTransform.transform(Rect{cropLayer->mScreenBounds})); } // Inherit the trusted state from the parent hierarchy, but don't clobber the trusted state @@ -2329,9 +2321,8 @@ WindowInfo Layer::fillInputInfo(const sp& display) { // If the layer is a clone, we need to crop the input region to cloned root to prevent // touches from going outside the cloned area. if (isClone()) { - sp clonedRoot = getClonedRoot(); - if (clonedRoot != nullptr) { - Rect rect = toPhysicalDisplay.transform(Rect{clonedRoot->mScreenBounds}); + if (const sp clonedRoot = getClonedRoot()) { + const Rect rect = displayTransform.transform(Rect{clonedRoot->mScreenBounds}); info.touchableRegion = info.touchableRegion.intersect(rect); } } @@ -2527,14 +2518,14 @@ scheduler::Seamlessness Layer::FrameRate::convertChangeFrameRateStrategy(int8_t } } -bool Layer::getPrimaryDisplayOnly() const { +bool Layer::isInternalDisplayOverlay() const { const State& s(mDrawingState); if (s.flags & layer_state_t::eLayerSkipScreenshot) { return true; } sp parent = mDrawingParent.promote(); - return parent == nullptr ? false : parent->getPrimaryDisplayOnly(); + return parent && parent->isInternalDisplayOverlay(); } void Layer::setClonedChild(const sp& clonedChild) { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index 41bdebdf10..982f246b00 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -151,12 +151,7 @@ public: Geometry requested_legacy; int32_t z; - // The identifier of the layer stack this layer belongs to. A layer can - // only be associated to a single layer stack. A layer stack is a - // z-ordered group of layers which can be associated to one or more - // displays. Using the same layer stack on different displays is a way - // to achieve mirroring. - uint32_t layerStack; + ui::LayerStack layerStack; uint32_t flags; uint8_t reserved[2]; @@ -403,8 +398,8 @@ public: virtual bool setTransparentRegionHint(const Region& transparent); virtual bool setTrustedOverlay(bool); virtual bool setFlags(uint32_t flags, uint32_t mask); - virtual bool setLayerStack(uint32_t layerStack); - virtual uint32_t getLayerStack() const; + virtual bool setLayerStack(ui::LayerStack); + virtual ui::LayerStack getLayerStack() const; virtual bool setMetadata(const LayerMetadata& data); virtual void setChildrenDrawingParent(const sp&); virtual bool reparent(const sp& newParentHandle); @@ -645,11 +640,6 @@ public: uint32_t getTransactionFlags(uint32_t flags); uint32_t setTransactionFlags(uint32_t flags); - // Deprecated, please use compositionengine::Output::belongsInOutput() - // instead. - // TODO(lpique): Move the remaining callers (screencap) to the new function. - bool belongsToDisplay(uint32_t layerStack) const { return getLayerStack() == layerStack; } - FloatRect getBounds(const Region& activeTransparentRegion) const; FloatRect getBounds() const; @@ -681,6 +671,14 @@ public: */ bool isHiddenByPolicy() const; + // True if the layer should be skipped in screenshots, screen recordings, + // and mirroring to external or virtual displays. + bool isInternalDisplayOverlay() const; + + ui::LayerFilter getOutputFilter() const { + return {getLayerStack(), isInternalDisplayOverlay()}; + } + bool isRemovedFromCurrentState() const; LayerProto* writeToProto(LayersProto& layersProto, uint32_t traceFlags, const DisplayDevice*); @@ -697,8 +695,6 @@ public: gui::WindowInfo::Type getWindowType() const { return mWindowType; } - bool getPrimaryDisplayOnly() const; - void updateMirrorInfo(); /* @@ -848,7 +844,8 @@ public: bool getPremultipledAlpha() const; void setInputInfo(const gui::WindowInfo& info); - gui::WindowInfo fillInputInfo(const sp& display); + gui::WindowInfo fillInputInfo(const DisplayDevice*); + /** * Returns whether this layer has an explicitly set input-info. */ @@ -1069,8 +1066,8 @@ private: // hasInputInfo() or no-op if no such parent is found. void fillTouchOcclusionMode(gui::WindowInfo& info); - // Fills in the frame and transform info for the gui::WindowInfo - void fillInputFrameInfo(gui::WindowInfo& info, const ui::Transform& toPhysicalDisplay); + // Fills in the frame and transform info for the gui::WindowInfo. + void fillInputFrameInfo(gui::WindowInfo&, const ui::Transform& displayTransform); // Cached properties computed from drawing state // Effective transform taking into account parent transforms and any parent scaling, which is diff --git a/services/surfaceflinger/LayerVector.cpp b/services/surfaceflinger/LayerVector.cpp index aee820a88c..f52e60deda 100644 --- a/services/surfaceflinger/LayerVector.cpp +++ b/services/surfaceflinger/LayerVector.cpp @@ -45,8 +45,8 @@ int LayerVector::do_compare(const void* lhs, const void* rhs) const const auto& lState = l->getDrawingState(); const auto& rState = r->getDrawingState(); - uint32_t ls = lState.layerStack; - uint32_t rs = rState.layerStack; + const auto ls = lState.layerStack; + const auto rs = rState.layerStack; if (ls != rs) return (ls > rs) ? 1 : -1; diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 2adfe14ef7..40d4c61401 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -281,7 +281,7 @@ void RefreshRateOverlay::setViewport(ui::Size viewport) { mFlinger.mTransactionFlags.fetch_or(eTransactionMask); } -void RefreshRateOverlay::setLayerStack(uint32_t stack) { +void RefreshRateOverlay::setLayerStack(ui::LayerStack stack) { mLayer->setLayerStack(stack); mFlinger.mTransactionFlags.fetch_or(eTransactionMask); } diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h index fd1e2df96d..63ae383c8c 100644 --- a/services/surfaceflinger/RefreshRateOverlay.h +++ b/services/surfaceflinger/RefreshRateOverlay.h @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -41,7 +42,7 @@ class RefreshRateOverlay { public: RefreshRateOverlay(SurfaceFlinger&, uint32_t lowFps, uint32_t highFps, bool showSpinner); - void setLayerStack(uint32_t stack); + void setLayerStack(ui::LayerStack); void setViewport(ui::Size); void changeRefreshRate(const Fps&); void onInvalidate(); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 99d9bb3463..417bffc998 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -2295,7 +2295,7 @@ void SurfaceFlinger::postComposition() { int32_t maxArea = 0; mDrawingState.traverse([&, compositionDisplay = compositionDisplay](Layer* layer) { const auto layerFe = layer->getCompositionEngineLayerFE(); - if (layer->isVisible() && compositionDisplay->belongsInOutput(layerFe)) { + if (layer->isVisible() && compositionDisplay->includesLayer(layerFe)) { const Dataspace transfer = static_cast(layer->getDataSpace() & Dataspace::TRANSFER_MASK); const bool isHdr = (transfer == Dataspace::TRANSFER_ST2084 || @@ -2426,8 +2426,8 @@ void SurfaceFlinger::computeLayerBounds() { const auto& displayDevice = pair.second; const auto display = displayDevice->getCompositionDisplay(); for (const auto& layer : mDrawingState.layersSortedByZ) { - // only consider the layers on the given layer stack - if (!display->belongsInOutput(layer->getLayerStack(), layer->getPrimaryDisplayOnly())) { + // Only consider the layers on this display. + if (!display->includesLayer(layer->getOutputFilter())) { continue; } @@ -2732,14 +2732,12 @@ void SurfaceFlinger::processDisplayAdded(const wp& displayToken, compositionengine::DisplayCreationArgsBuilder builder; if (const auto& physical = state.physical) { builder.setId(physical->id); - builder.setConnectionType(physical->type); } else { builder.setId(acquireVirtualDisplay(resolution, pixelFormat)); } builder.setPixels(resolution); builder.setIsSecure(state.isSecure); - builder.setLayerStackId(state.layerStack); builder.setPowerAdvisor(&mPowerAdvisor); builder.setName(state.displayName); auto compositionDisplay = getCompositionEngine().createDisplay(builder.build()); @@ -2942,16 +2940,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) { // Update transform hint if (transactionFlags & (eTransformHintUpdateNeeded | eDisplayTransactionNeeded)) { - // The transform hint might have changed for some layers - // (either because a display has changed, or because a layer - // as changed). - // - // Walk through all the layers in currentLayers, - // and update their transform hint. - // - // If a layer is visible only on a single display, then that - // display is used to calculate the hint, otherwise we use the - // default display. + // Layers and/or displays have changed, so update the transform hint for each layer. // // NOTE: we do this here, rather than when presenting the display so that // the hint is set before we acquire a buffer from the surface texture. @@ -2962,30 +2951,29 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) { // (soon to become the drawing state list). // sp hintDisplay; - uint32_t currentlayerStack = 0; - bool first = true; + ui::LayerStack layerStack; + mCurrentState.traverse([&](Layer* layer) REQUIRES(mStateLock) { // NOTE: we rely on the fact that layers are sorted by // layerStack first (so we don't have to traverse the list // of displays for every layer). - uint32_t layerStack = layer->getLayerStack(); - if (first || currentlayerStack != layerStack) { - currentlayerStack = layerStack; - // figure out if this layerstack is mirrored - // (more than one display) if so, pick the default display, - // if not, pick the only display it's on. + if (const auto filter = layer->getOutputFilter(); layerStack != filter.layerStack) { + layerStack = filter.layerStack; hintDisplay = nullptr; + + // Find the display that includes the layer. for (const auto& [token, display] : mDisplays) { - if (display->getCompositionDisplay() - ->belongsInOutput(layer->getLayerStack(), - layer->getPrimaryDisplayOnly())) { - if (hintDisplay) { - hintDisplay = nullptr; - break; - } else { - hintDisplay = display; - } + if (!display->getCompositionDisplay()->includesLayer(filter)) { + continue; } + + // Pick the primary display if another display mirrors the layer. + if (hintDisplay) { + hintDisplay = nullptr; + break; + } + + hintDisplay = display; } } @@ -2999,13 +2987,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags) { hintDisplay = getDefaultDisplayDeviceLocked(); } - // could be null if there is no display available at all to get - // the transform hint from. - if (hintDisplay) { - layer->updateTransformHint(hintDisplay->getTransformHint()); - } - - first = false; + layer->updateTransformHint(hintDisplay->getTransformHint()); }); } @@ -3069,9 +3051,12 @@ void SurfaceFlinger::notifyWindowInfos() { mDrawingState.traverseInReverseZOrder([&](Layer* layer) { if (!layer->needsInputInfo()) return; - sp display = enablePerWindowInputRotation() - ? ON_MAIN_THREAD(getDisplayWithInputByLayer(layer)) - : nullptr; + + const DisplayDevice* display = nullptr; + if (enablePerWindowInputRotation()) { + display = ON_MAIN_THREAD(getDisplayWithInputByLayer(layer)).get(); + } + // When calculating the screen bounds we ignore the transparent region since it may // result in an unwanted offset. windowInfos.push_back(layer->fillInputInfo(display)); @@ -3243,7 +3228,7 @@ void SurfaceFlinger::commitOffscreenLayers() { void SurfaceFlinger::invalidateLayerStack(const sp& layer, const Region& dirty) { for (const auto& [token, displayDevice] : ON_MAIN_THREAD(mDisplays)) { auto display = displayDevice->getCompositionDisplay(); - if (display->belongsInOutput(layer->getLayerStack(), layer->getPrimaryDisplayOnly())) { + if (display->includesLayer(layer->getOutputFilter())) { display->editState().dirtyRegion.orSelf(dirty); } } @@ -4464,7 +4449,7 @@ void SurfaceFlinger::onInitializeDisplays() { d.what = DisplayState::eDisplayProjectionChanged | DisplayState::eLayerStackChanged; d.token = token; - d.layerStack = 0; + d.layerStack = ui::DEFAULT_LAYER_STACK; d.orientation = ui::ROTATION_0; d.orientedDisplaySpaceRect.makeInvalid(); d.layerStackSpaceRect.makeInvalid(); @@ -4495,23 +4480,22 @@ void SurfaceFlinger::initializeDisplays() { } sp SurfaceFlinger::getDisplayWithInputByLayer(Layer* layer) const { - sp display; - for (const auto& pair : mDisplays) { - const auto& displayDevice = pair.second; - if (!displayDevice->receivesInput() || - !displayDevice->getCompositionDisplay() - ->belongsInOutput(layer->getLayerStack(), layer->getPrimaryDisplayOnly())) { + const auto filter = layer->getOutputFilter(); + sp inputDisplay; + + for (const auto& [_, display] : mDisplays) { + if (!display->receivesInput() || !display->getCompositionDisplay()->includesLayer(filter)) { continue; } // Don't return immediately so that we can log duplicates. - if (display) { - ALOGE("Multiple display devices claim to accept input for the same layerstack: %d", - layer->getLayerStack()); + if (inputDisplay) { + ALOGE("Multiple displays claim to accept input for the same layer stack: %u", + filter.layerStack.id); continue; } - display = displayDevice; + inputDisplay = display; } - return display; + return inputDisplay; } void SurfaceFlinger::setPowerModeInternal(const sp& display, hal::PowerMode mode) { @@ -6417,12 +6401,12 @@ void SurfaceFlinger::traverseLayersInLayerStack(ui::LayerStack layerStack, const // We loop through the first level of layers without traversing, // as we need to determine which layers belong to the requested display. for (const auto& layer : mDrawingState.layersSortedByZ) { - if (!layer->belongsToDisplay(layerStack)) { + if (layer->getLayerStack() != layerStack) { continue; } // relative layers are traversed in Layer::traverseInZOrder layer->traverseInZOrder(LayerVector::StateSet::Drawing, [&](Layer* layer) { - if (layer->getPrimaryDisplayOnly()) { + if (layer->isInternalDisplayOverlay()) { return; } if (!layer->isVisible()) { diff --git a/services/surfaceflinger/SurfaceInterceptor.cpp b/services/surfaceflinger/SurfaceInterceptor.cpp index 9be3abefab..0782fef8ea 100644 --- a/services/surfaceflinger/SurfaceInterceptor.cpp +++ b/services/surfaceflinger/SurfaceInterceptor.cpp @@ -323,11 +323,10 @@ void SurfaceInterceptor::addFlagsLocked(Transaction* transaction, int32_t layerI } void SurfaceInterceptor::addLayerStackLocked(Transaction* transaction, int32_t layerId, - uint32_t layerStack) -{ + ui::LayerStack layerStack) { SurfaceChange* change(createSurfaceChangeLocked(transaction, layerId)); LayerStackChange* layerStackChange(change->mutable_layer_stack()); - layerStackChange->set_layer_stack(layerStack); + layerStackChange->set_layer_stack(layerStack.id); } void SurfaceInterceptor::addCropLocked(Transaction* transaction, int32_t layerId, @@ -568,12 +567,11 @@ void SurfaceInterceptor::addDisplaySurfaceLocked(Transaction* transaction, int32 } } -void SurfaceInterceptor::addDisplayLayerStackLocked(Transaction* transaction, - int32_t sequenceId, uint32_t layerStack) -{ +void SurfaceInterceptor::addDisplayLayerStackLocked(Transaction* transaction, int32_t sequenceId, + ui::LayerStack layerStack) { DisplayChange* dispChange(createDisplayChangeLocked(transaction, sequenceId)); LayerStackChange* layerStackChange(dispChange->mutable_layer_stack()); - layerStackChange->set_layer_stack(layerStack); + layerStackChange->set_layer_stack(layerStack.id); } void SurfaceInterceptor::addDisplayFlagsLocked(Transaction* transaction, int32_t sequenceId, diff --git a/services/surfaceflinger/SurfaceInterceptor.h b/services/surfaceflinger/SurfaceInterceptor.h index 7b331b9f5f..970c3e5c27 100644 --- a/services/surfaceflinger/SurfaceInterceptor.h +++ b/services/surfaceflinger/SurfaceInterceptor.h @@ -160,7 +160,7 @@ private: void addTransparentRegionLocked(Transaction* transaction, int32_t layerId, const Region& transRegion); void addFlagsLocked(Transaction* transaction, int32_t layerId, uint8_t flags, uint8_t mask); - void addLayerStackLocked(Transaction* transaction, int32_t layerId, uint32_t layerStack); + void addLayerStackLocked(Transaction* transaction, int32_t layerId, ui::LayerStack); void addCropLocked(Transaction* transaction, int32_t layerId, const Rect& rect); void addCornerRadiusLocked(Transaction* transaction, int32_t layerId, float cornerRadius); void addBackgroundBlurRadiusLocked(Transaction* transaction, int32_t layerId, @@ -183,8 +183,7 @@ private: DisplayChange* createDisplayChangeLocked(Transaction* transaction, int32_t sequenceId); void addDisplaySurfaceLocked(Transaction* transaction, int32_t sequenceId, const sp& surface); - void addDisplayLayerStackLocked(Transaction* transaction, int32_t sequenceId, - uint32_t layerStack); + void addDisplayLayerStackLocked(Transaction* transaction, int32_t sequenceId, ui::LayerStack); void addDisplayFlagsLocked(Transaction* transaction, int32_t sequenceId, uint32_t flags); void addDisplaySizeLocked(Transaction* transaction, int32_t sequenceId, uint32_t w, uint32_t h); diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp index fa3f0e7239..d33bc1080c 100644 --- a/services/surfaceflinger/tests/Credentials_test.cpp +++ b/services/surfaceflinger/tests/Credentials_test.cpp @@ -93,7 +93,7 @@ protected: ASSERT_TRUE(mBGSurfaceControl->isValid()); Transaction t; - t.setDisplayLayerStack(mDisplay, 0); + t.setDisplayLayerStack(mDisplay, ui::DEFAULT_LAYER_STACK); ASSERT_EQ(NO_ERROR, t.setLayer(mBGSurfaceControl, INT_MAX - 3).show(mBGSurfaceControl).apply()); } diff --git a/services/surfaceflinger/tests/EffectLayer_test.cpp b/services/surfaceflinger/tests/EffectLayer_test.cpp index af00ec7fc9..1361ded67d 100644 --- a/services/surfaceflinger/tests/EffectLayer_test.cpp +++ b/services/surfaceflinger/tests/EffectLayer_test.cpp @@ -33,7 +33,7 @@ protected: mParentLayer = createColorLayer("Parent layer", Color::RED); asTransaction([&](Transaction& t) { - t.setDisplayLayerStack(display, 0); + t.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); t.setLayer(mParentLayer, INT32_MAX - 2).show(mParentLayer); t.setFlags(mParentLayer, layer_state_t::eLayerOpaque, layer_state_t::eLayerOpaque); }); diff --git a/services/surfaceflinger/tests/IPC_test.cpp b/services/surfaceflinger/tests/IPC_test.cpp index 9fa3d4c417..8d7e175cec 100644 --- a/services/surfaceflinger/tests/IPC_test.cpp +++ b/services/surfaceflinger/tests/IPC_test.cpp @@ -159,7 +159,7 @@ public: {0, 0, static_cast(width), static_cast(height)}, Color::RED); - transaction->setLayerStack(mSurfaceControl, 0) + transaction->setLayerStack(mSurfaceControl, ui::DEFAULT_LAYER_STACK) .setLayer(mSurfaceControl, std::numeric_limits::max()) .setBuffer(mSurfaceControl, gb) .setAcquireFence(mSurfaceControl, fence) @@ -232,7 +232,7 @@ public: mDisplayHeight = mode.resolution.getHeight(); Transaction setupTransaction; - setupTransaction.setDisplayLayerStack(mPrimaryDisplay, 0); + setupTransaction.setDisplayLayerStack(mPrimaryDisplay, ui::DEFAULT_LAYER_STACK); setupTransaction.apply(); } @@ -310,7 +310,7 @@ TEST_F(IPCTest, MergeBasic) { Color::RED); Transaction transaction; - transaction.setLayerStack(sc, 0) + transaction.setLayerStack(sc, ui::DEFAULT_LAYER_STACK) .setLayer(sc, std::numeric_limits::max() - 1) .setBuffer(sc, gb) .setAcquireFence(sc, fence) diff --git a/services/surfaceflinger/tests/LayerTransactionTest.h b/services/surfaceflinger/tests/LayerTransactionTest.h index 0bc8fe7aa0..6bd7920a62 100644 --- a/services/surfaceflinger/tests/LayerTransactionTest.h +++ b/services/surfaceflinger/tests/LayerTransactionTest.h @@ -266,7 +266,7 @@ protected: sp mDisplay; uint32_t mDisplayWidth; uint32_t mDisplayHeight; - uint32_t mDisplayLayerStack; + ui::LayerStack mDisplayLayerStack = ui::DEFAULT_LAYER_STACK; Rect mDisplayRect = Rect::INVALID_RECT; // leave room for ~256 layers @@ -294,8 +294,6 @@ private: // vsyncs. mBufferPostDelay = static_cast(1e6 / mode.refreshRate) * 3; - mDisplayLayerStack = 0; - mBlackBgSurface = createSurface(mClient, "BaseSurface", 0 /* buffer width */, 0 /* buffer height */, PIXEL_FORMAT_RGBA_8888, ISurfaceComposerClient::eFXSurfaceEffect); diff --git a/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp b/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp index 43d957cf7a..407625313c 100644 --- a/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp +++ b/services/surfaceflinger/tests/LayerTypeAndRenderTypeTransaction_test.cpp @@ -643,7 +643,8 @@ TEST_P(LayerTypeAndRenderTypeTransactionTest, SetLayerStackBasic) { ASSERT_NO_FATAL_FAILURE(layer = createLayer("test", 32, 32)); ASSERT_NO_FATAL_FAILURE(fillLayerColor(layer, Color::RED, 32, 32)); - Transaction().setLayerStack(layer, mDisplayLayerStack + 1).apply(); + const auto layerStack = ui::LayerStack::fromValue(mDisplayLayerStack.id + 1); + Transaction().setLayerStack(layer, layerStack).apply(); { SCOPED_TRACE("non-existing layer stack"); getScreenCapture()->expectColor(mDisplayRect, Color::BLACK); diff --git a/services/surfaceflinger/tests/LayerUpdate_test.cpp b/services/surfaceflinger/tests/LayerUpdate_test.cpp index ee4d367f3d..e1a7ecc03b 100644 --- a/services/surfaceflinger/tests/LayerUpdate_test.cpp +++ b/services/surfaceflinger/tests/LayerUpdate_test.cpp @@ -63,7 +63,7 @@ protected: TransactionUtils::fillSurfaceRGBA8(mSyncSurfaceControl, 31, 31, 31); asTransaction([&](Transaction& t) { - t.setDisplayLayerStack(display, 0); + t.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); t.setLayer(mBGSurfaceControl, INT32_MAX - 2).show(mBGSurfaceControl); diff --git a/services/surfaceflinger/tests/MirrorLayer_test.cpp b/services/surfaceflinger/tests/MirrorLayer_test.cpp index d02786504e..3ec6da9ff4 100644 --- a/services/surfaceflinger/tests/MirrorLayer_test.cpp +++ b/services/surfaceflinger/tests/MirrorLayer_test.cpp @@ -36,7 +36,7 @@ protected: mParentLayer = createColorLayer("Parent layer", Color::RED); mChildLayer = createColorLayer("Child layer", Color::GREEN, mParentLayer.get()); asTransaction([&](Transaction& t) { - t.setDisplayLayerStack(display, 0); + t.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); t.setLayer(mParentLayer, INT32_MAX - 2).show(mParentLayer); t.setCrop(mChildLayer, Rect(0, 0, 400, 400)).show(mChildLayer); t.setPosition(mChildLayer, 50, 50); diff --git a/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp b/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp index 08de01cdfb..1ed6c65afb 100644 --- a/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp +++ b/services/surfaceflinger/tests/MultiDisplayLayerBounds_test.cpp @@ -52,7 +52,7 @@ protected: mColorLayer = 0; } - void createDisplay(const ui::Size& layerStackSize, uint32_t layerStack) { + void createDisplay(const ui::Size& layerStackSize, ui::LayerStack layerStack) { mVirtualDisplay = SurfaceComposerClient::createDisplay(String8("VirtualDisplay"), false /*secure*/); asTransaction([&](Transaction& t) { @@ -63,7 +63,7 @@ protected: }); } - void createColorLayer(uint32_t layerStack) { + void createColorLayer(ui::LayerStack layerStack) { mColorLayer = createSurface(mClient, "ColorLayer", 0 /* buffer width */, 0 /* buffer height */, PIXEL_FORMAT_RGBA_8888, ISurfaceComposerClient::eFXSurfaceEffect); @@ -90,8 +90,9 @@ protected: }; TEST_F(MultiDisplayLayerBoundsTest, RenderLayerInVirtualDisplay) { - createDisplay(mMainDisplayState.layerStackSpaceRect, 1 /* layerStack */); - createColorLayer(1 /* layerStack */); + constexpr ui::LayerStack kLayerStack{1u}; + createDisplay(mMainDisplayState.layerStackSpaceRect, kLayerStack); + createColorLayer(kLayerStack); asTransaction([&](Transaction& t) { t.setPosition(mColorLayer, 10, 10); }); @@ -113,8 +114,8 @@ TEST_F(MultiDisplayLayerBoundsTest, RenderLayerInMirroredVirtualDisplay) { // Assumption here is that the new mirrored display has the same layer stack rect as the // primary display that it is mirroring. - createDisplay(mMainDisplayState.layerStackSpaceRect, 0 /* layerStack */); - createColorLayer(0 /* layerStack */); + createDisplay(mMainDisplayState.layerStackSpaceRect, ui::DEFAULT_LAYER_STACK); + createColorLayer(ui::DEFAULT_LAYER_STACK); asTransaction([&](Transaction& t) { t.setPosition(mColorLayer, 10, 10); }); diff --git a/services/surfaceflinger/tests/RelativeZ_test.cpp b/services/surfaceflinger/tests/RelativeZ_test.cpp index fde6e6eff8..50a4092ddb 100644 --- a/services/surfaceflinger/tests/RelativeZ_test.cpp +++ b/services/surfaceflinger/tests/RelativeZ_test.cpp @@ -43,7 +43,7 @@ protected: mForegroundLayer = createColorLayer("Foreground layer", Color::GREEN); asTransaction([&](Transaction& t) { - t.setDisplayLayerStack(display, 0); + t.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); t.setLayer(mBackgroundLayer, INT32_MAX - 2).show(mBackgroundLayer); t.setLayer(mForegroundLayer, INT32_MAX - 1).show(mForegroundLayer); }); diff --git a/services/surfaceflinger/tests/ScreenCapture_test.cpp b/services/surfaceflinger/tests/ScreenCapture_test.cpp index ab2064efd0..95301b3a37 100644 --- a/services/surfaceflinger/tests/ScreenCapture_test.cpp +++ b/services/surfaceflinger/tests/ScreenCapture_test.cpp @@ -53,7 +53,7 @@ protected: TransactionUtils::fillSurfaceRGBA8(mFGSurfaceControl, 195, 63, 63); asTransaction([&](Transaction& t) { - t.setDisplayLayerStack(display, 0); + t.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); t.setLayer(mBGSurfaceControl, INT32_MAX - 2).show(mBGSurfaceControl); @@ -563,7 +563,7 @@ TEST_F(ScreenCaptureTest, CaptureSecureLayer) { Transaction() .show(redLayer) .show(secureLayer) - .setLayerStack(redLayer, 0) + .setLayerStack(redLayer, ui::DEFAULT_LAYER_STACK) .setLayer(redLayer, INT32_MAX) .apply(); @@ -655,7 +655,7 @@ TEST_F(ScreenCaptureTest, CaptureDisplayPrimaryDisplayOnly) { Transaction() .show(layer) .hide(mFGSurfaceControl) - .setLayerStack(layer, 0) + .setLayerStack(layer, ui::DEFAULT_LAYER_STACK) .setLayer(layer, INT32_MAX) .setColor(layer, {layerColor.r / 255, layerColor.g / 255, layerColor.b / 255}) .setCrop(layer, bounds) @@ -702,7 +702,7 @@ TEST_F(ScreenCaptureTest, CaptureDisplayChildPrimaryDisplayOnly) { .show(layer) .show(childLayer) .hide(mFGSurfaceControl) - .setLayerStack(layer, 0) + .setLayerStack(layer, ui::DEFAULT_LAYER_STACK) .setLayer(layer, INT32_MAX) .setColor(layer, {layerColor.r / 255, layerColor.g / 255, layerColor.b / 255}) .setColor(childLayer, {childColor.r / 255, childColor.g / 255, childColor.b / 255}) diff --git a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp index a42405989f..ee16f40b6d 100644 --- a/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp +++ b/services/surfaceflinger/tests/SurfaceInterceptor_test.cpp @@ -283,7 +283,7 @@ void SurfaceInterceptorTest::setupBackgroundSurface() { ASSERT_TRUE(mFGSurfaceControl->isValid()); Transaction t; - t.setDisplayLayerStack(display, 0); + t.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ASSERT_EQ(NO_ERROR, t.setLayer(mBGSurfaceControl, INT_MAX - 3) .show(mBGSurfaceControl) @@ -380,7 +380,7 @@ void SurfaceInterceptorTest::transparentRegionHintUpdate(Transaction& t) { } void SurfaceInterceptorTest::layerStackUpdate(Transaction& t) { - t.setLayerStack(mBGSurfaceControl, STACK_UPDATE); + t.setLayerStack(mBGSurfaceControl, ui::LayerStack::fromValue(STACK_UPDATE)); } void SurfaceInterceptorTest::hiddenFlagUpdate(Transaction& t) { diff --git a/services/surfaceflinger/tests/TransactionTestHarnesses.h b/services/surfaceflinger/tests/TransactionTestHarnesses.h index 89f608645d..60cffb19df 100644 --- a/services/surfaceflinger/tests/TransactionTestHarnesses.h +++ b/services/surfaceflinger/tests/TransactionTestHarnesses.h @@ -65,7 +65,7 @@ public: SurfaceComposerClient::Transaction t; t.setDisplaySurface(vDisplay, producer); - t.setDisplayLayerStack(vDisplay, 0); + t.setDisplayLayerStack(vDisplay, ui::DEFAULT_LAYER_STACK); t.setDisplayProjection(vDisplay, displayState.orientation, Rect(displayState.layerStackSpaceRect), Rect(resolution)); t.apply(); diff --git a/services/surfaceflinger/tests/WindowInfosListener_test.cpp b/services/surfaceflinger/tests/WindowInfosListener_test.cpp index 89228d55e5..de116f29ca 100644 --- a/services/surfaceflinger/tests/WindowInfosListener_test.cpp +++ b/services/surfaceflinger/tests/WindowInfosListener_test.cpp @@ -84,7 +84,7 @@ TEST_F(WindowInfosListenerTest, WindowInfoAddedAndRemoved) { ISurfaceComposerClient::eFXSurfaceBufferState); Transaction() - .setLayerStack(surfaceControl, 0) + .setLayerStack(surfaceControl, ui::DEFAULT_LAYER_STACK) .show(surfaceControl) .setLayer(surfaceControl, INT32_MAX - 1) .setInputWindowInfo(surfaceControl, windowInfo) @@ -112,7 +112,7 @@ TEST_F(WindowInfosListenerTest, WindowInfoChanged) { ISurfaceComposerClient::eFXSurfaceBufferState); Transaction() - .setLayerStack(surfaceControl, 0) + .setLayerStack(surfaceControl, ui::DEFAULT_LAYER_STACK) .show(surfaceControl) .setLayer(surfaceControl, INT32_MAX - 1) .setInputWindowInfo(surfaceControl, windowInfo) diff --git a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp index a9e935df22..b3b4ec15cd 100644 --- a/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp +++ b/services/surfaceflinger/tests/fakehwc/SFFakeHwc_test.cpp @@ -363,7 +363,7 @@ protected: { TransactionScope ts(*mFakeComposerClient); - ts.setDisplayLayerStack(display, 0); + ts.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ts.setLayer(surfaceControl, INT32_MAX - 2).show(surfaceControl); } @@ -426,7 +426,7 @@ protected: { TransactionScope ts(*mFakeComposerClient); - ts.setDisplayLayerStack(display, 0); + ts.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ts.setLayer(surfaceControl, INT32_MAX - 2).show(surfaceControl); } @@ -479,7 +479,7 @@ protected: { TransactionScope ts(*mFakeComposerClient); - ts.setDisplayLayerStack(display, 0); + ts.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ts.setLayer(surfaceControl, INT32_MAX - 2).show(surfaceControl); } @@ -534,7 +534,7 @@ protected: { TransactionScope ts(*mFakeComposerClient); - ts.setDisplayLayerStack(display, 0); + ts.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ts.setLayer(surfaceControl, INT32_MAX - 2).show(surfaceControl); } @@ -586,7 +586,7 @@ protected: { TransactionScope ts(*mFakeComposerClient); - ts.setDisplayLayerStack(display, 0); + ts.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ts.setLayer(surfaceControl, INT32_MAX - 2).show(surfaceControl); } @@ -651,7 +651,7 @@ protected: { TransactionScope ts(*mFakeComposerClient); - ts.setDisplayLayerStack(display, 0); + ts.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ts.setLayer(surfaceControl, INT32_MAX - 2).show(surfaceControl); } @@ -703,7 +703,7 @@ protected: { TransactionScope ts(*mFakeComposerClient); - ts.setDisplayLayerStack(display, 0); + ts.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ts.setLayer(surfaceControl, INT32_MAX - 2).show(surfaceControl); } @@ -750,7 +750,7 @@ protected: { TransactionScope ts(*mFakeComposerClient); - ts.setDisplayLayerStack(display, 0); + ts.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ts.setLayer(surfaceControl, INT32_MAX - 2).show(surfaceControl); } @@ -797,7 +797,7 @@ protected: { TransactionScope ts(*mFakeComposerClient); - ts.setDisplayLayerStack(display, 0); + ts.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); ts.setLayer(surfaceControl, INT32_MAX - 2).show(surfaceControl); } @@ -1195,7 +1195,7 @@ protected: fillSurfaceRGBA8(mFGSurfaceControl, RED); Transaction t; - t.setDisplayLayerStack(display, 0); + t.setDisplayLayerStack(display, ui::DEFAULT_LAYER_STACK); t.setLayer(mBGSurfaceControl, INT32_MAX - 2); t.show(mBGSurfaceControl); @@ -1342,7 +1342,7 @@ protected: ALOGD("TransactionTest::SetLayerStack"); { TransactionScope ts(*sFakeComposer); - ts.setLayerStack(mFGSurfaceControl, 1); + ts.setLayerStack(mFGSurfaceControl, ui::LayerStack{1}); } // Foreground layer should have disappeared. diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 98c18890cf..0253c4cbbf 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -82,7 +82,7 @@ constexpr int DEFAULT_DISPLAY_WIDTH = 1920; constexpr int DEFAULT_DISPLAY_HEIGHT = 1024; constexpr int DEFAULT_TEXTURE_ID = 6000; -constexpr int DEFAULT_LAYER_STACK = 7000; +constexpr ui::LayerStack LAYER_STACK{7000u}; constexpr int DEFAULT_DISPLAY_MAX_LUMINANCE = 500; @@ -287,10 +287,8 @@ struct BaseDisplayVariant { auto ceDisplayArgs = compositionengine::DisplayCreationArgsBuilder() .setId(DEFAULT_DISPLAY_ID) - .setConnectionType(ui::DisplayConnectionType::Internal) .setPixels({DEFAULT_DISPLAY_WIDTH, DEFAULT_DISPLAY_HEIGHT}) .setIsSecure(Derived::IS_SECURE) - .setLayerStackId(DEFAULT_LAYER_STACK) .setPowerAdvisor(&test->mPowerAdvisor) .setName(std::string("Injected display for ") + test_info->test_case_name() + "." + test_info->name()) @@ -309,7 +307,7 @@ struct BaseDisplayVariant { .setPowerMode(Derived::INIT_POWER_MODE) .inject(); Mock::VerifyAndClear(test->mNativeWindow); - test->mDisplay->setLayerStack(DEFAULT_LAYER_STACK); + test->mDisplay->setLayerStack(LAYER_STACK); } template @@ -834,7 +832,7 @@ struct BaseLayerVariant { template static void initLayerDrawingStateAndComputeBounds(CompositionTest* test, sp layer) { auto& layerDrawingState = test->mFlinger.mutableLayerDrawingState(layer); - layerDrawingState.layerStack = DEFAULT_LAYER_STACK; + layerDrawingState.layerStack = LAYER_STACK; layerDrawingState.width = 100; layerDrawingState.height = 100; layerDrawingState.color = half4(LayerProperties::COLOR[0], LayerProperties::COLOR[1], diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp index 60b0f5334c..5a21e7be5b 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp @@ -139,20 +139,18 @@ sp DisplayTransactionTest::injectDefaultInternalDisplay( EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_SET_USAGE64)); EXPECT_CALL(*mNativeWindow, perform(NATIVE_WINDOW_API_DISCONNECT)).Times(AnyNumber()); - constexpr auto kConnectionType = ui::DisplayConnectionType::Internal; - constexpr bool kIsPrimary = true; - auto compositionDisplay = compositionengine::impl::createDisplay(mFlinger.getCompositionEngine(), compositionengine::DisplayCreationArgsBuilder() .setId(DEFAULT_DISPLAY_ID) - .setConnectionType(kConnectionType) .setPixels({DEFAULT_DISPLAY_WIDTH, DEFAULT_DISPLAY_HEIGHT}) .setPowerAdvisor(&mPowerAdvisor) .build()); - auto injector = FakeDisplayDeviceInjector(mFlinger, compositionDisplay, kConnectionType, + constexpr bool kIsPrimary = true; + auto injector = FakeDisplayDeviceInjector(mFlinger, compositionDisplay, + ui::DisplayConnectionType::Internal, DEFAULT_DISPLAY_HWC_DISPLAY_ID, kIsPrimary); injector.setNativeWindow(mNativeWindow); diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h index de058a4f68..0f1cc6765d 100644 --- a/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h +++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTestHelpers.h @@ -267,11 +267,6 @@ struct DisplayVariant { .setPixels({WIDTH, HEIGHT}) .setPowerAdvisor(&test->mPowerAdvisor); - const auto connectionType = CONNECTION_TYPE::value; - if (connectionType) { - ceDisplayArgs.setConnectionType(*connectionType); - } - auto compositionDisplay = compositionengine::impl::createDisplay(test->mFlinger.getCompositionEngine(), ceDisplayArgs.build()); @@ -279,7 +274,7 @@ struct DisplayVariant { auto injector = TestableSurfaceFlinger::FakeDisplayDeviceInjector(test->mFlinger, compositionDisplay, - connectionType, + CONNECTION_TYPE::value, HWC_DISPLAY_ID_OPT::value, static_cast(PRIMARY)); @@ -388,7 +383,6 @@ struct HwcDisplayVariant { auto ceDisplayArgs = compositionengine::DisplayCreationArgsBuilder() .setId(DisplayVariant::DISPLAY_ID::get()) - .setConnectionType(PhysicalDisplay::CONNECTION_TYPE) .setPixels({DisplayVariant::WIDTH, DisplayVariant::HEIGHT}) .setIsSecure(static_cast(DisplayVariant::SECURE)) .setPowerAdvisor(&test->mPowerAdvisor) diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp index 313ab032f9..ef53aed091 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HandleTransactionLockedTest.cpp @@ -534,8 +534,8 @@ TEST_F(HandleTransactionLockedTest, processesVirtualDisplayRemoval) { TEST_F(HandleTransactionLockedTest, processesDisplayLayerStackChanges) { using Case = NonHwcVirtualDisplayCase; - constexpr uint32_t oldLayerStack = 0u; - constexpr uint32_t newLayerStack = 123u; + constexpr ui::LayerStack oldLayerStack = ui::DEFAULT_LAYER_STACK; + constexpr ui::LayerStack newLayerStack{123u}; // -------------------------------------------------------------------- // Preconditions diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp index ef8b1493ae..bafa910270 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp @@ -63,15 +63,11 @@ TEST_F(OnInitializeDisplaysTest, onInitializeDisplaysSetsUpPrimaryDisplay) { // The primary display should have a current state ASSERT_TRUE(hasCurrentDisplayState(primaryDisplay.token())); const auto& primaryDisplayState = getCurrentDisplayState(primaryDisplay.token()); - // The layer stack state should be set to zero - EXPECT_EQ(0u, primaryDisplayState.layerStack); - // The orientation state should be set to zero - EXPECT_EQ(ui::ROTATION_0, primaryDisplayState.orientation); - // The orientedDisplaySpaceRect state should be set to INVALID + // The primary display state should be reset + EXPECT_EQ(ui::DEFAULT_LAYER_STACK, primaryDisplayState.layerStack); + EXPECT_EQ(ui::ROTATION_0, primaryDisplayState.orientation); EXPECT_EQ(Rect::INVALID_RECT, primaryDisplayState.orientedDisplaySpaceRect); - - // The layerStackSpaceRect state should be set to INVALID EXPECT_EQ(Rect::INVALID_RECT, primaryDisplayState.layerStackSpaceRect); // The width and height should both be zero @@ -99,4 +95,4 @@ TEST_F(OnInitializeDisplaysTest, onInitializeDisplaysSetsUpPrimaryDisplay) { } } // namespace -} // namespace android \ No newline at end of file +} // namespace android diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetDisplayStateTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetDisplayStateTest.cpp index fc40818ad1..7d9e22b23e 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetDisplayStateTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetDisplayStateTest.cpp @@ -25,6 +25,8 @@ namespace android { namespace { +constexpr ui::LayerStack LAYER_STACK{456u}; + class SetDisplayStateLockedTest : public DisplayTransactionTest {}; TEST_F(SetDisplayStateLockedTest, setDisplayStateLockedDoesNothingWithUnknownDisplay) { @@ -38,7 +40,7 @@ TEST_F(SetDisplayStateLockedTest, setDisplayStateLockedDoesNothingWithUnknownDis DisplayState state; state.what = DisplayState::eLayerStackChanged; state.token = displayToken; - state.layerStack = 456; + state.layerStack = LAYER_STACK; // -------------------------------------------------------------------- // Invocation @@ -167,13 +169,13 @@ TEST_F(SetDisplayStateLockedTest, setDisplayStateLockedDoesNothingIfLayerStackDi display.inject(); // The display has a layer stack set - display.mutableCurrentDisplayState().layerStack = 456u; + display.mutableCurrentDisplayState().layerStack = LAYER_STACK; // The incoming request sets the same layer stack DisplayState state; state.what = DisplayState::eLayerStackChanged; state.token = display.token(); - state.layerStack = 456u; + state.layerStack = LAYER_STACK; // -------------------------------------------------------------------- // Invocation @@ -187,7 +189,7 @@ TEST_F(SetDisplayStateLockedTest, setDisplayStateLockedDoesNothingIfLayerStackDi EXPECT_EQ(0u, flags); // The current display state is unchanged - EXPECT_EQ(456u, display.getCurrentDisplayState().layerStack); + EXPECT_EQ(LAYER_STACK, display.getCurrentDisplayState().layerStack); } TEST_F(SetDisplayStateLockedTest, setDisplayStateLockedRequestsUpdateIfLayerStackChanged) { @@ -201,13 +203,13 @@ TEST_F(SetDisplayStateLockedTest, setDisplayStateLockedRequestsUpdateIfLayerStac display.inject(); // The display has a layer stack set - display.mutableCurrentDisplayState().layerStack = 654u; + display.mutableCurrentDisplayState().layerStack = ui::LayerStack{LAYER_STACK.id + 1}; // The incoming request sets a different layer stack DisplayState state; state.what = DisplayState::eLayerStackChanged; state.token = display.token(); - state.layerStack = 456u; + state.layerStack = LAYER_STACK; // -------------------------------------------------------------------- // Invocation @@ -221,7 +223,7 @@ TEST_F(SetDisplayStateLockedTest, setDisplayStateLockedRequestsUpdateIfLayerStac EXPECT_EQ(eDisplayTransactionNeeded, flags); // The desired display state has been set to the new value. - EXPECT_EQ(456u, display.getCurrentDisplayState().layerStack); + EXPECT_EQ(LAYER_STACK, display.getCurrentDisplayState().layerStack); } TEST_F(SetDisplayStateLockedTest, setDisplayStateLockedDoesNothingIfFlagsNotChanged) { -- cgit v1.2.3-59-g8ed1b From 70aa7579ccc3284f77ad98178b6a5029a67e5999 Mon Sep 17 00:00:00 2001 From: chaviw Date: Wed, 22 Sep 2021 12:27:57 -0500 Subject: Use Transactions for RefreshRateOverlay RefreshRateOverlay currently calls directly in Layer to update properties. This is ends up adding confusing code to RefreshRateOverlay since it needs to notify SF to handle the updates. It also creates addtional confusion since there are multiple entries to layer properties. Instead, use the SCC Transaction requests to accomplish the same behavior. This simplifies the code and also allows SF to handle the updates instead of RefreshRateOverlay invoking force updates. Test: RefreshRateOverlay works with rotation, different refresh rates Test: RefreshRateOverlayTest Bug: 200820757 Change-Id: I8503c61456c4c14ce6cbcdb2b27035a340fecbf5 --- services/surfaceflinger/ClientCache.cpp | 11 +-- services/surfaceflinger/RefreshRateOverlay.cpp | 92 +++++++++++--------------- services/surfaceflinger/RefreshRateOverlay.h | 13 ++-- 3 files changed, 52 insertions(+), 64 deletions(-) (limited to 'services/surfaceflinger/RefreshRateOverlay.cpp') diff --git a/services/surfaceflinger/ClientCache.cpp b/services/surfaceflinger/ClientCache.cpp index 1ef8f78894..e7b8995703 100644 --- a/services/surfaceflinger/ClientCache.cpp +++ b/services/surfaceflinger/ClientCache.cpp @@ -82,10 +82,13 @@ bool ClientCache::add(const client_cache_t& cacheId, const sp& bu return false; } - status_t err = token->linkToDeath(mDeathRecipient); - if (err != NO_ERROR) { - ALOGE("failed to cache buffer: could not link to death"); - return false; + // Only call linkToDeath if not a local binder + if (token->localBinder() == nullptr) { + status_t err = token->linkToDeath(mDeathRecipient); + if (err != NO_ERROR) { + ALOGE("failed to cache buffer: could not link to death"); + return false; + } } auto [itr, success] = mBuffers.emplace(processToken, diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 0789e8dcb4..ec81e6317c 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -30,6 +30,8 @@ #include #include #include +#include +#include #undef LOG_TAG #define LOG_TAG "RefreshRateOverlay" @@ -190,35 +192,30 @@ RefreshRateOverlay::RefreshRateOverlay(SurfaceFlinger& flinger, uint32_t lowFps, } bool RefreshRateOverlay::createLayer() { - int32_t layerId; - const status_t ret = - mFlinger.createLayer(String8("RefreshRateOverlay"), mClient, - SevenSegmentDrawer::getWidth(), SevenSegmentDrawer::getHeight(), - PIXEL_FORMAT_RGBA_8888, - ISurfaceComposerClient::eFXSurfaceBufferState, LayerMetadata(), - &mIBinder, &mGbp, nullptr, &layerId); - if (ret) { + mSurfaceControl = + SurfaceComposerClient::getDefault() + ->createSurface(String8("RefreshRateOverlay"), SevenSegmentDrawer::getWidth(), + SevenSegmentDrawer::getHeight(), PIXEL_FORMAT_RGBA_8888, + ISurfaceComposerClient::eFXSurfaceBufferState); + + if (!mSurfaceControl) { ALOGE("failed to create buffer state layer"); return false; } - mLayer = mClient->getLayerUser(mIBinder); - mLayer->setFrameRate(Layer::FrameRate(Fps(0.0f), Layer::FrameRateCompatibility::NoVote)); - mLayer->setIsAtRoot(true); - - // setting Layer's Z requires resorting layersSortedByZ - ssize_t idx = mFlinger.mDrawingState.layersSortedByZ.indexOf(mLayer); - if (mLayer->setLayer(INT32_MAX - 2) && idx >= 0) { - mFlinger.mDrawingState.layersSortedByZ.removeAt(idx); - mFlinger.mDrawingState.layersSortedByZ.add(mLayer); - } + SurfaceComposerClient::Transaction t; + t.setFrameRate(mSurfaceControl, 0.0f, + static_cast(Layer::FrameRateCompatibility::NoVote), + static_cast(scheduler::Seamlessness::OnlySeamless)); + t.setLayer(mSurfaceControl, INT32_MAX - 2); + t.apply(); return true; } -const std::vector>& -RefreshRateOverlay::getOrCreateBuffers(uint32_t fps) { - ui::Transform::RotationFlags transformHint = mLayer->getTransformHint(); +const std::vector>& RefreshRateOverlay::getOrCreateBuffers(uint32_t fps) { + ui::Transform::RotationFlags transformHint = + static_cast(mSurfaceControl->getTransformHint()); // Tell SurfaceFlinger about the pre-rotation on the buffer. const auto transform = [&] { switch (transformHint) { @@ -230,7 +227,10 @@ RefreshRateOverlay::getOrCreateBuffers(uint32_t fps) { return ui::Transform::ROT_0; } }(); - mLayer->setTransform(transform); + + SurfaceComposerClient::Transaction t; + t.setTransform(mSurfaceControl, transform); + t.apply(); if (mBufferCache.find(transformHint) == mBufferCache.end() || mBufferCache.at(transformHint).find(fps) == mBufferCache.at(transformHint).end()) { @@ -249,16 +249,7 @@ RefreshRateOverlay::getOrCreateBuffers(uint32_t fps) { colorBase.fA = ALPHA; SkColor color = colorBase.toSkColor(); auto buffers = SevenSegmentDrawer::draw(fps, color, transformHint, mShowSpinner); - std::vector> textures; - std::transform(buffers.begin(), buffers.end(), std::back_inserter(textures), - [&](const auto& buffer) -> std::shared_ptr { - return std::make_shared< - renderengine::ExternalTexture>(buffer, - mFlinger.getRenderEngine(), - renderengine::ExternalTexture:: - Usage::READABLE); - }); - mBufferCache[transformHint].emplace(fps, textures); + mBufferCache[transformHint].emplace(fps, buffers); } return mBufferCache[transformHint][fps]; @@ -271,30 +262,26 @@ void RefreshRateOverlay::setViewport(ui::Size viewport) { Rect frame((3 * width) >> 4, height >> 5); frame.offsetBy(width >> 5, height >> 4); - layer_state_t::matrix22_t matrix; - matrix.dsdx = frame.getWidth() / static_cast(SevenSegmentDrawer::getWidth()); - matrix.dtdx = 0; - matrix.dtdy = 0; - matrix.dsdy = frame.getHeight() / static_cast(SevenSegmentDrawer::getHeight()); - mLayer->setMatrix(matrix, true); - mLayer->setPosition(frame.left, frame.top); - mFlinger.mTransactionFlags.fetch_or(eTransactionMask); + SurfaceComposerClient::Transaction t; + t.setMatrix(mSurfaceControl, + frame.getWidth() / static_cast(SevenSegmentDrawer::getWidth()), 0, 0, + frame.getHeight() / static_cast(SevenSegmentDrawer::getHeight())); + t.setPosition(mSurfaceControl, frame.left, frame.top); + t.apply(); } void RefreshRateOverlay::setLayerStack(ui::LayerStack stack) { - mLayer->setLayerStack(stack); - mFlinger.mTransactionFlags.fetch_or(eTransactionMask); + SurfaceComposerClient::Transaction t; + t.setLayerStack(mSurfaceControl, stack); + t.apply(); } void RefreshRateOverlay::changeRefreshRate(const Fps& fps) { mCurrentFps = fps.getIntValue(); auto buffer = getOrCreateBuffers(*mCurrentFps)[mFrame]; - mLayer->setBuffer(buffer, Fence::NO_FENCE, 0, 0, true, {}, - mLayer->getHeadFrameNumber(-1 /* expectedPresentTime */), - std::nullopt /* dequeueTime */, FrameTimelineInfo{}, - nullptr /* releaseBufferListener */, nullptr /* releaseBufferEndpoint */); - - mFlinger.mTransactionFlags.fetch_or(eTransactionMask); + SurfaceComposerClient::Transaction t; + t.setBuffer(mSurfaceControl, buffer); + t.apply(); } void RefreshRateOverlay::onInvalidate() { @@ -303,12 +290,9 @@ void RefreshRateOverlay::onInvalidate() { const auto& buffers = getOrCreateBuffers(*mCurrentFps); mFrame = (mFrame + 1) % buffers.size(); auto buffer = buffers[mFrame]; - mLayer->setBuffer(buffer, Fence::NO_FENCE, 0, 0, true, {}, - mLayer->getHeadFrameNumber(-1 /* expectedPresentTime */), - std::nullopt /* dequeueTime */, FrameTimelineInfo{}, - nullptr /* releaseBufferListener */, nullptr /* releaseBufferEndpoint */); - - mFlinger.mTransactionFlags.fetch_or(eTransactionMask); + SurfaceComposerClient::Transaction t; + t.setBuffer(mSurfaceControl, buffer); + t.apply(); } } // namespace android diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h index 63ae383c8c..354510a9d7 100644 --- a/services/surfaceflinger/RefreshRateOverlay.h +++ b/services/surfaceflinger/RefreshRateOverlay.h @@ -37,6 +37,7 @@ class IBinder; class IGraphicBufferProducer; class Layer; class SurfaceFlinger; +class SurfaceControl; class RefreshRateOverlay { public: @@ -70,18 +71,16 @@ private: }; bool createLayer(); - const std::vector>& getOrCreateBuffers( - uint32_t fps); + + const std::vector>& getOrCreateBuffers(uint32_t fps); SurfaceFlinger& mFlinger; const sp mClient; - sp mLayer; sp mIBinder; sp mGbp; - std::unordered_map< - ui::Transform::RotationFlags, - std::unordered_map>>> + std::unordered_map>>> mBufferCache; std::optional mCurrentFps; int mFrame = 0; @@ -94,6 +93,8 @@ private: // Interpolate the colors between these values. const uint32_t mLowFps; const uint32_t mHighFps; + + sp mSurfaceControl; }; } // namespace android -- cgit v1.2.3-59-g8ed1b From e0e0cde7fa7d4dc34fb68c862f4a255d7d3252c7 Mon Sep 17 00:00:00 2001 From: Dominik Laskowski Date: Fri, 30 Jul 2021 10:42:05 -0700 Subject: SF: Decouple MessageQueue Define an ICompositor interface against which MessageQueue (which ought to be an implementation detail of Scheduler) is implemented. Change the equivocal invalidate/refresh nomenclature to commit/composite. Schedule sampling only after composite. Bug: 185535769 Test: libsurfaceflinger_unittest Change-Id: I0c18f312459bae48531449f24f7b53c104fc5812 --- services/surfaceflinger/BufferLayer.cpp | 2 +- services/surfaceflinger/BufferQueueLayer.cpp | 8 +- services/surfaceflinger/BufferStateLayer.cpp | 4 +- .../compositionengine/CompositionRefreshArgs.h | 4 +- .../CompositionEngine/src/Output.cpp | 2 +- services/surfaceflinger/DisplayDevice.cpp | 4 +- services/surfaceflinger/DisplayDevice.h | 2 +- services/surfaceflinger/RefreshRateOverlay.cpp | 2 +- services/surfaceflinger/RefreshRateOverlay.h | 2 +- services/surfaceflinger/RegionSamplingThread.cpp | 2 +- services/surfaceflinger/Scheduler/MessageQueue.cpp | 93 ++++++------ services/surfaceflinger/Scheduler/MessageQueue.h | 81 +++++----- services/surfaceflinger/Scheduler/Scheduler.cpp | 14 +- services/surfaceflinger/Scheduler/Scheduler.h | 6 +- services/surfaceflinger/SurfaceFlinger.cpp | 166 +++++++-------------- services/surfaceflinger/SurfaceFlinger.h | 68 ++++----- .../SurfaceFlingerDefaultFactory.cpp | 4 +- .../surfaceflinger/SurfaceFlingerDefaultFactory.h | 2 +- services/surfaceflinger/SurfaceFlingerFactory.h | 7 +- .../tests/unittests/CompositionTest.cpp | 10 +- .../tests/unittests/MessageQueueTest.cpp | 101 ++++++------- .../tests/unittests/SetFrameRateTest.cpp | 18 +-- .../unittests/SurfaceFlinger_CreateDisplayTest.cpp | 10 +- .../SurfaceFlinger_DestroyDisplayTest.cpp | 4 +- .../tests/unittests/SurfaceFlinger_HotplugTest.cpp | 10 +- .../SurfaceFlinger_OnInitializeDisplaysTest.cpp | 5 +- .../SurfaceFlinger_SetPowerModeInternalTest.cpp | 2 +- .../tests/unittests/TestableSurfaceFlinger.h | 20 ++- .../tests/unittests/TransactionApplicationTest.cpp | 14 +- .../tests/unittests/mock/MockMessageQueue.h | 9 +- .../tests/unittests/mock/MockSchedulerCallback.h | 4 +- 31 files changed, 304 insertions(+), 376 deletions(-) (limited to 'services/surfaceflinger/RefreshRateOverlay.cpp') diff --git a/services/surfaceflinger/BufferLayer.cpp b/services/surfaceflinger/BufferLayer.cpp index ef05fe2138..160b2ea5c0 100644 --- a/services/surfaceflinger/BufferLayer.cpp +++ b/services/surfaceflinger/BufferLayer.cpp @@ -486,7 +486,7 @@ bool BufferLayer::latchBuffer(bool& recomputeVisibleRegions, nsecs_t latchTime, // try again later if (!fenceHasSignaled()) { ATRACE_NAME("!fenceHasSignaled()"); - mFlinger->signalLayerUpdate(); + mFlinger->onLayerUpdate(); return false; } diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp index 8bbe43865a..bf38177d99 100644 --- a/services/surfaceflinger/BufferQueueLayer.cpp +++ b/services/surfaceflinger/BufferQueueLayer.cpp @@ -228,7 +228,7 @@ status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t if (updateResult == BufferQueue::PRESENT_LATER) { // Producer doesn't want buffer to be displayed yet. Signal a // layer update so we check again at the next opportunity. - mFlinger->signalLayerUpdate(); + mFlinger->onLayerUpdate(); return BAD_VALUE; } else if (updateResult == BufferLayerConsumer::BUFFER_REJECTED) { // If the buffer has been rejected, remove it from the shadow queue @@ -309,7 +309,7 @@ status_t BufferQueueLayer::updateTexImage(bool& recomputeVisibleRegions, nsecs_t // Decrement the queued-frames count. Signal another event if we // have more frames pending. if ((queuedBuffer && more_frames_pending) || mAutoRefresh) { - mFlinger->signalLayerUpdate(); + mFlinger->onLayerUpdate(); } return NO_ERROR; @@ -412,7 +412,7 @@ void BufferQueueLayer::onFrameAvailable(const BufferItem& item) { mFlinger->mInterceptor->saveBufferUpdate(layerId, item.mGraphicBuffer->getWidth(), item.mGraphicBuffer->getHeight(), item.mFrameNumber); - mFlinger->signalLayerUpdate(); + mFlinger->onLayerUpdate(); mConsumer->onBufferAvailable(item); } @@ -458,7 +458,7 @@ void BufferQueueLayer::onSidebandStreamChanged() { bool sidebandStreamChanged = false; if (mSidebandStreamChanged.compare_exchange_strong(sidebandStreamChanged, true)) { // mSidebandStreamChanged was changed to true - mFlinger->signalLayerUpdate(); + mFlinger->onLayerUpdate(); } } diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp index 4eeaba154f..f0099c268d 100644 --- a/services/surfaceflinger/BufferStateLayer.cpp +++ b/services/surfaceflinger/BufferStateLayer.cpp @@ -595,7 +595,7 @@ bool BufferStateLayer::setSidebandStream(const sp& sidebandStream) setTransactionFlags(eTransactionNeeded); if (!mSidebandStreamChanged.exchange(true)) { // mSidebandStreamChanged was false - mFlinger->signalLayerUpdate(); + mFlinger->onLayerUpdate(); } return true; } @@ -713,7 +713,7 @@ bool BufferStateLayer::onPreComposition(nsecs_t refreshStartTime) { void BufferStateLayer::setAutoRefresh(bool autoRefresh) { if (!mAutoRefresh.exchange(autoRefresh)) { - mFlinger->signalLayerUpdate(); + mFlinger->onLayerUpdate(); } } diff --git a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h index 95d553d02f..93586eddd8 100644 --- a/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h +++ b/services/surfaceflinger/CompositionEngine/include/compositionengine/CompositionRefreshArgs.h @@ -85,8 +85,8 @@ struct CompositionRefreshArgs { // to prevent an early presentation of a frame. std::shared_ptr previousPresentFence; - // The predicted next invalidation time - std::optional nextInvalidateTime; + // If set, a frame has been scheduled for that time. + std::optional scheduledFrameTime; }; } // namespace android::compositionengine diff --git a/services/surfaceflinger/CompositionEngine/src/Output.cpp b/services/surfaceflinger/CompositionEngine/src/Output.cpp index 048d7c2b4a..fad1fa74dd 100644 --- a/services/surfaceflinger/CompositionEngine/src/Output.cpp +++ b/services/surfaceflinger/CompositionEngine/src/Output.cpp @@ -1314,7 +1314,7 @@ void Output::postFramebuffer() { void Output::renderCachedSets(const CompositionRefreshArgs& refreshArgs) { if (mPlanner) { - mPlanner->renderCachedSets(getState(), refreshArgs.nextInvalidateTime); + mPlanner->renderCachedSets(getState(), refreshArgs.scheduledFrameTime); } } diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index 3397118c05..fd09ae4066 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -466,9 +466,9 @@ bool DisplayDevice::onKernelTimerChanged(std::optional desiredMod return false; } -void DisplayDevice::onInvalidate() { +void DisplayDevice::animateRefreshRateOverlay() { if (mRefreshRateOverlay) { - mRefreshRateOverlay->onInvalidate(); + mRefreshRateOverlay->animate(); } } diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h index 4a731bddf1..4b9718f608 100644 --- a/services/surfaceflinger/DisplayDevice.h +++ b/services/surfaceflinger/DisplayDevice.h @@ -234,7 +234,7 @@ public: void enableRefreshRateOverlay(bool enable, bool showSpinner); bool isRefreshRateOverlayEnabled() const { return mRefreshRateOverlay != nullptr; } bool onKernelTimerChanged(std::optional, bool timerExpired); - void onInvalidate(); + void animateRefreshRateOverlay(); void onVsync(nsecs_t timestamp); nsecs_t getVsyncPeriodFromHWC() const; diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index ec81e6317c..2502d66a67 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -284,7 +284,7 @@ void RefreshRateOverlay::changeRefreshRate(const Fps& fps) { t.apply(); } -void RefreshRateOverlay::onInvalidate() { +void RefreshRateOverlay::animate() { if (!mCurrentFps.has_value()) return; const auto& buffers = getOrCreateBuffers(*mCurrentFps); diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h index 354510a9d7..65d446c751 100644 --- a/services/surfaceflinger/RefreshRateOverlay.h +++ b/services/surfaceflinger/RefreshRateOverlay.h @@ -46,7 +46,7 @@ public: void setLayerStack(ui::LayerStack); void setViewport(ui::Size); void changeRefreshRate(const Fps&); - void onInvalidate(); + void animate(); private: class SevenSegmentDrawer { diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index aa2fec56ad..9465b197b2 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -150,7 +150,7 @@ void RegionSamplingThread::checkForStaleLuma() { if (mSampleRequestTime.has_value()) { ATRACE_INT(lumaSamplingStepTag, static_cast(samplingStep::waitForSamplePhase)); mSampleRequestTime.reset(); - mFlinger.scheduleRegionSamplingThread(); + mFlinger.scheduleSample(); } } diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp index 4d51125156..043a536216 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.cpp +++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp @@ -27,49 +27,55 @@ #include "EventThread.h" #include "FrameTimeline.h" #include "MessageQueue.h" -#include "SurfaceFlinger.h" namespace android::impl { -void MessageQueue::Handler::dispatchRefresh() { - if ((mEventMask.fetch_or(eventMaskRefresh) & eventMaskRefresh) == 0) { - mQueue.mLooper->sendMessage(this, Message(MessageQueue::REFRESH)); +void MessageQueue::Handler::dispatchComposite() { + if ((mEventMask.fetch_or(kComposite) & kComposite) == 0) { + mQueue.mLooper->sendMessage(this, Message(kComposite)); } } -void MessageQueue::Handler::dispatchInvalidate(int64_t vsyncId, nsecs_t expectedVSyncTimestamp) { - if ((mEventMask.fetch_or(eventMaskInvalidate) & eventMaskInvalidate) == 0) { +void MessageQueue::Handler::dispatchCommit(int64_t vsyncId, nsecs_t expectedVsyncTime) { + if ((mEventMask.fetch_or(kCommit) & kCommit) == 0) { mVsyncId = vsyncId; - mExpectedVSyncTime = expectedVSyncTimestamp; - mQueue.mLooper->sendMessage(this, Message(MessageQueue::INVALIDATE)); + mExpectedVsyncTime = expectedVsyncTime; + mQueue.mLooper->sendMessage(this, Message(kCommit)); } } -bool MessageQueue::Handler::invalidatePending() { - constexpr auto pendingMask = eventMaskInvalidate | eventMaskRefresh; - return (mEventMask.load() & pendingMask) != 0; +bool MessageQueue::Handler::isFramePending() const { + constexpr auto kPendingMask = kCommit | kComposite; + return (mEventMask.load() & kPendingMask) != 0; } void MessageQueue::Handler::handleMessage(const Message& message) { + const nsecs_t frameTime = systemTime(); switch (message.what) { - case INVALIDATE: - mEventMask.fetch_and(~eventMaskInvalidate); - mQueue.mFlinger->onMessageReceived(message.what, mVsyncId, mExpectedVSyncTime); - break; - case REFRESH: - mEventMask.fetch_and(~eventMaskRefresh); - mQueue.mFlinger->onMessageReceived(message.what, mVsyncId, mExpectedVSyncTime); + case kCommit: + mEventMask.fetch_and(~kCommit); + if (!mQueue.mCompositor.commit(frameTime, mVsyncId, mExpectedVsyncTime)) { + return; + } + // Composite immediately, rather than after pending tasks through scheduleComposite. + [[fallthrough]]; + case kComposite: + mEventMask.fetch_and(~kComposite); + mQueue.mCompositor.composite(frameTime); + mQueue.mCompositor.sample(); break; } } -// --------------------------------------------------------------------------- +MessageQueue::MessageQueue(ICompositor& compositor) + : MessageQueue(compositor, sp::make(*this)) {} -void MessageQueue::init(const sp& flinger) { - mFlinger = flinger; - mLooper = new Looper(true); - mHandler = new Handler(*this); -} +constexpr bool kAllowNonCallbacks = true; + +MessageQueue::MessageQueue(ICompositor& compositor, sp handler) + : mCompositor(compositor), + mLooper(sp::make(kAllowNonCallbacks)), + mHandler(std::move(handler)) {} // TODO(b/169865816): refactor VSyncInjections to use MessageQueue directly // and remove the EventThread from MessageQueue @@ -110,11 +116,13 @@ void MessageQueue::vsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime, ns { std::lock_guard lock(mVsync.mutex); mVsync.lastCallbackTime = std::chrono::nanoseconds(vsyncTime); - mVsync.scheduled = false; + mVsync.scheduledFrameTime.reset(); } - mHandler->dispatchInvalidate(mVsync.tokenManager->generateTokenForPredictions( - {targetWakeupTime, readyTime, vsyncTime}), - vsyncTime); + + const auto vsyncId = mVsync.tokenManager->generateTokenForPredictions( + {targetWakeupTime, readyTime, vsyncTime}); + + mHandler->dispatchCommit(vsyncId, vsyncTime); } void MessageQueue::initVsync(scheduler::VSyncDispatch& dispatch, @@ -135,8 +143,8 @@ void MessageQueue::setDuration(std::chrono::nanoseconds workDuration) { ATRACE_CALL(); std::lock_guard lock(mVsync.mutex); mVsync.workDuration = workDuration; - if (mVsync.scheduled) { - mVsync.expectedWakeupTime = mVsync.registration->schedule( + if (mVsync.scheduledFrameTime) { + mVsync.scheduledFrameTime = mVsync.registration->schedule( {mVsync.workDuration.get().count(), /*readyDuration=*/0, mVsync.lastCallbackTime.count()}); } @@ -168,7 +176,7 @@ void MessageQueue::postMessage(sp&& handler) { mLooper->sendMessage(handler, Message()); } -void MessageQueue::invalidate() { +void MessageQueue::scheduleCommit() { ATRACE_CALL(); { @@ -181,15 +189,14 @@ void MessageQueue::invalidate() { } std::lock_guard lock(mVsync.mutex); - mVsync.scheduled = true; - mVsync.expectedWakeupTime = + mVsync.scheduledFrameTime = mVsync.registration->schedule({.workDuration = mVsync.workDuration.get().count(), .readyDuration = 0, .earliestVsync = mVsync.lastCallbackTime.count()}); } -void MessageQueue::refresh() { - mHandler->dispatchRefresh(); +void MessageQueue::scheduleComposite() { + mHandler->dispatchComposite(); } void MessageQueue::injectorCallback() { @@ -198,24 +205,22 @@ void MessageQueue::injectorCallback() { while ((n = DisplayEventReceiver::getEvents(&mInjector.tube, buffer, 8)) > 0) { for (int i = 0; i < n; i++) { if (buffer[i].header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) { - mHandler->dispatchInvalidate(buffer[i].vsync.vsyncId, - buffer[i].vsync.expectedVSyncTimestamp); + mHandler->dispatchCommit(buffer[i].vsync.vsyncId, + buffer[i].vsync.expectedVSyncTimestamp); break; } } } } -std::optional MessageQueue::nextExpectedInvalidate() { - if (mHandler->invalidatePending()) { - return std::chrono::steady_clock::now(); +auto MessageQueue::getScheduledFrameTime() const -> std::optional { + if (mHandler->isFramePending()) { + return Clock::now(); } std::lock_guard lock(mVsync.mutex); - if (mVsync.scheduled) { - LOG_ALWAYS_FATAL_IF(!mVsync.expectedWakeupTime.has_value(), "callback was never scheduled"); - const auto expectedWakeupTime = std::chrono::nanoseconds(*mVsync.expectedWakeupTime); - return std::optional(expectedWakeupTime); + if (const auto time = mVsync.scheduledFrameTime) { + return Clock::time_point(std::chrono::nanoseconds(*time)); } return std::nullopt; diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h index 58ce9b9a2f..2c908a6983 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.h +++ b/services/surfaceflinger/Scheduler/MessageQueue.h @@ -33,7 +33,14 @@ namespace android { -class SurfaceFlinger; +struct ICompositor { + virtual bool commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expectedVsyncTime) = 0; + virtual void composite(nsecs_t frameTime) = 0; + virtual void sample() = 0; + +protected: + ~ICompositor() = default; +}; template class Task : public MessageHandler { @@ -56,65 +63,65 @@ inline auto makeTask(F&& f) { class MessageQueue { public: - enum { - INVALIDATE = 0, - REFRESH = 1, - }; - virtual ~MessageQueue() = default; - virtual void init(const sp& flinger) = 0; virtual void initVsync(scheduler::VSyncDispatch&, frametimeline::TokenManager&, std::chrono::nanoseconds workDuration) = 0; virtual void setDuration(std::chrono::nanoseconds workDuration) = 0; virtual void setInjector(sp) = 0; virtual void waitMessage() = 0; virtual void postMessage(sp&&) = 0; - virtual void invalidate() = 0; - virtual void refresh() = 0; - virtual std::optional nextExpectedInvalidate() = 0; -}; + virtual void scheduleCommit() = 0; + virtual void scheduleComposite() = 0; -// --------------------------------------------------------------------------- + using Clock = std::chrono::steady_clock; + virtual std::optional getScheduledFrameTime() const = 0; +}; namespace impl { class MessageQueue : public android::MessageQueue { protected: class Handler : public MessageHandler { - enum : uint32_t { - eventMaskInvalidate = 0x1, - eventMaskRefresh = 0x2, - eventMaskTransaction = 0x4 - }; + static constexpr uint32_t kCommit = 0b1; + static constexpr uint32_t kComposite = 0b10; + MessageQueue& mQueue; - std::atomic mEventMask; - std::atomic mVsyncId; - std::atomic mExpectedVSyncTime; + std::atomic mEventMask = 0; + std::atomic mVsyncId = 0; + std::atomic mExpectedVsyncTime = 0; public: - explicit Handler(MessageQueue& queue) : mQueue(queue), mEventMask(0) {} + explicit Handler(MessageQueue& queue) : mQueue(queue) {} void handleMessage(const Message& message) override; - virtual void dispatchRefresh(); - virtual void dispatchInvalidate(int64_t vsyncId, nsecs_t expectedVSyncTimestamp); - virtual bool invalidatePending(); + + bool isFramePending() const; + + virtual void dispatchCommit(int64_t vsyncId, nsecs_t expectedVsyncTime); + void dispatchComposite(); }; friend class Handler; - sp mFlinger; - sp mLooper; + // For tests. + MessageQueue(ICompositor&, sp); + + void vsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime, nsecs_t readyTime); + +private: + ICompositor& mCompositor; + const sp mLooper; + const sp mHandler; struct Vsync { frametimeline::TokenManager* tokenManager = nullptr; std::unique_ptr registration; - std::mutex mutex; + mutable std::mutex mutex; TracedOrdinal workDuration GUARDED_BY(mutex) = {"VsyncWorkDuration-sf", std::chrono::nanoseconds(0)}; std::chrono::nanoseconds lastCallbackTime GUARDED_BY(mutex) = std::chrono::nanoseconds{0}; - bool scheduled GUARDED_BY(mutex) = false; - std::optional expectedWakeupTime GUARDED_BY(mutex); + std::optional scheduledFrameTime GUARDED_BY(mutex); TracedOrdinal value = {"VSYNC-sf", 0}; }; @@ -127,14 +134,11 @@ protected: Vsync mVsync; Injector mInjector; - sp mHandler; - - void vsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime, nsecs_t readyTime); void injectorCallback(); public: - ~MessageQueue() override = default; - void init(const sp& flinger) override; + explicit MessageQueue(ICompositor&); + void initVsync(scheduler::VSyncDispatch&, frametimeline::TokenManager&, std::chrono::nanoseconds workDuration) override; void setDuration(std::chrono::nanoseconds workDuration) override; @@ -143,13 +147,10 @@ public: void waitMessage() override; void postMessage(sp&&) override; - // sends INVALIDATE message at next VSYNC - void invalidate() override; - - // sends REFRESH message at next VSYNC - void refresh() override; + void scheduleCommit() override; + void scheduleComposite() override; - std::optional nextExpectedInvalidate() override; + std::optional getScheduledFrameTime() const override; }; } // namespace impl diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 12e741bbc2..c6a19de9c6 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -870,7 +870,7 @@ DisplayModePtr Scheduler::getPreferredDisplayMode() { void Scheduler::onNewVsyncPeriodChangeTimeline(const hal::VsyncPeriodChangeTimeline& timeline) { if (timeline.refreshRequired) { - mSchedulerCallback.scheduleRefresh(FrameHint::kNone); + mSchedulerCallback.scheduleComposite(FrameHint::kNone); } std::lock_guard lock(mVsyncTimelineLock); @@ -882,12 +882,12 @@ void Scheduler::onNewVsyncPeriodChangeTimeline(const hal::VsyncPeriodChangeTimel } } -void Scheduler::onDisplayRefreshed(nsecs_t timestamp) { - const bool refresh = [=] { +void Scheduler::onPostComposition(nsecs_t presentTime) { + const bool recomposite = [=] { std::lock_guard lock(mVsyncTimelineLock); if (mLastVsyncPeriodChangeTimeline && mLastVsyncPeriodChangeTimeline->refreshRequired) { - if (timestamp < mLastVsyncPeriodChangeTimeline->refreshTimeNanos) { - // We need to schedule another refresh as refreshTimeNanos is still in the future. + if (presentTime < mLastVsyncPeriodChangeTimeline->refreshTimeNanos) { + // We need to composite again as refreshTimeNanos is still in the future. return true; } @@ -896,8 +896,8 @@ void Scheduler::onDisplayRefreshed(nsecs_t timestamp) { return false; }(); - if (refresh) { - mSchedulerCallback.scheduleRefresh(FrameHint::kNone); + if (recomposite) { + mSchedulerCallback.scheduleComposite(FrameHint::kNone); } } diff --git a/services/surfaceflinger/Scheduler/Scheduler.h b/services/surfaceflinger/Scheduler/Scheduler.h index 3f3debe33b..0a33dbbe92 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.h +++ b/services/surfaceflinger/Scheduler/Scheduler.h @@ -59,7 +59,7 @@ struct ISchedulerCallback { // Indicates frame activity, i.e. whether commit and/or composite is taking place. enum class FrameHint { kNone, kActive }; - virtual void scheduleRefresh(FrameHint) = 0; + virtual void scheduleComposite(FrameHint) = 0; virtual void setVsyncEnabled(bool) = 0; virtual void changeRefreshRate(const scheduler::RefreshRateConfigs::RefreshRate&, scheduler::RefreshRateConfigEvent) = 0; @@ -162,8 +162,8 @@ public: // Notifies the scheduler about a refresh rate timeline change. void onNewVsyncPeriodChangeTimeline(const hal::VsyncPeriodChangeTimeline& timeline); - // Notifies the scheduler when the display was refreshed - void onDisplayRefreshed(nsecs_t timestamp); + // Notifies the scheduler post composition. + void onPostComposition(nsecs_t presentTime); // Notifies the scheduler when the display size has changed. Called from SF's main thread void onActiveDisplayAreaChanged(uint32_t displayArea); diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index e90af3a64a..bb65bae06b 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -371,7 +371,7 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory, SkipInitializationTag) mTimeStats(std::make_shared()), mFrameTracer(mFactory.createFrameTracer()), mFrameTimeline(mFactory.createFrameTimeline(mTimeStats, getpid())), - mEventQueue(mFactory.createMessageQueue()), + mEventQueue(mFactory.createMessageQueue(*this)), mCompositionEngine(mFactory.createCompositionEngine()), mHwcServiceName(base::GetProperty("debug.sf.hwc_service_name"s, "default"s)), mTunnelModeEnabledReporter(new TunnelModeEnabledReporter()), @@ -506,10 +506,6 @@ SurfaceFlinger::SurfaceFlinger(Factory& factory) : SurfaceFlinger(factory, SkipI SurfaceFlinger::~SurfaceFlinger() = default; -void SurfaceFlinger::onFirstRef() { - mEventQueue->init(this); -} - void SurfaceFlinger::binderDied(const wp&) { // the window manager died on us. prepare its eulogy. mBootFinished = false; @@ -1104,7 +1100,7 @@ void SurfaceFlinger::setDesiredActiveMode(const ActiveModeInfo& info) { } if (display->setDesiredActiveMode(info)) { - scheduleRefresh(FrameHint::kNone); + scheduleComposite(FrameHint::kNone); // Start receiving vsync samples now, so that we can detect a period // switch. @@ -1704,31 +1700,26 @@ sp SurfaceFlinger::createDisplayEventConnection( return mScheduler->createDisplayEventConnection(handle, eventRegistration); } -void SurfaceFlinger::scheduleInvalidate(FrameHint hint) { +void SurfaceFlinger::scheduleCommit(FrameHint hint) { if (hint == FrameHint::kActive) { mScheduler->resetIdleTimer(); } mPowerAdvisor.notifyDisplayUpdateImminent(); - mEventQueue->invalidate(); + mEventQueue->scheduleCommit(); } -void SurfaceFlinger::scheduleRefresh(FrameHint hint) { - mForceRefresh = true; - scheduleInvalidate(hint); +void SurfaceFlinger::scheduleComposite(FrameHint hint) { + mMustComposite = true; + scheduleCommit(hint); } void SurfaceFlinger::scheduleRepaint() { mGeometryDirty = true; - scheduleRefresh(FrameHint::kActive); -} - -void SurfaceFlinger::signalLayerUpdate() { - scheduleInvalidate(FrameHint::kActive); + scheduleComposite(FrameHint::kActive); } -void SurfaceFlinger::signalRefresh() { - mRefreshPending = true; - mEventQueue->refresh(); +void SurfaceFlinger::scheduleSample() { + static_cast(schedule([this] { sample(); })); } nsecs_t SurfaceFlinger::getVsyncPeriodFromHWC() const { @@ -1834,7 +1825,7 @@ void SurfaceFlinger::onComposerHalSeamlessPossible(hal::HWDisplayId) { void SurfaceFlinger::onComposerHalRefresh(hal::HWDisplayId) { Mutex::Autolock lock(mStateLock); - scheduleRefresh(FrameHint::kNone); + scheduleComposite(FrameHint::kNone); } void SurfaceFlinger::setVsyncEnabled(bool enabled) { @@ -1889,40 +1880,26 @@ nsecs_t SurfaceFlinger::calculateExpectedPresentTime(DisplayStatInfo stats) cons : stats.vsyncTime + stats.vsyncPeriod; } -void SurfaceFlinger::onMessageReceived(int32_t what, int64_t vsyncId, nsecs_t expectedVSyncTime) { - switch (what) { - case MessageQueue::INVALIDATE: { - onMessageInvalidate(vsyncId, expectedVSyncTime); - break; - } - case MessageQueue::REFRESH: { - onMessageRefresh(); - break; - } - } -} - -void SurfaceFlinger::onMessageInvalidate(int64_t vsyncId, nsecs_t expectedVSyncTime) { - const nsecs_t frameStart = systemTime(); +bool SurfaceFlinger::commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expectedVsyncTime) { // calculate the expected present time once and use the cached // value throughout this frame to make sure all layers are // seeing this same value. - if (expectedVSyncTime >= frameStart) { - mExpectedPresentTime = expectedVSyncTime; + if (expectedVsyncTime >= frameTime) { + mExpectedPresentTime = expectedVsyncTime; } else { - const DisplayStatInfo stats = mScheduler->getDisplayStatInfo(frameStart); + const DisplayStatInfo stats = mScheduler->getDisplayStatInfo(frameTime); mExpectedPresentTime = calculateExpectedPresentTime(stats); } const nsecs_t lastScheduledPresentTime = mScheduledPresentTime; - mScheduledPresentTime = expectedVSyncTime; + mScheduledPresentTime = expectedVsyncTime; const auto vsyncIn = [&] { if (!ATRACE_ENABLED()) return 0.f; return (mExpectedPresentTime - systemTime()) / 1e6f; }(); - ATRACE_FORMAT("onMessageInvalidate %" PRId64 " vsyncIn %.2fms%s", vsyncId, vsyncIn, - mExpectedPresentTime == expectedVSyncTime ? "" : " (adjusted)"); + ATRACE_FORMAT("%s %" PRId64 " vsyncIn %.2fms%s", __func__, vsyncId, vsyncIn, + mExpectedPresentTime == expectedVsyncTime ? "" : " (adjusted)"); // When Backpressure propagation is enabled we want to give a small grace period // for the present fence to fire instead of just giving up on this frame to handle cases @@ -1969,11 +1946,11 @@ void SurfaceFlinger::onMessageInvalidate(int64_t vsyncId, nsecs_t expectedVSyncT } // If we are in the middle of a mode change and the fence hasn't - // fired yet just wait for the next invalidate + // fired yet just wait for the next commit. if (mSetActiveModePending) { if (framePending) { - mEventQueue->invalidate(); - return; + mEventQueue->scheduleCommit(); + return false; } // We received the present fence from the HWC, so we assume it successfully updated @@ -1984,8 +1961,8 @@ void SurfaceFlinger::onMessageInvalidate(int64_t vsyncId, nsecs_t expectedVSyncT if (framePending) { if ((hwcFrameMissed && !gpuFrameMissed) || mPropagateBackpressureClientComposition) { - signalLayerUpdate(); - return; + scheduleCommit(FrameHint::kNone); + return false; } } @@ -1997,11 +1974,12 @@ void SurfaceFlinger::onMessageInvalidate(int64_t vsyncId, nsecs_t expectedVSyncT if (mRefreshRateOverlaySpinner) { Mutex::Autolock lock(mStateLock); if (const auto display = getDefaultDisplayDeviceLocked()) { - display->onInvalidate(); + display->animateRefreshRateOverlay(); } } - bool refreshNeeded = mForceRefresh.exchange(false); + // Composite if transactions were committed, or if requested by HWC. + bool mustComposite = mMustComposite.exchange(false); { mTracePostComposition = mTracing.flagIsSet(SurfaceTracing::TRACE_COMPOSITION) || mTracing.flagIsSet(SurfaceTracing::TRACE_SYNC) || @@ -2009,10 +1987,12 @@ void SurfaceFlinger::onMessageInvalidate(int64_t vsyncId, nsecs_t expectedVSyncT const bool tracePreComposition = mTracingEnabled && !mTracePostComposition; ConditionalLockGuard lock(mTracingLock, tracePreComposition); - mFrameTimeline->setSfWakeUp(vsyncId, frameStart, Fps::fromPeriodNsecs(stats.vsyncPeriod)); + mFrameTimeline->setSfWakeUp(vsyncId, frameTime, Fps::fromPeriodNsecs(stats.vsyncPeriod)); + + mustComposite |= flushAndCommitTransactions(); + mustComposite |= latchBuffers(); - refreshNeeded |= flushAndCommitTransactions(); - refreshNeeded |= handleMessageInvalidate(); + updateLayerGeometry(); if (tracePreComposition) { if (mVisibleRegionsDirty) { @@ -2035,25 +2015,7 @@ void SurfaceFlinger::onMessageInvalidate(int64_t vsyncId, nsecs_t expectedVSyncT updateCursorAsync(); updateInputFlinger(); - if (refreshNeeded && CC_LIKELY(mBootStage != BootStage::BOOTLOADER)) { - // Signal a refresh if a transaction modified the window state, - // a new buffer was latched, or if HWC has requested a full - // repaint - if (mFrameStartTime <= 0) { - // We should only use the time of the first invalidate - // message that signals a refresh as the beginning of the - // frame. Otherwise the real frame time will be - // underestimated. - mFrameStartTime = frameStart; - } - - // Run the refresh immediately after invalidate as there is no point going thru the message - // queue again, and to ensure that we actually refresh the screen instead of handling - // other messages that were queued us already in the MessageQueue. - mRefreshPending = true; - onMessageRefresh(); - } - notifyRegionSamplingThread(); + return mustComposite && CC_LIKELY(mBootStage != BootStage::BOOTLOADER); } bool SurfaceFlinger::flushAndCommitTransactions() { @@ -2068,6 +2030,9 @@ bool SurfaceFlinger::flushAndCommitTransactions() { commitTransactions(); } + // Invoke OnCommit callbacks. + mTransactionCallbackInvoker.sendCallbacks(); + if (transactionFlushNeeded()) { setTransactionFlags(eTransactionFlushNeeded); } @@ -2075,11 +2040,9 @@ bool SurfaceFlinger::flushAndCommitTransactions() { return shouldCommit; } -void SurfaceFlinger::onMessageRefresh() { +void SurfaceFlinger::composite(nsecs_t frameTime) { ATRACE_CALL(); - mRefreshPending = false; - compositionengine::CompositionRefreshArgs refreshArgs; const auto& displays = ON_MAIN_THREAD(mDisplays); refreshArgs.outputs.reserve(displays.size()); @@ -2123,18 +2086,16 @@ void SurfaceFlinger::onMessageRefresh() { const auto hwcMinWorkDuration = mVsyncConfiguration->getCurrentConfigs().hwcMinWorkDuration; refreshArgs.earliestPresentTime = prevVsyncTime - hwcMinWorkDuration; refreshArgs.previousPresentFence = mPreviousPresentFences[0].fenceTime; - refreshArgs.nextInvalidateTime = mEventQueue->nextExpectedInvalidate(); + refreshArgs.scheduledFrameTime = mEventQueue->getScheduledFrameTime(); // Store the present time just before calling to the composition engine so we could notify // the scheduler. const auto presentTime = systemTime(); mCompositionEngine->present(refreshArgs); - mTimeStats->recordFrameDuration(mFrameStartTime, systemTime()); - // Reset the frame start time now that we've recorded this frame. - mFrameStartTime = 0; + mTimeStats->recordFrameDuration(frameTime, systemTime()); - mScheduler->onDisplayRefreshed(presentTime); + mScheduler->onPostComposition(presentTime); postFrame(); postComposition(); @@ -2177,17 +2138,12 @@ void SurfaceFlinger::onMessageRefresh() { mVisibleRegionsDirty = false; if (mCompositionEngine->needsAnotherUpdate()) { - signalLayerUpdate(); + scheduleCommit(FrameHint::kNone); } } -bool SurfaceFlinger::handleMessageInvalidate() { +void SurfaceFlinger::updateLayerGeometry() { ATRACE_CALL(); - // Send on commit callbacks - mTransactionCallbackInvoker.sendCallbacks(); - - bool refreshNeeded = handlePageFlip(); - if (mVisibleRegionsDirty) { computeLayerBounds(); @@ -2199,7 +2155,6 @@ bool SurfaceFlinger::handleMessageInvalidate() { invalidateLayerStack(layer, visibleReg); } mLayersPendingRefresh.clear(); - return refreshNeeded; } void SurfaceFlinger::updateCompositorTiming(const DisplayStatInfo& stats, nsecs_t compositeTime, @@ -3301,11 +3256,10 @@ void SurfaceFlinger::invalidateLayerStack(const sp& layer, const Re } } -bool SurfaceFlinger::handlePageFlip() { +bool SurfaceFlinger::latchBuffers() { ATRACE_CALL(); - ALOGV("handlePageFlip"); - nsecs_t latchTime = systemTime(); + const nsecs_t latchTime = systemTime(); bool visibleRegions = false; bool frameQueued = false; @@ -3374,7 +3328,7 @@ bool SurfaceFlinger::handlePageFlip() { // queued frame that shouldn't be displayed during this vsync period, wake // up during the next vsync period to check again. if (frameQueued && (mLayersWithQueuedFrames.empty() || !newDataLatched)) { - signalLayerUpdate(); + scheduleCommit(FrameHint::kNone); } // enter boot animation on first buffer latch @@ -3453,7 +3407,7 @@ uint32_t SurfaceFlinger::setTransactionFlags(uint32_t mask, TransactionSchedule const sp& applyToken) { const uint32_t old = mTransactionFlags.fetch_or(mask); modulateVsync(&VsyncModulator::setTransactionSchedule, schedule, applyToken); - if ((old & mask) == 0) scheduleInvalidate(FrameHint::kActive); + if ((old & mask) == 0) scheduleCommit(FrameHint::kActive); return old; } @@ -4572,7 +4526,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp& display, hal: mVisibleRegionsDirty = true; mHasPoweredOff = true; - scheduleRefresh(FrameHint::kActive); + scheduleComposite(FrameHint::kActive); } else if (mode == hal::PowerMode::OFF) { // Turn off the display if (SurfaceFlinger::setSchedFifo(false) != NO_ERROR) { @@ -5430,8 +5384,8 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r } scheduleRepaint(); return NO_ERROR; - case 1004: // Force refresh ahead of next VSYNC. - scheduleRefresh(FrameHint::kActive); + case 1004: // Force composite ahead of next VSYNC. + scheduleComposite(FrameHint::kActive); return NO_ERROR; case 1005: { // Force commit ahead of next VSYNC. Mutex::Autolock lock(mStateLock); @@ -5439,8 +5393,8 @@ status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* r eTraversalNeeded); return NO_ERROR; } - case 1006: // Force refresh immediately. - signalRefresh(); + case 1006: // Force composite immediately. + mEventQueue->scheduleComposite(); return NO_ERROR; case 1007: // Unused. return NAME_NOT_FOUND; @@ -5834,7 +5788,7 @@ void SurfaceFlinger::kernelTimerChanged(bool expired) { const bool timerExpired = mKernelIdleTimerEnabled && expired; if (display->onKernelTimerChanged(desiredModeId, timerExpired)) { - mEventQueue->invalidate(); + mEventQueue->scheduleCommit(); } })); } @@ -6265,12 +6219,6 @@ status_t SurfaceFlinger::captureScreenCommon( bool canCaptureBlackoutContent = hasCaptureBlackoutContentPermission(); static_cast(schedule([=, renderAreaFuture = std::move(renderAreaFuture)]() mutable { - if (mRefreshPending) { - ALOGW("Skipping screenshot for now"); - captureScreenCommon(std::move(renderAreaFuture), traverseLayers, buffer, regionSampling, - grayscale, captureListener); - return; - } ScreenCaptureResults captureResults; std::unique_ptr renderArea = renderAreaFuture.get(); if (!renderArea) { @@ -6617,6 +6565,10 @@ void SurfaceFlinger::onLayerDestroyed(Layer* layer) { } } +void SurfaceFlinger::onLayerUpdate() { + scheduleCommit(FrameHint::kActive); +} + // WARNING: ONLY CALL THIS FROM LAYER DTOR // Here we add children in the current state to offscreen layers and remove the // layer itself from the offscreen layer list. Since @@ -6941,16 +6893,12 @@ sp SurfaceFlinger::handleLayerCreatedLocked(const sp& handle) { return layer; } -void SurfaceFlinger::scheduleRegionSamplingThread() { - static_cast(schedule([&] { notifyRegionSamplingThread(); })); -} - -void SurfaceFlinger::notifyRegionSamplingThread() { +void SurfaceFlinger::sample() { if (!mLumaSampling || !mRegionSamplingThread) { return; } - mRegionSamplingThread->onCompositionComplete(mEventQueue->nextExpectedInvalidate()); + mRegionSamplingThread->onCompositionComplete(mEventQueue->getScheduledFrameTime()); } void SurfaceFlinger::onActiveDisplaySizeChanged(const sp& activeDisplay) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 1217d6369b..43f7b36090 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -57,6 +57,7 @@ #include "Fps.h" #include "FrameTracker.h" #include "LayerVector.h" +#include "Scheduler/MessageQueue.h" #include "Scheduler/RefreshRateConfigs.h" #include "Scheduler/RefreshRateStats.h" #include "Scheduler/Scheduler.h" @@ -177,6 +178,7 @@ class SurfaceFlinger : public BnSurfaceComposer, public PriorityDumper, private IBinder::DeathRecipient, private HWC2::ComposerCallback, + private ICompositor, private ISchedulerCallback { public: struct SkipInitializationTag {}; @@ -287,11 +289,13 @@ public: [[nodiscard]] std::future schedule(F&&); // Schedule commit of transactions on the main thread ahead of the next VSYNC. - void scheduleInvalidate(FrameHint); - // As above, but also force refresh regardless if transactions were committed. - void scheduleRefresh(FrameHint) override; + void scheduleCommit(FrameHint); + // As above, but also force composite regardless if transactions were committed. + void scheduleComposite(FrameHint) override; // As above, but also force dirty geometry to repaint. void scheduleRepaint(); + // Schedule sampling independently from commit or composite. + void scheduleSample(); surfaceflinger::Factory& getFactory() { return mFactory; } @@ -305,11 +309,6 @@ public: // utility function to delete a texture on the main thread void deleteTextureAsync(uint32_t texture); - // called on the main thread by MessageQueue when an internal message - // is received - // TODO: this should be made accessible only to MessageQueue - void onMessageReceived(int32_t what, int64_t vsyncId, nsecs_t expectedVSyncTime); - renderengine::RenderEngine& getRenderEngine() const; bool authenticateSurfaceTextureLocked( @@ -317,6 +316,7 @@ public: void onLayerFirstRef(Layer*); void onLayerDestroyed(Layer*); + void onLayerUpdate(); void removeHierarchyFromOffscreenLayers(Layer* layer); void removeFromOffscreenLayers(Layer* layer); @@ -734,9 +734,6 @@ private: // Implements IBinder::DeathRecipient. void binderDied(const wp& who) override; - // Implements RefBase. - void onFirstRef() override; - // HWC2::ComposerCallback overrides: void onComposerHalVsync(hal::HWDisplayId, int64_t timestamp, std::optional) override; @@ -746,6 +743,19 @@ private: const hal::VsyncPeriodChangeTimeline&) override; void onComposerHalSeamlessPossible(hal::HWDisplayId) override; + // ICompositor overrides: + + // Commits transactions for layers and displays. Returns whether any state has been invalidated, + // i.e. whether a frame should be composited for each display. + bool commit(nsecs_t frameTime, int64_t vsyncId, nsecs_t expectedVsyncTime) override; + + // Composites a frame for each display. CompositionEngine performs GPU and/or HAL composition + // via RenderEngine and the Composer HAL, respectively. + void composite(nsecs_t frameTime) override; + + // Samples the composited frame via RegionSamplingThread. + void sample() override; + /* * ISchedulerCallback */ @@ -766,13 +776,6 @@ private: // Show spinner with refresh rate overlay bool mRefreshRateOverlaySpinner = false; - /* - * Message handling - */ - // Can only be called from the main thread or with mStateLock held - void signalLayerUpdate(); - void signalRefresh(); - // Called on the main thread in response to initializeDisplays() void onInitializeDisplays() REQUIRES(mStateLock); // Sets the desired active mode bit. It obtains the lock, and sets mDesiredActiveMode. @@ -797,10 +800,6 @@ private: const std::optional& policy, bool overridePolicy) EXCLUDES(mStateLock); - // Handle the INVALIDATE message queue event, latching new buffers and applying - // incoming transactions - void onMessageInvalidate(int64_t vsyncId, nsecs_t expectedVSyncTime); - // Returns whether transactions were committed. bool flushAndCommitTransactions() EXCLUDES(mStateLock); @@ -808,12 +807,10 @@ private: void commitTransactionsLocked(uint32_t transactionFlags) REQUIRES(mStateLock); void doCommitTransactions() REQUIRES(mStateLock); - // Handle the REFRESH message queue event, sending the current frame down to RenderEngine and - // the Composer HAL for presentation - void onMessageRefresh(); + // Returns whether a new buffer has been latched. + bool latchBuffers(); - // Returns whether a new buffer has been latched (see handlePageFlip()) - bool handleMessageInvalidate(); + void updateLayerGeometry(); void updateInputFlinger(); void notifyWindowInfos(); @@ -824,11 +821,6 @@ private: void updatePhaseConfiguration(const Fps&) REQUIRES(mStateLock); void setVsyncConfig(const VsyncModulator::VsyncConfig&, nsecs_t vsyncPeriod); - /* handlePageFlip - latch a new buffer if available and compute the dirty - * region. Returns whether a new buffer has been latched, i.e., whether it - * is necessary to perform a refresh during this vsync. - */ - bool handlePageFlip(); /* * Transactions @@ -1250,7 +1242,7 @@ private: bool mLayersRemoved = false; bool mLayersAdded = false; - std::atomic_bool mForceRefresh = false; + std::atomic_bool mMustComposite = false; std::atomic_bool mGeometryDirty = false; // constant members (no synchronization needed for access) @@ -1350,10 +1342,6 @@ private: mutable Mutex mDestroyedLayerLock; Vector mDestroyedLayers; - nsecs_t mRefreshStartTime = 0; - - std::atomic mRefreshPending = false; - // We maintain a pool of pre-generated texture names to hand out to avoid // layer creation needing to run on the main thread (which it would // otherwise need to do to access RenderEngine). @@ -1446,9 +1434,6 @@ private: Hwc2::impl::PowerAdvisor mPowerAdvisor; - // This should only be accessed on the main thread. - nsecs_t mFrameStartTime = 0; - void enableRefreshRateOverlay(bool enable) REQUIRES(mStateLock); // Flag used to set override desired display mode from backdoor @@ -1502,9 +1487,6 @@ private: std::atomic mActiveDisplayTransformHint; - void scheduleRegionSamplingThread(); - void notifyRegionSamplingThread(); - bool isRefreshRateOverlayEnabled() const REQUIRES(mStateLock) { return std::any_of(mDisplays.begin(), mDisplays.end(), [](std::pair, sp> display) { diff --git a/services/surfaceflinger/SurfaceFlingerDefaultFactory.cpp b/services/surfaceflinger/SurfaceFlingerDefaultFactory.cpp index 89d1c4d327..9a2f9107c3 100644 --- a/services/surfaceflinger/SurfaceFlingerDefaultFactory.cpp +++ b/services/surfaceflinger/SurfaceFlingerDefaultFactory.cpp @@ -51,8 +51,8 @@ std::unique_ptr DefaultFactory::createHWComposer(const std::string& return std::make_unique(serviceName); } -std::unique_ptr DefaultFactory::createMessageQueue() { - return std::make_unique(); +std::unique_ptr DefaultFactory::createMessageQueue(ICompositor& compositor) { + return std::make_unique(compositor); } std::unique_ptr DefaultFactory::createVsyncConfiguration( diff --git a/services/surfaceflinger/SurfaceFlingerDefaultFactory.h b/services/surfaceflinger/SurfaceFlingerDefaultFactory.h index b8bf2baa33..2be09ee788 100644 --- a/services/surfaceflinger/SurfaceFlingerDefaultFactory.h +++ b/services/surfaceflinger/SurfaceFlingerDefaultFactory.h @@ -27,7 +27,7 @@ public: virtual ~DefaultFactory(); std::unique_ptr createHWComposer(const std::string& serviceName) override; - std::unique_ptr createMessageQueue() override; + std::unique_ptr createMessageQueue(ICompositor&) override; std::unique_ptr createVsyncConfiguration( Fps currentRefreshRate) override; std::unique_ptr createScheduler( diff --git a/services/surfaceflinger/SurfaceFlingerFactory.h b/services/surfaceflinger/SurfaceFlingerFactory.h index 13c95ddd15..bca533b794 100644 --- a/services/surfaceflinger/SurfaceFlingerFactory.h +++ b/services/surfaceflinger/SurfaceFlingerFactory.h @@ -31,11 +31,11 @@ namespace android { typedef int32_t PixelFormat; class BufferQueueLayer; -class BufferStateLayer; class BufferLayerConsumer; -class EffectLayer; +class BufferStateLayer; class ContainerLayer; class DisplayDevice; +class EffectLayer; class FrameTracer; class GraphicBuffer; class HWComposer; @@ -50,6 +50,7 @@ class SurfaceInterceptor; class TimeStats; struct DisplayDeviceCreationArgs; +struct ICompositor; struct ISchedulerCallback; struct LayerCreationArgs; @@ -76,7 +77,7 @@ class NativeWindowSurface; class Factory { public: virtual std::unique_ptr createHWComposer(const std::string& serviceName) = 0; - virtual std::unique_ptr createMessageQueue() = 0; + virtual std::unique_ptr createMessageQueue(ICompositor&) = 0; virtual std::unique_ptr createVsyncConfiguration( Fps currentRefreshRate) = 0; virtual std::unique_ptr createScheduler( diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp index 5135ff952f..a0812912d4 100644 --- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp +++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp @@ -202,8 +202,7 @@ void CompositionTest::displayRefreshCompositionDirtyGeometry() { // -------------------------------------------------------------------- // Invocation - mFlinger.onMessageReceived(MessageQueue::INVALIDATE); - mFlinger.onMessageReceived(MessageQueue::REFRESH); + mFlinger.commitAndComposite(); LayerCase::cleanup(this); } @@ -215,8 +214,7 @@ void CompositionTest::displayRefreshCompositionDirtyFrame() { // -------------------------------------------------------------------- // Invocation - mFlinger.onMessageReceived(MessageQueue::INVALIDATE); - mFlinger.onMessageReceived(MessageQueue::REFRESH); + mFlinger.commitAndComposite(); LayerCase::cleanup(this); } @@ -537,7 +535,7 @@ struct BaseLayerProperties { ASSERT_EQ(NO_ERROR, err); Mock::VerifyAndClear(test->mRenderEngine); - EXPECT_CALL(*test->mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*test->mMessageQueue, scheduleCommit()).Times(1); enqueueBuffer(test, layer); Mock::VerifyAndClearExpectations(test->mMessageQueue); @@ -883,7 +881,7 @@ struct BaseLayerVariant { test->mFlinger.mutableDrawingState().layersSortedByZ.clear(); // Layer should be unregistered with scheduler. - test->mFlinger.onMessageReceived(MessageQueue::INVALIDATE); + test->mFlinger.commit(); EXPECT_EQ(0, test->mFlinger.scheduler()->layerHistorySize()); } }; diff --git a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp index dbd51fed78..17d1dd6b09 100644 --- a/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp +++ b/services/surfaceflinger/tests/unittests/MessageQueueTest.cpp @@ -31,62 +31,51 @@ using namespace testing; using CallbackToken = scheduler::VSyncDispatch::CallbackToken; +struct NoOpCompositor final : ICompositor { + bool commit(nsecs_t, int64_t, nsecs_t) override { return false; } + void composite(nsecs_t) override {} + void sample() override {} +} gNoOpCompositor; + class TestableMessageQueue : public impl::MessageQueue { -public: - class MockHandler : public MessageQueue::Handler { - public: - explicit MockHandler(MessageQueue& queue) : MessageQueue::Handler(queue) {} - ~MockHandler() override = default; - MOCK_METHOD2(dispatchInvalidate, void(int64_t vsyncId, nsecs_t expectedVSyncTimestamp)); + struct MockHandler : MessageQueue::Handler { + using MessageQueue::Handler::Handler; + + MOCK_METHOD(void, dispatchCommit, (int64_t, nsecs_t), (override)); }; - TestableMessageQueue() = default; - ~TestableMessageQueue() override = default; + explicit TestableMessageQueue(sp handler) + : impl::MessageQueue(gNoOpCompositor, handler), mHandler(std::move(handler)) {} - void initHandler(const sp& handler) { mHandler = handler; } +public: + TestableMessageQueue() : TestableMessageQueue(sp::make(*this)) {} - void triggerVsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime, nsecs_t readyTime) { - vsyncCallback(vsyncTime, targetWakeupTime, readyTime); - } -}; + using impl::MessageQueue::vsyncCallback; -class MockVSyncDispatch : public scheduler::VSyncDispatch { -public: - MockVSyncDispatch() = default; - ~MockVSyncDispatch() override = default; + const sp mHandler; +}; +struct MockVSyncDispatch : scheduler::VSyncDispatch { MOCK_METHOD2(registerCallback, - CallbackToken(std::function const&, std::string)); + CallbackToken(const std::function&, std::string)); MOCK_METHOD1(unregisterCallback, void(CallbackToken)); MOCK_METHOD2(schedule, scheduler::ScheduleResult(CallbackToken, ScheduleTiming)); MOCK_METHOD1(cancel, scheduler::CancelResult(CallbackToken token)); MOCK_CONST_METHOD1(dump, void(std::string&)); }; -class MockTokenManager : public frametimeline::TokenManager { -public: - MockTokenManager() = default; - ~MockTokenManager() override = default; - +struct MockTokenManager : frametimeline::TokenManager { MOCK_METHOD1(generateTokenForPredictions, int64_t(frametimeline::TimelineItem&& prediction)); MOCK_CONST_METHOD1(getPredictionsForToken, std::optional(int64_t)); }; -class MessageQueueTest : public testing::Test { -public: - MessageQueueTest() = default; - ~MessageQueueTest() override = default; - +struct MessageQueueTest : testing::Test { void SetUp() override { - EXPECT_NO_FATAL_FAILURE(mEventQueue.initHandler(mHandler)); - EXPECT_CALL(mVSyncDispatch, registerCallback(_, "sf")).WillOnce(Return(mCallbackToken)); EXPECT_NO_FATAL_FAILURE(mEventQueue.initVsync(mVSyncDispatch, mTokenManager, mDuration)); EXPECT_CALL(mVSyncDispatch, unregisterCallback(mCallbackToken)).Times(1); } - sp mHandler = - new TestableMessageQueue::MockHandler(mEventQueue); MockVSyncDispatch mVSyncDispatch; MockTokenManager mTokenManager; TestableMessageQueue mEventQueue; @@ -100,45 +89,49 @@ namespace { /* ------------------------------------------------------------------------ * Test cases */ -TEST_F(MessageQueueTest, invalidate) { +TEST_F(MessageQueueTest, commit) { const auto timing = scheduler::VSyncDispatch::ScheduleTiming{.workDuration = mDuration.count(), .readyDuration = 0, .earliestVsync = 0}; - EXPECT_FALSE(mEventQueue.nextExpectedInvalidate().has_value()); + EXPECT_FALSE(mEventQueue.getScheduledFrameTime()); EXPECT_CALL(mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(1234)); - EXPECT_NO_FATAL_FAILURE(mEventQueue.invalidate()); - EXPECT_TRUE(mEventQueue.nextExpectedInvalidate().has_value()); - EXPECT_EQ(1234, mEventQueue.nextExpectedInvalidate().value().time_since_epoch().count()); + EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleCommit()); + + ASSERT_TRUE(mEventQueue.getScheduledFrameTime()); + EXPECT_EQ(1234, mEventQueue.getScheduledFrameTime()->time_since_epoch().count()); } -TEST_F(MessageQueueTest, invalidateTwice) { +TEST_F(MessageQueueTest, commitTwice) { InSequence s; const auto timing = scheduler::VSyncDispatch::ScheduleTiming{.workDuration = mDuration.count(), .readyDuration = 0, .earliestVsync = 0}; EXPECT_CALL(mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(1234)); - EXPECT_NO_FATAL_FAILURE(mEventQueue.invalidate()); - EXPECT_TRUE(mEventQueue.nextExpectedInvalidate().has_value()); - EXPECT_EQ(1234, mEventQueue.nextExpectedInvalidate().value().time_since_epoch().count()); + EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleCommit()); + + ASSERT_TRUE(mEventQueue.getScheduledFrameTime()); + EXPECT_EQ(1234, mEventQueue.getScheduledFrameTime()->time_since_epoch().count()); EXPECT_CALL(mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(4567)); - EXPECT_NO_FATAL_FAILURE(mEventQueue.invalidate()); - EXPECT_TRUE(mEventQueue.nextExpectedInvalidate().has_value()); - EXPECT_EQ(4567, mEventQueue.nextExpectedInvalidate().value().time_since_epoch().count()); + EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleCommit()); + + ASSERT_TRUE(mEventQueue.getScheduledFrameTime()); + EXPECT_EQ(4567, mEventQueue.getScheduledFrameTime()->time_since_epoch().count()); } -TEST_F(MessageQueueTest, invalidateTwiceWithCallback) { +TEST_F(MessageQueueTest, commitTwiceWithCallback) { InSequence s; const auto timing = scheduler::VSyncDispatch::ScheduleTiming{.workDuration = mDuration.count(), .readyDuration = 0, .earliestVsync = 0}; EXPECT_CALL(mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(1234)); - EXPECT_NO_FATAL_FAILURE(mEventQueue.invalidate()); - EXPECT_TRUE(mEventQueue.nextExpectedInvalidate().has_value()); - EXPECT_EQ(1234, mEventQueue.nextExpectedInvalidate().value().time_since_epoch().count()); + EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleCommit()); + + ASSERT_TRUE(mEventQueue.getScheduledFrameTime()); + EXPECT_EQ(1234, mEventQueue.getScheduledFrameTime()->time_since_epoch().count()); const auto startTime = 100; const auto endTime = startTime + mDuration.count(); @@ -148,10 +141,10 @@ TEST_F(MessageQueueTest, invalidateTwiceWithCallback) { generateTokenForPredictions( frametimeline::TimelineItem(startTime, endTime, presentTime))) .WillOnce(Return(vsyncId)); - EXPECT_CALL(*mHandler, dispatchInvalidate(vsyncId, presentTime)).Times(1); - EXPECT_NO_FATAL_FAILURE(mEventQueue.triggerVsyncCallback(presentTime, startTime, endTime)); + EXPECT_CALL(*mEventQueue.mHandler, dispatchCommit(vsyncId, presentTime)).Times(1); + EXPECT_NO_FATAL_FAILURE(mEventQueue.vsyncCallback(presentTime, startTime, endTime)); - EXPECT_FALSE(mEventQueue.nextExpectedInvalidate().has_value()); + EXPECT_FALSE(mEventQueue.getScheduledFrameTime()); const auto timingAfterCallback = scheduler::VSyncDispatch::ScheduleTiming{.workDuration = mDuration.count(), @@ -159,10 +152,10 @@ TEST_F(MessageQueueTest, invalidateTwiceWithCallback) { .earliestVsync = presentTime}; EXPECT_CALL(mVSyncDispatch, schedule(mCallbackToken, timingAfterCallback)).WillOnce(Return(0)); - EXPECT_NO_FATAL_FAILURE(mEventQueue.invalidate()); + EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleCommit()); } -TEST_F(MessageQueueTest, invalidateWithDurationChange) { +TEST_F(MessageQueueTest, commitWithDurationChange) { EXPECT_NO_FATAL_FAILURE(mEventQueue.setDuration(mDifferentDuration)); const auto timing = @@ -171,7 +164,7 @@ TEST_F(MessageQueueTest, invalidateWithDurationChange) { .earliestVsync = 0}; EXPECT_CALL(mVSyncDispatch, schedule(mCallbackToken, timing)).WillOnce(Return(0)); - EXPECT_NO_FATAL_FAILURE(mEventQueue.invalidate()); + EXPECT_NO_FATAL_FAILURE(mEventQueue.scheduleCommit()); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp index 84fa1e290a..6c3b2fdc6c 100644 --- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp +++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp @@ -176,7 +176,7 @@ namespace { * Test cases */ TEST_P(SetFrameRateTest, SetAndGet) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); const auto& layerFactory = GetParam(); @@ -187,7 +187,7 @@ TEST_P(SetFrameRateTest, SetAndGet) { } TEST_P(SetFrameRateTest, SetAndGetParent) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); const auto& layerFactory = GetParam(); @@ -212,7 +212,7 @@ TEST_P(SetFrameRateTest, SetAndGetParent) { } TEST_P(SetFrameRateTest, SetAndGetParentAllVote) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); const auto& layerFactory = GetParam(); @@ -251,7 +251,7 @@ TEST_P(SetFrameRateTest, SetAndGetParentAllVote) { } TEST_P(SetFrameRateTest, SetAndGetChild) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); const auto& layerFactory = GetParam(); @@ -276,7 +276,7 @@ TEST_P(SetFrameRateTest, SetAndGetChild) { } TEST_P(SetFrameRateTest, SetAndGetChildAllVote) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); const auto& layerFactory = GetParam(); @@ -315,7 +315,7 @@ TEST_P(SetFrameRateTest, SetAndGetChildAllVote) { } TEST_P(SetFrameRateTest, SetAndGetChildAddAfterVote) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); const auto& layerFactory = GetParam(); @@ -345,7 +345,7 @@ TEST_P(SetFrameRateTest, SetAndGetChildAddAfterVote) { } TEST_P(SetFrameRateTest, SetAndGetChildRemoveAfterVote) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); const auto& layerFactory = GetParam(); @@ -376,7 +376,7 @@ TEST_P(SetFrameRateTest, SetAndGetChildRemoveAfterVote) { } TEST_P(SetFrameRateTest, SetAndGetParentNotInTree) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); const auto& layerFactory = GetParam(); @@ -475,7 +475,7 @@ TEST_P(SetFrameRateTest, SetOnParentActivatesTree) { } TEST_P(SetFrameRateTest, addChildForParentWithTreeVote) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); const auto& layerFactory = GetParam(); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp index 2362a31827..8c3034178f 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_CreateDisplayTest.cpp @@ -51,8 +51,8 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForNonsecureDisplay) { // -------------------------------------------------------------------- // Cleanup conditions - // Destroying the display invalidates the display state. - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + // Creating the display commits a display transaction. + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); } TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) { @@ -86,9 +86,9 @@ TEST_F(CreateDisplayTest, createDisplaySetsCurrentStateForSecureDisplay) { // -------------------------------------------------------------------- // Cleanup conditions - // Destroying the display invalidates the display state. - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + // Creating the display commits a display transaction. + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); } } // namespace -} // namespace android \ No newline at end of file +} // namespace android diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp index e2be074fc4..7087fb6568 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_DestroyDisplayTest.cpp @@ -40,8 +40,8 @@ TEST_F(DestroyDisplayTest, destroyDisplayClearsCurrentStateForDisplay) { // The call should notify the interceptor that a display was created. EXPECT_CALL(*mSurfaceInterceptor, saveDisplayDeletion(_)).Times(1); - // Destroying the display invalidates the display state. - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + // Destroying the display commits a display transaction. + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); // -------------------------------------------------------------------- // Invocation diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp index bd89397ef6..29ff0cd90b 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_HotplugTest.cpp @@ -38,9 +38,8 @@ TEST_F(HotplugTest, enqueuesEventsForDisplayTransaction) { // -------------------------------------------------------------------- // Call Expectations - // We expect invalidate() to be invoked once to trigger display transaction - // processing. - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + // We expect a scheduled commit for the display transaction. + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); // -------------------------------------------------------------------- // Invocation @@ -86,9 +85,8 @@ TEST_F(HotplugTest, processesEnqueuedEventsIfCalledOnMainThread) { // -------------------------------------------------------------------- // Call Expectations - // We expect invalidate() to be invoked once to trigger display transaction - // processing. - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + // We expect a scheduled commit for the display transaction. + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); // -------------------------------------------------------------------- // Invocation diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp index bafa910270..e1b44cf549 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_OnInitializeDisplaysTest.cpp @@ -46,9 +46,8 @@ TEST_F(OnInitializeDisplaysTest, onInitializeDisplaysSetsUpPrimaryDisplay) { // We expect a call to get the active display config. Case::Display::setupHwcGetActiveConfigCallExpectations(this); - // We expect invalidate() to be invoked once to trigger display transaction - // processing. - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + // We expect a scheduled commit for the display transaction. + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); EXPECT_CALL(*mVSyncTracker, nextAnticipatedVSyncTimeFrom(_)).WillRepeatedly(Return(0)); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp index 65024202b8..e55721dd6e 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_SetPowerModeInternalTest.cpp @@ -268,7 +268,7 @@ struct DisplayPowerCase { } static void setupRepaintEverythingCallExpectations(DisplayTransactionTest* test) { - EXPECT_CALL(*test->mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*test->mMessageQueue, scheduleCommit()).Times(1); } static void setupSurfaceInterceptorCallExpectations(DisplayTransactionTest* test, diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 7072439cea..de1caded50 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -73,8 +73,8 @@ public: return nullptr; } - std::unique_ptr createMessageQueue() override { - return std::make_unique(); + std::unique_ptr createMessageQueue(ICompositor& compositor) override { + return std::make_unique(compositor); } std::unique_ptr createVsyncConfiguration( @@ -296,6 +296,16 @@ public: * Forwarding for functions being tested */ + nsecs_t commit() { + constexpr int64_t kVsyncId = 123; + const nsecs_t now = systemTime(); + const nsecs_t expectedVsyncTime = now + 10'000'000; + mFlinger->commit(now, kVsyncId, expectedVsyncTime); + return now; + } + + void commitAndComposite() { mFlinger->composite(commit()); } + auto createDisplay(const String8& displayName, bool secure) { return mFlinger->createDisplay(displayName, secure); } @@ -343,10 +353,6 @@ public: return mFlinger->setPowerModeInternal(display, mode); } - auto onMessageReceived(int32_t what) { - return mFlinger->onMessageReceived(what, /*vsyncId=*/0, systemTime()); - } - auto renderScreenImplLocked(const RenderArea& renderArea, SurfaceFlinger::TraverseLayersFunction traverseLayers, const std::shared_ptr& buffer, @@ -761,7 +767,7 @@ public: }; private: - void scheduleRefresh(FrameHint) override {} + void scheduleComposite(FrameHint) override {} void setVsyncEnabled(bool) override {} void changeRefreshRate(const Scheduler::RefreshRate&, Scheduler::ModeEvent) override {} void kernelTimerChanged(bool) override {} diff --git a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp index 1a50427b93..d8e68b8966 100644 --- a/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp +++ b/services/surfaceflinger/tests/unittests/TransactionApplicationTest.cpp @@ -126,8 +126,7 @@ public: void NotPlacedOnTransactionQueue(uint32_t flags, bool syncInputWindows) { ASSERT_EQ(0u, mFlinger.getTransactionQueue().size()); - // called in SurfaceFlinger::signalTransaction - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); TransactionInfo transaction; setupSingle(transaction, flags, syncInputWindows, /*desiredPresentTime*/ systemTime(), /*isAutoTimestamp*/ true, @@ -157,8 +156,7 @@ public: void PlaceOnTransactionQueue(uint32_t flags, bool syncInputWindows) { ASSERT_EQ(0u, mFlinger.getTransactionQueue().size()); - // called in SurfaceFlinger::signalTransaction - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); // first check will see desired present time has not passed, // but afterwards it will look like the desired present time has passed @@ -187,12 +185,11 @@ public: void BlockedByPriorTransaction(uint32_t flags, bool syncInputWindows) { ASSERT_EQ(0u, mFlinger.getTransactionQueue().size()); - // called in SurfaceFlinger::signalTransaction nsecs_t time = systemTime(); if (!syncInputWindows) { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(2); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(2); } else { - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); } // transaction that should go on the pending thread TransactionInfo transactionA; @@ -255,8 +252,7 @@ public: TEST_F(TransactionApplicationTest, Flush_RemovesFromQueue) { ASSERT_EQ(0u, mFlinger.getTransactionQueue().size()); - // called in SurfaceFlinger::signalTransaction - EXPECT_CALL(*mMessageQueue, invalidate()).Times(1); + EXPECT_CALL(*mMessageQueue, scheduleCommit()).Times(1); TransactionInfo transactionA; // transaction to go on pending queue setupSingle(transactionA, /*flags*/ 0, /*syncInputWindows*/ false, diff --git a/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.h b/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.h index 0e7b320787..d68433760d 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.h +++ b/services/surfaceflinger/tests/unittests/mock/MockMessageQueue.h @@ -29,17 +29,18 @@ public: MessageQueue(); ~MessageQueue() override; - MOCK_METHOD1(init, void(const sp&)); MOCK_METHOD1(setInjector, void(sp)); MOCK_METHOD0(waitMessage, void()); MOCK_METHOD1(postMessage, void(sp&&)); - MOCK_METHOD0(invalidate, void()); - MOCK_METHOD0(refresh, void()); MOCK_METHOD3(initVsync, void(scheduler::VSyncDispatch&, frametimeline::TokenManager&, std::chrono::nanoseconds)); MOCK_METHOD1(setDuration, void(std::chrono::nanoseconds workDuration)); - MOCK_METHOD0(nextExpectedInvalidate, std::optional()); + + MOCK_METHOD(void, scheduleCommit, (), (override)); + MOCK_METHOD(void, scheduleComposite, (), (override)); + + MOCK_METHOD(std::optional, getScheduledFrameTime, (), (const, override)); }; } // namespace android::mock diff --git a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h index 291559f2f9..e241dc903f 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h +++ b/services/surfaceflinger/tests/unittests/mock/MockSchedulerCallback.h @@ -23,7 +23,7 @@ namespace android::mock { struct SchedulerCallback final : ISchedulerCallback { - MOCK_METHOD(void, scheduleRefresh, (FrameHint), (override)); + MOCK_METHOD(void, scheduleComposite, (FrameHint), (override)); MOCK_METHOD1(setVsyncEnabled, void(bool)); MOCK_METHOD2(changeRefreshRate, void(const scheduler::RefreshRateConfigs::RefreshRate&, @@ -33,7 +33,7 @@ struct SchedulerCallback final : ISchedulerCallback { }; struct NoOpSchedulerCallback final : ISchedulerCallback { - void scheduleRefresh(FrameHint) override {} + void scheduleComposite(FrameHint) override {} void setVsyncEnabled(bool) override {} void changeRefreshRate(const scheduler::RefreshRateConfigs::RefreshRate&, scheduler::RefreshRateConfigEvent) override {} -- cgit v1.2.3-59-g8ed1b From f6b4ba6692807b35e909efaca1d8201bcdd5c397 Mon Sep 17 00:00:00 2001 From: Dominik Laskowski Date: Tue, 9 Nov 2021 12:46:10 -0800 Subject: SF: Set up libscheduler headers Start pulling Scheduler sources into a libscheduler target akin to librenderengine and libcompositionengine. Bug: 185535769 Test: Build Change-Id: I8ee871cce96209c8c53601152501129b09c5e46f --- services/surfaceflinger/Android.bp | 2 +- .../surfaceflinger/DisplayHardware/DisplayMode.h | 15 +-- services/surfaceflinger/Fps.h | 128 --------------------- services/surfaceflinger/FrameTimeline/Android.bp | 3 + .../surfaceflinger/FrameTimeline/FrameTimeline.h | 15 ++- services/surfaceflinger/Layer.h | 7 +- services/surfaceflinger/RefreshRateOverlay.cpp | 2 +- services/surfaceflinger/RefreshRateOverlay.h | 12 +- services/surfaceflinger/Scheduler/Android.bp | 26 +++++ services/surfaceflinger/Scheduler/LayerInfo.h | 10 +- .../surfaceflinger/Scheduler/RefreshRateConfigs.h | 11 +- .../surfaceflinger/Scheduler/RefreshRateStats.h | 3 +- services/surfaceflinger/Scheduler/Seamlessness.h | 54 --------- services/surfaceflinger/Scheduler/VSyncTracker.h | 4 +- .../surfaceflinger/Scheduler/VsyncConfiguration.h | 3 +- .../Scheduler/include/scheduler/Fps.h | 128 +++++++++++++++++++++ .../Scheduler/include/scheduler/Seamlessness.h | 52 +++++++++ services/surfaceflinger/SurfaceFlinger.h | 5 +- services/surfaceflinger/SurfaceFlingerFactory.h | 10 +- services/surfaceflinger/TimeStats/Android.bp | 6 + services/surfaceflinger/TimeStats/TimeStats.h | 12 +- services/surfaceflinger/tests/unittests/Android.bp | 3 +- services/surfaceflinger/tests/unittests/FpsOps.h | 2 +- .../surfaceflinger/tests/unittests/FpsTest.cpp | 7 +- .../tests/unittests/LayerInfoTest.cpp | 3 +- 25 files changed, 289 insertions(+), 234 deletions(-) delete mode 100644 services/surfaceflinger/Fps.h create mode 100644 services/surfaceflinger/Scheduler/Android.bp delete mode 100644 services/surfaceflinger/Scheduler/Seamlessness.h create mode 100644 services/surfaceflinger/Scheduler/include/scheduler/Fps.h create mode 100644 services/surfaceflinger/Scheduler/include/scheduler/Seamlessness.h (limited to 'services/surfaceflinger/RefreshRateOverlay.cpp') diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp index fb42cc02eb..29636f84f5 100644 --- a/services/surfaceflinger/Android.bp +++ b/services/surfaceflinger/Android.bp @@ -78,10 +78,10 @@ cc_defaults { "libframetimeline", "libperfetto_client_experimental", "librenderengine", + "libscheduler", "libserviceutils", "libtonemap", "libtrace_proto", - "libaidlcommonsupport", ], header_libs: [ "android.hardware.graphics.composer@2.1-command-buffer", diff --git a/services/surfaceflinger/DisplayHardware/DisplayMode.h b/services/surfaceflinger/DisplayHardware/DisplayMode.h index 5de622b318..0ab9605fca 100644 --- a/services/surfaceflinger/DisplayHardware/DisplayMode.h +++ b/services/surfaceflinger/DisplayHardware/DisplayMode.h @@ -16,9 +16,9 @@ #pragma once -#include "DisplayHardware/Hal.h" -#include "Fps.h" -#include "Scheduler/StrongTyping.h" +#include +#include +#include #include #include @@ -27,9 +27,10 @@ #include #include -#include -#include -#include +#include + +#include "DisplayHardware/Hal.h" +#include "Scheduler/StrongTyping.h" namespace android { @@ -161,4 +162,4 @@ inline std::string to_string(const DisplayMode& mode) { mode.getDpiY(), mode.getGroup()); } -} // namespace android \ No newline at end of file +} // namespace android diff --git a/services/surfaceflinger/Fps.h b/services/surfaceflinger/Fps.h deleted file mode 100644 index 639b3e5ed8..0000000000 --- a/services/surfaceflinger/Fps.h +++ /dev/null @@ -1,128 +0,0 @@ -/* - * Copyright 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include -#include -#include -#include - -#include -#include - -namespace android { - -// Frames per second, stored as floating-point frequency. Provides conversion from/to period in -// nanoseconds, and relational operators with precision threshold. -// -// const Fps fps = 60_Hz; -// -// using namespace fps_approx_ops; -// assert(fps == Fps::fromPeriodNsecs(16'666'667)); -// -class Fps { -public: - constexpr Fps() = default; - - static constexpr Fps fromValue(float frequency) { - return frequency > 0.f ? Fps(frequency, static_cast(1e9f / frequency)) : Fps(); - } - - static constexpr Fps fromPeriodNsecs(nsecs_t period) { - return period > 0 ? Fps(1e9f / period, period) : Fps(); - } - - constexpr bool isValid() const { return mFrequency > 0.f; } - - constexpr float getValue() const { return mFrequency; } - int getIntValue() const { return static_cast(std::round(mFrequency)); } - - constexpr nsecs_t getPeriodNsecs() const { return mPeriod; } - -private: - constexpr Fps(float frequency, nsecs_t period) : mFrequency(frequency), mPeriod(period) {} - - float mFrequency = 0.f; - nsecs_t mPeriod = 0; -}; - -static_assert(std::is_trivially_copyable_v); - -constexpr Fps operator""_Hz(unsigned long long frequency) { - return Fps::fromValue(static_cast(frequency)); -} - -constexpr Fps operator""_Hz(long double frequency) { - return Fps::fromValue(static_cast(frequency)); -} - -inline bool isStrictlyLess(Fps lhs, Fps rhs) { - return lhs.getValue() < rhs.getValue(); -} - -// Does not satisfy equivalence relation. -inline bool isApproxEqual(Fps lhs, Fps rhs) { - // TODO(b/185536303): Replace with ULP distance. - return std::abs(lhs.getValue() - rhs.getValue()) < 0.001f; -} - -// Does not satisfy strict weak order. -inline bool isApproxLess(Fps lhs, Fps rhs) { - return isStrictlyLess(lhs, rhs) && !isApproxEqual(lhs, rhs); -} - -namespace fps_approx_ops { - -inline bool operator==(Fps lhs, Fps rhs) { - return isApproxEqual(lhs, rhs); -} - -inline bool operator<(Fps lhs, Fps rhs) { - return isApproxLess(lhs, rhs); -} - -inline bool operator!=(Fps lhs, Fps rhs) { - return !isApproxEqual(lhs, rhs); -} - -inline bool operator>(Fps lhs, Fps rhs) { - return isApproxLess(rhs, lhs); -} - -inline bool operator<=(Fps lhs, Fps rhs) { - return !isApproxLess(rhs, lhs); -} - -inline bool operator>=(Fps lhs, Fps rhs) { - return !isApproxLess(lhs, rhs); -} - -} // namespace fps_approx_ops - -struct FpsApproxEqual { - bool operator()(Fps lhs, Fps rhs) const { return isApproxEqual(lhs, rhs); } -}; - -inline std::string to_string(Fps fps) { - return base::StringPrintf("%.2f Hz", fps.getValue()); -} - -inline std::ostream& operator<<(std::ostream& stream, Fps fps) { - return stream << to_string(fps); -} - -} // namespace android diff --git a/services/surfaceflinger/FrameTimeline/Android.bp b/services/surfaceflinger/FrameTimeline/Android.bp index 10a58333f9..2d4ec04cfc 100644 --- a/services/surfaceflinger/FrameTimeline/Android.bp +++ b/services/surfaceflinger/FrameTimeline/Android.bp @@ -13,6 +13,9 @@ cc_library_static { srcs: [ "FrameTimeline.cpp", ], + header_libs: [ + "libscheduler_headers", + ], shared_libs: [ "android.hardware.graphics.composer@2.4", "libbase", diff --git a/services/surfaceflinger/FrameTimeline/FrameTimeline.h b/services/surfaceflinger/FrameTimeline/FrameTimeline.h index 139f91f3bc..d08344ef6e 100644 --- a/services/surfaceflinger/FrameTimeline/FrameTimeline.h +++ b/services/surfaceflinger/FrameTimeline/FrameTimeline.h @@ -16,8 +16,14 @@ #pragma once -#include <../Fps.h> -#include <../TimeStats/TimeStats.h> +#include +#include +#include +#include +#include +#include +#include + #include #include #include @@ -28,8 +34,9 @@ #include #include -#include -#include +#include + +#include "../TimeStats/TimeStats.h" namespace android::frametimeline { diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h index bda1c28d27..041b439ac7 100644 --- a/services/surfaceflinger/Layer.h +++ b/services/surfaceflinger/Layer.h @@ -18,7 +18,6 @@ #pragma once #include -#include #include #include #include @@ -39,6 +38,10 @@ #include #include +#include +#include +#include + #include #include #include @@ -49,13 +52,11 @@ #include "ClientCache.h" #include "DisplayHardware/ComposerHal.h" #include "DisplayHardware/HWComposer.h" -#include "Fps.h" #include "FrameTracker.h" #include "LayerVector.h" #include "MonitoredProducer.h" #include "RenderArea.h" #include "Scheduler/LayerInfo.h" -#include "Scheduler/Seamlessness.h" #include "SurfaceFlinger.h" #include "Tracing/LayerTracing.h" #include "TransactionCallbackInvoker.h" diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 2502d66a67..712ab168a2 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -276,7 +276,7 @@ void RefreshRateOverlay::setLayerStack(ui::LayerStack stack) { t.apply(); } -void RefreshRateOverlay::changeRefreshRate(const Fps& fps) { +void RefreshRateOverlay::changeRefreshRate(Fps fps) { mCurrentFps = fps.getIntValue(); auto buffer = getOrCreateBuffers(*mCurrentFps)[mFrame]; SurfaceComposerClient::Transaction t; diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h index 65d446c751..381df37903 100644 --- a/services/surfaceflinger/RefreshRateOverlay.h +++ b/services/surfaceflinger/RefreshRateOverlay.h @@ -16,6 +16,10 @@ #pragma once +#include +#include +#include + #include #include #include @@ -23,11 +27,7 @@ #include #include -#include -#include -#include - -#include "Fps.h" +#include namespace android { @@ -45,7 +45,7 @@ public: void setLayerStack(ui::LayerStack); void setViewport(ui::Size); - void changeRefreshRate(const Fps&); + void changeRefreshRate(Fps); void animate(); private: diff --git a/services/surfaceflinger/Scheduler/Android.bp b/services/surfaceflinger/Scheduler/Android.bp new file mode 100644 index 0000000000..2318a575eb --- /dev/null +++ b/services/surfaceflinger/Scheduler/Android.bp @@ -0,0 +1,26 @@ +cc_defaults { + name: "libscheduler_defaults", + defaults: ["surfaceflinger_defaults"], + cflags: [ + "-DLOG_TAG=\"Scheduler\"", + "-DATRACE_TAG=ATRACE_TAG_GRAPHICS", + ], + shared_libs: [ + "libbase", + "libutils", + ], +} + +cc_library_headers { + name: "libscheduler_headers", + defaults: ["libscheduler_defaults"], + export_include_dirs: ["include"], +} + +cc_library_static { + name: "libscheduler", + defaults: ["libscheduler_defaults"], + srcs: [], + local_include_dirs: ["include"], + export_include_dirs: ["include"], +} diff --git a/services/surfaceflinger/Scheduler/LayerInfo.h b/services/surfaceflinger/Scheduler/LayerInfo.h index 92abbae532..18ed95ec0f 100644 --- a/services/surfaceflinger/Scheduler/LayerInfo.h +++ b/services/surfaceflinger/Scheduler/LayerInfo.h @@ -16,15 +16,19 @@ #pragma once +#include +#include +#include +#include +#include + #include #include -#include -#include +#include #include "LayerHistory.h" #include "RefreshRateConfigs.h" -#include "Scheduler/Seamlessness.h" #include "SchedulerUtils.h" namespace android { diff --git a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h index 53472efae9..85c31e8d5b 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateConfigs.h +++ b/services/surfaceflinger/Scheduler/RefreshRateConfigs.h @@ -16,20 +16,21 @@ #pragma once -#include -#include - #include #include #include #include +#include +#include + +#include +#include + #include "DisplayHardware/DisplayMode.h" #include "DisplayHardware/HWComposer.h" -#include "Fps.h" #include "Scheduler/OneShotTimer.h" #include "Scheduler/SchedulerUtils.h" -#include "Scheduler/Seamlessness.h" #include "Scheduler/StrongTyping.h" namespace android::scheduler { diff --git a/services/surfaceflinger/Scheduler/RefreshRateStats.h b/services/surfaceflinger/Scheduler/RefreshRateStats.h index 80aa96fd63..23ebb061e4 100644 --- a/services/surfaceflinger/Scheduler/RefreshRateStats.h +++ b/services/surfaceflinger/Scheduler/RefreshRateStats.h @@ -23,7 +23,8 @@ #include #include -#include "Fps.h" +#include + #include "Scheduler/SchedulerUtils.h" #include "TimeStats/TimeStats.h" diff --git a/services/surfaceflinger/Scheduler/Seamlessness.h b/services/surfaceflinger/Scheduler/Seamlessness.h deleted file mode 100644 index 3e42a4db37..0000000000 --- a/services/surfaceflinger/Scheduler/Seamlessness.h +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2020 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#pragma once - -#include -#include - -namespace android { -namespace scheduler { - -// The seamlessness requirement of a Layer. -enum class Seamlessness { - // Indicates a requirement for a seamless mode switch. - OnlySeamless, - // Indicates that both seamless and seamed mode switches are allowed. - SeamedAndSeamless, - // Indicates no preference for seamlessness. For such layers the system will - // prefer seamless switches, but also non-seamless switches to the group of the - // default config are allowed. - Default -}; - -inline std::string toString(Seamlessness seamlessness) { - switch (seamlessness) { - case Seamlessness::OnlySeamless: - return "OnlySeamless"; - case Seamlessness::SeamedAndSeamless: - return "SeamedAndSeamless"; - case Seamlessness::Default: - return "Default"; - } -} - -// Used by gtest -inline std::ostream& operator<<(std::ostream& os, Seamlessness val) { - return os << toString(val); -} - -} // namespace scheduler -} // namespace android diff --git a/services/surfaceflinger/Scheduler/VSyncTracker.h b/services/surfaceflinger/Scheduler/VSyncTracker.h index 95750ad5cc..76315d2b5b 100644 --- a/services/surfaceflinger/Scheduler/VSyncTracker.h +++ b/services/surfaceflinger/Scheduler/VSyncTracker.h @@ -17,7 +17,9 @@ #pragma once #include -#include "Fps.h" + +#include + #include "VSyncDispatch.h" namespace android::scheduler { diff --git a/services/surfaceflinger/Scheduler/VsyncConfiguration.h b/services/surfaceflinger/Scheduler/VsyncConfiguration.h index 844751270e..02ebd70272 100644 --- a/services/surfaceflinger/Scheduler/VsyncConfiguration.h +++ b/services/surfaceflinger/Scheduler/VsyncConfiguration.h @@ -23,7 +23,8 @@ #include #include -#include "Fps.h" +#include + #include "VsyncModulator.h" namespace android::scheduler { diff --git a/services/surfaceflinger/Scheduler/include/scheduler/Fps.h b/services/surfaceflinger/Scheduler/include/scheduler/Fps.h new file mode 100644 index 0000000000..639b3e5ed8 --- /dev/null +++ b/services/surfaceflinger/Scheduler/include/scheduler/Fps.h @@ -0,0 +1,128 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include +#include + +#include +#include + +namespace android { + +// Frames per second, stored as floating-point frequency. Provides conversion from/to period in +// nanoseconds, and relational operators with precision threshold. +// +// const Fps fps = 60_Hz; +// +// using namespace fps_approx_ops; +// assert(fps == Fps::fromPeriodNsecs(16'666'667)); +// +class Fps { +public: + constexpr Fps() = default; + + static constexpr Fps fromValue(float frequency) { + return frequency > 0.f ? Fps(frequency, static_cast(1e9f / frequency)) : Fps(); + } + + static constexpr Fps fromPeriodNsecs(nsecs_t period) { + return period > 0 ? Fps(1e9f / period, period) : Fps(); + } + + constexpr bool isValid() const { return mFrequency > 0.f; } + + constexpr float getValue() const { return mFrequency; } + int getIntValue() const { return static_cast(std::round(mFrequency)); } + + constexpr nsecs_t getPeriodNsecs() const { return mPeriod; } + +private: + constexpr Fps(float frequency, nsecs_t period) : mFrequency(frequency), mPeriod(period) {} + + float mFrequency = 0.f; + nsecs_t mPeriod = 0; +}; + +static_assert(std::is_trivially_copyable_v); + +constexpr Fps operator""_Hz(unsigned long long frequency) { + return Fps::fromValue(static_cast(frequency)); +} + +constexpr Fps operator""_Hz(long double frequency) { + return Fps::fromValue(static_cast(frequency)); +} + +inline bool isStrictlyLess(Fps lhs, Fps rhs) { + return lhs.getValue() < rhs.getValue(); +} + +// Does not satisfy equivalence relation. +inline bool isApproxEqual(Fps lhs, Fps rhs) { + // TODO(b/185536303): Replace with ULP distance. + return std::abs(lhs.getValue() - rhs.getValue()) < 0.001f; +} + +// Does not satisfy strict weak order. +inline bool isApproxLess(Fps lhs, Fps rhs) { + return isStrictlyLess(lhs, rhs) && !isApproxEqual(lhs, rhs); +} + +namespace fps_approx_ops { + +inline bool operator==(Fps lhs, Fps rhs) { + return isApproxEqual(lhs, rhs); +} + +inline bool operator<(Fps lhs, Fps rhs) { + return isApproxLess(lhs, rhs); +} + +inline bool operator!=(Fps lhs, Fps rhs) { + return !isApproxEqual(lhs, rhs); +} + +inline bool operator>(Fps lhs, Fps rhs) { + return isApproxLess(rhs, lhs); +} + +inline bool operator<=(Fps lhs, Fps rhs) { + return !isApproxLess(rhs, lhs); +} + +inline bool operator>=(Fps lhs, Fps rhs) { + return !isApproxLess(lhs, rhs); +} + +} // namespace fps_approx_ops + +struct FpsApproxEqual { + bool operator()(Fps lhs, Fps rhs) const { return isApproxEqual(lhs, rhs); } +}; + +inline std::string to_string(Fps fps) { + return base::StringPrintf("%.2f Hz", fps.getValue()); +} + +inline std::ostream& operator<<(std::ostream& stream, Fps fps) { + return stream << to_string(fps); +} + +} // namespace android diff --git a/services/surfaceflinger/Scheduler/include/scheduler/Seamlessness.h b/services/surfaceflinger/Scheduler/include/scheduler/Seamlessness.h new file mode 100644 index 0000000000..d7667ec9c6 --- /dev/null +++ b/services/surfaceflinger/Scheduler/include/scheduler/Seamlessness.h @@ -0,0 +1,52 @@ +/* + * Copyright 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +namespace android::scheduler { + +// The seamlessness requirement of a Layer. +enum class Seamlessness { + // Indicates a requirement for a seamless mode switch. + OnlySeamless, + // Indicates that both seamless and seamed mode switches are allowed. + SeamedAndSeamless, + // Indicates no preference for seamlessness. For such layers the system will + // prefer seamless switches, but also non-seamless switches to the group of the + // default config are allowed. + Default +}; + +inline std::string toString(Seamlessness seamlessness) { + switch (seamlessness) { + case Seamlessness::OnlySeamless: + return "OnlySeamless"; + case Seamlessness::SeamedAndSeamless: + return "SeamedAndSeamless"; + case Seamlessness::Default: + return "Default"; + } +} + +// Used by gtest +inline std::ostream& operator<<(std::ostream& os, Seamlessness val) { + return os << toString(val); +} + +} // namespace android::scheduler diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 6093be91f9..3037593d6c 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -23,7 +23,6 @@ */ #include -#include #include #include #include @@ -47,13 +46,15 @@ #include #include +#include +#include + #include "ClientCache.h" #include "DisplayDevice.h" #include "DisplayHardware/HWC2.h" #include "DisplayHardware/PowerAdvisor.h" #include "DisplayIdGenerator.h" #include "Effects/Daltonizer.h" -#include "Fps.h" #include "FrameTracker.h" #include "LayerVector.h" #include "Scheduler/RefreshRateConfigs.h" diff --git a/services/surfaceflinger/SurfaceFlingerFactory.h b/services/surfaceflinger/SurfaceFlingerFactory.h index e670f37189..e509cc9385 100644 --- a/services/surfaceflinger/SurfaceFlingerFactory.h +++ b/services/surfaceflinger/SurfaceFlingerFactory.h @@ -16,16 +16,16 @@ #pragma once -#include "Fps.h" - -#include -#include - #include #include #include #include +#include +#include + +#include + namespace android { typedef int32_t PixelFormat; diff --git a/services/surfaceflinger/TimeStats/Android.bp b/services/surfaceflinger/TimeStats/Android.bp index bcc3e4e52a..4686eed54c 100644 --- a/services/surfaceflinger/TimeStats/Android.bp +++ b/services/surfaceflinger/TimeStats/Android.bp @@ -12,6 +12,9 @@ cc_library { srcs: [ "TimeStats.cpp", ], + header_libs: [ + "libscheduler_headers", + ], shared_libs: [ "android.hardware.graphics.composer@2.4", "libbase", @@ -24,6 +27,9 @@ cc_library { "libutils", ], export_include_dirs: ["."], + export_header_lib_headers: [ + "libscheduler_headers", + ], export_shared_lib_headers: [ "libtimestats_proto", ], diff --git a/services/surfaceflinger/TimeStats/TimeStats.h b/services/surfaceflinger/TimeStats/TimeStats.h index bdeaeb8468..23f19b5cfd 100644 --- a/services/surfaceflinger/TimeStats/TimeStats.h +++ b/services/surfaceflinger/TimeStats/TimeStats.h @@ -17,8 +17,12 @@ #pragma once #include +#include +#include +#include +#include +#include -#include <../Fps.h> #include #include #include @@ -27,11 +31,7 @@ #include #include -#include -#include -#include -#include -#include +#include using namespace android::surfaceflinger; diff --git a/services/surfaceflinger/tests/unittests/Android.bp b/services/surfaceflinger/tests/unittests/Android.bp index 3dc6d8b5c1..5c9e1c5ad9 100644 --- a/services/surfaceflinger/tests/unittests/Android.bp +++ b/services/surfaceflinger/tests/unittests/Android.bp @@ -135,8 +135,9 @@ cc_test { "libgui_mocks", "liblayers_proto", "libperfetto_client_experimental", - "librenderengine_mocks", "librenderengine", + "librenderengine_mocks", + "libscheduler", "libserviceutils", "libtimestats", "libtimestats_atoms_proto", diff --git a/services/surfaceflinger/tests/unittests/FpsOps.h b/services/surfaceflinger/tests/unittests/FpsOps.h index 23c2841efc..7c737dce28 100644 --- a/services/surfaceflinger/tests/unittests/FpsOps.h +++ b/services/surfaceflinger/tests/unittests/FpsOps.h @@ -16,7 +16,7 @@ #pragma once -#include "Fps.h" +#include namespace android { diff --git a/services/surfaceflinger/tests/unittests/FpsTest.cpp b/services/surfaceflinger/tests/unittests/FpsTest.cpp index b44dd89229..88b74d2a22 100644 --- a/services/surfaceflinger/tests/unittests/FpsTest.cpp +++ b/services/surfaceflinger/tests/unittests/FpsTest.cpp @@ -14,12 +14,13 @@ * limitations under the License. */ -#include "Fps.h" -#include "FpsOps.h" - #include #include +#include + +#include "FpsOps.h" + namespace android { TEST(FpsTest, construct) { diff --git a/services/surfaceflinger/tests/unittests/LayerInfoTest.cpp b/services/surfaceflinger/tests/unittests/LayerInfoTest.cpp index f25994e399..5c2d2e1f43 100644 --- a/services/surfaceflinger/tests/unittests/LayerInfoTest.cpp +++ b/services/surfaceflinger/tests/unittests/LayerInfoTest.cpp @@ -19,7 +19,8 @@ #include -#include "Fps.h" +#include + #include "FpsOps.h" #include "Scheduler/LayerHistory.h" #include "Scheduler/LayerInfo.h" -- cgit v1.2.3-59-g8ed1b From 78c4a242236a3c4dd20227d4fbd4b4ade67ebca2 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Tue, 30 Nov 2021 17:49:44 -0800 Subject: SF: make RefreshRateOverlay a trusted overlay Bug: 207057942 Test: 1.Settings->System->Developper options->Show refresh rate,open 2.Open any app for the first time 3.click the prompt box of permission Change-Id: I06a3c3df7ca59b319a3906e83c7bb7e9f0f85d32 --- services/surfaceflinger/RefreshRateOverlay.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'services/surfaceflinger/RefreshRateOverlay.cpp') diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 712ab168a2..81c1566d71 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -203,12 +203,13 @@ bool RefreshRateOverlay::createLayer() { return false; } - SurfaceComposerClient::Transaction t; - t.setFrameRate(mSurfaceControl, 0.0f, - static_cast(Layer::FrameRateCompatibility::NoVote), - static_cast(scheduler::Seamlessness::OnlySeamless)); - t.setLayer(mSurfaceControl, INT32_MAX - 2); - t.apply(); + SurfaceComposerClient::Transaction() + .setFrameRate(mSurfaceControl, 0.0f, + static_cast(Layer::FrameRateCompatibility::NoVote), + static_cast(scheduler::Seamlessness::OnlySeamless)) + .setLayer(mSurfaceControl, INT32_MAX - 2) + .setTrustedOverlay(mSurfaceControl, true) + .apply(); return true; } -- cgit v1.2.3-59-g8ed1b From 8c4356c0ac57003787c20c3e4afda9dbf4f8a324 Mon Sep 17 00:00:00 2001 From: Dominik Laskowski Date: Mon, 21 Mar 2022 08:19:54 -0700 Subject: SF: Clean up RefreshRateOverlay Flatten buffer cache, remove unused members, and fix conversion warnings. Skip animation transactions unless spinner is enabled. Bug: 185535769 Bug: 129481165 Test: Apply follow-up fix and toggle overlay. Change-Id: I14688f7b5d882f595322dfadd5cabbd5a8564301 --- services/surfaceflinger/DisplayDevice.cpp | 5 +- services/surfaceflinger/RefreshRateOverlay.cpp | 206 ++++++++++++++----------- services/surfaceflinger/RefreshRateOverlay.h | 67 +++----- 3 files changed, 139 insertions(+), 139 deletions(-) (limited to 'services/surfaceflinger/RefreshRateOverlay.cpp') diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp index f5a4b3d6ca..9116fd335e 100644 --- a/services/surfaceflinger/DisplayDevice.cpp +++ b/services/surfaceflinger/DisplayDevice.cpp @@ -471,9 +471,8 @@ void DisplayDevice::enableRefreshRateOverlay(bool enable, bool showSpinnner) { return; } - const auto [lowFps, highFps] = mRefreshRateConfigs->getSupportedRefreshRateRange(); - mRefreshRateOverlay = std::make_unique(*mFlinger, lowFps.getIntValue(), - highFps.getIntValue(), showSpinnner); + const auto fpsRange = mRefreshRateConfigs->getSupportedRefreshRateRange(); + mRefreshRateOverlay = std::make_unique(fpsRange, showSpinnner); mRefreshRateOverlay->setLayerStack(getLayerStack()); mRefreshRateOverlay->setViewport(getSize()); mRefreshRateOverlay->changeRefreshRate(getActiveMode()->getFps()); diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 81c1566d71..80aa07231f 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -14,22 +14,20 @@ * limitations under the License. */ -// TODO(b/129481165): remove the #pragma below and fix conversion issues -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wconversion" -#pragma clang diagnostic ignored "-Wextra" - #include #include "RefreshRateOverlay.h" #include "Client.h" #include "Layer.h" -#include +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wconversion" +#include #include +#pragma clang diagnostic pop +#include #include #include -#include #include #include @@ -37,29 +35,40 @@ #define LOG_TAG "RefreshRateOverlay" namespace android { +namespace { + +constexpr int kDigitWidth = 64; +constexpr int kDigitHeight = 100; +constexpr int kDigitSpace = 16; -void RefreshRateOverlay::SevenSegmentDrawer::drawSegment(Segment segment, int left, SkColor& color, +// Layout is digit, space, digit, space, digit, space, spinner. +constexpr int kBufferWidth = 4 * kDigitWidth + 3 * kDigitSpace; +constexpr int kBufferHeight = kDigitHeight; + +} // namespace + +void RefreshRateOverlay::SevenSegmentDrawer::drawSegment(Segment segment, int left, SkColor color, SkCanvas& canvas) { const SkRect rect = [&]() { switch (segment) { case Segment::Upper: - return SkRect::MakeLTRB(left, 0, left + DIGIT_WIDTH, DIGIT_SPACE); + return SkRect::MakeLTRB(left, 0, left + kDigitWidth, kDigitSpace); case Segment::UpperLeft: - return SkRect::MakeLTRB(left, 0, left + DIGIT_SPACE, DIGIT_HEIGHT / 2); + return SkRect::MakeLTRB(left, 0, left + kDigitSpace, kDigitHeight / 2); case Segment::UpperRight: - return SkRect::MakeLTRB(left + DIGIT_WIDTH - DIGIT_SPACE, 0, left + DIGIT_WIDTH, - DIGIT_HEIGHT / 2); + return SkRect::MakeLTRB(left + kDigitWidth - kDigitSpace, 0, left + kDigitWidth, + kDigitHeight / 2); case Segment::Middle: - return SkRect::MakeLTRB(left, DIGIT_HEIGHT / 2 - DIGIT_SPACE / 2, - left + DIGIT_WIDTH, DIGIT_HEIGHT / 2 + DIGIT_SPACE / 2); + return SkRect::MakeLTRB(left, kDigitHeight / 2 - kDigitSpace / 2, + left + kDigitWidth, kDigitHeight / 2 + kDigitSpace / 2); case Segment::LowerLeft: - return SkRect::MakeLTRB(left, DIGIT_HEIGHT / 2, left + DIGIT_SPACE, DIGIT_HEIGHT); + return SkRect::MakeLTRB(left, kDigitHeight / 2, left + kDigitSpace, kDigitHeight); case Segment::LowerRight: - return SkRect::MakeLTRB(left + DIGIT_WIDTH - DIGIT_SPACE, DIGIT_HEIGHT / 2, - left + DIGIT_WIDTH, DIGIT_HEIGHT); + return SkRect::MakeLTRB(left + kDigitWidth - kDigitSpace, kDigitHeight / 2, + left + kDigitWidth, kDigitHeight); case Segment::Bottom: - return SkRect::MakeLTRB(left, DIGIT_HEIGHT - DIGIT_SPACE, left + DIGIT_WIDTH, - DIGIT_HEIGHT); + return SkRect::MakeLTRB(left, kDigitHeight - kDigitSpace, left + kDigitWidth, + kDigitHeight); } }(); @@ -69,7 +78,7 @@ void RefreshRateOverlay::SevenSegmentDrawer::drawSegment(Segment segment, int le canvas.drawRect(rect, paint); } -void RefreshRateOverlay::SevenSegmentDrawer::drawDigit(int digit, int left, SkColor& color, +void RefreshRateOverlay::SevenSegmentDrawer::drawDigit(int digit, int left, SkColor color, SkCanvas& canvas) { if (digit < 0 || digit > 9) return; @@ -94,37 +103,45 @@ void RefreshRateOverlay::SevenSegmentDrawer::drawDigit(int digit, int left, SkCo drawSegment(Segment::Bottom, left, color, canvas); } -std::vector> RefreshRateOverlay::SevenSegmentDrawer::draw( - int number, SkColor& color, ui::Transform::RotationFlags rotation, bool showSpinner) { +auto RefreshRateOverlay::SevenSegmentDrawer::draw(int number, SkColor color, + ui::Transform::RotationFlags rotation, + bool showSpinner) -> Buffers { if (number < 0 || number > 1000) return {}; const auto hundreds = number / 100; const auto tens = (number / 10) % 10; const auto ones = number % 10; - std::vector> buffers; - const auto loopCount = showSpinner ? 6 : 1; - for (int i = 0; i < loopCount; i++) { + const size_t loopCount = showSpinner ? 6 : 1; + + Buffers buffers; + buffers.reserve(loopCount); + + for (size_t i = 0; i < loopCount; i++) { // Pre-rotate the buffer before it reaches SurfaceFlinger. SkMatrix canvasTransform = SkMatrix(); - auto [bufferWidth, bufferHeight] = [&] { + const auto [bufferWidth, bufferHeight] = [&]() -> std::pair { switch (rotation) { case ui::Transform::ROT_90: - canvasTransform.setTranslate(BUFFER_HEIGHT, 0); - canvasTransform.preRotate(90); - return std::make_tuple(BUFFER_HEIGHT, BUFFER_WIDTH); + canvasTransform.setTranslate(kBufferHeight, 0); + canvasTransform.preRotate(90.f); + return {kBufferHeight, kBufferWidth}; case ui::Transform::ROT_270: - canvasTransform.setRotate(270, BUFFER_WIDTH / 2.0, BUFFER_WIDTH / 2.0); - return std::make_tuple(BUFFER_HEIGHT, BUFFER_WIDTH); + canvasTransform.setRotate(270.f, kBufferWidth / 2.f, kBufferWidth / 2.f); + return {kBufferHeight, kBufferWidth}; default: - return std::make_tuple(BUFFER_WIDTH, BUFFER_HEIGHT); + return {kBufferWidth, kBufferHeight}; } }(); + sp buffer = - new GraphicBuffer(bufferWidth, bufferHeight, HAL_PIXEL_FORMAT_RGBA_8888, 1, + new GraphicBuffer(static_cast(bufferWidth), + static_cast(bufferHeight), HAL_PIXEL_FORMAT_RGBA_8888, + 1, GRALLOC_USAGE_SW_WRITE_RARELY | GRALLOC_USAGE_HW_COMPOSER | GRALLOC_USAGE_HW_TEXTURE, "RefreshRateOverlayBuffer"); + const status_t bufferStatus = buffer->initCheck(); LOG_ALWAYS_FATAL_IF(bufferStatus != OK, "RefreshRateOverlay: Buffer failed to allocate: %d", bufferStatus); @@ -137,15 +154,15 @@ std::vector> RefreshRateOverlay::SevenSegmentDrawer::draw( if (hundreds != 0) { drawDigit(hundreds, left, color, *canvas); } - left += DIGIT_WIDTH + DIGIT_SPACE; + left += kDigitWidth + kDigitSpace; if (tens != 0) { drawDigit(tens, left, color, *canvas); } - left += DIGIT_WIDTH + DIGIT_SPACE; + left += kDigitWidth + kDigitSpace; drawDigit(ones, left, color, *canvas); - left += DIGIT_WIDTH + DIGIT_SPACE; + left += kDigitWidth + kDigitSpace; if (showSpinner) { switch (i) { @@ -172,51 +189,48 @@ std::vector> RefreshRateOverlay::SevenSegmentDrawer::draw( void* pixels = nullptr; buffer->lock(GRALLOC_USAGE_SW_WRITE_RARELY, reinterpret_cast(&pixels)); + const SkImageInfo& imageInfo = surface->imageInfo(); - size_t dstRowBytes = buffer->getStride() * imageInfo.bytesPerPixel(); + const size_t dstRowBytes = + buffer->getStride() * static_cast(imageInfo.bytesPerPixel()); + canvas->readPixels(imageInfo, pixels, dstRowBytes, 0, 0); buffer->unlock(); - buffers.emplace_back(buffer); + buffers.push_back(std::move(buffer)); } return buffers; } -RefreshRateOverlay::RefreshRateOverlay(SurfaceFlinger& flinger, uint32_t lowFps, uint32_t highFps, - bool showSpinner) - : mFlinger(flinger), - mClient(new Client(&mFlinger)), +RefreshRateOverlay::RefreshRateOverlay(FpsRange fpsRange, bool showSpinner) + : mFpsRange(fpsRange), mShowSpinner(showSpinner), - mLowFps(lowFps), - mHighFps(highFps) { - createLayer(); -} - -bool RefreshRateOverlay::createLayer() { - mSurfaceControl = - SurfaceComposerClient::getDefault() - ->createSurface(String8("RefreshRateOverlay"), SevenSegmentDrawer::getWidth(), - SevenSegmentDrawer::getHeight(), PIXEL_FORMAT_RGBA_8888, - ISurfaceComposerClient::eFXSurfaceBufferState); - + mSurfaceControl(SurfaceComposerClient::getDefault() + ->createSurface(String8("RefreshRateOverlay"), kBufferWidth, + kBufferHeight, PIXEL_FORMAT_RGBA_8888, + ISurfaceComposerClient::eFXSurfaceBufferState)) { if (!mSurfaceControl) { - ALOGE("failed to create buffer state layer"); - return false; + ALOGE("%s: Failed to create buffer state layer", __func__); + return; } + constexpr float kFrameRate = 0.f; + constexpr int8_t kCompatibility = static_cast(Layer::FrameRateCompatibility::NoVote); + constexpr int8_t kSeamlessness = ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS; + SurfaceComposerClient::Transaction() - .setFrameRate(mSurfaceControl, 0.0f, - static_cast(Layer::FrameRateCompatibility::NoVote), - static_cast(scheduler::Seamlessness::OnlySeamless)) + .setFrameRate(mSurfaceControl, kFrameRate, kCompatibility, kSeamlessness) .setLayer(mSurfaceControl, INT32_MAX - 2) .setTrustedOverlay(mSurfaceControl, true) .apply(); - - return true; } -const std::vector>& RefreshRateOverlay::getOrCreateBuffers(uint32_t fps) { - ui::Transform::RotationFlags transformHint = +auto RefreshRateOverlay::getOrCreateBuffers(Fps fps) -> const Buffers& { + static const Buffers kNoBuffers; + if (!mSurfaceControl) return kNoBuffers; + + const auto transformHint = static_cast(mSurfaceControl->getTransformHint()); + // Tell SurfaceFlinger about the pre-rotation on the buffer. const auto transform = [&] { switch (transformHint) { @@ -233,40 +247,49 @@ const std::vector>& RefreshRateOverlay::getOrCreateBuffers(uin t.setTransform(mSurfaceControl, transform); t.apply(); - if (mBufferCache.find(transformHint) == mBufferCache.end() || - mBufferCache.at(transformHint).find(fps) == mBufferCache.at(transformHint).end()) { - // Ensure the range is > 0, so we don't divide by 0. - const auto rangeLength = std::max(1u, mHighFps - mLowFps); - // Clip values outside the range [mLowFps, mHighFps]. The current fps may be outside - // of this range if the display has changed its set of supported refresh rates. - fps = std::max(fps, mLowFps); - fps = std::min(fps, mHighFps); - const auto fpsScale = static_cast(fps - mLowFps) / rangeLength; - SkColor4f colorBase = SkColor4f::FromColor(HIGH_FPS_COLOR) * fpsScale; - SkColor4f lowFpsColor = SkColor4f::FromColor(LOW_FPS_COLOR) * (1 - fpsScale); - colorBase.fR = colorBase.fR + lowFpsColor.fR; - colorBase.fG = colorBase.fG + lowFpsColor.fG; - colorBase.fB = colorBase.fB + lowFpsColor.fB; - colorBase.fA = ALPHA; - SkColor color = colorBase.toSkColor(); - auto buffers = SevenSegmentDrawer::draw(fps, color, transformHint, mShowSpinner); - mBufferCache[transformHint].emplace(fps, buffers); + BufferCache::const_iterator it = mBufferCache.find({fps.getIntValue(), transformHint}); + if (it == mBufferCache.end()) { + const int minFps = mFpsRange.min.getIntValue(); + const int maxFps = mFpsRange.max.getIntValue(); + + // Clamp to the range. The current fps may be outside of this range if the display has + // changed its set of supported refresh rates. + const int intFps = std::clamp(fps.getIntValue(), minFps, maxFps); + + // Ensure non-zero range to avoid division by zero. + const float fpsScale = static_cast(intFps - minFps) / std::max(1, maxFps - minFps); + + constexpr SkColor kMinFpsColor = SK_ColorRED; + constexpr SkColor kMaxFpsColor = SK_ColorGREEN; + constexpr float kAlpha = 0.8f; + + SkColor4f colorBase = SkColor4f::FromColor(kMaxFpsColor) * fpsScale; + const SkColor4f minFpsColor = SkColor4f::FromColor(kMinFpsColor) * (1 - fpsScale); + + colorBase.fR = colorBase.fR + minFpsColor.fR; + colorBase.fG = colorBase.fG + minFpsColor.fG; + colorBase.fB = colorBase.fB + minFpsColor.fB; + colorBase.fA = kAlpha; + + const SkColor color = colorBase.toSkColor(); + + auto buffers = SevenSegmentDrawer::draw(intFps, color, transformHint, mShowSpinner); + it = mBufferCache.try_emplace({intFps, transformHint}, std::move(buffers)).first; } - return mBufferCache[transformHint][fps]; + return it->second; } void RefreshRateOverlay::setViewport(ui::Size viewport) { constexpr int32_t kMaxWidth = 1000; - const auto width = std::min(kMaxWidth, std::min(viewport.width, viewport.height)); + const auto width = std::min({kMaxWidth, viewport.width, viewport.height}); const auto height = 2 * width; Rect frame((3 * width) >> 4, height >> 5); frame.offsetBy(width >> 5, height >> 4); SurfaceComposerClient::Transaction t; - t.setMatrix(mSurfaceControl, - frame.getWidth() / static_cast(SevenSegmentDrawer::getWidth()), 0, 0, - frame.getHeight() / static_cast(SevenSegmentDrawer::getHeight())); + t.setMatrix(mSurfaceControl, frame.getWidth() / static_cast(kBufferWidth), 0, 0, + frame.getHeight() / static_cast(kBufferHeight)); t.setPosition(mSurfaceControl, frame.left, frame.top); t.apply(); } @@ -278,25 +301,22 @@ void RefreshRateOverlay::setLayerStack(ui::LayerStack stack) { } void RefreshRateOverlay::changeRefreshRate(Fps fps) { - mCurrentFps = fps.getIntValue(); - auto buffer = getOrCreateBuffers(*mCurrentFps)[mFrame]; + mCurrentFps = fps; + const auto buffer = getOrCreateBuffers(fps)[mFrame]; SurfaceComposerClient::Transaction t; t.setBuffer(mSurfaceControl, buffer); t.apply(); } void RefreshRateOverlay::animate() { - if (!mCurrentFps.has_value()) return; + if (!mShowSpinner || !mCurrentFps) return; const auto& buffers = getOrCreateBuffers(*mCurrentFps); mFrame = (mFrame + 1) % buffers.size(); - auto buffer = buffers[mFrame]; + const auto buffer = buffers[mFrame]; SurfaceComposerClient::Transaction t; t.setBuffer(mSurfaceControl, buffer); t.apply(); } } // namespace android - -// TODO(b/129481165): remove the #pragma below and fix conversion issues -#pragma clang diagnostic pop // ignored "-Wconversion -Wextra" diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h index 381df37903..a465a36747 100644 --- a/services/surfaceflinger/RefreshRateOverlay.h +++ b/services/surfaceflinger/RefreshRateOverlay.h @@ -16,32 +16,27 @@ #pragma once -#include #include -#include +#include -#include -#include +#include #include -#include #include +#include #include #include +class SkCanvas; + namespace android { -class Client; class GraphicBuffer; -class IBinder; -class IGraphicBufferProducer; -class Layer; -class SurfaceFlinger; class SurfaceControl; class RefreshRateOverlay { public: - RefreshRateOverlay(SurfaceFlinger&, uint32_t lowFps, uint32_t highFps, bool showSpinner); + RefreshRateOverlay(FpsRange, bool showSpinner); void setLayerStack(ui::LayerStack); void setViewport(ui::Size); @@ -49,52 +44,38 @@ public: void animate(); private: + using Buffers = std::vector>; + class SevenSegmentDrawer { public: - static std::vector> draw(int number, SkColor& color, - ui::Transform::RotationFlags, bool showSpinner); - static uint32_t getHeight() { return BUFFER_HEIGHT; } - static uint32_t getWidth() { return BUFFER_WIDTH; } + static Buffers draw(int number, SkColor, ui::Transform::RotationFlags, bool showSpinner); private: enum class Segment { Upper, UpperLeft, UpperRight, Middle, LowerLeft, LowerRight, Bottom }; - static void drawSegment(Segment segment, int left, SkColor& color, SkCanvas& canvas); - static void drawDigit(int digit, int left, SkColor& color, SkCanvas& canvas); - - static constexpr uint32_t DIGIT_HEIGHT = 100; - static constexpr uint32_t DIGIT_WIDTH = 64; - static constexpr uint32_t DIGIT_SPACE = 16; - static constexpr uint32_t BUFFER_HEIGHT = DIGIT_HEIGHT; - static constexpr uint32_t BUFFER_WIDTH = - 4 * DIGIT_WIDTH + 3 * DIGIT_SPACE; // Digit|Space|Digit|Space|Digit|Space|Spinner + static void drawSegment(Segment, int left, SkColor, SkCanvas&); + static void drawDigit(int digit, int left, SkColor, SkCanvas&); }; - bool createLayer(); + const Buffers& getOrCreateBuffers(Fps); - const std::vector>& getOrCreateBuffers(uint32_t fps); + struct Key { + int fps; + ui::Transform::RotationFlags flags; - SurfaceFlinger& mFlinger; - const sp mClient; - sp mIBinder; - sp mGbp; + bool operator==(Key other) const { return fps == other.fps && flags == other.flags; } + }; - std::unordered_map>>> - mBufferCache; - std::optional mCurrentFps; - int mFrame = 0; - static constexpr float ALPHA = 0.8f; - const SkColor LOW_FPS_COLOR = SK_ColorRED; - const SkColor HIGH_FPS_COLOR = SK_ColorGREEN; + using BufferCache = ftl::SmallMap; + BufferCache mBufferCache; - const bool mShowSpinner; + std::optional mCurrentFps; + size_t mFrame = 0; - // Interpolate the colors between these values. - const uint32_t mLowFps; - const uint32_t mHighFps; + const FpsRange mFpsRange; // For color interpolation. + const bool mShowSpinner; - sp mSurfaceControl; + const sp mSurfaceControl; }; } // namespace android -- cgit v1.2.3-59-g8ed1b From 1f6fc70ab0c6efdaa1c60dd2ced32fb6833c92e2 Mon Sep 17 00:00:00 2001 From: Dominik Laskowski Date: Mon, 21 Mar 2022 08:34:50 -0700 Subject: SF: Fix feedback loop with refresh rate overlay RefreshRateOverlay was refactored to use transactions instead of APIs internal to SF. A side effect is that the overlay layer feeds back into the frame rate detection and idle heuristics, which causes oscillation that trends to the high refresh rate. The transaction to setFrameRate failed to apply, as the NoVote argument was invalid. Make it valid, such that LayerHistory::summarize skips the overlay layer. Do not reset the idle timer for solely NO_VOTE transactions. Bug: 221081400 Test: flame is not stuck at 90 Hz when overlay is enabled. Test: Same with sf.debug.show_refresh_rate_overlay_spinner Test: SetFrameRateTest Change-Id: I6322c1c487672b602a0f974e8ecf445633dcc3a1 --- libs/gui/LayerState.cpp | 11 +++--- libs/nativewindow/include/apex/window.h | 13 ------- libs/nativewindow/include/system/window.h | 18 ++++++++++ services/surfaceflinger/Layer.cpp | 2 ++ services/surfaceflinger/RefreshRateOverlay.cpp | 42 ++++++++++------------ services/surfaceflinger/SurfaceFlinger.cpp | 19 ++++------ services/surfaceflinger/SurfaceFlinger.h | 3 +- services/surfaceflinger/TransactionState.h | 36 +++++++++++++++++-- .../tests/unittests/SetFrameRateTest.cpp | 21 +++++++---- 9 files changed, 101 insertions(+), 64 deletions(-) (limited to 'services/surfaceflinger/RefreshRateOverlay.cpp') diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index f7cd5c4f71..502031c8d8 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -16,8 +16,8 @@ #define LOG_TAG "LayerState" -#include -#include +#include +#include #include #include @@ -25,10 +25,9 @@ #include #include #include +#include #include -#include - namespace android { using gui::FocusRequest; @@ -679,7 +678,9 @@ bool ValidateFrameRate(float frameRate, int8_t compatibility, int8_t changeFrame if (compatibility != ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT && compatibility != ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_FIXED_SOURCE && - (!privileged || compatibility != ANATIVEWINDOW_FRAME_RATE_EXACT)) { + (!privileged || + (compatibility != ANATIVEWINDOW_FRAME_RATE_EXACT && + compatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE))) { ALOGE("%s failed - invalid compatibility value %d privileged: %s", functionName, compatibility, privileged ? "yes" : "no"); return false; diff --git a/libs/nativewindow/include/apex/window.h b/libs/nativewindow/include/apex/window.h index 0923438eec..2d1354cdf1 100644 --- a/libs/nativewindow/include/apex/window.h +++ b/libs/nativewindow/include/apex/window.h @@ -39,19 +39,6 @@ enum ANativeWindowPerform { // clang-format on }; -/* - * Internal extension of compatibility value for ANativeWindow_setFrameRate. */ -enum ANativeWindow_FrameRateCompatibilityInternal { - /** - * This surface belongs to an app on the High Refresh Rate Deny list, and needs the display - * to operate at the exact frame rate. - * - * This is used internally by the platform and should not be used by apps. - * @hide - */ - ANATIVEWINDOW_FRAME_RATE_EXACT = 100, -}; - /** * Prototype of the function that an ANativeWindow implementation would call * when ANativeWindow_cancelBuffer is called. diff --git a/libs/nativewindow/include/system/window.h b/libs/nativewindow/include/system/window.h index a319769148..a54af1fa62 100644 --- a/libs/nativewindow/include/system/window.h +++ b/libs/nativewindow/include/system/window.h @@ -1018,6 +1018,24 @@ static inline int native_window_set_auto_prerotation(struct ANativeWindow* windo return window->perform(window, NATIVE_WINDOW_SET_AUTO_PREROTATION, autoPrerotation); } +/* + * Internal extension of ANativeWindow_FrameRateCompatibility. + */ +enum { + /** + * This surface belongs to an app on the High Refresh Rate Deny list, and needs the display + * to operate at the exact frame rate. + * + * Keep in sync with Surface.java constant. + */ + ANATIVEWINDOW_FRAME_RATE_EXACT = 100, + + /** + * This surface is ignored while choosing the refresh rate. + */ + ANATIVEWINDOW_FRAME_RATE_NO_VOTE, +}; + static inline int native_window_set_frame_rate(struct ANativeWindow* window, float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) { return window->perform(window, NATIVE_WINDOW_SET_FRAME_RATE, (double)frameRate, diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 997b1a1b1a..f7e1d1ee95 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2596,6 +2596,8 @@ Layer::FrameRateCompatibility Layer::FrameRate::convertCompatibility(int8_t comp return FrameRateCompatibility::ExactOrMultiple; case ANATIVEWINDOW_FRAME_RATE_EXACT: return FrameRateCompatibility::Exact; + case ANATIVEWINDOW_FRAME_RATE_NO_VOTE: + return FrameRateCompatibility::NoVote; default: LOG_ALWAYS_FATAL("Invalid frame rate compatibility value %d", compatibility); return FrameRateCompatibility::Default; diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp index 80aa07231f..d4435c2818 100644 --- a/services/surfaceflinger/RefreshRateOverlay.cpp +++ b/services/surfaceflinger/RefreshRateOverlay.cpp @@ -45,6 +45,15 @@ constexpr int kDigitSpace = 16; constexpr int kBufferWidth = 4 * kDigitWidth + 3 * kDigitSpace; constexpr int kBufferHeight = kDigitHeight; +SurfaceComposerClient::Transaction createTransaction(const sp& surface) { + constexpr float kFrameRate = 0.f; + constexpr int8_t kCompatibility = ANATIVEWINDOW_FRAME_RATE_NO_VOTE; + constexpr int8_t kSeamlessness = ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS; + + return SurfaceComposerClient::Transaction().setFrameRate(surface, kFrameRate, kCompatibility, + kSeamlessness); +} + } // namespace void RefreshRateOverlay::SevenSegmentDrawer::drawSegment(Segment segment, int left, SkColor color, @@ -213,12 +222,7 @@ RefreshRateOverlay::RefreshRateOverlay(FpsRange fpsRange, bool showSpinner) return; } - constexpr float kFrameRate = 0.f; - constexpr int8_t kCompatibility = static_cast(Layer::FrameRateCompatibility::NoVote); - constexpr int8_t kSeamlessness = ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS; - - SurfaceComposerClient::Transaction() - .setFrameRate(mSurfaceControl, kFrameRate, kCompatibility, kSeamlessness) + createTransaction(mSurfaceControl) .setLayer(mSurfaceControl, INT32_MAX - 2) .setTrustedOverlay(mSurfaceControl, true) .apply(); @@ -243,9 +247,7 @@ auto RefreshRateOverlay::getOrCreateBuffers(Fps fps) -> const Buffers& { } }(); - SurfaceComposerClient::Transaction t; - t.setTransform(mSurfaceControl, transform); - t.apply(); + createTransaction(mSurfaceControl).setTransform(mSurfaceControl, transform).apply(); BufferCache::const_iterator it = mBufferCache.find({fps.getIntValue(), transformHint}); if (it == mBufferCache.end()) { @@ -287,25 +289,21 @@ void RefreshRateOverlay::setViewport(ui::Size viewport) { Rect frame((3 * width) >> 4, height >> 5); frame.offsetBy(width >> 5, height >> 4); - SurfaceComposerClient::Transaction t; - t.setMatrix(mSurfaceControl, frame.getWidth() / static_cast(kBufferWidth), 0, 0, - frame.getHeight() / static_cast(kBufferHeight)); - t.setPosition(mSurfaceControl, frame.left, frame.top); - t.apply(); + createTransaction(mSurfaceControl) + .setMatrix(mSurfaceControl, frame.getWidth() / static_cast(kBufferWidth), 0, 0, + frame.getHeight() / static_cast(kBufferHeight)) + .setPosition(mSurfaceControl, frame.left, frame.top) + .apply(); } void RefreshRateOverlay::setLayerStack(ui::LayerStack stack) { - SurfaceComposerClient::Transaction t; - t.setLayerStack(mSurfaceControl, stack); - t.apply(); + createTransaction(mSurfaceControl).setLayerStack(mSurfaceControl, stack).apply(); } void RefreshRateOverlay::changeRefreshRate(Fps fps) { mCurrentFps = fps; const auto buffer = getOrCreateBuffers(fps)[mFrame]; - SurfaceComposerClient::Transaction t; - t.setBuffer(mSurfaceControl, buffer); - t.apply(); + createTransaction(mSurfaceControl).setBuffer(mSurfaceControl, buffer).apply(); } void RefreshRateOverlay::animate() { @@ -314,9 +312,7 @@ void RefreshRateOverlay::animate() { const auto& buffers = getOrCreateBuffers(*mCurrentFps); mFrame = (mFrame + 1) % buffers.size(); const auto buffer = buffers[mFrame]; - SurfaceComposerClient::Transaction t; - t.setBuffer(mSurfaceControl, buffer); - t.apply(); + createTransaction(mSurfaceControl).setBuffer(mSurfaceControl, buffer).apply(); } } // namespace android diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 4c830307e8..64ba280f9c 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3629,11 +3629,11 @@ uint32_t SurfaceFlinger::clearTransactionFlags(uint32_t mask) { } void SurfaceFlinger::setTransactionFlags(uint32_t mask, TransactionSchedule schedule, - const sp& applyToken) { + const sp& applyToken, FrameHint frameHint) { modulateVsync(&VsyncModulator::setTransactionSchedule, schedule, applyToken); if (const bool scheduled = mTransactionFlags.fetch_or(mask) & mask; !scheduled) { - scheduleCommit(FrameHint::kActive); + scheduleCommit(frameHint); } } @@ -4005,7 +4005,7 @@ auto SurfaceFlinger::transactionIsReadyToBeApplied( } void SurfaceFlinger::queueTransaction(TransactionState& state) { - Mutex::Autolock _l(mQueueLock); + Mutex::Autolock lock(mQueueLock); // Generate a CountDownLatch pending state if this is a synchronous transaction. if ((state.flags & eSynchronous) || state.inputWindowCommands.syncInputWindows) { @@ -4024,7 +4024,9 @@ void SurfaceFlinger::queueTransaction(TransactionState& state) { return TransactionSchedule::Late; }(state.flags); - setTransactionFlags(eTransactionFlushNeeded, schedule, state.applyToken); + const auto frameHint = state.isFrameActive() ? FrameHint::kActive : FrameHint::kNone; + + setTransactionFlags(eTransactionFlushNeeded, schedule, state.applyToken, frameHint); } void SurfaceFlinger::waitForSynchronousTransaction( @@ -7172,15 +7174,6 @@ int SurfaceFlinger::getMaxAcquiredBufferCountForRefreshRate(Fps refreshRate) con return calculateMaxAcquiredBufferCount(refreshRate, presentLatency); } -void TransactionState::traverseStatesWithBuffers( - std::function visitor) { - for (const auto& state : states) { - if (state.state.hasBufferChanges() && state.state.hasValidBuffer() && state.state.surface) { - visitor(state.state); - } - } -} - void SurfaceFlinger::handleLayerCreatedLocked(const LayerCreatedState& state) { sp layer = state.layer.promote(); if (!layer) { diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 97b0e8db5c..3c7facfc12 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -784,7 +784,8 @@ private: // Sets the masked bits, and schedules a commit if needed. void setTransactionFlags(uint32_t mask, TransactionSchedule = TransactionSchedule::Late, - const sp& applyToken = nullptr); + const sp& applyToken = nullptr, + FrameHint = FrameHint::kActive); // Clears and returns the masked bits. uint32_t clearTransactionFlags(uint32_t mask); diff --git a/services/surfaceflinger/TransactionState.h b/services/surfaceflinger/TransactionState.h index 04ca347b2f..bab5326093 100644 --- a/services/surfaceflinger/TransactionState.h +++ b/services/surfaceflinger/TransactionState.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2021 The Android Open Source Project + * Copyright 2021 The Android Open Source Project * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,12 +16,21 @@ #pragma once +#include +#include +#include +#include + #include +#include namespace android { + class CountDownLatch; struct TransactionState { + TransactionState() = default; + TransactionState(const FrameTimelineInfo& frameTimelineInfo, const Vector& composerStates, const Vector& displayStates, uint32_t transactionFlags, @@ -47,9 +56,30 @@ struct TransactionState { originUid(originUid), id(transactionId) {} - TransactionState() {} + // Invokes `void(const layer_state_t&)` visitor for matching layers. + template + void traverseStatesWithBuffers(Visitor&& visitor) const { + for (const auto& [state] : states) { + if (state.hasBufferChanges() && state.hasValidBuffer() && state.surface) { + visitor(state); + } + } + } + + // TODO(b/185535769): Remove FrameHint. Instead, reset the idle timer (of the relevant physical + // display) on the main thread if commit leads to composite. Then, RefreshRateOverlay should be + // able to setFrameRate once, rather than for each transaction. + bool isFrameActive() const { + if (!displays.empty()) return true; + + for (const auto& [state] : states) { + if (state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) { + return true; + } + } - void traverseStatesWithBuffers(std::function visitor); + return false; + } FrameTimelineInfo frameTimelineInfo; Vector states; diff --git a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp index 825f145ecd..b9a5f36794 100644 --- a/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp +++ b/services/surfaceflinger/tests/unittests/SetFrameRateTest.cpp @@ -337,10 +337,22 @@ TEST_F(SetFrameRateTest, ValidateFrameRate) { ANATIVEWINDOW_CHANGE_FRAME_RATE_ALWAYS, "")); EXPECT_TRUE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_FIXED_SOURCE, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); + + // Privileged APIs. + EXPECT_FALSE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_EXACT, + ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); + EXPECT_FALSE(ValidateFrameRate(0.0f, ANATIVEWINDOW_FRAME_RATE_NO_VOTE, + ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); + + constexpr bool kPrivileged = true; EXPECT_TRUE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_EXACT, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "", - /*privileged=*/true)); + kPrivileged)); + EXPECT_TRUE(ValidateFrameRate(0.0f, ANATIVEWINDOW_FRAME_RATE_NO_VOTE, + ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "", + kPrivileged)); + // Invalid frame rate. EXPECT_FALSE(ValidateFrameRate(-1, ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); EXPECT_FALSE(ValidateFrameRate(1.0f / 0.0f, ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT, @@ -348,15 +360,12 @@ TEST_F(SetFrameRateTest, ValidateFrameRate) { EXPECT_FALSE(ValidateFrameRate(0.0f / 0.0f, ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); - EXPECT_FALSE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_EXACT, - ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); - - // Invalid compatibility + // Invalid compatibility. EXPECT_FALSE( ValidateFrameRate(60.0f, -1, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); EXPECT_FALSE(ValidateFrameRate(60.0f, 2, ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS, "")); - // Invalid change frame rate strategy + // Invalid change frame rate strategy. EXPECT_FALSE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_EXACT, -1, "")); EXPECT_FALSE(ValidateFrameRate(60.0f, ANATIVEWINDOW_FRAME_RATE_EXACT, 2, "")); } -- cgit v1.2.3-59-g8ed1b