New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 614147 link

Starred by 3 users

Issue metadata

Status: WontFix
Owner:
Closed: Nov 16
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug

Blocking:
issue 614154



Sign in to add a comment

Implement frame statistics for direct composition

Project Member Reported by stanisc@chromium.org, May 23 2016

Issue description

Frame statistics is needed to allow us to accurately implement smoothness benchmark on Windows.

See the following TODO in ImageTransportSurface::CreateNativeSurface:

    // TODO(jbauman): Get frame statistics from DirectComposition
    std::unique_ptr<gfx::VSyncProvider> vsync_provider(
        new gfx::VSyncProviderWin(surface_handle));

Here are some extra details from jbauman@:

Chromium supports using EGL_CHROMIUM_sync_control for getting the vsync times from the EGL driver. That EGL extension is undocumented, but it's pretty similar to GLX_OML_sync_control.

That extension supports getting the UST, graphics MSC, and SBC, which are pretty similar to what we get from GetFrameStatistics. The UST is the time of the last vsync, the MSC is the number of screen refreshes since some arbitrary point, and the SBC is the number of swaps from the current surface that have finished executing on the GPU.

Ideally we could reuse that extension on windows, but I'm not certain it would work for our purposes. We'll have to see if we can convert the input from GetFrameStatistics into those numbers, and if those numbers would actually be useful for measuring the latency.
 
Blocking: 614154
Owner: stanisc@chromium.org
Status: Assigned (was: Untriaged)
I am assigning this to myself for now. I'll see if I can make progress with it.
See https://bugs.chromium.org/p/angleproject/issues/detail?id=1402 for the ANGLE change that implements EGL_CHROMIUM_sync_control.
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b5df120a181df5743a1ebd5f7cc6537a00097ee3

commit b5df120a181df5743a1ebd5f7cc6537a00097ee3
Author: stanisc <stanisc@chromium.org>
Date: Wed Oct 12 21:35:40 2016

Route DXGI Frame Statistics to VSyncProvider

This leverages new ANGLE implementation of
eglGetSyncValuesCHROMIUM and replaces the current DWM
specific version of VSyncProvider with the default version
which is now mostly shared with Linux implementation.
This doesn't add any new functionality but makes a step
in the right direction with the goal of passing actual
frame statistics back to the browser.

The next step will be to use the VSync system time
returned to VSyncProvider by eglGetSyncValuesCHROMIUM
and expose it as frame time to be used by smoothness
benchmarks.
BUG= 614147 
CQ_INCLUDE_TRYBOTS=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

Review-Url: https://codereview.chromium.org/2411803003
Cr-Commit-Position: refs/heads/master@{#424868}

[modify] https://crrev.com/b5df120a181df5743a1ebd5f7cc6537a00097ee3/gpu/ipc/service/image_transport_surface_win.cc
[modify] https://crrev.com/b5df120a181df5743a1ebd5f7cc6537a00097ee3/ui/gl/gl_surface_egl.cc
[modify] https://crrev.com/b5df120a181df5743a1ebd5f7cc6537a00097ee3/ui/gl/gl_surface_egl.h
[modify] https://crrev.com/b5df120a181df5743a1ebd5f7cc6537a00097ee3/ui/gl/sync_control_vsync_provider.cc
[modify] https://crrev.com/b5df120a181df5743a1ebd5f7cc6537a00097ee3/ui/gl/sync_control_vsync_provider.h

Cc: brucedaw...@chromium.org
Here is my thoughts about finishing this work. jbauman@, feel free to chime in.

1) Partially or fully revert the change above in comment #5. That change was intended to leverage eglGetSyncValuesCHROMIUM as a source for VSyncProvider to match Linux implementation. That didn't work as expected because EGLVSyncProvider derives VSync interval from consecutive VSync timestamps - I think that resulted in delay based VSync time oscillation on the compositor side. Since there is a plan to use true D3D VSync there is no need anymore to use eglGetSyncValuesCHROMIUM to get VSync parameters on Windows.

2) Make sure that on Windows there is no dependency of VSync provider on presence of eglGetSyncValuesCHROMIUM. I think the code that was causing issues with not sticking to VSyncProviderWin when I tried to block usage of ANGLE eglGetSyncValuesCHROMIUM is the following one:

  if (sync_provider)
    vsync_provider_ = std::move(sync_provider);
  else if (g_egl_sync_control_supported)
    vsync_provider_.reset(new EGLSyncControlVSyncProvider(surface_));

Normally on Windows the first branch runs on initialization which assigns VSyncProviderWin.
None of the branches run on re-init keeping the previously assigned instance of VSyncProviderWin.
However with ANGLE eglGetSyncValuesCHROMIUM the second branch would run and override the provider on actions like maximizing the window.

3) Reintroduce eglGetSyncValuesCHROMIUM in ANGLE. As explained above it wouldn't be used by VSync provider anymore so it should have no impact on existing smoothness benchmarks.

