AF Manager class for dynamic animation frame request/cancel#1407
AF Manager class for dynamic animation frame request/cancel#1407phoenixbf wants to merge 21 commits intoNASA-AMMOS:masterfrom
Conversation
gkjohnson
left a comment
gkjohnson
left a comment
There was a problem hiding this comment.
Great, thanks! A couple comments but I think this is the right direction
| @@ -0,0 +1,43 @@ | |||
| class AFManager { | |||
There was a problem hiding this comment.
Lets call this FrameScheduler for now.
| setXRSession( xrsession ) { | ||
|
|
||
| this.xrsession = xrsession; | ||
|
|
||
| } | ||
|
|
||
| removeXRSession() { | ||
|
|
||
| this.xrsession = null; | ||
|
|
||
| } |
There was a problem hiding this comment.
When changing the system being used for callbacks we will have to cancel all previous callbacks and reregister them with the new function. Eg if the "LRUCache" schedules an unload event it will be waiting until that event fires before scheduling another one. So if the last event is scheduled using the "xrSession" callback and then the session closes, the scheduled callback will never be fired.
So we'll need to keep track of all scheduled callbacks and rAF handles to swap over the scheduled events.
|
@phoenixbf - just wanted to check in to see if you had any more questions on the changes needed here. |
Hi, performing some local testing in order to have a clean rAF tracking system and handling swaps |
|
I've added management of pending AF. So far I tried the above logic in both local and online setups without any evident issues, also with mutiple tsets. If this is the right direction, it should be probably instantiated in |
|
Thanks the direction of this looks great. I think we can cut down on some code redundancy in the implementation but we can deal with that once everything is working.
This sounds good to me. Looks like the "LRUCache", "PriorityQueue", the "throttle" function, and "QueryManager" are all places where rAF is used (though the throttle function can maybe be changed to "queueMicrotask") |
…ryManager; updated Dingo Gap VR sample
gkjohnson
left a comment
gkjohnson
left a comment
There was a problem hiding this comment.
Looking good! Made a few comments -
|
|
||
| setXRSession( xrsession ) { | ||
|
|
||
| if ( ! this.framescheduler ) return; |
There was a problem hiding this comment.
this should never be "null" so we should be able to remove this check.
| this.isLoading = false; | ||
|
|
||
| // FrameScheduler referenced by LRUCache, PriorityQueue | ||
| this.framescheduler = new FrameScheduler(); |
There was a problem hiding this comment.
lets use camel casing to align with the class naming:
this.framescheduler -> this.frameScheduler|
|
||
| this._unloadPriorityCallback = null; | ||
|
|
||
| this.framescheduler = null; |
There was a problem hiding this comment.
Lets initialize these to new FrameScheduler() so the classes are in a usable state when constructed. Then we can remove the null checks, as well. This should help the tests pass, too.
- remove unused function - remove use of "window" - ensure "flushPending" clears all pending handles - code style update
|
I've just made some of the remaining changes to fix tests, add new tests for FrameScheduler, and address some code styling and other issues. I think this should be ready to merge but if you could test it in a real XR environment that would be great. |
Yep, sure. Today I'll carry out additional online tests with a few HMDs |
2 participants
See #1342