Skip to content

Commit

Permalink
gpu: Fix reduced frame rate when animating multiple windows on Linux.
Browse files Browse the repository at this point in the history
Synchronizing with vsync has the advantage of not tearing but reduces
frame rate when multiple windows animate. All Chrome windows share one
thread (GPU main thread) for issuing swap buffers. Therefore, if one
window's swap buffers blocks the other windows' frame rates fall.

This CL ports the force swap interval zero for multiple window swaps
workaround used on Windows. This can cause tearing without a compositing
manager but seems like a reasonable tradeoff.

Also included are minor fixes to conform to the style guide.

R=kbr
BUG=528803

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Change-Id: If23b2fd7562376fa2fc02391fa2aa3a3c3644dd5
Reviewed-on: https://chromium-review.googlesource.com/479566
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#465889}
  • Loading branch information
sunnyps authored and Commit Bot committed Apr 20, 2017
1 parent 731836e commit 43bd95a
Show file tree
Hide file tree
Showing 17 changed files with 124 additions and 148 deletions.
4 changes: 2 additions & 2 deletions gpu/ipc/service/image_transport_surface_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ scoped_refptr<gl::GLSurface> ImageTransportSurface::CreateNativeSurface(
if (!initialize_success)
return scoped_refptr<gl::GLSurface>();

return scoped_refptr<gl::GLSurface>(
new PassThroughImageTransportSurface(delegate, surface.get()));
return scoped_refptr<gl::GLSurface>(new PassThroughImageTransportSurface(
delegate, surface.get(), kMultiWindowSwapIntervalDefault));
}

} // namespace gpu
12 changes: 9 additions & 3 deletions gpu/ipc/service/image_transport_surface_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "gpu/ipc/service/image_transport_surface.h"

#include "gpu/ipc/service/pass_through_image_transport_surface.h"
#include "ui/gl/gl_surface_glx.h"
#include "ui/gl/init/gl_factory.h"

namespace gpu {
Expand All @@ -16,15 +17,20 @@ scoped_refptr<gl::GLSurface> ImageTransportSurface::CreateNativeSurface(
gl::GLSurfaceFormat format) {
DCHECK_NE(surface_handle, kNullSurfaceHandle);
scoped_refptr<gl::GLSurface> surface;
MultiWindowSwapInterval multi_window_swap_interval =
kMultiWindowSwapIntervalDefault;
#if defined(USE_OZONE)
surface = gl::init::CreateSurfacelessViewGLSurface(surface_handle);
#endif
if (!surface)
if (!surface) {
surface = gl::init::CreateViewGLSurface(surface_handle);
if (gl::GetGLImplementation() == gl::kGLImplementationDesktopGL)
multi_window_swap_interval = kMultiWindowSwapIntervalForceZero;
}
if (!surface)
return surface;
return scoped_refptr<gl::GLSurface>(
new PassThroughImageTransportSurface(delegate, surface.get()));
return scoped_refptr<gl::GLSurface>(new PassThroughImageTransportSurface(
delegate, surface.get(), multi_window_swap_interval));
}

} // namespace gpu
3 changes: 2 additions & 1 deletion gpu/ipc/service/image_transport_surface_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@
if (!surface.get() || !surface->Initialize(format))
return surface;
return make_scoped_refptr<gl::GLSurface>(
new PassThroughImageTransportSurface(delegate, surface.get()));
new PassThroughImageTransportSurface(
delegate, surface.get(), kMultiWindowSwapIntervalDefault));
}
}

Expand Down
10 changes: 8 additions & 2 deletions gpu/ipc/service/image_transport_surface_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ scoped_refptr<gl::GLSurface> ImageTransportSurface::CreateNativeSurface(
DCHECK_NE(surface_handle, kNullSurfaceHandle);

scoped_refptr<gl::GLSurface> surface;
MultiWindowSwapInterval multi_window_swap_interval =
kMultiWindowSwapIntervalDefault;
if (gl::GetGLImplementation() == gl::kGLImplementationEGLGLES2) {
std::unique_ptr<gfx::VSyncProvider> vsync_provider;

Expand Down Expand Up @@ -72,6 +74,10 @@ scoped_refptr<gl::GLSurface> ImageTransportSurface::CreateNativeSurface(
} else {
surface = gl::init::CreateNativeViewGLSurfaceEGL(
surface_handle, std::move(vsync_provider));
// This is unnecessary with DirectComposition because that doesn't block
// swaps, but instead blocks the first draw into a surface during the next
// frame.
multi_window_swap_interval = kMultiWindowSwapIntervalForceZero;
if (!surface)
return nullptr;
}
Expand All @@ -81,8 +87,8 @@ scoped_refptr<gl::GLSurface> ImageTransportSurface::CreateNativeSurface(
return nullptr;
}

return scoped_refptr<gl::GLSurface>(
new PassThroughImageTransportSurface(delegate, surface.get()));
return scoped_refptr<gl::GLSurface>(new PassThroughImageTransportSurface(
delegate, surface.get(), multi_window_swap_interval));
}

} // namespace gpu
64 changes: 50 additions & 14 deletions gpu/ipc/service/pass_through_image_transport_surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,23 @@

