Update explainer.md (stage 2)#96
Conversation
As outlined in #94, added content for stage 2 of re-writing the explainer. This covers Proposed Approach (basic testing pattern and a few primer JS examples on getting started).
alcooper91
left a comment
alcooper91
left a comment
There was a problem hiding this comment.
Sorry for the delay, I was unexpectedly OOO the last three weeks. I have one minor concern with the "Next animation frame" logic being described here; relevant to some logic we do in the tests. LMK your thoughts.
| 1. **Connect a fake device** with the desired capabilities (session types, views, supported features, etc.). | ||
| 2. **Use WebXR entry points** to obtain a session (e.g. `navigator.xr.requestSession(. . .)`). | ||
| 3. **Drive device state** (viewer pose, tracking loss, bounds/floor origin, visibility state etc.). | ||
| 4. **Advance one frame**, then assert on data returned by WebXR (poses, events, hit test results etc.). |
There was a problem hiding this comment.
I think in some cases in the spec we require two frames to pass before state is guaranteed. This is because there may be a delay in getting data updated. e.g. If you submit the data update in the returned animation frame it may not take effect before the next frame has been sent to you, but it should be fixed by that following frame (e.g. an update made during frame N is only guaranteed to be present in the data for frame N+2).
There was a problem hiding this comment.
(While this is a chrome link, this is the shared infra file that has this comment: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/webxr/resources/webxr_util.js;drc=47be9eaf6640ae2376cdb76053ebc46e4df31191;l=91)
There was a problem hiding this comment.
Thanks for the feedback. Just to make sure I understand correctly, is this referring specifically to cases where the test state is updated from within the XR animation frame callback, so the change may then only be guaranteed in the N+2 frame?
In order to address this, would it be better to change the explainer language throughout from "wait at least one frame" to "wait for up to two frames"? I am thinking of the examples below (and future ones) which are mostly framed around asserting expected outcomes on the next frame.
There was a problem hiding this comment.
Well, I guess the comment on the code says only if it's outside of the animation frame that the update method is called, so I guess we may still want it to be the next frame? (And of course there's no guarantees for methods that return promises).
There was a problem hiding this comment.
Thanks for the extra insights, makes sense - I still think it may be worth adding a note about this at the top to align with what's on the spec and better manage expectations. I made another PR with a short note after step 4 in the basic testing flow section. LMK your thoughts on the wording
Added a note after step 4 in the basic testing flow section to address the next animation frame logic described.
3 participants
As outlined in #94, added content for stage 2 of re-writing the explainer. This covers Proposed Approach (basic testing pattern and a few primer JS examples on getting started).