You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Apologies that this has taken as long as it has to get a PR up. Doing some GH and meeting minutes archeology, the primary concerns about the algorithm as it currently is written seem to be that Alan needs it to allow for non-blocking communication to the XR hardware and otherwise myself and a couple of others just want to ensure that the timing of when the changes apply is reasonably predictable.
This change moves the apply the pending render state execution to immediately after the frame loop executes, and queues a task to do it. Then it also moves the execution of the frame loop into a task on the same queue, which should implicitly ensure that the render state update must finish before the next frame executes. (I believe that the frame loop execution was effectively taking place in a task before anyway, so this just makes that explicit. Please correct me if I'm wrong!)
This gives us the behavior that any changes will apply some time after the frame finishes executing, but the only guarantee is that they will be finished applying by the time the next frame begins executing. This means the most predictable way to interact with the render state is to inspect and update it within a frame, with the expectation that any changes will take effect in the next frame.
This is technically a backwards-compatibility break, but I have a hard time imagining that any developers are strongly dependent on the current timing.
1. [=Queue a task=] to perform the following steps:
1. Set |activeState| to |newState|.
1. If |oldLayer| is not equal to |activeState|'s {{XRRenderState/baseLayer}}, or the dimensions of |oldLayer| are different from |baseLayer|, [=update the viewports=] for |session|.
The reason will be displayed to describe this comment to others. Learn more.
Might be worth making sure that we've put the right hooks in for the layers spec to make changes? E.g. in this case, the layers spec is going to want to do this when the layers array has changed.
The reason will be displayed to describe this comment to others. Learn more.
Good point! In this case it seems like, since the layers attribute is defined in this spec, we can check for differences without doing the same sort of "create an algorithm to be overridden" pattern. Gave it a pass with some fairly broad language around detecting changes, let me know what you think!
Merging this now, given that Manish and Alan have both signed off on it. I think I'll leave it on the agenda to mention as a "heads up" for other implementers.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1051. Would love @asajeffrey's input on this!
Apologies that this has taken as long as it has to get a PR up. Doing some GH and meeting minutes archeology, the primary concerns about the algorithm as it currently is written seem to be that Alan needs it to allow for non-blocking communication to the XR hardware and otherwise myself and a couple of others just want to ensure that the timing of when the changes apply is reasonably predictable.
This change moves the
apply the pending render state
execution to immediately after the frame loop executes, and queues a task to do it. Then it also moves the execution of the frame loop into a task on the same queue, which should implicitly ensure that the render state update must finish before the next frame executes. (I believe that the frame loop execution was effectively taking place in a task before anyway, so this just makes that explicit. Please correct me if I'm wrong!)This gives us the behavior that any changes will apply some time after the frame finishes executing, but the only guarantee is that they will be finished applying by the time the next frame begins executing. This means the most predictable way to interact with the render state is to inspect and update it within a frame, with the expectation that any changes will take effect in the next frame.
This is technically a backwards-compatibility break, but I have a hard time imagining that any developers are strongly dependent on the current timing.
Preview | Diff