namespace gpu {

namespace {
// Number of swap generations before vsync is reenabled after we've stopped
// doing multiple swaps per frame.
const int kMultiWindowSwapEnableVSyncDelay = 60;

int g_current_swap_generation_ = 0;
int g_num_swaps_in_current_swap_generation_ = 0;
int g_last_multi_window_swap_generation_ = 0;
} // anonymous namespace

PassThroughImageTransportSurface::PassThroughImageTransportSurface(
base::WeakPtr<ImageTransportSurfaceDelegate> delegate,
gl::GLSurface* surface)
gl::GLSurface* surface,
MultiWindowSwapInterval multi_window_swap_interval)
: GLSurfaceAdapter(surface),
delegate_(delegate),
did_set_swap_interval_(false),
multi_window_swap_interval_(multi_window_swap_interval),
weak_ptr_factory_(this) {}

bool PassThroughImageTransportSurface::Initialize(
Expand Down Expand Up @@ -110,18 +121,6 @@ void PassThroughImageTransportSurface::CommitOverlayPlanesAsync(
weak_ptr_factory_.GetWeakPtr(), base::Passed(&latency_info), callback));
}

bool PassThroughImageTransportSurface::OnMakeCurrent(gl::GLContext* context) {
if (!did_set_swap_interval_) {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableGpuVsync))
context->ForceSwapIntervalZero(true);
else
context->SetSwapInterval(1);
did_set_swap_interval_ = true;
}
return true;
}

PassThroughImageTransportSurface::~PassThroughImageTransportSurface() {
if (delegate_) {
delegate_->SetLatencyInfoCallback(
Expand All @@ -143,12 +142,49 @@ void PassThroughImageTransportSurface::SendVSyncUpdateIfAvailable() {
}
}

void PassThroughImageTransportSurface::UpdateSwapInterval() {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableGpuVsync)) {
gl::GLContext::GetCurrent()->ForceSwapIntervalZero(true);
return;
}

gl::GLContext::GetCurrent()->SetSwapInterval(1);

if (multi_window_swap_interval_ == kMultiWindowSwapIntervalForceZero) {
// This code is a simple way of enforcing that we only vsync if one surface
// is swapping per frame. This provides single window cases a stable refresh
// while allowing multi-window cases to not slow down due to multiple syncs
// on a single thread. A better way to fix this problem would be to have
// each surface present on its own thread.

if (g_current_swap_generation_ == swap_generation_) {
// No other surface has swapped since we swapped last time.
if (g_num_swaps_in_current_swap_generation_ > 1)
g_last_multi_window_swap_generation_ = g_current_swap_generation_;
g_num_swaps_in_current_swap_generation_ = 0;
g_current_swap_generation_++;
}

swap_generation_ = g_current_swap_generation_;
g_num_swaps_in_current_swap_generation_++;

bool should_override_vsync =
(g_num_swaps_in_current_swap_generation_ > 1) &&
(g_current_swap_generation_ - g_last_multi_window_swap_generation_ <
kMultiWindowSwapEnableVSyncDelay);
gl::GLContext::GetCurrent()->ForceSwapIntervalZero(should_override_vsync);
}
}

