Commit 0cd016da authored by Bo Liu's avatar Bo Liu Committed by Commit Bot

aw: Remove synchronous draw to ensure GL binding init

Previously, BrowserViewRenderer made sure to use synchronous draws until
it receives a non-empty frame from the compositor, before it starts
using async draws. This is to ensure that GL bindings are initialized by
the compositor before calling into draw functor.

GL init has been made thread safe now. See gpu::InitializeGLThreadSafe.

However, we may have accidentally come to depend on sending a
synchronous IPC on init before start using async IPCs (which may have
more places to be dropped randomly than sync IPCs). So out of an
abudance of caution, change the behavior to send a single synchronous
IPC (without waiting for non-empty frame).

Note this does have the side effect that we may send empty or null
frames to render thread, so draw functor may be called before receiving
first non-null/non-empty CompositorFrame.

One step closer to getting rid of synchronous hardware draw altogether.

Change-Id: I30167e74df7bb0f2a880401f0545148e160fe001
Reviewed-on: https://chromium-review.googlesource.com/941544Reviewed-by: default avatarTobias Sargeant <tobiasjs@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541507}
parent 1ab62f15
...@@ -110,8 +110,7 @@ BrowserViewRenderer::BrowserViewRenderer( ...@@ -110,8 +110,7 @@ BrowserViewRenderer::BrowserViewRenderer(
max_page_scale_factor_(0.f), max_page_scale_factor_(0.f),
on_new_picture_enable_(false), on_new_picture_enable_(false),
clear_view_(false), clear_view_(false),
offscreen_pre_raster_(false), offscreen_pre_raster_(false) {}
allow_async_draw_(false) {}
BrowserViewRenderer::~BrowserViewRenderer() { BrowserViewRenderer::~BrowserViewRenderer() {
DCHECK(compositor_map_.empty()); DCHECK(compositor_map_.empty());
...@@ -237,13 +236,13 @@ bool BrowserViewRenderer::OnDrawHardware() { ...@@ -237,13 +236,13 @@ bool BrowserViewRenderer::OnDrawHardware() {
scoped_refptr<content::SynchronousCompositor::FrameFuture> future; // Async. scoped_refptr<content::SynchronousCompositor::FrameFuture> future; // Async.
content::SynchronousCompositor::Frame frame; // Sync. content::SynchronousCompositor::Frame frame; // Sync.
bool async = !sync_on_draw_hardware_ && allow_async_draw_; if (sync_on_draw_hardware_) {
if (async) { // TODO(boliu): Remove the synchronous call path here.
future = compositor_->DemandDrawHwAsync(
size_, viewport_rect_for_tile_priority, transform_for_tile_priority);
} else {
frame = compositor_->DemandDrawHw(size_, viewport_rect_for_tile_priority, frame = compositor_->DemandDrawHw(size_, viewport_rect_for_tile_priority,
transform_for_tile_priority); transform_for_tile_priority);
} else {
future = compositor_->DemandDrawHwAsync(
size_, viewport_rect_for_tile_priority, transform_for_tile_priority);
} }
if (!frame.frame && !future) { if (!frame.frame && !future) {
...@@ -252,7 +251,6 @@ bool BrowserViewRenderer::OnDrawHardware() { ...@@ -252,7 +251,6 @@ bool BrowserViewRenderer::OnDrawHardware() {
return current_compositor_frame_consumer_->HasFrameOnUI(); return current_compositor_frame_consumer_->HasFrameOnUI();
} }
allow_async_draw_ = true;
std::unique_ptr<ChildFrame> child_frame = std::make_unique<ChildFrame>( std::unique_ptr<ChildFrame> child_frame = std::make_unique<ChildFrame>(
std::move(future), frame.layer_tree_frame_sink_id, std::move(frame.frame), std::move(future), frame.layer_tree_frame_sink_id, std::move(frame.frame),
compositor_id_, viewport_rect_for_tile_priority.IsEmpty(), compositor_id_, viewport_rect_for_tile_priority.IsEmpty(),
......
...@@ -211,14 +211,6 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient, ...@@ -211,14 +211,6 @@ class BrowserViewRenderer : public content::SynchronousCompositorClient,
bool offscreen_pre_raster_; bool offscreen_pre_raster_;
// Must do a synchronous draw first to ensure GL bindings are initialized.
// TODO(boliu): Wait on render thread and remove this. When the
// first synchronous draw requirement is removed,
// RenderThreadManager::DeleteHardwareRendererOnUI will need to
// change, because it will no longer be true that having received a
// frame means that GL bindings have been initialized.
bool allow_async_draw_;
gfx::Vector2d last_on_draw_scroll_offset_; gfx::Vector2d last_on_draw_scroll_offset_;
gfx::Rect last_on_draw_global_visible_rect_; gfx::Rect last_on_draw_global_visible_rect_;
......
...@@ -297,6 +297,9 @@ void RenderThreadManager::DrawGL(AwDrawGLInfo* draw_info) { ...@@ -297,6 +297,9 @@ void RenderThreadManager::DrawGL(AwDrawGLInfo* draw_info) {
return; return;
} }
// Force GL binding init if it's not yet initialized.
DeferredGpuCommandService::GetInstance();
// kModeProcessNoContext should never happen because we tear down hardware // kModeProcessNoContext should never happen because we tear down hardware
// in onTrimMemory. However that guarantee is maintained outside of chromium // in onTrimMemory. However that guarantee is maintained outside of chromium
// code. Not notifying shared state in kModeProcessNoContext can lead to // code. Not notifying shared state in kModeProcessNoContext can lead to
...@@ -359,8 +362,6 @@ void RenderThreadManager::DeleteHardwareRendererOnUI() { ...@@ -359,8 +362,6 @@ void RenderThreadManager::DeleteHardwareRendererOnUI() {
// If the WebView gets onTrimMemory >= MODERATE twice in a row, the 2nd // If the WebView gets onTrimMemory >= MODERATE twice in a row, the 2nd
// onTrimMemory will result in an unnecessary Render Thread InvokeGL call. // onTrimMemory will result in an unnecessary Render Thread InvokeGL call.
// TODO(boliu): removing the requirement that the first frame be
// synchronous will require changing the following test.
if (has_received_frame_) { if (has_received_frame_) {
// Receiving at least one frame is a precondition for // Receiving at least one frame is a precondition for
// initialization (such as looing up GL bindings and constructing // initialization (such as looing up GL bindings and constructing
......
...@@ -291,7 +291,8 @@ SynchronousCompositorHost::DemandDrawHwAsync( ...@@ -291,7 +291,8 @@ SynchronousCompositorHost::DemandDrawHwAsync(
const gfx::Rect& viewport_rect_for_tile_priority, const gfx::Rect& viewport_rect_for_tile_priority,
const gfx::Transform& transform_for_tile_priority) { const gfx::Transform& transform_for_tile_priority) {
scoped_refptr<FrameFuture> frame_future = new FrameFuture(); scoped_refptr<FrameFuture> frame_future = new FrameFuture();
if (compute_scroll_needs_synchronous_draw_) { if (compute_scroll_needs_synchronous_draw_ || !allow_async_draw_) {
allow_async_draw_ = allow_async_draw_ || IsReadyForSynchronousCall();
compute_scroll_needs_synchronous_draw_ = false; compute_scroll_needs_synchronous_draw_ = false;
auto frame_ptr = std::make_unique<Frame>(); auto frame_ptr = std::make_unique<Frame>();
*frame_ptr = DemandDrawHw(viewport_size, viewport_rect_for_tile_priority, *frame_ptr = DemandDrawHw(viewport_size, viewport_rect_for_tile_priority,
......
...@@ -123,6 +123,13 @@ class SynchronousCompositorHost : public SynchronousCompositor, ...@@ -123,6 +123,13 @@ class SynchronousCompositorHost : public SynchronousCompositor,
size_t bytes_limit_; size_t bytes_limit_;
std::unique_ptr<SharedMemoryWithSize> software_draw_shm_; std::unique_ptr<SharedMemoryWithSize> software_draw_shm_;
// Make sure to send a synchronous IPC that succeeds first before sending
// asynchronous ones. This shouldn't be needed. However we may have come
// to rely on sending a synchronous message first on initialization. So
// with an abundance of caution, keep that behavior until we are sure this
// isn't required.
bool allow_async_draw_ = false;
// Indicates the next draw needs to be synchronous // Indicates the next draw needs to be synchronous
bool compute_scroll_needs_synchronous_draw_ = false; bool compute_scroll_needs_synchronous_draw_ = false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment