Add support for targetFrameRate and supportedFrameRates #1201
Conversation
Merge with main repo
|
@thetuvix I updated my proposal. |
index.bs
Outdated
| The <dfn attribute for="XRSession">targetFrameRate</dfn> attribute reflects the internal [=XRSession/framerate=]. If the {{XRSession}} has no [=XRSession/framerate=], return `null`. The <dfn attribute for="XRSession">ontargetframeratechange</dfn> attribute is an [=Event handler IDL attribute=] for the {{targetframeratechange}} event type. If the {{XRSession}}'s frame rate is updated for any reason, it MUST [=apply the target frame rate=] with the new frame rate. | ||
|
|
||
| The <dfn attribute for="XRSession">supportedFrameRates</dfn> attribute returns a list of supported frame rates. This attribute is optional and MUST NOT be present for {{XRSessionMode/inline}} sessions. If the user agent can dynamically change the frame rate, this should be reflected as a frame rate of <code>0</code>. If the {{XRSession}} has an internal [=XRSession/framerate=], it also MUST support the {{XRSession/supportedFrameRates}} attribute. |
There was a problem hiding this comment.
If the {{XRSession}}'s frame rate is updated for any reason, it MUST [=apply the target frame rate=] with the new frame rate.
I don't fully follow when this applies.
For example, on HoloLens, the app cannot affect the frame rate in any way - in normal usage, the app's render rate will always be 60Hz. If the app misses frames, previous frames can be reprojected, but the app continues to be expected to keep up. Specifically, I would expect supportedFrameRates to only contain 60, because the app cannot request any other frame timings.
However, if the user starts a Mixed Reality Capture video recording, the system will drop to 30Hz all-up, and the app will start to receive 30Hz frame timing. Is this sentence saying that because the effective frame rate got updated "for any reason", that an ontargetframeratechange event must be fired and targetFrameRate must now return 30? This seems wrong, since 30 is not an element within supportedFrameRates.
If the user agent can dynamically change the frame rate, this should be reflected as a frame rate of
0.
This clause sets the wrong mental model for developers. This makes it sound like the target frame timing is fixed if the app chooses a non-0 value, and dynamic if the app chooses 0. In reality, things are always somewhat dynamic regardless - the UA can make use of the targetFrameRate hint provided by the page, but any given frame can have a duration that either matches or doesn't match that target for various platform-specific reasons.
It seems more flexible and consistent to treat targetFrameRate as a hint to the UA, with the page able to observe for any given frame what the effective frame rate/duration is, which can change whether the page has set a non-0 target or not.
Perhaps an answer here is to change the API to requestTargetFrameRate, which returns a promise that completes when the UA has taken action on the request to its best ability. No guarantee is made that the frame rate won't change further for other reasons after the request (e.g. halved due to system overlay, background app, this app not hitting framerate, thermal condition, etc.). However, if the app is waiting until the frame rate logically switches to 72Hz before starting video playback, the requestTargetFrameRate promise completion is the time to start. We could then choose to additionally surface on XRFrame a given frame's duration, without promising that it will be an element of supportedFrameRates, although perhaps it is cleaner first to see if devs are satisfied with just the promise.
There was a problem hiding this comment.
If the {{XRSession}}'s frame rate is updated for any reason, it MUST [=apply the target frame rate=] with the new frame rate.
I don't fully follow when this applies.
For example, on HoloLens, the app cannot affect the frame rate in any way - in normal usage, the app's render rate will always be 60Hz. If the app misses frames, previous frames can be reprojected, but the app continues to be expected to keep up. Specifically, I would expect
supportedFrameRatesto only contain60, because the app cannot request any other frame timings.
supportedFrameRates is the list of framerates that the author can set. They are passed to the system and it will decide how (or if) to apply it. In the case of Hololens, this list could just 60 or it can decide not to support this function which means that the app can't control frame rate.
However, if the user starts a Mixed Reality Capture video recording, the system will drop to 30Hz all-up, and the app will start to receive 30Hz frame timing. Is this sentence saying that because the effective frame rate got updated "for any reason", that an
ontargetframeratechangeevent must be fired andtargetFrameRatemust now return30? This seems wrong, since30is not an element withinsupportedFrameRates.
Since the rate the author sets is just a hint for the compositor, it is free to change it.
Quest also has cases where it drops frame rates. For instance, if the user presses the Oculus button on the controller to bring up the menu, the rate is cut by half.
If the user agent can dynamically change the frame rate, this should be reflected as a frame rate of
0.This clause sets the wrong mental model for developers. This makes it sound like the target frame timing is fixed if the app chooses a non-
0value, and dynamic if the app chooses0. In reality, things are always somewhat dynamic regardless - the UA can make use of thetargetFrameRatehint provided by the page, but any given frame can have a duration that either matches or doesn't match that target for various platform-specific reasons.
I'm happy to remove this special case.
It seems more flexible and consistent to treat
targetFrameRateas a hint to the UA, with the page able to observe for any given frame what the effective frame rate/duration is, which can change whether the page has set a non-0target or not.
Yes, this is what I intended to do. Maybe I need it to make it more explicit and also add a note?
Perhaps an answer here is to change the API to
requestTargetFrameRate, which returns a promise that completes when the UA has taken action on the request to its best ability. No guarantee is made that the frame rate won't change further for other reasons after the request (e.g. halved due to system overlay, background app, this app not hitting framerate, thermal condition, etc.). However, if the app is waiting until the frame rate logically switches to 72Hz before starting video playback, therequestTargetFrameRatepromise completion is the time to start. We could then choose to additionally surface onXRFramea given frame's duration, without promising that it will be an element ofsupportedFrameRates, although perhaps it is cleaner first to see if devs are satisfied with just the promise.
I prefer to have an event instead of a promise to give the UA the flexibility to change the rate at any point, even without setting a target frame rate.
For instance, on Hololens this event would fire if Mixed Reality Capture starts.
There was a problem hiding this comment.
If the user agent can dynamically change the frame rate, this should be reflected as a frame rate of 0.
This clause sets the wrong mental model for developers. This makes it sound like the target frame timing is fixed if the app chooses a non-0 value, and dynamic if the app chooses 0. In reality, things are always somewhat dynamic regardless - the UA can make use of the targetFrameRate hint provided by the page, but any given frame can have a duration that either matches or doesn't match that target for various platform-specific reasons.
I'm happy to remove this special case.
Yea - I would eliminate ever enumerating 0 from supportedFrameRates. We still need to define the default null state for targetFrameRate (and allow apps to set it again when video playback ends), though that can just mean that the app is expressing no preference to the UA.
I prefer to have an event instead of a promise to give the UA the flexibility to change the rate at any point, even without setting a target frame rate.
For instance, on Hololens this event would fire if Mixed Reality Capture starts.
Got it - the name ontargetframeratechange suggested to me that the event could only return values from supportedFrameRates, and perhaps only triggered when the app had requested a change to the target frame rate.
Perhaps just call this onframeratechange to make clear that it's about the effective frame rate, not the target?
There was a problem hiding this comment.
Got it - the name
ontargetframeratechangesuggested to me that the event could only return values fromsupportedFrameRates, and perhaps only triggered when the app had requested a change to the target frame rate.Perhaps just call this
onframeratechangeto make clear that it's about the effective frame rate, not the target?
In the case of Oculus, the system still draws at the full frame rate. It's just the browser that is asked to slow down. Given that, I'm leaning toward ontargetframeratechange but I would be fine either way.
|
@thetuvix I updated the spec text a bit per your comments. |
|
Not clear from the spec (perhaps only for me), but can UA change If UA can internally change Also, what happens in this scenario:
If the described scenario is good, then for a developer to know what |
It's a read only attribute so the author can't use it to change the rate. They have to use The value of
Yes, this is a feature for advanced users and I can see that if there is a chance of confusion if they don't understand how it works. Do you have a suggestion how we can avoid that?
Yes, I would expect that it would happen like this. Should I add some wording to state that the XR System should try to match the frame rate that the author set? |
Hm... this is confusing to me, because
That would be good. It seems like if possible the system should always be trying to hit the framerate that the developer specified, because presumably they did so because their content works best at that rate. Obviously sometimes that's not practical, but it should always be the goal. |
Agreed - perhaps just call the event @cabanier: My concern is that their code would look like this:
Another risk is if the app codes its The advantage I see of making the update a promise-returning |
There was a problem hiding this comment.
Added some notes from the conversation on the call today.
Additionally, given the discussion about it being difficult to give developers actionable performance feedback, I wonder if it would be sensible to split this proposal into two parts:
- The
supportedFrameRatesattribute and theupdateTargetFrameRate()method. - Communicating the current effective frame rate and an event when it changes.
Mostly because it seems like the former is pretty well agreed upon at this point, while the latter still needs a bit of discussion and iteration.
index.bs
Outdated
| [SameObject] readonly attribute XRRenderState renderState; | ||
| [SameObject] readonly attribute XRInputSourceArray inputSources; | ||
|
|
||
| // Methods | ||
| undefined updateRenderState(optional XRRenderStateInit state = {}); | ||
| undefined updateTargetFrameRate(float rate); |
There was a problem hiding this comment.
From call: Should be updated to return a Promise<void> that resolves when the requested update has take effect (even if the effective frame rate does not match at the time.) We probably want the promise to reject if a value that's not present in supportedFrameRates is passed.
It would also be good to determine a way to allow developers to revert to a state of "no preference". Passing 0 may be effective here, though we explicitly don't want supportedFrameRates to enumerate 0.
index.bs
Outdated
| @@ -614,11 +614,14 @@ enum XRVisibilityState { | |||
| [SecureContext, Exposed=Window] interface XRSession : EventTarget { | |||
| // Attributes | |||
| readonly attribute XRVisibilityState visibilityState; | |||
| readonly attribute float? targetFrameRate; | |||
There was a problem hiding this comment.
From call: This value actually represents the effective frame rate of the UA/system, and should be distinct from the developer-provided target frame rate. The UA should never change the target framerate that the developer provides (unless it's been set to a "no preference" state, I suppose) thought it does not have to honor it.
I believe Alex mentioned something about exposing this on the XRFrame instead, which is something I would support as well.
The current target frame rate does not need to be reported by the session, since the developer set it and can track it on their own if needed.
There was a problem hiding this comment.
See #1201 (comment)
The UA can change or ignore frame rate changes so that author will need to know.
They can keep track of what rate they set and compare it to the one on XRSession but I'm unsure what they would do with that information.
index.bs
Outdated
| @@ -636,6 +639,7 @@ enum XRVisibilityState { | |||
| attribute EventHandler onsqueezestart; | |||
| attribute EventHandler onsqueezeend; | |||
| attribute EventHandler onvisibilitychange; | |||
| attribute EventHandler ontargetframeratechange; | |||
There was a problem hiding this comment.
From call: Whatever naming we choose for the targetFrameRate attribute above should also be reflected here, as this event indicates changes to the effective frame rate and not the developer-provided target frame rate. (Though the effective framerate may change in response to a change to the target framerate.)
No, this does not reflect the frame rate of the system; it's supposed to be the rate that the system wants the experience to run. |
Since it's the rate that the system wants the experience to run and not the system rate, we shouldn't call it so.
There aren't many attributes on XRFrame. Also, if it's put on XRFrame you can't control or read the frame rate outside of a Raf.
We covered this in the meeting and I will add a promise in addition to the event. |
I agree. I'll update the PR with the promise |
|
Do we really need an event/promise here? What are real cases where the developer needs to know that UA decided to render something at a different frame rate? The actual frame rate is well sufficient for it. I believe UA's ability to change Event when change happens due to UA, might be also not necessary.
Then what value does event/promise brings? Or even changing If UA changes the frame rate, this has nothing to do with the developer wanting of frame rate to hit the target.
This might lead to confusion, as from the naming, a developer could expect it to fire every frame, as almost no frame framerate is the same. So when it changes due to performance reasons from 60 to 59, will that throw an event, that is a "frame rate change".
The |
The feature is mainly meant for performance tuning.
Correct. It is a hint to help with performance/media playback
Indeed. Except in this case, the logic to determine the frame rate changes are made by the developer.
I'm unsure if it would be confusing but I agree it makes more sense on
Yes, I also didn't think the promise was absolutely necessary but other people felt strongly that is was. |
They are. By tracking actual frame rate (time passed since last frame). I'm afraid some people got confused by specifics of this proposal. Mixing "a hint for a desired target frame rate" and "actual frame rate". What was also suggested by @toji. What would be real world use case for tracking target frame rate changes? |
Unfortunately, that data is very noisy so it's not a good indication of the actual framerate.
It's not about tracking performance. This feature is designed to relieve the load on the system for experience that can't make framerate or have headroom to have a higher framerate.
I noted this earlier. |
Developers smooth out the data through previous frames. And make decisions of that.
That definitely will make more confusion. As it will not be an actual frame rate. I'm really keen to see an actual example of using events/promise, for updating frame rate, that is applicable to real-world applications. |
Those decisions would be incorrect. :-(
Does it matter it is not the actual framerate? All the author should care about is how quickly they should produce frames.
As opposed to polling the rate? |
There are various frame rates we could refer to in various places in the API here: (I'll use the names that I've been hearing in my own head)
I believe the confusion we've had in this discussion is whether 1 or 3 makes sense as "target frame rate". Happy to just call the function For an additional polling/event API to read 1, OpenXR worked around this whole naming concern around "frame rate" by calling it
The fact that the UA may never achieve the app's requested target frame rate is exactly the reason for the promise - apps that use it are less likely to fall into various traps:
I agree that Developers for whom the |
|
Thanks for this great writeup! Are you suggesting that we should drop the |
index.bs
Outdated
| 1. The [=XR Compositor=] MAY use |rate| to update the {{XRSystem}}'s frame rate. | ||
| 1. Set the |session|'s internal [=XRSession/framerate=] to the {{XRSystem}}'s recommended frame rate. It is up to the {{XRSystem}} to determine this recommended frame rate. | ||
| 1. [=Queue a task=] to perform the following steps: | ||
| 1. Await until the {{XRSystem}}'s frame rate has taken effect. |
There was a problem hiding this comment.
We should perhaps be more precise here that we are waiting until the XR compositor makes all changes it intends to make at this time in response to the request. This accommodates the case where the device is already at 30fps and stays there - no change was made, but the XR compositor is ready to respect that request when the temporary condition is lifted.
| 1. Await until the {{XRSystem}}'s frame rate has taken effect. | |
| 1. If the [=XRSession/framerate=] was changed, await until the {{XRSystem}}'s frame rate has taken effect. |
It might be worth trying that on for size, to see if it resolves some of the differing interpretations we all had where some folks (including me) assumed In the spec language, we could still define |
|
It would be still good to see a real-world concept for a promise/event handling and application logic. What would be the use cases. As discussion maybe went into promise/event path due to misunderstanding of the scope of this proposal ( |
|
The primary use case I recall hearing on the call:
For this scenario, the promise approach is cleaner than an event or polling approach for the reasons discussed above. Note that if this was the only scenario, it would be cleaner for the function to be |
That would not be possible, due to uncertainty of a platform, which cannot guarantee frameret set. In such scenario if promise/event happens always, it will happen next frame? As "UA acknowledged a hint". |
…frame rate, updated spec to account for them
|
The promise would be signaled when the request is completed, either honoring the request or not:
The purpose of the promise is to be an attractive alternative to the codepaths apps would be tempted to write without it - unlike the promise, those alternate codepaths are wrong on some devices. For example, without this promise, the app is likely to instead write code that polls to see when their specific target frame timing is actually achieved - on devices temporarily in some 30fps mode, they could be stuck waiting for minutes. The promise gives the UA a chance to signal that the requested frame rate has been logically achieved: the browser has taken note of the hint and may apply it later, but no change is coming within the next 1 second, and so you should just start your activity now. The app benefit of waiting to start video playback until the frame rate change has completed is to avoid playing the first few seconds of the video while the display is blanked out actively changing frame rates. If this was simply a case of starting while lower-res like with streaming video, the user could at least tell what's going on in the video for those few seconds - however, if some VR device needs to blank to change frame rates, the user might miss part of the video entirely. |
|
FYI I've been publishing the wip on https://cabanier.github.io/webxr/ |
Does this need to be specified by the spec? How does the developer know how long it can take? If there is a guarantee of some time limit, or it is up to a developer to code their own logic with timeouts? Is blanking - is a current behaviour by existing platforms when changing the screen refresh rates? |
The spec is silent how long it takes for any promise to be fulfilled (ie
What is "blanking"? |
I assume it is when the hardware is changing resolution/refresh rate, it can lead to the screen blinking to black and back. |
Indeed - and in the case of a permissions prompt, a function like In this case, we likely want to make a stronger spec guarantee around
Basically, we can write the spec language for the promise qualitatively around whether the XR Compositor is able to start effecting the frame rate change now (await that completion) or not yet (complete now).
Yes - that's what I'd meant. Displays are not always able to change refresh rates seamlessly with no visual artifacts - I recall from @cabanier that the app developers he was working with had a concern that they wanted to hold off on video playback until the transition to the new frame rate was finished. He can share more info on the specifics of what they were exactly worried about there. |
index.bs
Outdated
|
|
||
| <div class="algorithm" data-algorithm="update-frame-rate"> | ||
|
|
||
| The <dfn method for="XRSession">updateFrameRate(|rate|)</dfn> method passes the [=target frame rate=] |rate| to the {{XRSession}}. |
There was a problem hiding this comment.
I feel like there's a bug here because while this description states that the method is being passed the target framerate the algorithm doesn't actually set or store that anywhere, which makes it appear that this is setting the internal framerate directly with an opportunity for the compositor to veto, at which point any target frame rate expressed by the developer would effectively be lost.
Can we add an explicit target framerate slot to the session, like the existing internal framerate, that this algorithm sets to the value passed to updateFrameRate() as long as it's one of the supported framerates? That way it's made explicit that the target frame rate hint provided by developers doesn't simply vanish the next time the system updates it's internal framerate.
There was a problem hiding this comment.
I feel like there's a bug here because while this description states that the method is being passed the
target frameratethe algorithm doesn't actually set or store that anywhere, which makes it appear that this is setting theinternal frameratedirectly with an opportunity for the compositor to veto, at which point any target frame rate expressed by the developer would effectively be lost.
Does the line [=Apply the frame rate=] with |rate|, |promise| and |session| not use this rate?
It's indeed true that the value that the developer set is indeed lost. I believe we discussed that they could store this themselves if they wanted to
Can we add an explicit
target framerateslot to the session, like the existinginternal framerate, that this algorithm sets to the value passed toupdateFrameRate()as long as it's one of the supported framerates? That way it's made explicit that thetarget frame ratehint provided by developers doesn't simply vanish the next time the system updates it's internal framerate.
Yes, that sounds reasonable.
There was a problem hiding this comment.
Does the line
[=Apply the frame rate=] with |rate|, |promise| and |session|not use this rate?
It only uses it when the developer initially calls updateFrameRate() and will be overwritten the next time the system has to adjust the rate for any reason.
It's indeed true that the value that the developer set is indeed lost. I believe we discussed that they could store this themselves if they wanted to.
I don't care about being able to report the value the user set back to them. They definitely can track that themselves if they want. I'm just pointing out that the spec doesn't appear to require the implementation to remember the user-set target, which also implies that it wouldn't factor into any future frame rate adjustments.
I can see developers reading this and assuming that their expressed target framerate gets lost after the next time that onframeratechange is called, which may encourage them to constantly reset the target frame rate in response to onframeratechange events, which is probably not a pattern we want to encourage.
index.bs
Outdated
| [SameObject] readonly attribute XRRenderState renderState; | ||
| [SameObject] readonly attribute XRInputSourceArray inputSources; | ||
|
|
||
| // Methods | ||
| undefined updateRenderState(optional XRRenderStateInit state = {}); | ||
| Promise<undefined> updateFrameRate(float rate); |
There was a problem hiding this comment.
I know this was discussed earlier in the thread, but @Manishearth and I would prefer that this function be named updateTargetFrameRate() to disambiguate which frame rate it is setting and make it explicit that it may not be reflected in the frameRate attribute.
Closes #1193
Preview | Diff