std::unique_ptr<std::vector<ui::LatencyInfo>>
PassThroughImageTransportSurface::StartSwapBuffers() {
// GetVsyncValues before SwapBuffers to work around Mali driver bug:
// crbug.com/223558.
SendVSyncUpdateIfAvailable();

UpdateSwapInterval();

base::TimeTicks swap_time = base::TimeTicks::Now();
for (auto& latency : latency_info_) {
latency.AddLatencyNumberWithTimestamp(
Expand Down
19 changes: 16 additions & 3 deletions gpu/ipc/service/pass_through_image_transport_surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,22 @@

namespace gpu {

enum MultiWindowSwapInterval {
// Use the default swap interval of 1 even if multiple windows are swapping.
// This can reduce frame rate if the swap buffers calls block.
kMultiWindowSwapIntervalDefault,
// Force swap interval to 0 when multiple windows are swapping.
kMultiWindowSwapIntervalForceZero
};

// An implementation of ImageTransportSurface that implements GLSurface through
// GLSurfaceAdapter, thereby forwarding GLSurface methods through to it.
class PassThroughImageTransportSurface : public gl::GLSurfaceAdapter {
public:
PassThroughImageTransportSurface(
base::WeakPtr<ImageTransportSurfaceDelegate> delegate,
gl::GLSurface* surface);
gl::GLSurface* surface,
MultiWindowSwapInterval multi_window_swap_interval);

// GLSurface implementation.
bool Initialize(gl::GLSurfaceFormat format) override;
Expand All @@ -43,7 +52,6 @@ class PassThroughImageTransportSurface : public gl::GLSurfaceAdapter {
gfx::SwapResult CommitOverlayPlanes() override;
void CommitOverlayPlanesAsync(
const SwapCompletionCallback& callback) override;
bool OnMakeCurrent(gl::GLContext* context) override;

private:
~PassThroughImageTransportSurface() override;
Expand All @@ -52,6 +60,8 @@ class PassThroughImageTransportSurface : public gl::GLSurfaceAdapter {
// the browser.
void SendVSyncUpdateIfAvailable();

void UpdateSwapInterval();

// Add |latency_info| to be reported and augumented with GPU latency
// components next time there is a GPU buffer swap.
void AddLatencyInfo(const std::vector<ui::LatencyInfo>& latency_info);
Expand All @@ -65,8 +75,11 @@ class PassThroughImageTransportSurface : public gl::GLSurfaceAdapter {
gfx::SwapResult result);

base::WeakPtr<ImageTransportSurfaceDelegate> delegate_;
bool did_set_swap_interval_;
std::vector<ui::LatencyInfo> latency_info_;
MultiWindowSwapInterval multi_window_swap_interval_ =
kMultiWindowSwapIntervalDefault;
int swap_generation_ = 0;

base::WeakPtrFactory<PassThroughImageTransportSurface> weak_ptr_factory_;

DISALLOW_COPY_AND_ASSIGN(PassThroughImageTransportSurface);
Expand Down
14 changes: 5 additions & 9 deletions ui/gl/gl_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,9 @@ void GLContext::ScopedReleaseCurrent::Cancel() {
canceled_ = true;
}

GLContext::GLContext(GLShareGroup* share_group)
: static_bindings_initialized_(false),
dynamic_bindings_initialized_(false),
share_group_(share_group),
current_virtual_context_(nullptr),
state_dirtied_externally_(false),
swap_interval_(1),
force_swap_interval_zero_(false) {
GLContext::GLContext(GLShareGroup* share_group) : share_group_(share_group) {
if (!share_group_.get())
share_group_ = new gl::GLShareGroup();

share_group_->AddContext(this);
}

Expand Down Expand Up @@ -223,11 +215,15 @@ void GLContext::SetGLStateRestorer(GLStateRestorer* state_restorer) {
}

void GLContext::SetSwapInterval(int interval) {
if (swap_interval_ == interval)
return;
swap_interval_ = interval;
OnSetSwapInterval(force_swap_interval_zero_ ? 0 : swap_interval_);
}

void GLContext::ForceSwapIntervalZero(bool force) {
if (force_swap_interval_zero_ == force)
return;
force_swap_interval_zero_ = force;
OnSetSwapInterval(force_swap_interval_zero_ ? 0 : swap_interval_);
}
Expand Down
13 changes: 7 additions & 6 deletions ui/gl/gl_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {

std::unique_ptr<GLVersionInfo> GenerateGLVersionInfo();

bool static_bindings_initialized_;
bool dynamic_bindings_initialized_;
bool static_bindings_initialized_ = false;
bool dynamic_bindings_initialized_ = false;
std::unique_ptr<DriverGL> driver_gl_;
std::unique_ptr<GLApi> gl_api_;
std::unique_ptr<TraceGLApi> trace_gl_api_;
Expand All @@ -209,13 +209,14 @@ class GL_EXPORT GLContext : public base::RefCounted<GLContext> {
RealGLApi* real_gl_api_ = nullptr;

scoped_refptr<GLShareGroup> share_group_;
GLContext* current_virtual_context_;
bool state_dirtied_externally_;
GLContext* current_virtual_context_ = nullptr;
bool state_dirtied_externally_ = false;
std::unique_ptr<GLStateRestorer> state_restorer_;
std::unique_ptr<GLVersionInfo> version_info_;

int swap_interval_;
bool force_swap_interval_zero_;
// Start with an invalid value so that the first SetSwapInterval isn't a nop.
int swap_interval_ = -1;
bool force_swap_interval_zero_ = false;

DISALLOW_COPY_AND_ASSIGN(GLContext);
};
Expand Down
9 changes: 1 addition & 8 deletions ui/gl/gl_context_egl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,7 @@ GLContextEGL::GLContextEGL(GLShareGroup* share_group)
context_(nullptr),
display_(nullptr),
config_(nullptr),
unbind_fbo_on_makecurrent_(false),
swap_interval_(1) {
}
unbind_fbo_on_makecurrent_(false) {}

bool GLContextEGL::Initialize(GLSurface* compatible_surface,
const GLContextAttribs& attribs) {
Expand Down Expand Up @@ -211,8 +209,6 @@ bool GLContextEGL::MakeCurrent(GLSurface* surface) {
return false;
}

surface->OnSetSwapInterval(swap_interval_);

release_current.Cancel();
return true;
}
Expand Down Expand Up @@ -271,9 +267,6 @@ void GLContextEGL::OnSetSwapInterval(int interval) {
if (!eglSwapInterval(display_, interval)) {
LOG(ERROR) << "eglSwapInterval failed with error "
<< GetLastEGLErrorString();
} else {
swap_interval_ = interval;
GLSurface::GetCurrent()->OnSetSwapInterval(interval);
}
}

Expand Down
1 change: 0 additions & 1 deletion ui/gl/gl_context_egl.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ class GL_EXPORT GLContextEGL : public GLContextReal {
EGLDisplay display_;
EGLConfig config_;
bool unbind_fbo_on_makecurrent_;
int swap_interval_;

DISALLOW_COPY_AND_ASSIGN(GLContextEGL);
};
Expand Down
19 changes: 6 additions & 13 deletions ui/gl/gl_context_glx.cc
Original file line number Diff line number Diff line change
Expand Up @@ -294,20 +294,13 @@ void* GLContextGLX::GetHandle() {

void GLContextGLX::OnSetSwapInterval(int interval) {
DCHECK(IsCurrent(nullptr));
if (HasExtension("GLX_EXT_swap_control") &&
g_driver_glx.fn.glXSwapIntervalEXTFn) {
glXSwapIntervalEXT(
display_,
glXGetCurrentDrawable(),
interval);
} else if (HasExtension("GLX_MESA_swap_control") &&
g_driver_glx.fn.glXSwapIntervalMESAFn) {
if (GLSurfaceGLX::IsEXTSwapControlSupported()) {
glXSwapIntervalEXT(display_, glXGetCurrentDrawable(), interval);
} else if (GLSurfaceGLX::IsMESASwapControlSupported()) {
glXSwapIntervalMESA(interval);
} else {
if(interval == 0)
LOG(WARNING) <<
"Could not disable vsync: driver does not "
"support GLX_EXT_swap_control";
} else if (interval == 0) {
LOG(WARNING)
<< "Could not disable vsync: driver does not support swap control";
}
}

Expand Down
7 changes: 0 additions & 7 deletions ui/gl/gl_surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,6 @@ bool GLSurface::ExtensionsContain(const char* c_extensions, const char* name) {
return extensions.find(delimited_name) != std::string::npos;
}

void GLSurface::OnSetSwapInterval(int interval) {
}

GLSurfaceAdapter::GLSurfaceAdapter(GLSurface* surface) : surface_(surface) {}

bool GLSurfaceAdapter::Initialize(GLSurfaceFormat format) {
Expand Down Expand Up @@ -392,10 +389,6 @@ gfx::Vector2d GLSurfaceAdapter::GetDrawOffset() const {
return surface_->GetDrawOffset();
}

void GLSurfaceAdapter::OnSetSwapInterval(int interval) {
surface_->OnSetSwapInterval(interval);
}

GLSurfaceAdapter::~GLSurfaceAdapter() {}

scoped_refptr<GLSurface> InitializeGLSurfaceWithFormat(
Expand Down
Loading

0 comments on commit 43bd95a

Please sign in to comment.