VRLayer should be a real interface and not a Dictionary

[esprehn]

The API exposes getLayers() which returns a sequence, and the API would also benefit from allowing real methods on VRLayers. In general returning dictionaries has a bunch of drawbacks, lets use real interfaces instead. :)

@tabatkins @ojan @bfgeek

[DigiTec]

What APIs would be useful to add on VRLayer? Can you give some examples. This would help inform the overall design. Specifically for backwards compat we have to continue to take the dictionary in requestPresent, but could also take an array of "some other object" that the user could construct (but then we'd also have to add constructors).

[cvan]

I'm thinking something like this:

window.addEventListener('vrdisplaypresentactivate', e => {
  if (!e.display) { return; }
  if (e.reason === 'navigation') {
    e.display.requestPresent([
      {source: webglCanvasVRContent}
    ]);
  } else if (e.reason === 'mounted') {
    e.display.requestPresent([
      {source: webglCanvasVRContent}
    ]);
  }
});

window.addEventListener('vrdisplaypresentblur', e => {
  if (!e.display) { return; }
    e.display.requestPresent([
      {source: webglCanvasVRPauseScreen},
      {source: webglCanvasVRContent}
    ]);
  }
});

[tabatkins]

@DigiTec Elliot is talking about the methods that return a VRLayer dictionary. This results in you returning a plain JS object with values set on it as data properties. Instead using an interface type for return values gives you a more standard sort of object, which has getters/setters on the prototype, and which can actually have methods added to it in the future. (We should make returning dictionary types be an error in WebIDL instead; they're never a good idea.)

But also:

Specifically for backwards compat we have to

Uh, what backwards compat? There's no shipping API to be back-compatible with.

[toji]

Uh, what backwards compat? There's no shipping API to be back-compatible with.

True, but there's a non-trivial amount of content built against experimental builds and flagged features. We can break it when necessary, (and have a few times) but in general we'd like to be as kind as possible to developers who have invested in the feature early.

So to clarify, even if we continue accepting dictionaries in requestPresent it would be preferable to return an actual interface from getLayers that represents the same values, correct? I don't have a problem with that. I'd like to continue consuming dictionaries, though. This particular mechanism is intended to be used as an API extension point in the future, so flexibility in what it accepts will be beneficial.

[DigiTec]

Still, we haven't shown an example of getLayers() returning a "Platform Layer Object" and what we'd put on it. What controls do we want to add to layers? Why would those controls not simply be future calls to requestPresent with an updated dictionary?

I'm expecting someone to come back and start talking about threading ;-) That would perhaps convince me in the direction of a platform object for a layer. Since it may be necessary to represent a layer which was attached on the main thread in some sort of background thread or worker.

For now, the capability in requestPresent is already enough. If you getLayers(), modify the returned dictionary, and then pass it right back to requestPresent, you can update your layering state. This should be relatively rare.

I think our biggest value add with layers is being able to establish a retained mode composition structure, with some basic rules, for when the main thread or rendering threads are under pressure but the VR composition thread can continue to run. The current API already allows for this, once we define what those extra layers are and how the rules work. For now, we have a restriction of only a single layer, which we'd likely relax once we start building 1.2 or later versions of the API.

[tabatkins]

True, but there's a non-trivial amount of content built against experimental builds and flagged features. We can break it when necessary, (and have a few times) but in general we'd like to be as kind as possible to developers who have invested in the feature early.

While I feel you, the numbers are extremely trivial compared to our normal thresholds for breaking things. It's okay to not change without without reason, but the tech is definitely still in its early incubation stages where we can change anything about it without worrying about web-compat. (Origin trials and experimental flags are not a way to ship early; they're a guarantee that we will break you, with the benefit that you can influence how the tech develops and ensure it meets your needs.)

I'd like to continue consuming dictionaries, though.

Yes, consuming dictionaries is A-OK. They were originally designed to be consumed! The ability to return a dictionary, tho, is an anti-pattern that I, Elliot, and others think should be backed out of the spec; it's a footgun that makes your API harder to use-counter and harder to extend in common ways.

Still, we haven't shown an example of getLayers() returning a "Platform Layer Object" and what we'd put on it.

Right now, it would just look exactly like the dictionary. The difference is just in details of how the object works - returning a dictionary produces a plain JS object with data properties, while returning an interface produces an object with a named prototype with getters and setters instead.

There are a lot of benefits to the latter:

