-
Notifications
You must be signed in to change notification settings - Fork 377
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
XRFrame attributes for an inactive frame #1207
Comments
Seems like there is no holy-grail solution here, and it varies on the scale of giving to devs responsibility (unsafe) or prevent them from shooting the foot (safe). Unsafe:XRFrame provides pointers to objects, arrays, sets, etc. Which are "live" objects. If the reference is the same across the frames, then I believe it is not a problem, that data on those objects is updated to the most recent. But it is not historical (unsafe). As mentioned before: if the spec states clearly that XRFrame should be used only within the same tick, and not persisted for long-term use, does not restrict devs from misuse though. Safer:Another option would be to make XRFrame defunct as you've mentioned and nullify all the pointers to live objects. Making it clear that data is not available anymore. This leads to code branching, and sometimes not obvious logic "why it is null now?". Safe:Throw an exception when accessing live-object references on outdated XRFrame, it would also educate through exception message, basically: don't use an outdated XRFrame. This prevents code branching and helps devs earlier. All options seem fine 😄 |
In retrospect, it was a mistake to have these as attributes and we should have a function instead or they should be on
It's weird the that arrays are live objects. I see that they are defined as
I think it's the latter. For instance, we already recycle |
I discussed this with @toji today and he has some thoughts about this he plans to post but just to clarify one point: the reason The general pattern is that methods should error on invalidity, attributes should not error on invalidity but if you want them to be invalid when the producing object is invalid they should store a reference to the object (e.g. the |
If you tear it off, you don't expect that it will change over time. |
Sorry, what I meant was that the objects contained in the arrays (well, sets in this case) are live, but the containers themselves are not live.
Ouch, in that case those ( |
I see that the change to re-use |
The XRFrames can / are recycled I believe - the spec mandates recycling by reusing animation frame that is set on an XRSession. Alright, so given the above, I think we are forced to go with option 3: we will keep returning a collection that has the same contents for as long as it is possible (so when an XRFrame is recycled, the app will start getting latest state). It'd mean that when the attributes are queried from an inactive frame, the app will get the state that was valid as of last time that particular frame instance was active. This leaves the footgun in place, so I wouldn't mind just saying that all accesses (be it a method call or attribute access) on inactive XRFrames throw InvalidStateErrors (or at least log a diagnostic to console) to save the devs potential grief if others think it may be a good idea to do. |
So in looking through this with Manish earlier, a distinction that I noticed between What makes TL;DR: If an API object is created external to the frame then there's not much sense in pretending it's not a live object and the frame can reference it as needed. If the API object is only surfaced by the frame, though, it may be best to treat them as immutable.
This is still practical, even given the above. If the |
That's correct.
The main reason for returning "live" objects instead of immutable object every time we're being asked for it is that we'd then have to expose new attribute ( Quick example: // ... in rAFcb ...
let previous_planes = ...; // obtained in previous rAFcb or input source event
let some_interesing_plane = previous_planes[i];
if (frame.detectedPlanes.has(some_interesting_plane)) ... // yay, a plane we want is still present! Compare to: // ... in rAFcb ...
let previous_planes = ...; // obtained in previous rAFcb or input source event
let some_interesing_plane = previous_planes[i];
let current_plane_ids = new Set(frame.detectedPlanes.map(plane => plane.id)); // boilerplate?
if (current_plane_ids.has(some_interesing_plane.id)) ... // yay, a plane we want is still present! Additionally, (feel free to move this part of the discussion to plane detection repo) In any case, I think we're fine leaving the XRFrame spec'd the way it is now in the core spec (seems like it's equivalent to option 3). I'll need to make sure the anchors and planes specs have their frame update algorithms written correctly. The things that I'd propose we consider for core spec would be:
|
/agenda to figure out what to do here |
From today's meeting: I think this issue is no longer that big of a concern given that the spec mandates that the XRFrame is recycled, and other comments in this thread. At the very least, putting my "implementer" hat on, I know how to make progress to ensure the implementation is compliant. Putting on my "spec editor" hat: to reiterate my previous reply, I think we should consider adding a recommendation ("SHOULD") for the user agents to log a diagnostics message when a live object is accessed while a frame is inactive. We guarantee that objects are in "valid, but unspecified state" when the frame that produced them is not active, but accessing them can lead to interop risks I think. |
During implementation of one of extension modules, we've run into a situation where it seems reasonable to either throw or return a default-constructed value from an XRFrame that is no longer active (examples are
XRFrame.trackedAnchors
andXRFrame.detectedPlanes
, along with already-existingXRFrame.session
).My read on the current behavior is that there is no special-casing of what happens when XRFrame attributes are accessed for an inactive frame, so e.g.
XRFrame.session
would just return an instance ofXRSession
irrespective of the frame's state (even when the session has already ended). TheXRFrame.trackedAnchors
/XRFrame.detectedPlanes
are a bit more complicated, and I think we could take one of the 3 approaches here:Ideally, approach 3 is something we should go with, but the problem with this is that the instances present in the setlike are all "live" objects that could be changing, so the applications would have no guarantee about the state of those objects when inspecting them outside of an active frame. This is less of a concern for anchors since they only contain XRSpace that is useable only during active frames, but is very relevant for planes. The only useful information the apps would be able to get would be to observe how the composition of the sets have been changing over time (e.g. they'd be able to inspect which anchor objects have been present in
frame_n_minus_ten.detectedPlanes
set & see if those same objects are inframe_n.detectedPlanes
set). Accessing a specific attribute on the "live" objects would return the most recently available data for that object, not the historic data that the app could reasonably expect to get (hence, The Footgun).So the specific question is: what do people think the approach to the above problem for those attributes should be? Did I miss some other viable option?
And more general question is: maybe we should start treating inactive XRFrames as defunct objects which do not return anything useful? Is there any use case that I am missing that actually takes advantage of inactive XRFrames, or is this just a side-effect of having to operate in JavaScript which allows the apps to store a reference to XRFrame in a variable that's accessible in a wider scope, outside of our callbacks? If the latter, it may actually be viable to throw on inactive XRFrame attribute access.
The text was updated successfully, but these errors were encountered: