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

Issue 728772 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug
Proj-XR


Show other hotlists

Hotlists containing this issue:
VR-Automated-Tests


Sign in to add a comment

Ensure that window.innerWidth is set to the correct value in webvr and vr shell

Project Member Reported by billorr@chromium.org, Jun 1 2017

Issue description

I've observed that innerWidth doesn't appear to be returning the expected value during automated tests.

We should add a test that validates window.innerWidth/innerHeight are returning the expected value, and debug why they aren't.
 
Cc: billorr@chromium.org
Labels: Proj-VR
Cc: mthiesse@chromium.org bajones@chromium.org
Components: Blink>WebVR Test>Missing UI>Browser>VR
Labels: VR-Test
First, we need to define the expected values. For WebVR, presentation is not really the window. The presentation is happening in another "display" that doesn't have a window. (This is easier to think about for desktop.)
We should probably not be changing the display size for webVR pages at all, other than to the size we set for 2D pages in Vr Shell.

We originally changed webVR page size because of the fullscreen hacks.

However, we should be careful because there are some gotchas here. For example, if we don't set the page's DPR to 1.0, then sites may naively create canvases with too many pixels (if they set the CSS width/height, but not the canvas width/height). I think we probably still don't want to change the page's DPR so that we load faster, and we should just expect web devs to do the right thing if they care about perf.
Cc: mscales@chromium.org
mscales: See the comment above for a best practice for authors.
I should clarify that I'm also not at all sure what the backing size would be if the author set the css size, rather than the canvas width/height. I'm sure Brandon would know.

In any case, best practice would be to set the canvas width/height, the css size can be anything.
None of the properties of window should change because of WebVR. In the desktop case the window will still be visible while the display is presenting, so everything needs to carry on as usual. A behaviour difference between desktop and mobile would be confusing.

AFAIK, changing the CSS size of a canvas does nothing to the backing store. 

We just need to make sure that developers do the right thing, which is set the dimensions of the canvas to be the same as the dimensions of the VR display.
>>None of the properties of window should change because of WebVR.

I'm not sure about that.  There are several cases for how we enter WebVR:
* Desktop 2d -> WebVr.  The window size is the 2d window size.
* Phone 2d -> WebVR.  The window size is mostly the size of the phone (minus scaling/controls).
* VR browser -> WebVR.  The window size is the "vrshell" window size.

Given that users can go [2d -> vr browser -> WebVR] or [2d -> WebVR] and get different sizes, we should be careful that these give the same experience.
The window dimensions are unrelated to WebVR. I'm having trouble seeing how any of the three scenarios warrant changing anything about the window.

The transition from 2d to VR browser is more interesting, but again not WebVR related - the window really does change size, but the VR display does not.
For [2d -> WebVR], the window size shouldn't change (ie - we still report the 2d window size).

For [2d -> vr browser -> WebVR], should the reported window size be the VR browser window size, or the 2d window size?
Owner: mthiesse@chromium.org
Status: Assigned (was: Untriaged)
I think we want to minimize the number of times we change the size. We should leave it unchanged when entering webVR, and only change it when entering/exiting the VR browsing.

I'll pick this up.
On second thought this is all much simpler and more consistent if we just change the size whenever we enter VR, and leave it set while in VR. Avoiding crazy page size changes when the user hits the app button is probably the right thing to do here.
Status: Started (was: Assigned)
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 12 2017

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

commit 9b394e806ade71fd109c680a489e25e1a803097a
Author: Michael Thiessen <mthiesse@chromium.org>
Date: Wed Jul 12 18:27:42 2017

VR: Don't set webVR pages to a different size than normal pages.

This was a legacy workaround from when we used the normal compositor and
fullscreened the layer to render webVR. With the mailbox path this is
long dead and we don't need to set the page size differently for webVR.

Bug: 728772
Change-Id: I0fb22859f2250f1192bcbf8d52dd6366c78217ea
Reviewed-on: https://chromium-review.googlesource.com/567497
Reviewed-by: Yash Malik <ymalik@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486031}
[modify] https://crrev.com/9b394e806ade71fd109c680a489e25e1a803097a/chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java
[modify] https://crrev.com/9b394e806ade71fd109c680a489e25e1a803097a/chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellTransitionTest.java

