New issue
Advanced search Search tips

Issue 725684 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Proj-XR
Proj-XR-VR

Blocking:
issue 723962



Sign in to add a comment

Use high-priority GL context for VrShell / WebVR composition

Project Member Reported by klausw@chromium.org, May 23 2017

Issue description

VrShell's VR composition should happen as fast as possible to avoid getting backed up behind potentially slow operations such as 2D content composition or user GL code for WebVR.

Delays in VrShellGl's operations can cause inconsistent timing if the thread gets blocked and is unable to handle other events such as sending vsync to WebVR.
 

Comment 1 by klausw@chromium.org, May 23 2017

Status: Started (was: Untriaged)

Comment 2 by klausw@chromium.org, May 23 2017

Blocking: 723962
Project Member

Comment 3 by bugdroid1@chromium.org, May 24 2017

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

commit 9f8c6768f7dff7a712449cc3b526ece8d3ad43a9
Author: klausw <klausw@chromium.org>
Date: Wed May 24 00:32:24 2017

Implement EGL context priority if supported, use in VrShell

On devices where the EGL context priority extension is supported,
make this available when creating GL contexts, and use it for VrShell's
VR compositing. The extension is documented here:

https://www.khronos.org/registry/EGL/extensions/IMG/EGL_IMG_context_priority.txt

If the extension is not supported, ignore the requested priority.

As a special case, assume the extension is present if other
VR-specific extensions are available. It was erroneously not reported
as present on Daydream devices even though it was implemented, see
https://github.com/googlevr/gvr-android-sdk/issues/330 .

(This functionality was previously implemented in http://crrev.com/2586803003
but was reverted due to synchronization issues with non-virtualized contexts.
In this new version, only VrShell's private native rendering context uses this
option, so WebGL can continue using virtualized contexts as before.)

BUG= 725684 
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

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

[modify] https://crrev.com/9f8c6768f7dff7a712449cc3b526ece8d3ad43a9/chrome/browser/android/vr_shell/vr_shell_gl.cc
[modify] https://crrev.com/9f8c6768f7dff7a712449cc3b526ece8d3ad43a9/ui/gl/gl_context.h
[modify] https://crrev.com/9f8c6768f7dff7a712449cc3b526ece8d3ad43a9/ui/gl/gl_context_egl.cc
[modify] https://crrev.com/9f8c6768f7dff7a712449cc3b526ece8d3ad43a9/ui/gl/gl_surface_egl.cc
[modify] https://crrev.com/9f8c6768f7dff7a712449cc3b526ece8d3ad43a9/ui/gl/gl_surface_egl.h

Comment 4 by klausw@chromium.org, May 25 2017

Status: Fixed (was: Started)

Comment 5 by klausw@chromium.org, May 25 2017

Labels: Proj-VR M-60
Project Member

Comment 6 by bugdroid1@chromium.org, May 30 2017

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

commit 01693c7c9e98c2a4f3809cb1b4d6ae7b97eab02f
Author: klausw <klausw@chromium.org>
Date: Tue May 30 19:59:04 2017

Use standard-priority GL context for VrShellGl.

The high-priority GL context appears to be conflicting with GVR's
internal high priority context for async reprojection, so we
can't use that in other VR code.

BUG= 727800 , 725684 

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

[modify] https://crrev.com/01693c7c9e98c2a4f3809cb1b4d6ae7b97eab02f/chrome/browser/android/vr_shell/vr_shell_gl.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 30 2017

Labels: merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/05bac91e966e8be06039390c1cb44a51f0e5957f

commit 05bac91e966e8be06039390c1cb44a51f0e5957f
Author: klausw <klausw@chromium.org>
Date: Tue May 30 21:18:04 2017

Use standard-priority GL context for VrShellGl.

The high-priority GL context appears to be conflicting with GVR's
internal high priority context for async reprojection, so we
can't use that in other VR code.

BUG= 727800 , 725684 
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2909163003
Cr-Original-Commit-Position: refs/heads/master@{#475634}
Review-Url: https://codereview.chromium.org/2909393002
Cr-Commit-Position: refs/branch-heads/3112@{#40}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}

[modify] https://crrev.com/05bac91e966e8be06039390c1cb44a51f0e5957f/chrome/browser/android/vr_shell/vr_shell_gl.cc

Project Member

Comment 8 by bugdroid1@chromium.org, May 31 2017

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

commit a54463e34bc265ab1c63a192f449042136ab1528
Author: klausw <klausw@chromium.org>
Date: Wed May 31 04:28:39 2017

Add usage guideline comments for custom GL context priority.

Due to conflicts with the GVR library, we had to stop using this for the VR
browser mode. I think this attribute is still useful for the future, but we
should use it to reduce priority of possibly-slow user GL code instead of using
"high" priority ourselves.

BUG= 725684 
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

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

[modify] https://crrev.com/a54463e34bc265ab1c63a192f449042136ab1528/ui/gl/gl_context.h

Components: Blink>WebXR

Sign in to add a comment