Merge "SF: Update metadata of unvisible layers to backend" into main
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index ca7acf9..ebdf232 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -279,6 +279,9 @@
layer_state_t::eDropInputModeChanged | layer_state_t::eTrustedOverlayChanged |
layer_state_t::eLayerStackChanged;
+ // Changes requiring a composition pass.
+ static constexpr uint64_t REQUIRES_COMPOSITION = layer_state_t::CONTENT_DIRTY;
+
// Changes that affect the visible region on a display.
static constexpr uint64_t VISIBLE_REGION_CHANGES = layer_state_t::GEOMETRY_CHANGES |
layer_state_t::HIERARCHY_CHANGES | layer_state_t::eAlphaChanged;
diff --git a/libs/renderengine/skia/SkiaRenderEngine.cpp b/libs/renderengine/skia/SkiaRenderEngine.cpp
index 325a911..b973211 100644
--- a/libs/renderengine/skia/SkiaRenderEngine.cpp
+++ b/libs/renderengine/skia/SkiaRenderEngine.cpp
@@ -91,7 +91,6 @@
// Debugging settings
static const bool kPrintLayerSettings = false;
static const bool kGaneshFlushAfterEveryLayer = kPrintLayerSettings;
-static constexpr bool kEnableLayerBrightening = true;
} // namespace
@@ -714,8 +713,7 @@
// ...and compute the dimming ratio if dimming is requested
const float displayDimmingRatio = display.targetLuminanceNits > 0.f &&
- maxLayerWhitePoint > 0.f &&
- (kEnableLayerBrightening || display.targetLuminanceNits > maxLayerWhitePoint)
+ maxLayerWhitePoint > 0.f && display.targetLuminanceNits > maxLayerWhitePoint
? maxLayerWhitePoint / display.targetLuminanceNits
: 1.f;
diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
index 4b0618e..52f8bea 100644
--- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
@@ -41,7 +41,8 @@
return;
}
- mGlobalChanges |= RequestedLayerState::Changes::Hierarchy;
+ mGlobalChanges |= RequestedLayerState::Changes::Hierarchy |
+ RequestedLayerState::Changes::RequiresComposition;
for (auto& newLayer : newLayers) {
RequestedLayerState& layer = *newLayer.get();
auto [it, inserted] = mIdToLayer.try_emplace(layer.id, References{.owner = layer});
@@ -104,7 +105,8 @@
if (!layer.canBeDestroyed()) {
continue;
}
- layer.changes |= RequestedLayerState::Changes::Destroyed;
+ layer.changes |= RequestedLayerState::Changes::Destroyed |
+ RequestedLayerState::Changes::RequiresComposition;
layersToBeDestroyed.emplace_back(layerId);
}
@@ -112,7 +114,8 @@
return;
}
- mGlobalChanges |= RequestedLayerState::Changes::Hierarchy;
+ mGlobalChanges |= RequestedLayerState::Changes::Hierarchy |
+ RequestedLayerState::Changes::RequiresComposition;
for (size_t i = 0; i < layersToBeDestroyed.size(); i++) {
uint32_t layerId = layersToBeDestroyed[i];
auto it = mIdToLayer.find(layerId);
@@ -142,7 +145,8 @@
if (linkedLayer->parentId == layer.id) {
linkedLayer->parentId = UNASSIGNED_LAYER_ID;
if (linkedLayer->canBeDestroyed()) {
- linkedLayer->changes |= RequestedLayerState::Changes::Destroyed;
+ linkedLayer->changes |= RequestedLayerState::Changes::Destroyed |
+ RequestedLayerState::Changes::RequiresComposition;
layersToBeDestroyed.emplace_back(linkedLayer->id);
}
}
@@ -249,7 +253,8 @@
layer_state_t::eDataspaceChanged | layer_state_t::eAlphaChanged;
bgColorLayer->changes |= RequestedLayerState::Changes::Content;
mChangedLayers.push_back(bgColorLayer);
- mGlobalChanges |= RequestedLayerState::Changes::Content;
+ mGlobalChanges |= RequestedLayerState::Changes::Content |
+ RequestedLayerState::Changes::RequiresComposition;
}
}
@@ -407,7 +412,8 @@
layer.relativeParentId = unlinkLayer(layer.relativeParentId, layer.id);
layer.changes |=
RequestedLayerState::Changes::Hierarchy | RequestedLayerState::Changes::RelativeParent;
- mGlobalChanges |= RequestedLayerState::Changes::Hierarchy;
+ mGlobalChanges |= RequestedLayerState::Changes::Hierarchy |
+ RequestedLayerState::Changes::RequiresComposition;
}
// Some layers mirror the entire display stack. Since we don't have a single root layer per display
diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
index 5631fac..f5e5b02 100644
--- a/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
+++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.cpp
@@ -58,7 +58,8 @@
parentId(args.parentId),
layerIdToMirror(args.layerIdToMirror) {
layerId = static_cast<int32_t>(args.sequence);
- changes |= RequestedLayerState::Changes::Created;
+ changes |= RequestedLayerState::Changes::Created |
+ RequestedLayerState::Changes::RequiresComposition;
metadata.merge(args.metadata);
changes |= RequestedLayerState::Changes::Metadata;
handleAlive = true;
@@ -248,7 +249,8 @@
if (hadSomethingToDraw != hasSomethingToDraw()) {
changes |= RequestedLayerState::Changes::Visibility |
- RequestedLayerState::Changes::VisibleRegion;
+ RequestedLayerState::Changes::VisibleRegion |
+ RequestedLayerState::Changes::RequiresComposition;
}
if (clientChanges & layer_state_t::HIERARCHY_CHANGES)
changes |= RequestedLayerState::Changes::Hierarchy;
@@ -258,6 +260,8 @@
changes |= RequestedLayerState::Changes::Geometry;
if (clientChanges & layer_state_t::AFFECTS_CHILDREN)
changes |= RequestedLayerState::Changes::AffectsChildren;
+ if (clientChanges & layer_state_t::REQUIRES_COMPOSITION)
+ changes |= RequestedLayerState::Changes::RequiresComposition;
if (clientChanges & layer_state_t::INPUT_CHANGES)
changes |= RequestedLayerState::Changes::Input;
if (clientChanges & layer_state_t::VISIBLE_REGION_CHANGES)
diff --git a/services/surfaceflinger/FrontEnd/RequestedLayerState.h b/services/surfaceflinger/FrontEnd/RequestedLayerState.h
index 09f33de..4829d4c 100644
--- a/services/surfaceflinger/FrontEnd/RequestedLayerState.h
+++ b/services/surfaceflinger/FrontEnd/RequestedLayerState.h
@@ -57,6 +57,7 @@
BufferSize = 1u << 18,
GameMode = 1u << 19,
BufferUsageFlags = 1u << 20,
+ RequiresComposition = 1u << 21,
};
static Rect reduce(const Rect& win, const Region& exclude);
RequestedLayerState(const LayerCreationArgs&);
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index f0c7ff0..28d8018 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1411,11 +1411,12 @@
}
}
-void SurfaceFlinger::initiateDisplayModeChanges() {
+bool SurfaceFlinger::initiateDisplayModeChanges() {
ATRACE_CALL();
std::optional<PhysicalDisplayId> displayToUpdateImmediately;
+ bool mustComposite = false;
for (const auto& [id, physical] : mPhysicalDisplays) {
const auto display = getDisplayDeviceLocked(id);
if (!display) continue;
@@ -1472,7 +1473,11 @@
mScheduler->onNewVsyncPeriodChangeTimeline(outTimeline);
if (outTimeline.refreshRequired) {
- scheduleComposite(FrameHint::kNone);
+ if (FlagManager::getInstance().vrr_bugfix_24q4()) {
+ mustComposite = true;
+ } else {
+ scheduleComposite(FrameHint::kNone);
+ }
} else {
// TODO(b/255635711): Remove `displayToUpdateImmediately` to `finalizeDisplayModeChange`
// for all displays. This was only needed when the loop iterated over `mDisplays` rather
@@ -1490,6 +1495,8 @@
applyActiveMode(display);
}
}
+
+ return mustComposite;
}
void SurfaceFlinger::disableExpensiveRendering() {
@@ -2439,7 +2446,12 @@
mUpdateAttachedChoreographer = true;
}
outTransactionsAreEmpty = mLayerLifecycleManager.getGlobalChanges().get() == 0;
- mustComposite |= mLayerLifecycleManager.getGlobalChanges().get() != 0;
+ if (FlagManager::getInstance().vrr_bugfix_24q4()) {
+ mustComposite |= mLayerLifecycleManager.getGlobalChanges().test(
+ frontend::RequestedLayerState::Changes::RequiresComposition);
+ } else {
+ mustComposite |= mLayerLifecycleManager.getGlobalChanges().get() != 0;
+ }
bool newDataLatched = false;
ATRACE_NAME("DisplayCallbackAndStatsUpdates");
@@ -2664,7 +2676,7 @@
? &mLayerHierarchyBuilder.getHierarchy()
: nullptr,
updateAttachedChoreographer);
- initiateDisplayModeChanges();
+ mustComposite |= initiateDisplayModeChanges();
}
updateCursorAsync();
@@ -7507,14 +7519,11 @@
auto future = mScheduler->schedule(
[&]() FTL_FAKE_GUARD(mStateLock) FTL_FAKE_GUARD(kMainThreadContext) {
n = data.readInt32();
- mHdrSdrRatioOverlay = n != 0;
- switch (n) {
- case 0:
- case 1:
- enableHdrSdrRatioOverlay(mHdrSdrRatioOverlay);
- break;
- default:
- reply->writeBool(isHdrSdrRatioOverlayEnabled());
+ if (n == 0 || n == 1) {
+ mHdrSdrRatioOverlay = n != 0;
+ enableHdrSdrRatioOverlay(mHdrSdrRatioOverlay);
+ } else {
+ reply->writeBool(isHdrSdrRatioOverlayEnabled());
}
});
future.wait();
@@ -8334,10 +8343,7 @@
renderArea->getHintForSeamlessTransition());
sdrWhitePointNits = state.sdrWhitePointNits;
- // TODO(b/298219334): Clean this up once we verify this doesn't break anything
- static constexpr bool kScreenshotsDontDim = true;
-
- if (kScreenshotsDontDim && !captureResults.capturedHdrLayers) {
+ if (!captureResults.capturedHdrLayers) {
displayBrightnessNits = sdrWhitePointNits;
} else {
displayBrightnessNits = state.displayBrightnessNits;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index edcc8d3..a198768 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -736,7 +736,7 @@
status_t setActiveModeFromBackdoor(const sp<display::DisplayToken>&, DisplayModeId, Fps minFps,
Fps maxFps);
- void initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext);
+ bool initiateDisplayModeChanges() REQUIRES(mStateLock, kMainThreadContext);
void finalizeDisplayModeChange(DisplayDevice&) REQUIRES(mStateLock, kMainThreadContext);
// TODO(b/241285191): Replace DisplayDevice with DisplayModeRequest, and move to Scheduler.
diff --git a/services/surfaceflinger/TransactionState.h b/services/surfaceflinger/TransactionState.h
index 89a8f92..e5d6481 100644
--- a/services/surfaceflinger/TransactionState.h
+++ b/services/surfaceflinger/TransactionState.h
@@ -23,6 +23,7 @@
#include "FrontEnd/LayerCreationArgs.h"
#include "renderengine/ExternalTexture.h"
+#include <common/FlagManager.h>
#include <gui/LayerState.h>
#include <system/window.h>
@@ -108,9 +109,22 @@
for (const auto& state : states) {
const bool frameRateChanged = state.state.what & layer_state_t::eFrameRateChanged;
- if (!frameRateChanged ||
- state.state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) {
- return true;
+ if (FlagManager::getInstance().vrr_bugfix_24q4()) {
+ const bool frameRateIsNoVote = frameRateChanged &&
+ state.state.frameRateCompatibility == ANATIVEWINDOW_FRAME_RATE_NO_VOTE;
+ const bool frameRateCategoryChanged =
+ state.state.what & layer_state_t::eFrameRateCategoryChanged;
+ const bool frameRateCategoryIsNoPreference = frameRateCategoryChanged &&
+ state.state.frameRateCategory ==
+ ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE;
+ if (!frameRateIsNoVote && !frameRateCategoryIsNoPreference) {
+ return true;
+ }
+ } else {
+ if (!frameRateChanged ||
+ state.state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) {
+ return true;
+ }
}
}
diff --git a/services/surfaceflinger/common/FlagManager.cpp b/services/surfaceflinger/common/FlagManager.cpp
index e73cf5d..121629f 100644
--- a/services/surfaceflinger/common/FlagManager.cpp
+++ b/services/surfaceflinger/common/FlagManager.cpp
@@ -134,6 +134,7 @@
DUMP_READ_ONLY_FLAG(screenshot_fence_preservation);
DUMP_READ_ONLY_FLAG(vulkan_renderengine);
DUMP_READ_ONLY_FLAG(renderable_buffer_usage);
+ DUMP_READ_ONLY_FLAG(vrr_bugfix_24q4);
DUMP_READ_ONLY_FLAG(restore_blur_step);
DUMP_READ_ONLY_FLAG(dont_skip_on_early_ro);
DUMP_READ_ONLY_FLAG(protected_if_client);
@@ -145,6 +146,7 @@
DUMP_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter);
DUMP_READ_ONLY_FLAG(detached_mirror);
DUMP_READ_ONLY_FLAG(commit_not_composited);
+ DUMP_READ_ONLY_FLAG(local_tonemap_screenshots);
#undef DUMP_READ_ONLY_FLAG
#undef DUMP_SERVER_FLAG
@@ -233,6 +235,7 @@
FLAG_MANAGER_READ_ONLY_FLAG(restore_blur_step, "debug.renderengine.restore_blur_step")
FLAG_MANAGER_READ_ONLY_FLAG(dont_skip_on_early_ro, "")
FLAG_MANAGER_READ_ONLY_FLAG(protected_if_client, "")
+FLAG_MANAGER_READ_ONLY_FLAG(vrr_bugfix_24q4, "");
FLAG_MANAGER_READ_ONLY_FLAG(ce_fence_promise, "");
FLAG_MANAGER_READ_ONLY_FLAG(graphite_renderengine, "debug.renderengine.graphite")
FLAG_MANAGER_READ_ONLY_FLAG(latch_unsignaled_with_auto_refresh_changed, "");
@@ -240,6 +243,7 @@
FLAG_MANAGER_READ_ONLY_FLAG(allow_n_vsyncs_in_targeter, "");
FLAG_MANAGER_READ_ONLY_FLAG(detached_mirror, "");
FLAG_MANAGER_READ_ONLY_FLAG(commit_not_composited, "");
+FLAG_MANAGER_READ_ONLY_FLAG(local_tonemap_screenshots, "debug.sf.local_tonemap_screenshots");
/// Trunk stable server flags ///
FLAG_MANAGER_SERVER_FLAG(refresh_rate_overlay_on_external_display, "")
diff --git a/services/surfaceflinger/common/include/common/FlagManager.h b/services/surfaceflinger/common/include/common/FlagManager.h
index 327cc4a..4cf4453 100644
--- a/services/surfaceflinger/common/include/common/FlagManager.h
+++ b/services/surfaceflinger/common/include/common/FlagManager.h
@@ -72,6 +72,7 @@
bool enable_layer_command_batching() const;
bool screenshot_fence_preservation() const;
bool vulkan_renderengine() const;
+ bool vrr_bugfix_24q4() const;
bool renderable_buffer_usage() const;
bool restore_blur_step() const;
bool dont_skip_on_early_ro() const;
@@ -84,6 +85,7 @@
bool allow_n_vsyncs_in_targeter() const;
bool detached_mirror() const;
bool commit_not_composited() const;
+ bool local_tonemap_screenshots() const;
protected:
// overridden for unit tests
diff --git a/services/surfaceflinger/surfaceflinger_flags_new.aconfig b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
index f1ec3e1..5b94f07 100644
--- a/services/surfaceflinger/surfaceflinger_flags_new.aconfig
+++ b/services/surfaceflinger/surfaceflinger_flags_new.aconfig
@@ -76,4 +76,23 @@
}
} # latch_unsignaled_with_auto_refresh_changed
+flag {
+ name: "local_tonemap_screenshots"
+ namespace: "core_graphics"
+ description: "Enables local tonemapping when capturing screenshots"
+ bug: "329464641"
+ is_fixed_read_only: true
+} # local_tonemap_screenshots
+
+flag {
+ name: "vrr_bugfix_24q4"
+ namespace: "core_graphics"
+ description: "bug fixes for VRR"
+ bug: "331513837"
+ is_fixed_read_only: true
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+} # vrr_bugfix_24q4
+
# IMPORTANT - please keep alphabetize to reduce merge conflicts
diff --git a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
index 867ff55..158db75 100644
--- a/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerLifecycleManagerTest.cpp
@@ -461,8 +461,9 @@
HAL_PIXEL_FORMAT_RGBA_8888,
GRALLOC_USAGE_PROTECTED /*usage*/));
EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(),
- ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer |
- RequestedLayerState::Changes::Content)
+ ftl::Flags<RequestedLayerState::Changes>(
+ RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content |
+ RequestedLayerState::Changes::RequiresComposition)
.get());
mLifecycleManager.commitChanges();
@@ -493,10 +494,11 @@
HAL_PIXEL_FORMAT_RGB_888,
GRALLOC_USAGE_PROTECTED /*usage*/));
EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(),
- ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer |
- RequestedLayerState::Changes::Content |
- RequestedLayerState::Changes::VisibleRegion |
- RequestedLayerState::Changes::Visibility)
+ ftl::Flags<RequestedLayerState::Changes>(
+ RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content |
+ RequestedLayerState::Changes::VisibleRegion |
+ RequestedLayerState::Changes::Visibility |
+ RequestedLayerState::Changes::RequiresComposition)
.get());
mLifecycleManager.commitChanges();
}
@@ -520,7 +522,8 @@
RequestedLayerState::Changes::AffectsChildren |
RequestedLayerState::Changes::Content |
RequestedLayerState::Changes::Geometry |
- RequestedLayerState::Changes::VisibleRegion)
+ RequestedLayerState::Changes::VisibleRegion |
+ RequestedLayerState::Changes::RequiresComposition)
.get());
mLifecycleManager.commitChanges();
}
@@ -538,7 +541,8 @@
ftl::Flags<RequestedLayerState::Changes>(
RequestedLayerState::Changes::Content |
RequestedLayerState::Changes::AffectsChildren |
- RequestedLayerState::Changes::VisibleRegion)
+ RequestedLayerState::Changes::VisibleRegion |
+ RequestedLayerState::Changes::RequiresComposition)
.string());
EXPECT_EQ(mLifecycleManager.getChangedLayers()[0]->color.a, static_cast<half>(startingAlpha));
mLifecycleManager.commitChanges();
@@ -551,7 +555,8 @@
ftl::Flags<RequestedLayerState::Changes>(
RequestedLayerState::Changes::Content |
RequestedLayerState::Changes::AffectsChildren |
- RequestedLayerState::Changes::VisibleRegion)
+ RequestedLayerState::Changes::VisibleRegion |
+ RequestedLayerState::Changes::RequiresComposition)
.string());
EXPECT_EQ(mLifecycleManager.getChangedLayers()[0]->color.a, static_cast<half>(endingAlpha));
mLifecycleManager.commitChanges();
@@ -580,8 +585,9 @@
HAL_PIXEL_FORMAT_RGBA_8888,
GRALLOC_USAGE_SW_READ_NEVER /*usage*/));
EXPECT_EQ(mLifecycleManager.getGlobalChanges().get(),
- ftl::Flags<RequestedLayerState::Changes>(RequestedLayerState::Changes::Buffer |
- RequestedLayerState::Changes::Content)
+ ftl::Flags<RequestedLayerState::Changes>(
+ RequestedLayerState::Changes::Buffer | RequestedLayerState::Changes::Content |
+ RequestedLayerState::Changes::RequiresComposition)
.get());
mLifecycleManager.commitChanges();
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index 0011c12..ce4d798 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -254,7 +254,9 @@
TEST_F(LayerSnapshotTest, FastPathClearsPreviousChangeStates) {
setColor(11, {1._hf, 0._hf, 0._hf});
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
- EXPECT_EQ(getSnapshot(11)->changes, RequestedLayerState::Changes::Content);
+ EXPECT_EQ(getSnapshot(11)->changes,
+ RequestedLayerState::Changes::Content |
+ RequestedLayerState::Changes::RequiresComposition);
EXPECT_EQ(getSnapshot(11)->clientChanges, layer_state_t::eColorChanged);
EXPECT_EQ(getSnapshot(1)->changes.get(), 0u);
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
@@ -264,7 +266,9 @@
TEST_F(LayerSnapshotTest, FastPathSetsChangeFlagToContent) {
setColor(1, {1._hf, 0._hf, 0._hf});
UPDATE_AND_VERIFY(mSnapshotBuilder, STARTING_ZORDER);
- EXPECT_EQ(getSnapshot(1)->changes, RequestedLayerState::Changes::Content);
+ EXPECT_EQ(getSnapshot(1)->changes,
+ RequestedLayerState::Changes::Content |
+ RequestedLayerState::Changes::RequiresComposition);
EXPECT_EQ(getSnapshot(1)->clientChanges, layer_state_t::eColorChanged);
}