webxr-polyfill icon indicating copy to clipboard operation
webxr-polyfill copied to clipboard

make XRWebGLLayer spec compliant

Open AAwouters opened this issue 2 years ago • 2 comments

Fixes #167

Relevant spec change

AAwouters avatar Feb 07 '23 15:02 AAwouters

Can you add a unit test?

matthargett avatar Mar 13 '23 21:03 matthargett

Can you add a unit test?

I'll look into adding a test. However currently the tests won't run and it seems lots of imports are no longer correct. I could fix the tests and then add a unit test but polluting this PR (which has about half a line of changes) with lots of unrelated changes in the tests seems less than ideal.

AAwouters avatar Mar 14 '23 08:03 AAwouters

Can this PR be reviewed? What is needed to merge this?

ceyhun-o avatar Mar 04 '24 09:03 ceyhun-o

@AAwouters worked on this during his internship at @THEOplayer. I'm happy to take over this PR if needed.

Change looks fine. It would be good to add a unit tests as previously requested.

We looked at the unit tests, but they are completely broken. They haven't been touched since 2018 in https://github.com/immersive-web/webxr-polyfill/commit/39289985c085133fbf729c3a16637f70e5c213c3, and I suspect no-one has tried running them since then. I tried running npm test locally, but I didn't get very far:

  • https://github.com/immersive-web/webxr-polyfill/commit/fa824a9e79b2bf101512b6c72432037bd9758a30 moved XRDevice.js from src/api/ to src/devices/, but did not update the imports inside the tests (1, 2, 3). It also changed the API of XRDevice.requestSession({ immersive, outputContext }) to XRDevice.requestSession(mode), but test-xr-device.js still uses the old API.
  • https://github.com/immersive-web/webxr-polyfill/commit/d04ca99bb3a1805a08f6f6d4449cf76a16cb31c2 renamed XRDevicePose to XRViewerPose, but again failed to update the tests (1).

I don't currently have the bandwidth to fix all of these tests.

MattiasBuelens avatar Mar 05 '24 10:03 MattiasBuelens

sigh, yeah. This repo is pretty neglected these days, and you certainly shouldn't be held accountable for picking up after bit-rotted tests.

Because the scope of this is pretty small, I'm just going to land it.

toji avatar Mar 05 '24 23:03 toji