• getters and setters are vastly easier for us to do Use Counters on, because we can just insert them into the getter/setter code; with plain objects you have to instead return proxies, which'll slow down data access significantly.
• named prototypes mean users can type-test - they can tell later on whether something is a VRLayer or something else with x instanceof VRLayer. You can't do this at all with plain objects; the best you can do is a ducktyping check, seeing if the object has all the expected properties on it.
• named prototypes mean we, or users, can add methods to the object in the future. Adding methods to things is very common as APIs develop. Authors like being able to do this sort of thing too; they regularly innovate in surprising and insightful ways. Plain objects can't easily handle this: if we want to add methods to it, we have to either generate function objects for every single object (rather than a single copy on the prototype) or put the method on a namespacing object as a plain function and have it take the object, which is a very foreign pattern in JS (we only do it with a few things, like Object methods, for special reasons (so the methods don't show up on literally every object in the world)); if authors want to add methods, they need to either do the namespaced-function thing, or wrap the function that returns the objects, so they can instead return wrapper objects.

This isn't a request for any particular functionality at the moment. We've just experienced a lot of APIs, and this is a foreign pattern with a lot of potential downsides.

[toji]

Thanks for the background and clarifications. I don't see any problem with this. I'll try to put up a pull request for it later today.

[esprehn]

Awesome!

[toji]

Pull request (#122) is up, I'll merge it in the next day or so if nobody has any strenuous objections.

[annevk]

I don't really think there's agreement on returning dictionaries being an anti-pattern. @domenic @bzbarsky @tobie? If it is, we should change IDL, but I'm not particularly convinced by the arguments in #107 (comment).

[tobie]

I don't really think there's agreement on returning dictionaries being an anti-pattern. @domenic @bzbarsky @tobie? If it is, we should change IDL, but I'm not particularly convinced by the arguments in #107 (comment).

Do we examples of specs that do so already? What's the benefit over returning interfaces?

[annevk]

I'd think the main rationale would be that returning a class (for which you'd maybe have to define a constructor and such too if you want to "explain the platform") is rather heavy-weight when you just need to expose some properties. I haven't found an example, but I believe there are some.

[tobie]

Seems that benefits the editor at the expense of platform-consistency, no? Or are there other benefits that I'm missing?

[domenic]

In general there are definitely places in the platform where methods should return dictionaries (or "POJSOs", plain old JS objects). Any place where the object being returned is not an important entity in the domain, but instead just a "record" (in the informal sense, not Web IDL sense) of interesting fields, would be such a place. A good test would be if you could ever see the class growing a method.

Examples would be anything that reflects a set of options (e.g. a hypothetical el.getEventListenerOptions(f)), or serializes a data structure (I think Web Crypto has a lot of these), or cases where instead of/in addition to two separate methods you want something that returns a tuple (e.g. decode(bytes, fallbackEnconding) -> { string, usedEncoding } instead of separate usedEncoding(bytes, fallbackEncoding) -> usedEncoding + decode(bytes, fallbackEncoding) -> string).

I don't have enough background in this spec to know whether VRLayer is an important domain entity that deserves its own class, but just from the name, it kind of sounds like one. I just want us to be cautious of issuing a general prohibition against dictionary return values.

[DigiTec]

This was my point earlier. Is there anything we can think of to add to VRLayer that we think has a high probability of sticking that would make it benefit from being a platform object.

The only thing I've come up with is an toggle method that allows me to asynchronously turn layers on and off, even if I'm unable to submit a frame. This could be handy to show an overlay if I detect I'm blowing my frame budget, but arguably there are other ways to achieve this which might have better API ergonomics anyway.

Specifically, I think that getLayers() is fairly constrained in usage. Today you would use it for debugging, but be unlikely to use it for anything else. Why? Because you already know the layers, you submitted them with requestPresent. What would you do with the layers returned other than printing them to the console?

I'd almost support removal or deprecation of getLayers before I'd propose we move to returning another new object, which isn't free since it exists in the global namespace and therefore takes up a global slot, a constructor and a prototype object. Gimme my microseconds of init time back ;-)

Now, totally convinced if we have a valid use case. Let's hear one!

[DigiTec]

Note: I say this and I'll be talking extensively on potential updates to the VRLayer dictionary. During which I've not found any reason to add a platform object for it instead of its existing form.

[bzbarsky]

I'm a bit late to the party, but I wanted to emphasize that there is a perfectly good use case for returning a dictionary type: if you have some sort of "configuration options" dictionary and just want to hand out what your current configuration is. This is what webgl does, for example, and it seems totally reasonable.

As far as examples of specs or drafts that return dictionaries...

