Skip to content

Conversation

@phoenixbf
Copy link
Contributor

See #1342

Copy link
Contributor

@gkjohnson gkjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks! A couple comments but I think this is the right direction

@@ -0,0 +1,43 @@
class AFManager {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets call this FrameScheduler for now.

Comment on lines 11 to 21
setXRSession( xrsession ) {

this.xrsession = xrsession;

}

removeXRSession() {

this.xrsession = null;

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@gkjohnson
Copy link
Contributor

@phoenixbf - just wanted to check in to see if you had any more questions on the changes needed here.

@phoenixbf
Copy link
Contributor Author

@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

@phoenixbf
Copy link
Contributor Author

I've added management of pending AF.
It basically keeps track of requested AFs via handle as suggested, providing methods to cancel or flush all pending AFs. When entering XR session, it internally flushes and completes all pending, switching to this.xrsession.requestAnimationFrame for new requests.

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 TilesRendererBase and then referenced by LRUCache, PriorityQueue, etc. during their initializations

@gkjohnson
Copy link
Contributor

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.

If this is the right direction, it should be probably instantiated in TilesRendererBase and then referenced by LRUCache, PriorityQueue, etc. during their initializations

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")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants