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

Issue 714304 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature
Proj-VR
Proj-XR
Proj-XR-VR

Blocked on:
issue 834784



Sign in to add a comment

Investigate using Video Viewports for VRShell's page content

Project Member Reported by bajones@chromium.org, Apr 21 2017

Issue description

Daydream has a technique, used by Play Movies among other things, to render a quad surface as part of the reprojection phase with higher quality than usual. It's described here:

https://developers.google.com/vr/android/ndk/gvr-ndk-rendering#using_video_viewports

This seems like it would be a natural fit for VRShell's page content, potentially yielding higher quality for what is often very text-heavy content.
 
Status: Available (was: Untriaged)
Cc: bajones@chromium.org vollick@chromium.org
 Issue 817077  has been merged into this issue.
Labels: -Pri-3 M-67 Pri-1
Owner: vollick@chromium.org
Status: Assigned (was: Available)
Owner: mthiesse@chromium.org
I'm going to see if I can get land this and get it merged to 67...
Labels: -Pri-1 Hotlist-VRB-MVP Pri-2

Comment 6 by tiborg@chromium.org, Apr 19 2018

Blockedon: 834784
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 19 2018

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

commit 52565af78dec4bb3f6d9d0f44a841e836ef4d3ba
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Thu Apr 19 16:20:57 2018

VR: Clean up viewports, don't use texture id 0 when drawing content quad.

This CL cleans up our bufferspec/viewport initialization, and properly
updates viewports when we refresh the viewer parameters.

It also fixed some probably undefined behaviour when there's no content
overlay texture and we were using texture 0.

Bug:  714304 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: I69e2b075b91ac7e36f046de1ef3ff089d82e8929
Reviewed-on: https://chromium-review.googlesource.com/1016029
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552037}
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/android/vr/vr_shell_gl.h
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/elements/content_element.cc
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/elements/content_element.h
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/elements/textured_element.cc
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/renderers/external_textured_quad_renderer.cc
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/renderers/textured_quad_renderer.cc
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/renderers/textured_quad_renderer.h
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/test/fake_ui_element_renderer.cc
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/test/fake_ui_element_renderer.h
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/testapp/test_keyboard_renderer.cc
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/ui_element_renderer.cc
[modify] https://crrev.com/52565af78dec4bb3f6d9d0f44a841e836ef4d3ba/chrome/browser/vr/ui_element_renderer.h

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 20 2018

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

commit 1e24d20aa32e6c27eb7439b7a8521f835c591b44
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Fri Apr 20 00:19:07 2018

VR: Implement quad buffer for content quad.

Huge thanks to bajones for building the prototype I built this CL
off of, and for debugging for me ;)

Uses a separate viewport with transform to avoid resampling the
content quad when drawing it into our scene. The viewport is placed
underneath the viewport for the other browser UI, and a transparent
hole is punched through to the underlying viewport.

Also avoids toggling multisampling at runtime (which recreates the
swap chain) by re-using the two buffers (one multisampled and one
not) across browsing and webVR modes.

Bug:  714304 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Change-Id: Ibcbb2752aaa957c3f904d4b76bf6dd5fdf9164c8
Reviewed-on: https://chromium-review.googlesource.com/1017589
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Ian Vollick <vollick@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552212}
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/android/vr/vr_shell_gl.h
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/BUILD.gn
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/elements/content_element.cc
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/elements/content_element.h
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/elements/ui_element.cc
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/elements/ui_element.h
[add] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/renderers/transparent_quad_renderer.cc
[add] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/renderers/transparent_quad_renderer.h
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/renderers/web_vr_renderer.cc
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/renderers/web_vr_renderer.h
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/test/ui_pixel_test.cc
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/testapp/vr_test_context.cc
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/ui.cc
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/ui.h
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/ui_element_renderer.cc
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/ui_element_renderer.h
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/ui_input_manager_unittest.cc
[modify] https://crrev.com/1e24d20aa32e6c27eb7439b7a8521f835c591b44/chrome/browser/vr/ui_renderer.h

Status: Started (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Apr 20 2018

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

commit bae94e3a2e216da0b2c8308777257c765ff886ea
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Fri Apr 20 14:49:20 2018

Revert "VR: Implement quad buffer for content quad."

This reverts commit 1e24d20aa32e6c27eb7439b7a8521f835c591b44.

Reason for revert: Tanks performance. Quad layers are apparently expensive, yo.

Original change's description:
> VR: Implement quad buffer for content quad.
> 
> Huge thanks to bajones for building the prototype I built this CL
> off of, and for debugging for me ;)
> 
> Uses a separate viewport with transform to avoid resampling the
> content quad when drawing it into our scene. The viewport is placed
> underneath the viewport for the other browser UI, and a transparent
> hole is punched through to the underlying viewport.
> 
> Also avoids toggling multisampling at runtime (which recreates the
> swap chain) by re-using the two buffers (one multisampled and one
> not) across browsing and webVR modes.
> 
> Bug:  714304 
> 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
> Change-Id: Ibcbb2752aaa957c3f904d4b76bf6dd5fdf9164c8
> Reviewed-on: https://chromium-review.googlesource.com/1017589
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Reviewed-by: Ian Vollick <vollick@chromium.org>
> Reviewed-by: Brandon Jones <bajones@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#552212}

TBR=vollick@chromium.org,mthiesse@chromium.org,bajones@chromium.org