• Anything using iterables (setlike, maplike, etc): under the hood the iterator next is returning a dictionary, fundamentally. Not sure whether people want to count this.
• http://w3c.github.io/web-animations/#animationeffectreadonly getComputedTiming.
• http://w3c.github.io/encrypted-media/#mediakeysystemaccess-interface getConfiguration.
• https://w3c.github.io/mediacapture-main/getusermedia.html#idl-def-mediastreamtrack getCapabilities, getConstraints, getSettings.
• https://w3c.github.io/mediacapture-main/getusermedia.html#idl-def-mediadevices-partial-1 getSupportedConstraints.
• Things with custom serializers, though they use a dedicated syntax right now. But we're considering maybe getting rid of that syntax and just having APIs that need a custom serializer (e.g. https://w3c.github.io/push-api/#idl-def-PushSubscription) define a toJSON method which would then need to return a dictionary. That's how Gecko implements this API, fwiw.
• http://w3c.github.io/webrtc-pc/#interface-definition getConfiguration.
• https://fidoalliance.org/specs/fido-u2f-v1.0-nfc-bt-amendment-20150514/fido-u2f-javascript-api.html#high-level-javascript-api -- ignoring the invalid IDL, they are defining callback methods which you pass in which the browser later calls with a dictionary argument, so this is basically an async dictionary-typed return value.
• https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14 getContextAttributes.
• http://w3c.github.io/webrtc-pc/#rtcrtpsender-interface getParameters.
• The VRLayer thing under discussion in this bug.

That seems to be it for things Gecko implements that have actual public specs; we have various internal-ish APIs that return dictionaries too, typically describing the state of various objects.

[annevk]

@toji why did you change this? I feel like you've just ignored a bunch of feedback.

[toji]

I changed it because I got positive feedback on the pull request. I've been drowning in email recently, and so it's not easy to keep track of all the various conversations around a feature. Sorry. Not my intent to ignore anybody.

I've reverted the change for now because there are several reasonable arguments against it in this thread, but to be blunt I simply don't care which way this goes. There are far more pressing issues to be addressed in the spec, and this proposed change doesn't fundamentally change how users interact with the API for better or for worse. As a result if it looks like we're going to be blocked from shipping by the platform owners because of it I'll push the change regardless.

[annevk]

That does not sound like a good plan to resolve this. What if platform owners from a different engine pull the same trick? Meanwhile, would be good to reopen this.

[RafaelCintron]

Dictionaries for configuration properties work great if the properties are fairly independent of one another and have reasonable defaults. If we expect to have additional types of layers with properties that are very different from the ones we currently have, then I think it makes sense to have VRLayer be an interface and for requestPresent to, thus, take a sequence of different kinds of layers.

I agree with @DigiTec about getLayers. Since the web developer passes in the layers they want to present with, I do not see a need for getLayers.

[toji]

Closing since this is planned to be addressed in 2.0. If anyone has issues with the 2.0 proposal, please file a new issue. For details see: https://github.com/w3c/webvr/blob/master/explainer.md#appendix-b-proposed-idl

[bzbarsky]

Wait, I don't follow. Changing it to an interface later will be an incompatible change. @toji why do you think that change will be possible to make at all?

[toji]

Sorry, to clarify: We're not intending to change the existing behavior in 1.1, but in the 2.0 version of the spec it will be an interface from start. 2.0 is intentionally backwards compatibility breaking in order to allow us to address exactly these sort of issues.

[bzbarsky]

Yes, but how does that work in practice? A page gets a VRLayer, it tries to work with it, that fails because it doesn't have the properties expected. Similarly, if some other API gets specced that takes VRLayer as input suddenly it will be getting handed totally different objects.

I don't think you understand that issues I'm having here if you think that intentionally changing the meaning of and IDL identifier will address them....

The "page gets a VRLayer" thing might be addressed if getLayers ceases to exist, I guess.

[toji]

I'm afraid I may not be understanding the issue. Is the primary concern regarding name collisions? That's something I've brought up in #192, but as @slightlyoff points out in that issue it's not clear that we should be avoiding name conflicts between a deprecated version of an API and the version we intend to replace it.

It does sound like Mozilla is planning on having both APIs available simultaneously (Google is not, nor have I heard as much from the other vendors) and if that's the case then we'll want to take care to avoid naming conflicts. I tend to see this issue as primarily focusing on the API shape, however, and not the exact interface name, which is why I consider it to be addressed in 2.0.

[bzbarsky]

My understanding is that both APIs would be available at the same time, possibly with a transitional period, across various browsers. Obviously if something is not shipped it doesn't matter what it is or how it's specced.