Skip to content

Commit bbe26c2

Browse files
RamIndaniAndroid (Google) Code Review
authored andcommitted
Merge "[SF] vrr_bugfix_24q4 flag cleanup" into main
2 parents 3937ce2 + 9427792 commit bbe26c2

File tree

7 files changed

+19
-70
lines changed

7 files changed

+19
-70
lines changed

services/surfaceflinger/QueuedTransactionState.h

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -105,22 +105,15 @@ struct QueuedTransactionState {
105105

106106
for (const auto& state : states) {
107107
const bool frameRateChanged = state.state.what & layer_state_t::eFrameRateChanged;
108-
if (FlagManager::getInstance().vrr_bugfix_24q4()) {
109-
const bool frameRateIsNoVote = frameRateChanged &&
110-
state.state.frameRateCompatibility == ANATIVEWINDOW_FRAME_RATE_NO_VOTE;
111-
const bool frameRateCategoryChanged =
112-
state.state.what & layer_state_t::eFrameRateCategoryChanged;
113-
const bool frameRateCategoryIsNoPreference = frameRateCategoryChanged &&
114-
state.state.frameRateCategory ==
115-
ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE;
116-
if (!frameRateIsNoVote && !frameRateCategoryIsNoPreference) {
117-
return true;
118-
}
119-
} else {
120-
if (!frameRateChanged ||
121-
state.state.frameRateCompatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE) {
122-
return true;
123-
}
108+
const bool frameRateIsNoVote = frameRateChanged &&
109+
state.state.frameRateCompatibility == ANATIVEWINDOW_FRAME_RATE_NO_VOTE;
110+
const bool frameRateCategoryChanged =
111+
state.state.what & layer_state_t::eFrameRateCategoryChanged;
112+
const bool frameRateCategoryIsNoPreference = frameRateCategoryChanged &&
113+
state.state.frameRateCategory ==
114+
ANATIVEWINDOW_FRAME_RATE_CATEGORY_NO_PREFERENCE;
115+
if (!frameRateIsNoVote && !frameRateCategoryIsNoPreference) {
116+
return true;
124117
}
125118
}
126119

services/surfaceflinger/Scheduler/VSyncPredictor.cpp

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -374,11 +374,7 @@ bool VSyncPredictor::isVSyncInPhase(nsecs_t timePoint, Fps frameRate) {
374374
purgeTimelines(now);
375375

376376
for (auto& timeline : mTimelines) {
377-
const bool isVsyncValid = FlagManager::getInstance().vrr_bugfix_24q4()
378-
? timeline.isWithin(TimePoint::fromNs(vsync)) ==
379-
VsyncTimeline::VsyncOnTimeline::Unique
380-
: timeline.validUntil() && timeline.validUntil()->ns() > vsync;
381-
if (isVsyncValid) {
377+
if (timeline.isWithin(TimePoint::fromNs(vsync)) == VsyncTimeline::VsyncOnTimeline::Unique) {
382378
return timeline.isVSyncInPhase(model, vsync, frameRate);
383379
}
384380
}
@@ -413,15 +409,10 @@ void VSyncPredictor::setRenderRate(Fps renderRate, bool applyImmediately) {
413409
mLastCommittedVsync = TimePoint::fromNs(0);
414410

415411
} else {
416-
if (FlagManager::getInstance().vrr_bugfix_24q4()) {
417-
// We need to freeze the timeline at the committed vsync, and
418-
// then use with threshold adjustments when required to avoid
419-
// marginal errors when checking the vsync on the timeline.
420-
mTimelines.back().freeze(mLastCommittedVsync);
421-
} else {
422-
mTimelines.back().freeze(
423-
TimePoint::fromNs(mLastCommittedVsync.ns() + mIdealPeriod.ns() / 2));
424-
}
412+
// We need to freeze the timeline at the committed vsync, and
413+
// then use with threshold adjustments when required to avoid
414+
// marginal errors when checking the vsync on the timeline.
415+
mTimelines.back().freeze(mLastCommittedVsync);
425416
}
426417
mTimelines.emplace_back(mLastCommittedVsync, mIdealPeriod, renderRate);
427418
purgeTimelines(TimePoint::fromNs(mClock->now()));
@@ -646,11 +637,7 @@ void VSyncPredictor::purgeTimelines(android::TimePoint now) {
646637
}
647638

648639
while (mTimelines.size() > 1) {
649-
const auto validUntilOpt = mTimelines.front().validUntil();
650-
const bool isTimelineOutDated = FlagManager::getInstance().vrr_bugfix_24q4()
651-
? mTimelines.front().isWithin(now) == VsyncTimeline::VsyncOnTimeline::Outside
652-
: validUntilOpt && *validUntilOpt < now;
653-
if (isTimelineOutDated) {
640+
if (mTimelines.front().isWithin(now) == VsyncTimeline::VsyncOnTimeline::Outside) {
654641
mTimelines.pop_front();
655642
} else {
656643
break;
@@ -702,12 +689,6 @@ std::optional<TimePoint> VSyncPredictor::VsyncTimeline::nextAnticipatedVSyncTime
702689
SFTRACE_FORMAT_INSTANT("lastFrameMissed");
703690
}
704691
} else if (mightBackpressure && lastVsyncOpt) {
705-
if (!FlagManager::getInstance().vrr_bugfix_24q4()) {
706-
// lastVsyncOpt does not need to be corrected with the new rate, and
707-
// it should be used as is to avoid skipping a frame when changing rates are
708-
// aligned at vsync time.
709-
lastVsyncOpt = snapToVsyncAlignedWithRenderRate(model, *lastVsyncOpt);
710-
}
711692
const auto vsyncDiff = vsyncTime - *lastVsyncOpt;
712693
if (vsyncDiff <= minFramePeriodOpt->ns() - threshold) {
713694
// avoid a duplicate vsync
@@ -726,10 +707,7 @@ std::optional<TimePoint> VSyncPredictor::VsyncTimeline::nextAnticipatedVSyncTime
726707
}
727708

728709
SFTRACE_FORMAT_INSTANT("vsync in %.2fms", float(vsyncTime - TimePoint::now().ns()) / 1e6f);
729-
const bool isVsyncInvalid = FlagManager::getInstance().vrr_bugfix_24q4()
730-
? isWithin(TimePoint::fromNs(vsyncTime)) == VsyncOnTimeline::Outside
731-
: mValidUntil && vsyncTime > mValidUntil->ns();
732-
if (isVsyncInvalid) {
710+
if (isWithin(TimePoint::fromNs(vsyncTime)) == VsyncOnTimeline::Outside) {
733711
SFTRACE_FORMAT_INSTANT("no longer valid for vsync in %.2f",
734712
static_cast<float>(vsyncTime - TimePoint::now().ns()) / 1e6f);
735713
return std::nullopt;
@@ -785,9 +763,7 @@ bool VSyncPredictor::VsyncTimeline::isVSyncInPhase(Model model, nsecs_t vsync, F
785763
return ticks<std::milli, float>(TimePoint::fromNs(timePoint) - now);
786764
};
787765

788-
Fps displayFps = !FlagManager::getInstance().vrr_bugfix_24q4() && mRenderRateOpt
789-
? *mRenderRateOpt
790-
: Fps::fromPeriodNsecs(mIdealPeriod.ns());
766+
Fps displayFps = Fps::fromPeriodNsecs(mIdealPeriod.ns());
791767
const auto divisor = RefreshRateSelector::getFrameRateDivisor(displayFps, frameRate);
792768
const auto now = TimePoint::now();
793769

services/surfaceflinger/SurfaceFlinger.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2593,12 +2593,8 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs,
25932593
mUpdateAttachedChoreographer = true;
25942594
}
25952595
outTransactionsAreEmpty = mLayerLifecycleManager.getGlobalChanges().get() == 0;
2596-
if (FlagManager::getInstance().vrr_bugfix_24q4()) {
2597-
mustComposite |= mLayerLifecycleManager.getGlobalChanges().any(
2598-
frontend::RequestedLayerState::kMustComposite);
2599-
} else {
2600-
mustComposite |= mLayerLifecycleManager.getGlobalChanges().get() != 0;
2601-
}
2596+
mustComposite |= mLayerLifecycleManager.getGlobalChanges().any(
2597+
frontend::RequestedLayerState::kMustComposite);
26022598

26032599
bool newDataLatched = false;
26042600
SFTRACE_NAME("DisplayCallbackAndStatsUpdates");

services/surfaceflinger/common/FlagManager.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,6 @@ void FlagManager::dump(std::string& result) const {
179179
DUMP_ACONFIG_FLAG(trace_frame_rate_override);
180180
DUMP_ACONFIG_FLAG(true_hdr_screenshots);
181181
DUMP_ACONFIG_FLAG(use_known_refresh_rate_for_fps_consistency);
182-
DUMP_ACONFIG_FLAG(vrr_bugfix_24q4);
183182
DUMP_ACONFIG_FLAG(vrr_bugfix_dropped_frame);
184183
DUMP_ACONFIG_FLAG(vrr_config);
185184
DUMP_ACONFIG_FLAG(vulkan_renderengine);
@@ -277,7 +276,6 @@ FLAG_MANAGER_ACONFIG_FLAG(restore_blur_step, "debug.renderengine.restore_blur_st
277276
FLAG_MANAGER_ACONFIG_FLAG(dont_skip_on_early_ro, "")
278277
FLAG_MANAGER_ACONFIG_FLAG(no_vsyncs_on_screen_off, "debug.sf.no_vsyncs_on_screen_off")
279278
FLAG_MANAGER_ACONFIG_FLAG(protected_if_client, "")
280-
FLAG_MANAGER_ACONFIG_FLAG(vrr_bugfix_24q4, "");
281279
FLAG_MANAGER_ACONFIG_FLAG(vrr_bugfix_dropped_frame, "")
282280
FLAG_MANAGER_ACONFIG_FLAG(graphite_renderengine, "debug.renderengine.graphite")
283281
FLAG_MANAGER_ACONFIG_FLAG(filter_frames_before_trace_starts, "")

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ class FlagManager {
114114
bool trace_frame_rate_override() const;
115115
bool true_hdr_screenshots() const;
116116
bool use_known_refresh_rate_for_fps_consistency() const;
117-
bool vrr_bugfix_24q4() const;
118117
bool vrr_bugfix_dropped_frame() const;
119118
bool vrr_config() const;
120119
bool vulkan_renderengine() const;

services/surfaceflinger/surfaceflinger_flags_new.aconfig

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -317,17 +317,6 @@ flag {
317317
}
318318
} # override_trusted_overlay
319319

320-
flag {
321-
name: "vrr_bugfix_24q4"
322-
namespace: "core_graphics"
323-
description: "bug fixes for VRR"
324-
bug: "331513837"
325-
is_fixed_read_only: true
326-
metadata {
327-
purpose: PURPOSE_BUGFIX
328-
}
329-
} # vrr_bugfix_24q4
330-
331320
flag {
332321
name: "vrr_bugfix_dropped_frame"
333322
namespace: "core_graphics"

services/surfaceflinger/tests/unittests/VSyncPredictorTest.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,6 @@ TEST_F(VSyncPredictorTest, isVSyncInPhase) {
579579
}
580580

581581
TEST_F(VSyncPredictorTest, isVSyncInPhaseWithRenderRate) {
582-
SET_FLAG_FOR_TEST(flags::vrr_bugfix_24q4, true);
583582
auto last = mNow;
584583
for (auto i = 0u; i < kMinimumSamplesForPrediction; i++) {
585584
EXPECT_THAT(tracker.nextAnticipatedVSyncTimeFrom(mNow), Eq(last + mPeriod));
@@ -718,7 +717,6 @@ TEST_F(VSyncPredictorTest, setRenderRateIsIgnoredIfNotDivisor) {
718717

719718
TEST_F(VSyncPredictorTest, setRenderRateWhenRenderRateGoesDown) {
720719
SET_FLAG_FOR_TEST(flags::vrr_config, true);
721-
SET_FLAG_FOR_TEST(flags::vrr_bugfix_24q4, true);
722720

723721
const int32_t kGroup = 0;
724722
const auto kResolution = ui::Size(1920, 1080);

0 commit comments

Comments
 (0)