1368990 - Implement WebVR reftest for verifying onVRPresentChange

Status: RESOLVED FIXED

(Core :: WebVR, enhancement)

Description

[Daosheng Mu[:daoshengmu]]

For the onVRPresentChange() testcase, we will ask to resize while vrDisplay is presenting and make webglCanvas.width and height to be the eye width and height of VRDisplay.

Please refer, https://github.com/toji/webvr.info/blob/e00559c5b8bd1bc7ad050e9be07991ccf8c986ad/samples/XX-vr-controllers.html#L228.

Comment 1

[Daosheng Mu[:daoshengmu]]

Attachment: Bug 1368990 - Add reftest for verifying onVRPresentChange;

Review commit: https://reviewboard.mozilla.org/r/146616/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/146616/

Comment 2

[Daosheng Mu[:daoshengmu]]

Need to check why the submit frame image is skewed.

Comment 3

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8875207 [details]
Bug 1368990 - Add reftest for verifying onVRPresentChange;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/146616/diff/1-2/

Comment 4

[Daosheng Mu[:daoshengmu]]

Try looks good, https://treeherder.mozilla.org/#/jobs?repo=try&revision=f19c3976aab7f0d49f19204294395ab5cd7b5513

Comment 5

[:kip (Kearwood Gilbert)]

Comment on attachment 8875207 [details]
Bug 1368990 - Add reftest for verifying onVRPresentChange;

https://reviewboard.mozilla.org/r/146616/#review154722

Looks great, thanks!

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf25fd50974c
Add reftest for verifying onVRPresentChange; r=kip

Comment 7

[Daosheng Mu[:daoshengmu]]

This is a reftest, and I recommend to uplift it to beta for preventing other regressions.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/cf25fd50974c

Comment 9

[Daosheng Mu[:daoshengmu]]

Comment on attachment 8875207 [details]
Bug 1368990 - Add reftest for verifying onVRPresentChange;

Approval Request Comment
[Feature/Bug causing the regression]: This is a new reftest for protecting from regression.
[User impact if declined]: Nope.
[Is this code covered by automated tests?]: yes.
[Has the fix been verified in Nightly?]: yes.
[Needs manual test from QE? If yes, steps to reproduce]: nope.
[List of other uplifts needed for the feature/fix]: nope.
[Is the change risky?]: nope.
[Why is the change risky/not risky?]: just a reftest.
[String changes made/needed]: nope.

Comment 10

[Julien Cristau [:jcristau]]

Comment on attachment 8875207 [details]
Bug 1368990 - Add reftest for verifying onVRPresentChange;

I'm confused by the gfxVRPuppet.cpp change, which doesn't seem to be covered by "add reftest"?

Comment 11

[Daosheng Mu[:daoshengmu]]

(In reply to Julien Cristau [:jcristau] from comment #10)
> Comment on attachment 8875207 [details]
> Bug 1368990 - Add reftest for verifying onVRPresentChange;
> 
> I'm confused by the gfxVRPuppet.cpp change, which doesn't seem to be covered
> by "add reftest"?

The code from gfxVRPuppet.cpp is only run by reftest or mochitest. It is behind pref::"dom.vr.test". We didn't enable it from our build.

Comment 12

[Julien Cristau [:jcristau]]

Comment on attachment 8875207 [details]
Bug 1368990 - Add reftest for verifying onVRPresentChange;

Thanks :daoshengmu.  Add reftest for VR issue, beta55+

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/911dc640c42c