Skip to content

Commit

Permalink
aw: Implement new invalidation heurstic
Browse files Browse the repository at this point in the history
This CL has to main changes:
Add an uncommitted queue to viz::Surface. Uncommitted is a new state
before the frame becomes pending. When a frame is submitted it goes to
uncommitted queue and go to active or pending on commit. Queue length is
limited and CF that doesn't fit are returned to the client. Queue length
zero means everything is committed automatically.

Hook this up to webview and implement new invalidation heuristic.
Key moments:
* All renderer embedded clients are forced to be frame behind. This
  means when we draw for Frame N, we commit their compositor frames
  for frame N-1.
* We start observing begin frames when clients do and keep observing
  them until we know there is nothing to draw.
* On BF we check if there any visible surfaces that had pending
  activation dependency. In this case we always invalidate, to make
  sure we embed everything.
* If everything if embedded we check if there are uncommitted frames
  that we didn't invalidate for. If so, we invalidate.
* If we got BF when no clients need it and we didn't invalidate we
  unsubscribe.
* If we received new CF while we don't subscribe for BF, we will
  request BF and invalidate if needed. This is to ensure last frame
  of slow clients or clients that submit without BF are still drawn.
* For overlays we commit frame immediately.

Bug: 1233542
Change-Id: Ib6a4dd26b129ad2f022cfd1ccf7ace21b9f7ed94
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3514852
Reviewed-by: Bo Liu <boliu@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: Kyle Charbonneau <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1002676}
  • Loading branch information
vasilyt authored and Chromium LUCI CQ committed May 12, 2022
1 parent fba75a3 commit 18f3406
Show file tree
Hide file tree
Showing 22 changed files with 594 additions and 73 deletions.
37 changes: 36 additions & 1 deletion android_webview/browser/gfx/display_scheduler_webview.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,25 @@
#include "android_webview/browser/gfx/display_scheduler_webview.h"

#include "android_webview/browser/gfx/root_frame_sink.h"
#include "android_webview/browser/gfx/viz_compositor_thread_runner_webview.h"
#include "base/trace_event/trace_event.h"
#include "components/viz/common/features.h"
#include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"

namespace android_webview {
DisplaySchedulerWebView::DisplaySchedulerWebView(
RootFrameSink* root_frame_sink,
OverlaysInfoProvider* overlays_info_provider)
: root_frame_sink_(root_frame_sink),
overlays_info_provider_(overlays_info_provider) {}
overlays_info_provider_(overlays_info_provider),
use_new_invalidate_heuristic_(base::FeatureList::IsEnabled(
features::kWebViewNewInvalidateHeuristic)) {
surface_manager_observation_.Observe(
VizCompositorThreadRunnerWebView::GetInstance()
->GetFrameSinkManager()
->surface_manager());
}

DisplaySchedulerWebView::~DisplaySchedulerWebView() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
}
Expand All @@ -28,6 +39,10 @@ void DisplaySchedulerWebView::SetNeedsOneBeginFrame(bool needs_draw) {
void DisplaySchedulerWebView::DidSwapBuffers() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);

// Code below is part of old invalidation heuristic.
if (use_new_invalidate_heuristic_)
return;

bool needs_draw = false;
for (auto it = damaged_frames_.begin(); it != damaged_frames_.end();) {
DCHECK_GT(it->second, 0);
Expand Down Expand Up @@ -59,6 +74,11 @@ bool DisplaySchedulerWebView::IsFrameSinkOverlayed(

void DisplaySchedulerWebView::OnDisplayDamaged(viz::SurfaceId surface_id) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);

// Code below is part of old invalidation heuristic.
if (use_new_invalidate_heuristic_)
return;

// We don't need to track damage of root frame sink as we submit frame to it
// at DrawAndSwap and Root Renderer sink because Android View.Invalidation is
// handled by SynchronousCompositorHost.
Expand All @@ -81,4 +101,19 @@ void DisplaySchedulerWebView::OnDisplayDamaged(viz::SurfaceId surface_id) {
root_frame_sink_->SetNeedsDraw(true);
}
}

void DisplaySchedulerWebView::OnSurfaceHasNewUncommittedFrame(
const viz::SurfaceId& surface_id) {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);

// We don't need to track damage of root frame sink as we submit frame to it
// at DrawAndSwap and Root Renderer sink because Android View.Invalidation is
// handled by SynchronousCompositorHost.
if (surface_id.frame_sink_id() != root_frame_sink_->root_frame_sink_id() &&
!root_frame_sink_->IsChildSurface(surface_id.frame_sink_id()) &&
!IsFrameSinkOverlayed(surface_id.frame_sink_id())) {
root_frame_sink_->OnNewUncommittedFrame(surface_id);
}
}

} // namespace android_webview
15 changes: 14 additions & 1 deletion android_webview/browser/gfx/display_scheduler_webview.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@
#include <map>

