Commit bd7d5c4d authored by enne's avatar enne Committed by Commit bot

Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes vsync parameters differently than other platforms.  Other
platforms go directly through BrowserCompositorOutputSurface, which then
routes it to the vsync manager (without begin frame scheduling) or just
to the root BeginFrameSource for that BrowserCompositorOutputsurface
(with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info
from the DisplayLink, which forwards it to the BrowserCompositorMac.
This would then forward that to the ui::Compositor's vsync manager which
would then be used to tick the browser and then indirectly would forward
to the begin frame source for the BrowserCompositorOutputSurface to tick
the display.

--enable-begin-frame-scheduling turns off the vsync manager, which
previously left the browser and then display compositors to tick at
default intervals and timebases on Mac, leading to power regressions.
OOPS.

This patch adds an additional route where the BrowserCompositorMac
forwards vsync info to the BrowserCompositorOutputSurface's
BeginFrameSource via ui::Compositor's ui::ContextFactory (aka
GpuProcessTransportFactory).  As the BrowserCompositorOutputSurface's
begin frame source is the source of all frames when using
--enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the
vsync info and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when
using --enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead
of from 2.0W -> 2.3W.

R=ccameron@chromium.org

Review-Url: https://codereview.chromium.org/2170053002
Cr-Commit-Position: refs/heads/master@{#407632}
parent 349039d0
...@@ -45,6 +45,9 @@ class StubContextFactory : public ui::ContextFactory { ...@@ -45,6 +45,9 @@ class StubContextFactory : public ui::ContextFactory {
const gfx::ColorSpace& color_space) override; const gfx::ColorSpace& color_space) override;
void SetAuthoritativeVSyncInterval(ui::Compositor* compositor, void SetAuthoritativeVSyncInterval(ui::Compositor* compositor,
base::TimeDelta interval) override {} base::TimeDelta interval) override {}
void SetDisplayVSyncParameters(ui::Compositor* compositor,
base::TimeTicks timebase,
base::TimeDelta interval) override {}
void SetOutputIsSecure(ui::Compositor* compositor, bool secure) override {} void SetOutputIsSecure(ui::Compositor* compositor, bool secure) override {}
void AddObserver(ui::ContextFactoryObserver* observer) override {} void AddObserver(ui::ContextFactoryObserver* observer) override {}
void RemoveObserver(ui::ContextFactoryObserver* observer) override {} void RemoveObserver(ui::ContextFactoryObserver* observer) override {}
......
...@@ -730,6 +730,19 @@ void GpuProcessTransportFactory::SetAuthoritativeVSyncInterval( ...@@ -730,6 +730,19 @@ void GpuProcessTransportFactory::SetAuthoritativeVSyncInterval(
data->begin_frame_source->SetAuthoritativeVSyncInterval(interval); data->begin_frame_source->SetAuthoritativeVSyncInterval(interval);
} }
void GpuProcessTransportFactory::SetDisplayVSyncParameters(
ui::Compositor* compositor,
base::TimeTicks timebase,
base::TimeDelta interval) {
PerCompositorDataMap::iterator it = per_compositor_data_.find(compositor);
if (it == per_compositor_data_.end())
return;
PerCompositorData* data = it->second;
DCHECK(data);
if (data->begin_frame_source)
data->begin_frame_source->OnUpdateVSyncParameters(timebase, interval);
}
void GpuProcessTransportFactory::SetOutputIsSecure(ui::Compositor* compositor, void GpuProcessTransportFactory::SetOutputIsSecure(ui::Compositor* compositor,
bool secure) { bool secure) {
PerCompositorDataMap::iterator it = per_compositor_data_.find(compositor); PerCompositorDataMap::iterator it = per_compositor_data_.find(compositor);
......
...@@ -68,6 +68,9 @@ class GpuProcessTransportFactory ...@@ -68,6 +68,9 @@ class GpuProcessTransportFactory
const gfx::ColorSpace& color_space) override; const gfx::ColorSpace& color_space) override;
void SetAuthoritativeVSyncInterval(ui::Compositor* compositor, void SetAuthoritativeVSyncInterval(ui::Compositor* compositor,
base::TimeDelta interval) override; base::TimeDelta interval) override;
void SetDisplayVSyncParameters(ui::Compositor* compositor,
base::TimeTicks timebase,
base::TimeDelta interval) override;
void SetOutputIsSecure(ui::Compositor* compositor, bool secure) override; void SetOutputIsSecure(ui::Compositor* compositor, bool secure) override;
void AddObserver(ui::ContextFactoryObserver* observer) override; void AddObserver(ui::ContextFactoryObserver* observer) override;
void RemoveObserver(ui::ContextFactoryObserver* observer) override; void RemoveObserver(ui::ContextFactoryObserver* observer) override;
......
...@@ -303,6 +303,10 @@ void BrowserCompositorMac::UpdateVSyncParameters( ...@@ -303,6 +303,10 @@ void BrowserCompositorMac::UpdateVSyncParameters(
recyclable_compositor_->compositor() recyclable_compositor_->compositor()
->vsync_manager() ->vsync_manager()
->UpdateVSyncParameters(timebase, interval); ->UpdateVSyncParameters(timebase, interval);
recyclable_compositor_->compositor()
->context_factory()
->SetDisplayVSyncParameters(recyclable_compositor_->compositor(),
timebase, interval);
} }
} }
......
...@@ -144,6 +144,11 @@ class COMPOSITOR_EXPORT ContextFactory { ...@@ -144,6 +144,11 @@ class COMPOSITOR_EXPORT ContextFactory {
virtual void SetAuthoritativeVSyncInterval(ui::Compositor* compositor, virtual void SetAuthoritativeVSyncInterval(ui::Compositor* compositor,
base::TimeDelta interval) = 0; base::TimeDelta interval) = 0;
// Mac path for transporting vsync parameters to the display. Other platforms
// update it via the BrowserCompositorOutputSurface directly.
virtual void SetDisplayVSyncParameters(ui::Compositor* compositor,
base::TimeTicks timebase,
base::TimeDelta interval) = 0;
virtual void SetOutputIsSecure(Compositor* compositor, bool secure) = 0; virtual void SetOutputIsSecure(Compositor* compositor, bool secure) = 0;
......
...@@ -68,6 +68,9 @@ class InProcessContextFactory : public ContextFactory { ...@@ -68,6 +68,9 @@ class InProcessContextFactory : public ContextFactory {
const gfx::ColorSpace& color_space) override {} const gfx::ColorSpace& color_space) override {}
void SetAuthoritativeVSyncInterval(ui::Compositor* compositor, void SetAuthoritativeVSyncInterval(ui::Compositor* compositor,
base::TimeDelta interval) override {} base::TimeDelta interval) override {}
void SetDisplayVSyncParameters(ui::Compositor* compositor,
base::TimeTicks timebase,
base::TimeDelta interval) override {}
void SetOutputIsSecure(ui::Compositor* compositor, bool secure) override {} void SetOutputIsSecure(ui::Compositor* compositor, bool secure) override {}
void AddObserver(ContextFactoryObserver* observer) override; void AddObserver(ContextFactoryObserver* observer) override;
void RemoveObserver(ContextFactoryObserver* observer) override; void RemoveObserver(ContextFactoryObserver* observer) override;
......
...@@ -55,6 +55,9 @@ class VIEWS_MUS_EXPORT SurfaceContextFactory : public ui::ContextFactory { ...@@ -55,6 +55,9 @@ class VIEWS_MUS_EXPORT SurfaceContextFactory : public ui::ContextFactory {
const gfx::ColorSpace& color_space) override {} const gfx::ColorSpace& color_space) override {}
void SetAuthoritativeVSyncInterval(ui::Compositor* compositor, void SetAuthoritativeVSyncInterval(ui::Compositor* compositor,
base::TimeDelta interval) override {} base::TimeDelta interval) override {}
void SetDisplayVSyncParameters(ui::Compositor* compositor,
base::TimeTicks timebase,
base::TimeDelta interval) override {}
void SetOutputIsSecure(ui::Compositor* compositor, bool secure) override {} void SetOutputIsSecure(ui::Compositor* compositor, bool secure) override {}
void AddObserver(ui::ContextFactoryObserver* observer) override {} void AddObserver(ui::ContextFactoryObserver* observer) override {}
void RemoveObserver(ui::ContextFactoryObserver* observer) override {} void RemoveObserver(ui::ContextFactoryObserver* observer) override {}
......
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