VRR for DelayBasedBeginFrameSource
This change modifies DelayBasedBeginFrameSource in order to make it
compatible with the variable refresh rate feature. By using the max
VRR interval for the BeginFrameArgs, the scheduler will utilize a
delayed presentation deadline consistent with the display's maximum
vblank extension. The OnUpdateVsyncParameters callback keeps the time
source aligned with the page flips as they occur, such that each
begin frame is issued with a delay relative to the previous one by an
interval equal to the nominal rate.
Prior to this CL, the VRR feature required the
RootCompositorFrameSinkImpl to be initialized with a
BackToBackBeginFrameSource, which prevented VRR from being toggled
after startup. This change resolves that issue by updating
DelayBasedBeginFrameSource to work with VRR.
This change should not have any impact when the VRR feature is disabled.
The DelayBasedBeginFrameSource should fallback to the nominal refresh
interval which is consistent with the preexisting behavior.
BUG=b:290265668
Change-Id: I38498276f6e1f9780ce2e8b5275f95fd9228b896
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4799472
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Andrew Wolfers <aswolfers@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1190734}
diff --git a/components/viz/common/frame_sinks/begin_frame_source.cc b/components/viz/common/frame_sinks/begin_frame_source.cc
index 3dcb576..0851fa9 100644
--- a/components/viz/common/frame_sinks/begin_frame_source.cc
+++ b/components/viz/common/frame_sinks/begin_frame_source.cc
@@ -110,7 +110,7 @@
BeginFrameSource::BeginFrameArgsGenerator::GenerateBeginFrameArgs(
uint64_t source_id,
base::TimeTicks frame_time,
- base::TimeTicks next_frame_time,
+ base::TimeTicks deadline,
base::TimeDelta vsync_interval) {
uint64_t sequence_number =
next_sequence_number_ +
@@ -125,12 +125,12 @@
base::TimeDelta deadline_offset =
dynamic_begin_frame_deadline_offset_source_->GetDeadlineOffset(
vsync_interval);
- next_frame_time -= deadline_offset;
+ deadline -= deadline_offset;
}
- next_expected_frame_time_ = next_frame_time;
+ next_expected_frame_time_ = deadline;
next_sequence_number_ = sequence_number + 1;
return BeginFrameArgs::Create(BEGINFRAME_FROM_HERE, source_id,
- sequence_number, frame_time, next_frame_time,
+ sequence_number, frame_time, deadline,
vsync_interval, BeginFrameArgs::NORMAL);
}
@@ -302,7 +302,7 @@
void BackToBackBeginFrameSource::SetMaxVrrInterval(
const absl::optional<base::TimeDelta>& max_vrr_interval) {
- DCHECK(!max_vrr_interval.has_value() || !max_vrr_interval->is_zero());
+ DCHECK(!max_vrr_interval.has_value() || max_vrr_interval->is_positive());
max_vrr_interval_ = max_vrr_interval;
}
@@ -351,9 +351,14 @@
BeginFrameArgs DelayBasedBeginFrameSource::CreateBeginFrameArgs(
base::TimeTicks frame_time) {
- base::TimeDelta interval = time_source_->Interval();
+ base::TimeDelta interval =
+ max_vrr_interval_.value_or(time_source_->Interval());
+ // Use `Next-` instead of `LastTickTime` because it is snapped to
+ // `last_timebase_`
+ base::TimeTicks deadline =
+ time_source_->NextTickTime() - time_source_->Interval() + interval;
return begin_frame_args_generator_.GenerateBeginFrameArgs(
- source_id(), frame_time, time_source_->NextTickTime(), interval);
+ source_id(), frame_time, deadline, interval);
}
void DelayBasedBeginFrameSource::AddObserver(BeginFrameObserver* obs) {
@@ -372,10 +377,13 @@
// sufficient time has passed since the last tick.
base::TimeTicks last_or_missed_tick_time =
time_source_->NextTickTime() - time_source_->Interval();
+ const base::TimeDelta double_tick_margin =
+ max_vrr_interval_.has_value()
+ ? base::TimeDelta()
+ : time_source_->Interval() / kDoubleTickDivisor;
if (!last_begin_frame_args_.IsValid() ||
last_or_missed_tick_time >
- last_begin_frame_args_.frame_time +
- last_begin_frame_args_.interval / kDoubleTickDivisor) {
+ last_begin_frame_args_.frame_time + double_tick_margin) {
last_begin_frame_args_ = CreateBeginFrameArgs(last_or_missed_tick_time);
}
BeginFrameArgs missed_args = last_begin_frame_args_;
@@ -403,6 +411,12 @@
dynamic_begin_frame_deadline_offset_source);
}
+void DelayBasedBeginFrameSource::SetMaxVrrInterval(
+ const absl::optional<base::TimeDelta>& max_vrr_interval) {
+ DCHECK(!max_vrr_interval.has_value() || max_vrr_interval->is_positive());
+ max_vrr_interval_ = max_vrr_interval;
+}
+
void DelayBasedBeginFrameSource::OnTimerTick() {
if (RequestCallbackOnGpuAvailable())
return;
@@ -425,9 +439,11 @@
BeginFrameObserver* obs,
const BeginFrameArgs& args) {
BeginFrameArgs last_args = obs->LastUsedBeginFrameArgs();
+ const base::TimeDelta double_tick_margin =
+ max_vrr_interval_.has_value() ? base::TimeDelta()
+ : args.interval / kDoubleTickDivisor;
if (!last_args.IsValid() ||
- (args.frame_time >
- last_args.frame_time + args.interval / kDoubleTickDivisor)) {
+ (args.frame_time > last_args.frame_time + double_tick_margin)) {
if (args.type == BeginFrameArgs::MISSED) {
DCHECK(!last_args.frame_id.IsNextInSequenceTo(args.frame_id))
<< "missed " << args.ToString() << ", last " << last_args.ToString();
diff --git a/components/viz/common/frame_sinks/begin_frame_source.h b/components/viz/common/frame_sinks/begin_frame_source.h
index 9cee914..d80de778 100644
--- a/components/viz/common/frame_sinks/begin_frame_source.h
+++ b/components/viz/common/frame_sinks/begin_frame_source.h
@@ -150,7 +150,7 @@
BeginFrameArgs GenerateBeginFrameArgs(uint64_t source_id,
base::TimeTicks frame_time,
- base::TimeTicks next_frame_time,
+ base::TimeTicks deadline,
base::TimeDelta vsync_interval);
void set_dynamic_begin_frame_deadline_offset_source(
@@ -335,8 +335,6 @@
void OnTimerTick() override;
private:
- void SetActive(bool active);
-
std::unique_ptr<DelayBasedTimeSource> time_source_;
base::flat_set<BeginFrameObserver*> observers_;
base::flat_set<BeginFrameObserver*> pending_begin_frame_observers_;
@@ -373,7 +371,8 @@
// SyntheticBeginFrameSource implementation.
void OnUpdateVSyncParameters(base::TimeTicks timebase,
base::TimeDelta interval) override;
- void SetMaxVrrInterval(const absl::optional<base::TimeDelta>&) override {}
+ void SetMaxVrrInterval(
+ const absl::optional<base::TimeDelta>& max_vrr_interval) override;
// DelayBasedTimeSourceClient implementation.
void OnTimerTick() override;
@@ -397,8 +396,8 @@
std::unique_ptr<DelayBasedTimeSource> time_source_;
base::flat_set<BeginFrameObserver*> observers_;
base::TimeTicks last_timebase_;
+ absl::optional<base::TimeDelta> max_vrr_interval_ = absl::nullopt;
BeginFrameArgs last_begin_frame_args_;
-
BeginFrameArgsGenerator begin_frame_args_generator_;
};
diff --git a/components/viz/common/frame_sinks/begin_frame_source_unittest.cc b/components/viz/common/frame_sinks/begin_frame_source_unittest.cc
index 0c86616..1176c97 100644
--- a/components/viz/common/frame_sinks/begin_frame_source_unittest.cc
+++ b/components/viz/common/frame_sinks/begin_frame_source_unittest.cc
@@ -685,7 +685,7 @@
source_->RemoveObserver(&obs);
// New args created 8 intervals later.
- // Sequence number should increase bt this much.
+ // Sequence number should increase by this much.
EXPECT_BEGIN_FRAME_SOURCE_PAUSED(obs, false);
EXPECT_BEGIN_FRAME_USED_MISSED(obs, source_->source_id(), 10, 90000, 100000,
10000);
@@ -693,6 +693,36 @@
source_->AddObserver(&obs);
}
+TEST_F(DelayBasedBeginFrameSourceTest, WithVrrInterval) {
+ NiceMock<MockBeginFrameObserver> obs;
+ source_->SetMaxVrrInterval(base::Microseconds(25000));
+
+ EXPECT_BEGIN_FRAME_SOURCE_PAUSED(obs, false);
+ EXPECT_BEGIN_FRAME_USED_MISSED(obs, source_->source_id(), 1, 0, 25000, 25000);
+ source_->AddObserver(&obs);
+
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 2, 10000, 35000, 25000);
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 3, 20000, 45000, 25000);
+ task_runner_->FastForwardTo(TicksFromMicroseconds(21000));
+ source_->OnUpdateVSyncParameters(TicksFromMicroseconds(21000),
+ base::Microseconds(10000));
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 4, 30000, 46000, 25000);
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 5, 31000, 56000, 25000);
+ task_runner_->FastForwardTo(TicksFromMicroseconds(32000));
+ source_->OnUpdateVSyncParameters(TicksFromMicroseconds(32000),
+ base::Microseconds(10000));
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 6, 41000, 57000, 25000);
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 7, 42000, 67000, 25000);
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 8, 52000, 77000, 25000);
+ task_runner_->FastForwardTo(TicksFromMicroseconds(53000));
+ source_->OnUpdateVSyncParameters(TicksFromMicroseconds(53000),
+ base::Microseconds(10000));
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 9, 62000, 78000, 25000);
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 10, 63000, 88000, 25000);
+ EXPECT_BEGIN_FRAME_USED(obs, source_->source_id(), 11, 73000, 98000, 25000);
+ task_runner_->FastForwardTo(TicksFromMicroseconds(73000));
+}
+
// ExternalBeginFrameSource testing
// --------------------------------------------
class MockExternalBeginFrameSourceClient
diff --git a/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc b/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
index 5d1205be..c70b0af7 100644
--- a/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
+++ b/components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc
@@ -147,10 +147,7 @@
external_begin_frame_source =
std::make_unique<ExternalBeginFrameSourceIOS>(restart_id);
#else
- // TODO(b/221220344): Support dynamically choosing the BeginFrameSource per
- // VRR state changes.
- if (params->disable_frame_rate_limit ||
- params->enable_variable_refresh_rate) {
+ if (params->disable_frame_rate_limit) {
synthetic_begin_frame_source =
std::make_unique<BackToBackBeginFrameSource>(
std::make_unique<DelayBasedTimeSource>(