#include "base/memory/raw_ptr.h"
#include "base/scoped_observation.h"
#include "base/synchronization/waitable_event.h"
#include "base/threading/thread_checker.h"
#include "components/viz/service/display/display_scheduler.h"
#include "components/viz/service/surfaces/surface_manager.h"
#include "components/viz/service/surfaces/surface_observer.h"

namespace android_webview {
class RootFrameSink;
Expand All @@ -20,7 +23,8 @@ class OverlaysInfoProvider {
virtual bool IsFrameSinkOverlayed(viz::FrameSinkId frame_sink_id) = 0;
};

class DisplaySchedulerWebView : public viz::DisplaySchedulerBase {
class DisplaySchedulerWebView : public viz::DisplaySchedulerBase,
public viz::SurfaceObserver {
public:
DisplaySchedulerWebView(RootFrameSink* root_frame_sink,
OverlaysInfoProvider* overlays_info_provider);
Expand All @@ -42,6 +46,10 @@ class DisplaySchedulerWebView : public viz::DisplaySchedulerBase {
void OnRootFrameMissing(bool missing) override {}
void OnPendingSurfacesChanged() override {}

// SurfaceObserver implementation.
void OnSurfaceHasNewUncommittedFrame(
const viz::SurfaceId& surface_id) override;

private:
bool IsFrameSinkOverlayed(viz::FrameSinkId frame_sink_id);

Expand All @@ -55,6 +63,11 @@ class DisplaySchedulerWebView : public viz::DisplaySchedulerBase {
// destructor of this class.
const raw_ptr<OverlaysInfoProvider> overlays_info_provider_;

base::ScopedObservation<viz::SurfaceManager, viz::SurfaceObserver>
surface_manager_observation_{this};

const bool use_new_invalidate_heuristic_;

THREAD_CHECKER(thread_checker_);
};
} // namespace android_webview
Expand Down
25 changes: 24 additions & 1 deletion android_webview/browser/gfx/display_webview.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "android_webview/browser/gfx/overlay_processor_webview.h"
#include "base/memory/ptr_util.h"
#include "components/viz/common/features.h"
#include "components/viz/service/display/overlay_processor_stub.h"
#include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
#include "gpu/config/gpu_finch_features.h"
Expand Down Expand Up @@ -65,7 +66,10 @@ DisplayWebView::DisplayWebView(
std::move(overlay_processor),
std::move(scheduler),
/*current_task_runner=*/nullptr),
overlay_processor_webview_(overlay_processor_webview) {
overlay_processor_webview_(overlay_processor_webview),
frame_sink_manager_(frame_sink_manager),
use_new_invalidate_heuristic_(base::FeatureList::IsEnabled(
features::kWebViewNewInvalidateHeuristic)) {
if (overlay_processor_webview_) {
frame_sink_manager_observation_.Observe(frame_sink_manager);
}
Expand All @@ -80,6 +84,18 @@ void DisplayWebView::OnFrameSinkDidFinishFrame(
auto surface_id =
overlay_processor_webview_->GetOverlaySurfaceId(frame_sink_id);
if (surface_id.is_valid()) {
auto* surface =
frame_sink_manager_->surface_manager()->GetSurfaceForId(surface_id);
DCHECK(surface);

if (use_new_invalidate_heuristic_) {
// For overlays we are going to display this frame immediately, so commit
// it.
surface->CommitFramesRecursively(
base::BindRepeating([](const viz::SurfaceId&,
const viz::BeginFrameId&) { return true; }));
}

// TODO(vasilyt): We don't need full aggregation here as we don't need
// aggregated frame.
aggregator_->Aggregate(current_surface_id_, base::TimeTicks::Now(),
Expand All @@ -93,4 +109,11 @@ void DisplayWebView::OnFrameSinkDidFinishFrame(
}
}

std::vector<viz::SurfaceId> DisplayWebView::GetContainedSurfaceIds() {
std::vector<viz::SurfaceId> surfaces;
for (auto& surface : aggregator_->previous_contained_surfaces())
surfaces.push_back(surface.first);
return surfaces;
}

} // namespace android_webview
5 changes: 5 additions & 0 deletions android_webview/browser/gfx/display_webview.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class DisplayWebView : public viz::Display, public viz::FrameSinkObserver {
return overlay_processor_webview_;
}

std::vector<viz::SurfaceId> GetContainedSurfaceIds();

// viz::FrameSinkObserver implenentation:
void OnRegisteredFrameSinkId(const viz::FrameSinkId& frame_sink_id) override {
}
Expand Down Expand Up @@ -67,8 +69,11 @@ class DisplayWebView : public viz::Display, public viz::FrameSinkObserver {
viz::FrameSinkManagerImpl* frame_sink_manager);

const raw_ptr<OverlayProcessorWebView> overlay_processor_webview_;
const raw_ptr<viz::FrameSinkManagerImpl> frame_sink_manager_;
base::ScopedObservation<viz::FrameSinkManagerImpl, viz::FrameSinkObserver>
frame_sink_manager_observation_{this};

const bool use_new_invalidate_heuristic_;
};

} // namespace android_webview
Expand Down
50 changes: 49 additions & 1 deletion android_webview/browser/gfx/hardware_renderer_viz.cc
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ class HardwareRendererViz::OnViz : public viz::DisplayClient {
std::unique_ptr<viz::HitTestAggregator> hit_test_aggregator_;
viz::SurfaceId child_surface_id_;
const bool viz_frame_submission_;
const bool use_new_invalidate_heuristic_;
bool expect_context_loss_ = false;

// Initialized in ctor and never changes, so it's safe to access from both
Expand All @@ -131,7 +132,9 @@ HardwareRendererViz::OnViz::OnViz(
const scoped_refptr<RootFrameSink>& root_frame_sink)
: without_gpu_(root_frame_sink),
frame_sink_id_(without_gpu_->root_frame_sink_id()),
viz_frame_submission_(features::IsUsingVizFrameSubmissionForWebView()) {
viz_frame_submission_(features::IsUsingVizFrameSubmissionForWebView()),
use_new_invalidate_heuristic_(base::FeatureList::IsEnabled(
features::kWebViewNewInvalidateHeuristic)) {
DCHECK_CALLED_ON_VALID_THREAD(viz_thread_checker_);

std::unique_ptr<viz::DisplayCompositorMemoryAndTaskController>
Expand Down Expand Up @@ -252,6 +255,49 @@ void HardwareRendererViz::OnViz::DrawAndSwapOnViz(
const auto& local_surface_id =
without_gpu_->SubmitRootCompositorFrame(std::move(frame));

if (use_new_invalidate_heuristic_) {
auto root_surface_id =
viz::SurfaceId(without_gpu_->root_frame_sink_id(), local_surface_id);

auto commit_predicate = base::BindRepeating(
[](const viz::BeginFrameId& current_frame_id,
const viz::FrameSinkId& root_frame_sink_id,
const viz::FrameSinkId& child_frame_sink_id,
const viz::SurfaceId& surface_id,
const viz::BeginFrameId& frame_id) {
// Always commit frame from different begin frame sources, because we
// can't order with them.
if (frame_id.source_id != current_frame_id.source_id) {
// We always should have single source_id except for the manual
// acks.
DCHECK_EQ(frame_id.source_id, viz::BeginFrameArgs::kManualSourceId);
return true;
}

// Commit all frames that are older than current one.
if (frame_id.sequence_number < current_frame_id.sequence_number)
return true;

// All clients except root renderer and root surface are frame behind.
const bool is_frame_behind =
surface_id.frame_sink_id() != root_frame_sink_id &&
surface_id.frame_sink_id() != child_frame_sink_id;

// If this surface is not frame behind, commit it for current frame
// too.
if (!is_frame_behind &&
frame_id.sequence_number == current_frame_id.sequence_number)
return true;

return false;
},
child_frame->begin_frame_args.frame_id, root_surface_id.frame_sink_id(),
child_surface_id_.frame_sink_id());

GetFrameSinkManager()->surface_manager()->CommitFramesInRangeRecursively(
viz::SurfaceRange(root_surface_id), commit_predicate);
}

if (root_local_surface_id_ != local_surface_id) {
root_local_surface_id_ = local_surface_id;
display_->SetLocalSurfaceId(local_surface_id, device_scale_factor);
Expand All @@ -260,6 +306,8 @@ void HardwareRendererViz::OnViz::DrawAndSwapOnViz(
display_->Resize(viewport);
auto now = base::TimeTicks::Now();
display_->DrawAndSwap({now, now});

without_gpu_->SetContainedSurfaces(display_->GetContainedSurfaceIds());
}

void HardwareRendererViz::OnViz::PostDrawOnViz(
Expand Down

0 comments on commit 18f3406

Please sign in to comment.