Restrict "Use prefered interval for video" to multi-video cases only.

Bug: 976583
Change-Id: I1967e144fba5eca4d2b0d25aab755c0510030ce4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2117083
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754858}

components/viz/service/display/display.cc

@@ -313,7 +313,8 @@ Display::~Display() {
 
 void Display::Initialize(DisplayClient* client,
                          SurfaceManager* surface_manager,
-                         bool enable_shared_images) {
+                         bool enable_shared_images,
+                         bool using_synthetic_bfs) {
   DCHECK(client);
   DCHECK(surface_manager);
   gpu::ScopedAllowScheduleGpuTask allow_schedule_gpu_task;
@@ -324,8 +325,8 @@ void Display::Initialize(DisplayClient* client,
   if (output_surface_->software_device())
     output_surface_->software_device()->BindToClient(this);
 
-  frame_rate_decider_ =
-      std::make_unique<FrameRateDecider>(surface_manager_, this);
+  frame_rate_decider_ = std::make_unique<FrameRateDecider>(
+      surface_manager_, this, using_synthetic_bfs);
 
   InitializeRenderer(enable_shared_images);
 

components/viz/service/display/display.h

@@ -103,7 +103,8 @@ class VIZ_SERVICE_EXPORT Display : public DisplaySchedulerClient,
 #endif
   void Initialize(DisplayClient* client,
                   SurfaceManager* surface_manager,
-                  bool enable_shared_images = kEnableSharedImages);
+                  bool enable_shared_images = kEnableSharedImages,
+                  bool using_synthetic_bfs = false);
 
   void AddObserver(DisplayObserver* observer);
   void RemoveObserver(DisplayObserver* observer);

components/viz/service/display/frame_rate_decider.cc

@@ -20,10 +20,12 @@ FrameRateDecider::ScopedAggregate::~ScopedAggregate() {
 }
 
 FrameRateDecider::FrameRateDecider(SurfaceManager* surface_manager,
-                                   Client* client)
+                                   Client* client,
+                                   bool using_synthetic_bfs)
     : supported_intervals_{BeginFrameArgs::DefaultInterval()},
       surface_manager_(surface_manager),
-      client_(client) {
+      client_(client),
+      using_synthetic_bfs_(using_synthetic_bfs) {
   surface_manager_->AddObserver(this);
 }
 
@@ -70,6 +72,7 @@ void FrameRateDecider::OnSurfaceWillBeDrawn(Surface* surface) {
       it->second != active_index) {
     frame_sinks_updated_in_previous_frame_.insert(surface_id.frame_sink_id());
   }
+  frame_sinks_drawn_in_previous_frame_.insert(surface_id.frame_sink_id());
 }
 
 void FrameRateDecider::StartAggregation() {
@@ -77,6 +80,7 @@ void FrameRateDecider::StartAggregation() {
 
   inside_surface_aggregation_ = true;
   frame_sinks_updated_in_previous_frame_.clear();
+  frame_sinks_drawn_in_previous_frame_.clear();
 }
 
 void FrameRateDecider::EndAggregation() {
@@ -93,6 +97,30 @@ void FrameRateDecider::UpdatePreferredFrameIntervalIfNeeded() {
   if (!multiple_refresh_rates_supported())
     return;
 
+  // If lowering the refresh rate is supported by the platform then we do this
+  // in all cases where the content drawing onscreen animates at a fixed rate.
+  // This allows the platform to refresh the screen at a lower rate which is
+  // power efficient.
+  //
+  // But if we're using a synthetic begin frame source, then there is no benefit
+  // in ticking at a lower rate unless there are multiple frame sinks animating
+  // at a fixed rate. Ticking at a lower rate in this case ensures that updates
+  // from the frame sinks are aligned to the same vsync, allowing the compositor
+  // to draw at a lower rate.
+  if (using_synthetic_bfs_) {
+    int num_of_frame_sinks_with_fixed_interval = 0;
+    for (const auto& frame_sink_id : frame_sinks_drawn_in_previous_frame_) {
+      if (client_->GetPreferredFrameIntervalForFrameSinkId(frame_sink_id) !=
+          BeginFrameArgs::MinInterval())
+        num_of_frame_sinks_with_fixed_interval++;
+    }
+
+    if (num_of_frame_sinks_with_fixed_interval < 2) {
+      SetPreferredInterval(UnspecifiedFrameInterval());
+      return;
+    }
+  }
+
   // The code below picks the optimal frame interval for the display based on
   // the frame sinks which were updated in this frame. This is because we want
   // the display's update rate to be decided based on onscreen content that is
@@ -132,6 +160,11 @@ void FrameRateDecider::UpdatePreferredFrameIntervalIfNeeded() {
     }
   }
 
+  SetPreferredInterval(new_preferred_interval);
+}
+
+void FrameRateDecider::SetPreferredInterval(
+    base::TimeDelta new_preferred_interval) {
   if (new_preferred_interval == last_computed_preferred_frame_interval_) {
     num_of_frames_since_preferred_interval_changed_++;
   } else {

components/viz/service/display/frame_rate_decider.h

@@ -52,7 +52,9 @@ class VIZ_SERVICE_EXPORT FrameRateDecider : public SurfaceObserver {
     FrameRateDecider* const decider_;
   };
 
-  FrameRateDecider(SurfaceManager* surface_manager, Client* client);
+  FrameRateDecider(SurfaceManager* surface_manager,
+                   Client* client,
+                   bool using_synthetic_bfs);
   ~FrameRateDecider() override;
 
   void SetSupportedFrameIntervals(
@@ -69,6 +71,7 @@ class VIZ_SERVICE_EXPORT FrameRateDecider : public SurfaceObserver {
   void StartAggregation();
   void EndAggregation();
   void UpdatePreferredFrameIntervalIfNeeded();
+  void SetPreferredInterval(base::TimeDelta new_preferred_interval);
   bool multiple_refresh_rates_supported() const {
     return supported_intervals_.size() > 1u;
   }
@@ -77,6 +80,7 @@ class VIZ_SERVICE_EXPORT FrameRateDecider : public SurfaceObserver {
   base::flat_map<SurfaceId, uint64_t> current_surface_id_to_active_index_;
 
   base::flat_set<FrameSinkId> frame_sinks_updated_in_previous_frame_;
+  base::flat_set<FrameSinkId> frame_sinks_drawn_in_previous_frame_;
   base::flat_map<SurfaceId, uint64_t> prev_surface_id_to_active_index_;
 
   std::vector<base::TimeDelta> supported_intervals_;
@@ -88,6 +92,7 @@ class VIZ_SERVICE_EXPORT FrameRateDecider : public SurfaceObserver {
   size_t min_num_of_frames_to_toggle_interval_ = 60u;
   SurfaceManager* const surface_manager_;
   Client* const client_;
+  const bool using_synthetic_bfs_;
 };
 
 }  // namespace viz

components/viz/service/display/frame_rate_decider_unittest.cc

@@ -28,7 +28,7 @@ class FrameRateDeciderTest : public testing::Test,
   void SetUp() override {
     surface_manager_ = std::make_unique<SurfaceManager>(this, base::nullopt);
     frame_rate_decider_ =
-        std::make_unique<FrameRateDecider>(surface_manager_.get(), this);
+        std::make_unique<FrameRateDecider>(surface_manager_.get(), this, false);
     frame_rate_decider_->set_min_num_of_frames_to_toggle_interval_for_testing(
         0u);
   }
@@ -315,5 +315,53 @@ TEST_F(FrameRateDeciderTest, TogglesAfterMinNumOfFrames) {
   EXPECT_EQ(display_interval_, min_supported_interval);
 }
 
+TEST_F(FrameRateDeciderTest, TogglesWithSyntheticBFS) {
+  frame_rate_decider_ =
+      std::make_unique<FrameRateDecider>(surface_manager_.get(), this, true);
+  frame_rate_decider_->set_min_num_of_frames_to_toggle_interval_for_testing(0u);
+  base::TimeDelta min_supported_interval = base::TimeDelta::FromSeconds(1);
+  const std::vector<base::TimeDelta> supported_intervals = {
+      min_supported_interval * 2, min_supported_interval};
+  frame_rate_decider_->SetSupportedFrameIntervals(supported_intervals);
+  EXPECT_EQ(display_interval_, FrameRateDecider::UnspecifiedFrameInterval());
+
+  FrameSinkId frame_sink_id1(1u, 1u);
+  preferred_intervals_[frame_sink_id1] = min_supported_interval * 2;
+  auto* surface1 = CreateAndDrawSurface(frame_sink_id1);
+
+  // If there is only one framesink with non-minimum interval,
+  // UnspecifiedFrameInterval() should be used
+  UpdateFrame(surface1);
+  {
+    FrameRateDecider::ScopedAggregate scope(frame_rate_decider_.get());
+    frame_rate_decider_->OnSurfaceWillBeDrawn(surface1);
+  }
+  EXPECT_EQ(display_interval_, FrameRateDecider::UnspecifiedFrameInterval());
+
+  FrameSinkId frame_sink_id2(1u, 2u);
+  preferred_intervals_[frame_sink_id2] = min_supported_interval * 2;
+  auto* surface2 = CreateAndDrawSurface(frame_sink_id2);
+
+  // With two or more framesinks with non-minimum interval, calculate the
+  // optimal interval
+  UpdateFrame(surface1);
+  UpdateFrame(surface2);
+  {
+    FrameRateDecider::ScopedAggregate scope(frame_rate_decider_.get());
+    frame_rate_decider_->OnSurfaceWillBeDrawn(surface1);
+    frame_rate_decider_->OnSurfaceWillBeDrawn(surface2);
+  }
+  EXPECT_EQ(display_interval_, min_supported_interval * 2);
+
+  // Make sure it goes back to UnspecifiedFrameInterval() if we have only one
+  // framesink with non-minimum interval left
+  UpdateFrame(surface1);
+  {
+    FrameRateDecider::ScopedAggregate scope(frame_rate_decider_.get());
+    frame_rate_decider_->OnSurfaceWillBeDrawn(surface1);
+  }
+  EXPECT_EQ(display_interval_, FrameRateDecider::UnspecifiedFrameInterval());
+}
+
 }  // namespace
 }  // namespace viz

components/viz/service/frame_sinks/root_compositor_frame_sink_impl.cc

@@ -243,7 +243,8 @@ void RootCompositorFrameSinkImpl::SetDisplayVSyncParameters(
 
 void RootCompositorFrameSinkImpl::UpdateVSyncParameters() {
   base::TimeTicks timebase = display_frame_timebase_;
-  // Overwrite the interval here if |use_preferred_interval_|
+  // Overwrite the interval with a meaningful one here if
+  // |use_preferred_interval_|
   base::TimeDelta interval =
       use_preferred_interval_ &&
               preferred_frame_interval_ !=
@@ -377,11 +378,14 @@ RootCompositorFrameSinkImpl::RootCompositorFrameSinkImpl(
       display_(std::move(display)) {
   DCHECK(display_);
   DCHECK(begin_frame_source());
+  bool using_synthetic_bfs =
+      begin_frame_source() == synthetic_begin_frame_source_.get();
   frame_sink_manager->RegisterBeginFrameSource(begin_frame_source(),
                                                support_->frame_sink_id());
-  display_->Initialize(this, support_->frame_sink_manager()->surface_manager());
+  display_->Initialize(this, support_->frame_sink_manager()->surface_manager(),
+                       Display::kEnableSharedImages, using_synthetic_bfs);
   support_->SetUpHitTest(display_.get());
-  if (use_preferred_interval_for_video && synthetic_begin_frame_source_) {
+  if (use_preferred_interval_for_video && using_synthetic_bfs) {
     display_->SetSupportedFrameIntervals(
         {display_frame_interval_, display_frame_interval_ * 2});
     use_preferred_interval_ = true;