Cc: -billorr@chromium.org
Owner: billorr@chromium.org
Bill, what did you want to do with this bug at this point?

As discussed, the reason we see wrong window.innerWidth/Height values is that the compositor is paused. Once the compositor is unpaused the values propagate correctly.
I think Michael's fix for this is good for M-61.  The followup is whether we want to do anything to deal with the window.innerWidth/window.innerHeight not getting updated in response to pausing the compositor.  Given that we aren't changing the window size as often, it is more likely to be correct while in WebVR.

Applications shouldn't use window.innerWidth for WebVR, because the ideal canvas texture size is instead exposed through VRDisplay.


Labels: -M-61 M-62
Followup for what still needs to be addressed:
window.innerwidth, screen.width, window.outerwidth should report consistent values when we are in WebVR.  This could mean that none of them change from out of VR, or that they report vrshell values, but it should be consistent regardless how vr was entered.
More generally, these values should reflect the state of the 2D browser window and screen. Thus, the values should not change when entering (or exiting) WebVR presentation, regardless of whether presentation was initiated from 2D Chrome, the VR browser, or a desktop browser with an external headset. The one exception would be if presentation started in 2D Chrome but the user exited presentation into the VR browser, which has a different size. In theory, all of this should just work as long as we aren't manipulating these values or changing something we shouldn't as part of presentation.

billorr@ notes that if applications are using these values for something in WebVR presentation, which they shouldn't, the results will be different depending on how the user entered presentation. I don't think there's much we can or should do about this.
Cc: billorr@chromium.org
Owner: ----
Status: Available (was: Started)
Labels: -M-62 M-63
Just checked current status in Canary on a pixel.  Before entering webvr, I'm seeing 412x660 for window.innerWidth, window.outerWidth, and screen.width (and height).

After entering vr browsing, I'm seeing 617x412.

When I enter webvr from 2d, sometimes I see 617x412, but I've also seen other values for window.innerWidth, including 1269x714, 732x412.  This appears to be a race condition for how far we get through plumbing/updating the size before we stop the vsync timer.  window.outerWidth and screen.width appear to have the correct size.
Labels: -Pri-2 -M-63 Pri-3
Thanks, billorr@.

These values should represent the current state of the page. In the mobile case where the page his hidden, I would think that these values should represent the last state of the page and not change until the page changes (and a new layout occurs). Thus, 2D->WebVR should have the 2D dimensions and VR browser->WebVR should have the VR browser dimensions. Upon exiting WebVR presentation, the values should be updated to the dimensions of the resulting UI (2D or VR browser). (This is another way of saying what I said in #18.)

It sounds like we sometimes report the VR browser dimensions in the 2D->WebVR case and report unrelated dimensions in other cases.

Since applications should not depend on this, however, this seems low priority.
I think we should avoid differences in behavior depending on how you entered WebVR. 2D->WebVR and VR browser->WebVR should probably report the same size, IMO. Otherwise, web authors could start using this as a way to detect how VR was entered and whether the user was browsing in VR or not. Though maybe we don't care if sites detect this? I'm not sure what nefarious things they could do with that information.
That's a good point, and something to think through. However, since the app would already be running before the transition, I'm not sure they wouldn't already have this information since the VR browser window is probably a unique size. As long as there is not presentation-to-presentation navigation, any navigation to a _new_ page will be in the browser, which should (and we should ensure does) have the new dimensions before loading.
Components: Blink>WebXR
Removing Blink>WebVR component and assigning to Blink>WebXR 
Components: -Blink>WebVR
Components: Tests>Missing
Components: -Test>Missing

Comment 30 by samdrazin@chromium.org, Jan 17 (6 days ago)

Cc: btebbs@chromium.org ddorwin@chromium.org
Components: -UI>Browser>VR
VRB team triage: this does not seem to directly impact VRB.  Ping if we are mistaken.

Comment 31 by billorr@chromium.org, Jan 17 (6 days ago)

I don't believe it impacts VRB (the experience).  However it is tightly integrated with the UI>Browser>VR code component.

While the different window/viewport sizes may be wrong still in WebXR, I believe it is less likely to cause issues.  In WebXR, render target size is controlled/recommended in a different way.

Sign in to add a comment