Disable Vsync when Aero Glass is enabled.

Disable Vsync when Aero Glass is enabled.

The vsync is unnecessary for ratelimiting or tear prevention, and can cause issues when multiple windows are displayed.

BUG=313104
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=236150

[Ken Russell (switch to Gerrit), 2013-11-19 18:47:06]

LGTM

+brianderson in case there is any unexpected interaction with the compositor's
scheduling.

[brianderson, 2013-11-19 19:10:35]

I have some reservations about this patch.

What issues are there when multiple windows are present?
How does this help performance?
How does this behave if we start out with Aero enabled, but the user disables
Aero without re-launching Chrome? Do we tear?

Although the compositor rate-limits itself, I think we still run the risk of
swapping two frames within a single vsync period, which will cause us to skip
displaying a frame. Although this is good for latency, it is bad for
correctness. A better policy would be to skip a frame at the beginning of the
pipeline (i.e. a commit) rather than at the end of the pipeline so that 1) we
don't waste work and 1) every commit gets displayed.

However, if a different fix is going to take too long and this patch is needed
for WinAura, it may be a good temporary solution.

[jbauman, 2013-11-19 22:27:10]

On 2013/11/19 19:10:35, Brian Anderson wrote:
> I have some reservations about this patch.
> 
> What issues are there when multiple windows are present?

With two windows you'll get 30fps max, with 3 windows 20 fps max, etc. This
might seem rare, but it's common when resizing a window or loading a page, due
to the status bubble being its own top-level window.

> How does this help performance?

The multiple window case, and also canvases can start work on the next frame
instead of being blocked waiting for vsync, which may help in some cases. I'm
not sure about this, which is why I'm interested in trying it on the bots at
least.

> How does this behave if we start out with Aero enabled, but the user disables
> Aero without re-launching Chrome? Do we tear?

Yeah, it'll tear in that case. However that's pretty rare (particularly on
Windows 8, where that's not supported).
> 
> Although the compositor rate-limits itself, I think we still run the risk of
> swapping two frames within a single vsync period, which will cause us to skip
> displaying a frame. Although this is good for latency, it is bad for
> correctness. A better policy would be to skip a frame at the beginning of the
> pipeline (i.e. a commit) rather than at the end of the pipeline so that 1) we
> don't waste work and 1) every commit gets displayed.

Yeah, that's definitely a risk, but I can't see a way to avoid that but improve
the multi-window case.
> 
> However, if a different fix is going to take too long and this patch is needed
> for WinAura, it may be a good temporary solution.

[brianderson, 2013-11-19 22:49:53]

I see. Is the problem that we have a single gpu thread issuing the swaps for all
the windows and a single blocking swap slows everyone else down?

lgtm as a short-term solution, but I think we need to come up with a better
answer eventually.

[jbauman, 2013-11-19 22:54:56]

On 2013/11/19 22:49:53, Brian Anderson wrote:
> I see. Is the problem that we have a single gpu thread issuing the swaps for
all
> the windows and a single blocking swap slows everyone else down?

Right. With the image transport surface this was working (because it presented
from 4 different threads), but that caused its own problems.
> 
> lgtm as a short-term solution, but I think we need to come up with a better
> answer eventually.

[commit-bot: I haz the power, 2013-11-19 23:48:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbauman@chromium.org/75723004/1

[commit-bot: I haz the power, 2013-11-20 04:38:43]

Change committed as 236150