4) Add some windows specific code that calls eglGetSyncValuesCHROMIUM once per frame and traces last frame swap number and its time. We could use these trace entries for deriving accurate smoothness benchmarking. 

5) We could also use frame statistics to calculate VSync latency - by comparing actual D3D frame times with begin frame times on compositor side.
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/858b96d1f311506dd39d5a881cfb9fe73cf85b77

commit 858b96d1f311506dd39d5a881cfb9fe73cf85b77
Author: stanisc <stanisc@chromium.org>
Date: Thu Feb 16 22:27:32 2017

Revert of "Route DXGI Frame Statistics to VSyncProvider"

This reverts the following commits related to this change.
http://crrev.com/b5df120a181df5743a1ebd5f7cc6537a00097ee3
http://crrev.com/bc7ba6d60345cbd6b1c55945238e474b0164bee8
http://crrev.com/d5c055882dc5bf06a8930c6e7d6f52dee3489560
http://crrev.com/dd1d76988f2d7ae8f76a610bcbb03243e244436c

This was an attempt to share eglGetSyncValuesCHROMIUM
based VSyncProvider between Linux and Windows.
The feature wasn't working anyway because eglGetSyncValuesCHROMIUM
had to be reverted on ANGLE side due to video playback
smoothness regression that it caused.

That approach is now irrelevant because we are going
to provide a mechanism for delivering a true GPU VSync
signal.

BUG= 614147 
CQ_INCLUDE_TRYBOTS=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
NOTRY=true

Review-Url: https://codereview.chromium.org/2696203004
Cr-Commit-Position: refs/heads/master@{#451120}

[modify] https://crrev.com/858b96d1f311506dd39d5a881cfb9fe73cf85b77/gpu/ipc/service/image_transport_surface_win.cc
[modify] https://crrev.com/858b96d1f311506dd39d5a881cfb9fe73cf85b77/ui/gl/gl_surface_egl.cc
[modify] https://crrev.com/858b96d1f311506dd39d5a881cfb9fe73cf85b77/ui/gl/gl_surface_egl.h
[modify] https://crrev.com/858b96d1f311506dd39d5a881cfb9fe73cf85b77/ui/gl/sync_control_vsync_provider.cc
[modify] https://crrev.com/858b96d1f311506dd39d5a881cfb9fe73cf85b77/ui/gl/sync_control_vsync_provider.h

Project Member

Comment 8 by bugdroid1@chromium.org, Mar 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b27dad7e468ddaaf74d9b483bf14015c9f848e3

commit 1b27dad7e468ddaaf74d9b483bf14015c9f848e3
Author: stanisc <stanisc@chromium.org>
Date: Thu Mar 23 03:26:27 2017

More reliable check for whether EGLSyncControlVSyncProvider should be instantiated.

This change improves a check in NativeViewGLSurfaceEGL::Initialize
for whether it should reset vsync provider. This is done in
preparation for re-introducing ANGLE eglGetSyncValuesCHROMIUM
extension.

The current logic overrides sync provider with a new instance of
EGLSyncControlVSyncProvider when the extension is present and we
want to avoid that since EGLSyncControlVSyncProvider isn't
implemented on Windows.

There is no plan to switch to EGLSyncControlVSyncProvider on Windows.
eglGetSyncValuesCHROMIUM will be used to provide accurate frame
statistics data for instrumentation and benchmarking only.

BUG= 614147 

Review-Url: https://codereview.chromium.org/2767293002
Cr-Commit-Position: refs/heads/master@{#458996}

[modify] https://crrev.com/1b27dad7e468ddaaf74d9b483bf14015c9f848e3/ui/gl/gl_surface_egl.cc
[modify] https://crrev.com/1b27dad7e468ddaaf74d9b483bf14015c9f848e3/ui/gl/sync_control_vsync_provider.h

Cc: chadversary@chromium.org
FYI to the ANGLE devs here. Mesa has a native implementation of EGL_CHROMIUM_sync_control. Maybe ANGLE could use it when running on Linux. (I don't know ANGLE well enough, though, to know if that suggestion makes any sense).
Cc: -stanisc@chromium.org -jbau...@chromium.org
Owner: ----
Status: Untriaged (was: Assigned)
I won't be able to work on this.
Status: Available (was: Untriaged)
Project Member

Comment 12 by sheriffbot@chromium.org, Nov 5

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Cc: sadrul@chromium.org sunn...@chromium.org
Owner: zmo@chromium.org
Status: Assigned (was: Untriaged)
zmo: Could you please check if this issue is still relevant?
Labels: -Pri-2 Pri-3
Owner: dalecur...@chromium.org
Dale, how is the quality of the current video smoothness benchmark? If you feel the current one is good, please close this bug as WontFix. Otherwise, we can discuss about moving forward based on ANGLE's EGL_CHROMIUM_sync_control implementation.
It's quite good but local only. Something in UMA would be cool, but hard to debug I think :/
I prototyped this a bit and filed issue 892874 using frame statistics for presentation feedback on Windows
Should we close this one as WontFix then?
Status: WontFix (was: Assigned)

Sign in to add a comment