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

Commit 1e431a4b authored by Melody Hsu's avatar Melody Hsu
Browse files

Reject duplicate layer stacks in SF 

Different displays could try to assign the same layer stack ID,
causing potential race conditions when releasing fences.

Fixes: b/370358572
Test: atest SurfaceFlinger_test, atest MultiDisplayTest
Flag: com.android.graphics.surfaceflinger.flags.reject_dupe_layerstacks
Change-Id: I2e59daa184d888a1804d8250ba6ce5037542a7d1
parent cb47c6c9

services/surfaceflinger/SurfaceFlinger.cpp

+35 −2
Original line number Diff line number Diff line
@@ -2785,13 +2785,44 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( 
    const auto& displays = FTL_FAKE_GUARD(mStateLock, mDisplays);

    refreshArgs.outputs.reserve(displays.size());



    // Track layer stacks of physical displays that might be added to CompositionEngine

    // output. Layer stacks are not tracked in Display when we iterate through

    // frameTargeters. Cross-referencing layer stacks allows us to filter out displays

    // by ID with duplicate layer stacks before adding them to CompositionEngine output.

    ui::DisplayMap<DisplayId, ui::LayerStack> physicalDisplayLayerStacks;

    for (auto& [_, display] : displays) {

        const auto id = PhysicalDisplayId::tryCast(display->getId());

        if (id && frameTargeters.contains(*id)) {

            physicalDisplayLayerStacks.try_emplace(*id, display->getLayerStack());

        }

    }



    // Tracks layer stacks of displays that are added to CompositionEngine output.

    ui::DisplayMap<ui::LayerStack, ftl::Unit> outputLayerStacks;

    auto isOutputLayerStack = [&outputLayerStacks](DisplayId id, ui::LayerStack layerStack) {

        if (FlagManager::getInstance().reject_dupe_layerstacks() &&

            outputLayerStacks.contains(layerStack)) {

            // TODO: remove log and DisplayId from params once reject_dupe_layerstacks flag is

            // removed

            ALOGD("Existing layer stack ID %d output to another display %" PRIu64

                  ", dropping display from outputs",

                  layerStack.id, id.value);

            return true;

        }

        outputLayerStacks.try_emplace(layerStack);

        return false;

    };



    // Add outputs for physical displays.

    for (const auto& [id, targeter] : frameTargeters) {

        ftl::FakeGuard guard(mStateLock);



        if (const auto display = getCompositionDisplayLocked(id)) {

            const auto layerStack = physicalDisplayLayerStacks.get(id)->get();

            if (!isOutputLayerStack(display->getId(), layerStack)) {

                refreshArgs.outputs.push_back(display);

            }

        }



        refreshArgs.frameTargets.try_emplace(id, &targeter->target());

    }
@@ -2807,10 +2838,12 @@ CompositeResultsPerDisplay SurfaceFlinger::composite( 


            if (!refreshRate.isValid() ||

                mScheduler->isVsyncInPhase(pacesetterTarget.frameBeginTime(), refreshRate)) {

                if (!isOutputLayerStack(display->getId(), display->getLayerStack())) {

                    refreshArgs.outputs.push_back(display->getCompositionDisplay());

                }

            }

        }

    }

    mPowerAdvisor->setDisplays(displayIds);



    const bool updateTaskMetadata = mCompositionEngine->getFeatureFlags().test(

services/surfaceflinger/common/FlagManager.cpp

+2 −0
Original line number Diff line number Diff line
@@ -166,6 +166,7 @@ void FlagManager::dump(std::string& result) const { 
    DUMP_ACONFIG_FLAG(skip_invisible_windows_in_input);

    DUMP_ACONFIG_FLAG(begone_bright_hlg);

    DUMP_ACONFIG_FLAG(window_blur_kawase2);

    DUMP_ACONFIG_FLAG(reject_dupe_layerstacks);



#undef DUMP_ACONFIG_FLAG

#undef DUMP_LEGACY_SERVER_FLAG
@@ -266,6 +267,7 @@ FLAG_MANAGER_ACONFIG_FLAG(deprecate_frame_tracker, ""); 
FLAG_MANAGER_ACONFIG_FLAG(skip_invisible_windows_in_input, "");

FLAG_MANAGER_ACONFIG_FLAG(begone_bright_hlg, "debug.sf.begone_bright_hlg");

FLAG_MANAGER_ACONFIG_FLAG(window_blur_kawase2, "");

FLAG_MANAGER_ACONFIG_FLAG(reject_dupe_layerstacks, "");



/// Trunk stable server (R/W) flags ///

FLAG_MANAGER_ACONFIG_FLAG(refresh_rate_overlay_on_external_display, "")

services/surfaceflinger/common/include/common/FlagManager.h

+1 −0
Original line number Diff line number Diff line
@@ -104,6 +104,7 @@ public: 
    bool begone_bright_hlg() const;

    bool luts_api() const;

    bool window_blur_kawase2() const;

    bool reject_dupe_layerstacks() const;



protected:

    // overridden for unit tests

services/surfaceflinger/surfaceflinger_flags_new.aconfig

+11 −0
Original line number Diff line number Diff line
@@ -216,6 +216,17 @@ flag { 
  is_fixed_read_only: true

} # no_vsyncs_on_screen_off



flag {

  name: "reject_dupe_layerstacks"

  namespace: "window_surfaces"

  description: "Reject duplicate layerstacks for displays"

  bug: "370358572"

  is_fixed_read_only: true

  metadata {

    purpose: PURPOSE_BUGFIX

  }

 } # reject_dupe_layerstacks



flag {

  name: "single_hop_screenshot"

  namespace: "window_surfaces"

services/surfaceflinger/tests/Android.bp

+1 −1
Original line number Diff line number Diff line
@@ -52,7 +52,7 @@ cc_test { 
        "LayerTypeTransaction_test.cpp",

        "LayerUpdate_test.cpp",

        "MirrorLayer_test.cpp",

        "MultiDisplayLayerBounds_test.cpp",

        "MultiDisplay_test.cpp",

        "RefreshRateOverlay_test.cpp",

        "RelativeZ_test.cpp",

        "ReleaseBufferCallback_test.cpp",