Skip to content
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

Does XRAnchor.detach() imply that the unattached anchor lives on? #34

Closed
thetuvix opened this issue Mar 18, 2020 · 8 comments · Fixed by #40
Closed

Does XRAnchor.detach() imply that the unattached anchor lives on? #34

thetuvix opened this issue Mar 18, 2020 · 8 comments · Fixed by #40

Comments

@thetuvix
Copy link
Contributor

Agreed on having an explicit method for destroying an anchor synchronously. However, I also agree with @blairmacintyre that the name detach() implies to me that this newly-detached object will live in on some meaningful unattached way, when in fact there's no longer anything meaningful you can do with that anchor.

I wonder if destroy(), drop(), forget() or simply stopTracking() (to align with trackedAnchors) makes it more clear when you'd call this method: when you're no longer interested at all in that anchor.

I'll make a specific issue for bikeshedding detach() so those with more web API experience than me can call out similar cases in other web APIs.

Originally posted by @thetuvix in #10 (comment)

@thetuvix thetuvix changed the title Does XRAnchor.detach imply that the unattached anchor lives on? Does XRAnchor.detach() imply that the unattached anchor lives on? Mar 18, 2020
@bialpio
Copy link
Contributor

bialpio commented Mar 19, 2020

The newly detached object is roughly as useful as the anchor whose tracking is forever lost - you could theoretically obtain an XRSpace out of it, but you'd never get a meaningful pose out of it. I'm not sure if we could argue that this makes the object alive in a meaningful way.

Additionally, in case of detaching an anchor, I have decided to throw an InvalidStateError if the applications decide to access its properties - the reasoning was that detaching an anchor is app-driven, so it should know what it already detached and not try to access the properties. This may influence the name we decide on.

@thetuvix
Copy link
Contributor Author

Yea, after you call detach(), it's like a zombie anchor - seems to be alive but will only bite you if you interact with it 🧟 We should find a name that makes that clear!

I do agree with the functional design of having implicit anchor loss from the UA side leave the object functional as a dummy object, to avoid timing issues, but having the object properly go dead if you explicitly call detach().

@bialpio
Copy link
Contributor

bialpio commented Apr 6, 2020

Given that we do not really have a way of consuming the object on which a method is called (Rust-style), delete(), destroy(), stopTracking() or detach() all sound reasonable to me, with varying degrees of preference.

Some more notes:

  • delete() would mirror for example WebGLRenderingContext.deleteBuffer() with the exception that it'd be an instance method instead of a method on an XRSession - this would keep it in line with the approach taken by hit-test (cancel() method). Also implies (to me) that the object that remains behind is not in a usable state.
  • destroy() to me is like delete(), but harsher. :)
  • stopTracking() - I like that it is clear that the anchor will not show up in trackedAnchors, but it does not suggest to me that the anchor object is in zombie state.
  • detach() - I initially chose this because I think of anchors as "pins" that tie virtual world to the real world and it seemed to me that "detaching" that pin made sense here. Suffers from the same problem that stopTracking() has.

I'd say that delete() should be the way to go here, unless anyone disagrees.

@thetuvix
Copy link
Contributor Author

thetuvix commented Apr 8, 2020

I had initially preferred destroy() to delete() because it somehow felt better as an instance method, but I can't really find much web precedent to back up my feeling 😄

In C#, there is an explicit pattern for an instance method that releases external resources: Dispose(). Is there any such precedent on the web? If not, delete() seems reasonable to me here.

@bialpio
Copy link
Contributor

bialpio commented Apr 17, 2020

There's also remove(), for example ChildNode.remove(). In general, based on the number of results that caniuse.com returns, there does not seem to be a well-established pattern for the name, both return similar amount of results, with small advantage going to remove (49 vs 51):
https://caniuse.com/#search=remove
https://caniuse.com/#search=delete

I still prefer delete() so if there's no opposition, I'll issue a PR soon.

@raviramachandra
Copy link

Not sure, if this is the right place. But from the spec it was not clear if we can arbitrarily call for "anchor removal" (Section 6) or should we call only within a RAF callback ?

@tencircles
Copy link

Destroying things on the web is pretty uncommon. I think childNode.remove() is perhaps not the best example. This just detaches the node from its parent, the node is still perfectly alive. Regarding delete, generally this is used in cases where an attribute is removed from a collection, not where something is being disposed of.

This brings up a good point however, disposing of things on the web is typically left to the browser implementation. Even web audio nodes are left up to the implementation to dispose of when they need to be garbage collected. To this end, would it be an option that rather than having to explicitly dispose of an anchor, it can be disposed of by the implementation after all references to it have been deleted?

@bialpio
Copy link
Contributor

bialpio commented Apr 17, 2020

Not sure, if this is the right place. But from the spec it was not clear if we can arbitrarily call for "anchor removal" (Section 6) or should we call only within a RAF callback ?

I think we don't have to require removal to happen within rAF callback - the implementation can immediately mark the anchor internally as no longer fully functional (to ensure that accessing properties is failing after it was removed) and notify the device as soon as it can.

This brings up a good point however, disposing of things on the web is typically left to the browser implementation.

In general I agree with the approach, but I think this is one of the cases where we should expose a method to proactively mark the object as no longer needed (there is a cost associated with maintaining anchors so the application has a way of reducing such costs early instead of relying on GC). There is also a precedent - the already mentioned WebGLRenderingContext.deleteBuffer(), which would behave similarly (and I was actually bitten by the assumption that GC takes care of things in a test site - all was fine except for 0.25s freezes every now and then because GPU was busy deleting 100's of buffers to which I just dropped references). Without proactive mechanism to delete the anchor, the site has no way to control the behavior (as web APIs are not supposed to expose the GC in any way, so there is no way to trigger GC collection) & runs into the risk of paying for something it no longer needs.

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 a pull request may close this issue.

4 participants