WebXR - Fixed bug where the wrong number of views is being added #5063
Conversation
src/framework/xr/xr-manager.js
Outdated
| @@ -696,7 +696,8 @@ class XrManager extends EventHandler { | |||
|
|
|||
| if (lengthNew > this.views.length) { | |||
| // add new views into list | |||
| for (let i = 0; i <= (lengthNew - this.views.length); i++) { | |||
| const viewAddCount = (lengthNew - this.views.length); | |||
There was a problem hiding this comment.
I was looking at recently when you were refactoring around there, and though it'd be best to replace the if above as as well as the loop with while (lengthNew > this.views.length) {
and similar for the else / for loop in the else branch.
There was a problem hiding this comment.
Done. I don't have a way of testing the logic in the else branch so can you double check the code please 😅
yaustar
changed the title
src/framework/xr/xr-manager.js
Outdated
| @@ -696,8 +696,7 @@ class XrManager extends EventHandler { | |||
|
|
|||
| if (lengthNew > this.views.length) { | |||
| // add new views into list | |||
| const viewAddCount = (lengthNew - this.views.length); | |||
| for (let i = 0; i <= viewAddCount; i++) { | |||
| while (lengthNew > this.views.length) { | |||
There was a problem hiding this comment.
now get rid of that if, and also else?
the code should just be
while (..) {
}
while (..) {
}
no need for ifs as they just do the same as the first iteration of the loop.
As I recall, when session starts views might be not available straight away. Also, they might get removed due to platform specific behaviours (like minimizing or going into passthrough, and can be other edge cases). Then can get available again, and in different order within an array. Here is the source of specs: https://www.w3.org/TR/webxr/#xrview-interface
|
* Fixed bug where the number of XR views to add was changing * Replaced with while loop * Removed if statements from review feedback --------- Co-authored-by: Steven Yau <syau@snapchat.com>
Fixes an error in WebXR on some devices that have more than 2 viewports about view being undefined
This is because when we add views to an array in the loop, we are also using the length of that array to calculate how many to add which means the total number to add gets smaller each loop.
I confirm I have read the contributing guidelines and signed the Contributor License Agreement.