[webvr] Remove pose parameter from VRDisplay.submitFrame

RESOLVED FIXED in Firefox 52

Status

()

Core
Graphics
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: kip, Assigned: kip)

Tracking

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

a year ago
The WebVR 1.1 spec update removes the pose parameter from VRDisplay.submitFrame.

This parameter is currently optional in our WebVR 1.0 implementation.  The fallback path we have implemented for cases when the pose is not passed matches the processing model described in the WebVR 1.1 spec.

We should remove the pose parameter from VRDisplay.webidl to match the WebVR 1.1 spec.
(Assignee)

Comment 1

a year ago
Bug 1306486 includes processing model changes that ensure that sites will behave consistently when multiple calls to VRDisplay.getPose or VRDisplay.getFrameParameters are called within a single frame.
Blocks: 1306486
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Blocks: 1306415
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Attachment #8796339 - Flags: review?(gwright)
Attachment #8796339 - Flags: review?(bugs)
(Assignee)

Comment 4

a year ago
smaug: Would you be able to review the small webidl change here?

Comment 5

a year ago
mozreview-review
Comment on attachment 8796339 [details]
Bug 1306427 - Remove pose parameter from VRDisplay.submitFrame

https://reviewboard.mozilla.org/r/82224/#review82634
Attachment #8796339 - Flags: review?(bugs) → review+

Comment 6

a year ago
mozreview-review
Comment on attachment 8796339 [details]
Bug 1306427 - Remove pose parameter from VRDisplay.submitFrame

https://reviewboard.mozilla.org/r/82224/#review83930

::: gfx/vr/VRDisplayHost.cpp:85
(Diff revision 2)
> +
>    int32_t inputFrameID = aInputFrameID;
>    if (inputFrameID == 0) {
>      inputFrameID = mInputFrameID;
>    }
>    if (inputFrameID < 0) {

If inputFrameID isn't supposed to be negative, should this be an unsigned int instead?
Attachment #8796339 - Flags: review?(gwright) → review+
(Assignee)

Comment 7

a year ago
mozreview-review-reply
Comment on attachment 8796339 [details]
Bug 1306427 - Remove pose parameter from VRDisplay.submitFrame

https://reviewboard.mozilla.org/r/82224/#review83930

> If inputFrameID isn't supposed to be negative, should this be an unsigned int instead?

Before removing the VRPose parameter from submitFrame, this allowed -1 to be passed indicating that a frame was submitted without requesting a pose first.

Now that this can no longer occur, we could probably change all of the inputFrameID values to be unsigned.  There was a potential future use case of presenting layers that are head-locked or a "quad in space" in which case there is no associated pose, but we are not using this functionality now.

I wouldn't be opposed to making them all unsigned, as nothing will ever send a -1 in currently, and we could perhaps create an overloaded function without the inputFrameID parameter for these future use cases.
OK, well it's up to you :) I mentioned it because it looked a bit odd, but I understand if you consider this a refactoring which is unrelated to this patch, in which case it might be better to file a follow up bug to change to unsigned and fix that there, and land this separately.
(Assignee)

Updated

a year ago
See Also: → bug 1309988
(Assignee)

Comment 9

a year ago
(In reply to George Wright (:gw280) (:gwright) from comment #8)
> OK, well it's up to you :) I mentioned it because it looked a bit odd, but I
> understand if you consider this a refactoring which is unrelated to this
> patch, in which case it might be better to file a follow up bug to change to
> unsigned and fix that there, and land this separately.

I have added Bug 1309988 to change the values to unsigned and then remove the no longer needed "< 0" check.
(Assignee)

Comment 10

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/45bb8db1e48f4c0c0052f8dafe96922781243539
Bug 1306427 - Remove pose parameter from VRDisplay.submitFrame,r=smaug,r=gw280

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/45bb8db1e48f
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.