Change-Id: Iea71c2d624193c5f90c7554ee212d28fb526bcfc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  714304 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Reviewed-on: https://chromium-review.googlesource.com/1021972
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552333}
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/android/vr/vr_shell_gl.h
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/BUILD.gn
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/elements/content_element.cc
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/elements/content_element.h
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/elements/ui_element.cc
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/elements/ui_element.h
[delete] https://crrev.com/b2f81897274e4c7f9bc5d2c8c13d14201504d6c5/chrome/browser/vr/renderers/transparent_quad_renderer.cc
[delete] https://crrev.com/b2f81897274e4c7f9bc5d2c8c13d14201504d6c5/chrome/browser/vr/renderers/transparent_quad_renderer.h
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/renderers/web_vr_renderer.cc
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/renderers/web_vr_renderer.h
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/test/ui_pixel_test.cc
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/testapp/vr_test_context.cc
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/ui.cc
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/ui.h
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/ui_element_renderer.cc
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/ui_element_renderer.h
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/ui_input_manager_unittest.cc
[modify] https://crrev.com/bae94e3a2e216da0b2c8308777257c765ff886ea/chrome/browser/vr/ui_renderer.h

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 20 2018

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

commit bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Fri Apr 20 20:39:17 2018

Reland "VR: Implement quad buffer for content quad."

This reverts commit bae94e3a2e216da0b2c8308777257c765ff886ea.

Found the reason for the perf regressions, was an oversight that caused our glClear to do nothing.

Original change's description:
> Revert "VR: Implement quad buffer for content quad."
> 
> This reverts commit 1e24d20aa32e6c27eb7439b7a8521f835c591b44.
> 
> Reason for revert: Tanks performance. Quad layers are apparently expensive, yo.
> 
> Original change's description:
> > VR: Implement quad buffer for content quad.
> > 
> > Huge thanks to bajones for building the prototype I built this CL
> > off of, and for debugging for me ;)
> > 
> > Uses a separate viewport with transform to avoid resampling the
> > content quad when drawing it into our scene. The viewport is placed
> > underneath the viewport for the other browser UI, and a transparent
> > hole is punched through to the underlying viewport.
> > 
> > Also avoids toggling multisampling at runtime (which recreates the
> > swap chain) by re-using the two buffers (one multisampled and one
> > not) across browsing and webVR modes.
> > 
> > Bug:  714304 
> > 
> > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
> > Change-Id: Ibcbb2752aaa957c3f904d4b76bf6dd5fdf9164c8
> > Reviewed-on: https://chromium-review.googlesource.com/1017589
> > Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> > Reviewed-by: Ian Vollick <vollick@chromium.org>
> > Reviewed-by: Brandon Jones <bajones@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#552212}
> 
> TBR=vollick@chromium.org,mthiesse@chromium.org,bajones@chromium.org
> 
> Change-Id: Iea71c2d624193c5f90c7554ee212d28fb526bcfc
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  714304 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
> Reviewed-on: https://chromium-review.googlesource.com/1021972
> Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
> Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#552333}

TBR=vollick@chromium.org,mthiesse@chromium.org,bajones@chromium.org

Bug:  714304 
Change-Id: Iba87b74983d4b65d2067a8390cf45f4864ebf32a
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_vr
Reviewed-on: https://chromium-review.googlesource.com/1022550
Reviewed-by: Ian Vollick <vollick@chromium.org>
Reviewed-by: Brandon Jones <bajones@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552459}
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/android/vr/vr_shell_gl.h
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/BUILD.gn
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/elements/content_element.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/elements/content_element.h
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/elements/environment/background.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/elements/ui_element.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/elements/ui_element.h
[add] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/renderers/transparent_quad_renderer.cc
[add] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/renderers/transparent_quad_renderer.h
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/renderers/web_vr_renderer.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/renderers/web_vr_renderer.h
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/test/ui_pixel_test.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/testapp/vr_test_context.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/ui.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/ui.h
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/ui_element_renderer.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/ui_element_renderer.h
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/ui_input_manager_unittest.cc
[modify] https://crrev.com/bc382ac7f270e622b5c2211cc9ef6bb5b7ce960a/chrome/browser/vr/ui_renderer.h

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 24 2018

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

commit cddc0376c08a4adbec6a44e9084f76cfb6431517
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Tue Apr 24 01:03:12 2018

VR: Decrease UI buffer render target size.

With Quad layers keeping the content quad high-res and crisp, we can
lower the buffer size of the rest of our UI which doesn't need to be as
high quality as the content quad.

The quality decrease is only barely noticeable on an A/B toggle, and
the performance win is huge (and necessary to get us back to 60fps on a
Pixel XL device).

Bug:  714304 
Change-Id: I0f352f88789584e51815baafe0f25d69fb7e8780
Reviewed-on: https://chromium-review.googlesource.com/1024881
Reviewed-by: Ian Vollick <vollick@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552934}
[modify] https://crrev.com/cddc0376c08a4adbec6a44e9084f76cfb6431517/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/cddc0376c08a4adbec6a44e9084f76cfb6431517/chrome/browser/android/vr/vr_gl_thread.cc
[modify] https://crrev.com/cddc0376c08a4adbec6a44e9084f76cfb6431517/chrome/browser/android/vr/vr_gl_thread.h
[modify] https://crrev.com/cddc0376c08a4adbec6a44e9084f76cfb6431517/chrome/browser/android/vr/vr_shell.cc
[modify] https://crrev.com/cddc0376c08a4adbec6a44e9084f76cfb6431517/chrome/browser/android/vr/vr_shell.h
[modify] https://crrev.com/cddc0376c08a4adbec6a44e9084f76cfb6431517/chrome/browser/android/vr/vr_shell_gl.cc
[modify] https://crrev.com/cddc0376c08a4adbec6a44e9084f76cfb6431517/chrome/browser/android/vr/vr_shell_gl.h

Labels: -M-67 M-68
Too much going on in 67. Will push this out :(
Status: Fixed (was: Started)
Labels: Test-Complete

Sign in to add a comment