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

Remove LegacyNoInterfaceObject and expose extension objects #3366

Open
saschanaz opened this issue Dec 1, 2021 · 19 comments · May be fixed by #3381
Open

Remove LegacyNoInterfaceObject and expose extension objects #3366

saschanaz opened this issue Dec 1, 2021 · 19 comments · May be fixed by #3381

Comments

@saschanaz
Copy link
Contributor

The [LegacyNoInterfaceObject] extended attribute has been deprecated for quite some time and as of now WebGL is the only user of it. I think it's time to consider removing it here too.

@kenrussell
Copy link
Member

Sorry, I disagree. For the past 10+ years the WebGL working group has relied on being able to extend capabilities without polluting the global namespace. The extension mechanism was designed with great care, requiring applications to request exactly the capabilities that they desire. It's infeasible to accurately reflect the state of these extensions (i.e., whether they're actually supported on the current graphics card) via presence or absence of the extensions' type names in the global namespace, which would lead to great developer confusion if they were exposed.

Please do not remove this extended attribute from Web IDL.

@annevk
Copy link

annevk commented Dec 2, 2021

I think this warrants a longer discussion or at least some elaboration on the unique concerns WebGL faces that are not present for all other web platform APIs.

Searching for LegacyNoInterfaceObject in WebGL 1.0 and 2.0 yields no results. Where is it used currently?

@kenrussell
Copy link
Member

It's used in every WebGL extension in https://www.khronos.org/registry/webgl/extensions/ . The sources for these extensions are in https://github.com/KhronosGroup/WebGL/tree/main/extensions .

WebGL requires a way to return objects from its getExtension API that don't have named prototypes in the global namespace. If there is another Web IDL mechanism to achieve the same result, the working group will certainly consider moving to it.

@annevk
Copy link

annevk commented Dec 2, 2021

I see, that is quite a novel use case that I'm not sure got due consideration. I'd still appreciate if you could reopen this issue so we'd have some way to track this on the WebGL side as well.

@kenrussell kenrussell reopened this Dec 2, 2021
@kdashg
Copy link
Contributor

kdashg commented Feb 11, 2022

I discussed this some in one of our meeting calls, but I'll post about it here too.

I have found it useful in some of my webgl wrappers to be able to pre-emptively do prototype method overrides. However as it stands today, I always need to handle dynamically intercepting getExtension in order to hook functions on extension objects. If I could more statically override the prototype for extensions, that would save complexity, and match other web apis, including WebGLRenderingContext.

I think if the reason for being shy about exposing new prototypes is a fear of polluting the global namespace, we should just ask the owners of said namespace if that's ok! In particular, if e.g. the TAG would prefer to have new (global) prototypes in exchange for us giving up LegacyNoInterfaceObject, I think we should make that trade. :)

@annevk
Copy link

annevk commented Feb 11, 2022

cc @domenic

@domenic
Copy link
Contributor

domenic commented Feb 11, 2022

Yeah, from the perspective of HTML and IDL which somewhat "own" the global namespace, I have no concerns about polluting them, as long as the identifiers are unlikely to cause collisions with something we might use in the future. The global namespace exists in order for people to put things into it!

I think such collisions are not a concern here, at all, because the names are all things like EXT_texture_filter_anisotropic and not something like cookieStore or navigation that a future API might be interested in using.

@domenic
Copy link
Contributor

domenic commented Feb 11, 2022

If there is still any concern about such collisions or pollution, we have some informal precedents to follow, such as: CSSWG gets the CSS* "sub-namespace" reserved for them, or the HTML Standard gets the HTML*Element "sub-namespace" reserved for them, or WebRTC gets the RTC* "sub-namespace" reserved for them. In the sense that nobody would really ever think of creating another interface that matches those patterns, and the community has zero concerns about those groups introducing an unlimited number of new interfaces matching those patterns.

You could do something similar with WebGL Extensions, either by reserving the OES_*, WEBGL_*, EXT_*, ANGLE_*, KHR_*, and OVR_* sub-namespaces, or (at the risk of very-unlikely compat issues) renaming things to consolidate under something like a WebGLExt* sub-namespace.

@greggman
Copy link
Contributor

I hope this will come across as just another person's opinion and not in any way a negative judgement.

First question, will this make adding new extensions require getting them approved by more committees?

Otherwise, to me, WebGL is a 10yr old standard and changing it now feels like a waste of time. It's not just changing the IDL, it's all the stuff that comes later. Tests to prove they're exposed. Filing and responding to bugs on browser that don't implement it yet. etc...

While I agree that it would be useful to see the objects before getting the extensions, if we add this in now, in a year 30-50% of your users still won't have it so anything written for the next several years will require assuming they aren't there. If it was 2012 I'd be all for this but in 2021 what's really the point? People will have moved on to WebGPU by the time this is usable without giving up too many users.

We have finite resources. Let's spend them wisely. Even though this is relatively small, small things add up and every little distraction is one less other thing done.

@saschanaz
Copy link
Contributor Author

Tests to prove they're exposed.

This should be done automatically by WPT idlharness.

Filing and responding to bugs on browser that don't implement it yet.

I can do that if no one else does. Fortunately there are only three major engines.

etc...

I'm not sure there can be anything else, unless this ever turns out to be a webcompat issue?

kdashg added a commit to kdashg/WebGL that referenced this issue Feb 12, 2022
Rename prototypes like EXT_frag_depth => WebGLExtensionExtFragDepth.
Remove `noobject:true` xml annotations.
Leave some proposals and all rejected extensions untouched.
Closes KhronosGroup#3366.
@kainino0x
Copy link
Contributor

kainino0x commented Feb 24, 2022

Chiming in with one more argument against this change: it's misleading when it comes to feature detection. WebGL extensions are hardware-specific, not browser-specific - the browser would "expose" all of the extension interfaces it knows about, regardless of the underlying hardware.

If extension interfaces are exposed in the global scope, people will inevitably write the standard feature-detection line if (typeof EXT_texture_filter_anisotropic !== 'undefined') and think that means there is support, when there is not.

WebGPU has no optional-feature-specific objects yet, but it will, and when it does I would strongly consider using [LegacyNoInterfaceObject] for this reason.

I get the desire to reduce the maintenance cost of keeping this attribute around, but I also think it is valuable despite the fact that most APIs don't need it.

@domenic
Copy link
Contributor

domenic commented Feb 24, 2022

I don't think that argument holds very much; plenty of hardware-specific APIs are exposed, and not hidden using [LegacyNoInterfaceObject]. Hardware-specific feature detection is done via separate APIs, not via interface testing.

@kenrussell
Copy link
Member

On the contrary, I think @kainino0x 's argument is sound. There are currently 38 shipping WebGL extensions and many of them are exposed only on certain classes of hardware - for example, the compressed texture formats, which diverge between mobile and desktop.

The WebGL working group made a conscious design decision over 10 years ago to not expose these interfaces in order to ensure feature detection worked correctly, and that decision has worked well - all existing WebGL applications properly detect, enable, and use extensions. Many C/C++ OpenGL applications incorrectly feature detect and use said extensions, causing them to only work on certain operating systems, or even certain vendors' graphics cards, whose mechanisms for no-op'ing unsupported extensions differ from others.

On our working group's conference call today and subsequent discussions, I and others remain of the opinion that we would prefer the Web IDL group reconsider its position on supporting this feature in the long term.

@domenic
Copy link
Contributor

domenic commented Feb 24, 2022

OK. It's already been reconsidered several times; using the presence of absence of interface objects as a feature detection mechanism is not supported by Web IDL and won't be going forward.

@kainino0x
Copy link
Contributor

What does "not supported" mean? It's detectable, and a clear signal of whether an interface is implemented/exposed by the browser. For example I would definitely use typeof OffscreenCanvas !== 'undefined' to detect whether the browser has OffscreenCanvas support (which is not hardware dependent).

@domenic
Copy link
Contributor

domenic commented Feb 24, 2022

It may or may not stay as a clear signal of whether an interface is implemented/exposed by a browser, e.g. as new contexts are introduced, or new models of toggling features on or off are introduced, or restricting them in different contexts (e.g. contexts which don't meet certain security requirements).

Of course, you can indeed count on a browser which has no code in its source tree for a given feature, evaluating typeof InterfaceName === 'undefined' as true. But that's the only invariant we plan to uphold.

Similarly, as a deprecated construct and not-architecturally-correct way of providing/preventing feature detection, I suspect using [LegacyNoInterfaceObject] will make it harder for your spec to pass various reviews, e.g. TAG reviews, Blink API owner reviews, etc.

@kdashg
Copy link
Contributor

kdashg commented Feb 24, 2022

We had an in-depth discussion of this topic in our meeting today:

WebGL Meeting 2022-02-24 Minutes * KG: if Dominic agrees - if we just pick a prefix and go with it - we can do that * Agree we don't want to go through 2 standards bodies * KR: please reserve all the sub-namespaces - OES_*, WEBGL_*, EXT_*, ANGLE_*, KHR_*, and OVR_* * KG: I prefer to prefix things with WebGLExt* * GT: if you change the names of the prototypes it'll break code I wrote. * KR: pretty sure in Chrome these come out as e.g. OESTextureFloat * E.g. “Rename prototypes like EXT_frag_depth => WebGLExtensionExtFragDepth.” * KR: why not make the prototype names exactly the same as the extension names requested by the API? * KG: less webby - prefer 1 prefix * Autocomplete in DevTools won't work well with EXT_ prefix * KR: can't readily enumerate the extensions * KG: what about WebGLExtension_…? * KR: kind of verbose - better than nothing * KG: Today, Firefox tells you it's the underscore name, and Chrome tells you it's the camelCase name… * WebGLExtension_EXT_frag_depth * WebGLExtension_WEBGL_debug_renderer_info * KK: in principle, doesn't sound like a technically hard thing - more is this a direction we want to go policy-wise * Apple doesn't have a signal for the browser supporting the feature but the device not supporting it. * Not entirely convinced by argument that developers might feature detect by presence of the interface, and fail to request the extension * From our perspective, we're undecided * We have bigger things to worry about in the WebGL space. It would be simpler to not do anything at the moment if this isn't blocking anything big. * Still don't understand what would break if we don't do this. * KG: people who maintain Web IDL who are trying to remove [LegacyNoInterfaceObject] will be sad. * KK: we use it a lot. I'm not sure if it's something where we haven't updated things in WebKit, or if these are internal / testing interfaces. Didn't seem like that. I'm not sure the argument that someone wants to streamline their tooling should be the driving force of the spec work. * KG: 2 people from Mozilla who did the original post on #3366. WebGL right now is the only user of it. * KK: I can take an action item to dig more deeply into this and comment on #3366. But I don't fully understand - if the direction is that the global namespace is for things to be put there - not the strongest argument for making a change. Helpful for me to understand who are the people making the request, and what the implications are. * KR: frustrated that the same spec people have pushed back on exposing e.g. ImageBitmap creation options in a way that is enumerable. * Also: process issues around shipping new WebGL extensions. See [crbug.com/1048907](http://crbug.com/1048907). * Add comment about WebGL extensions' development process. * https://chromium-review.googlesource.com/c/chromium/src/+/2040445 * Arrived at after long discussion with Blink developers. * RC: if we put these in the global namespace, nobody can just "new" up a new extension object? * KG: no. You have to use a Web IDL constructor annotation. * KK: what about the other functions that have constructors? Call it even though you don't have the extension instantiated? * KG: they fail if you call them on the wrong prototype. * KK: depends on the annotation? Some functions are generic, some you can only call with "this" of the correct type? * KG: you mean statics? I don't think we have any of those. * KK: need to ensure if you capture a function off the prototype, you can invoke it against the extension object later when you enable and fetch it.

Some of the main points, in no particular order:

  1. There is a risk that people will start misunderstanding anew how to test for extensions.
  2. This would allow pages to realize "the browser (generally) supports this extension, but it's just not exposed on this machine"
  3. Code that people write to recognize a particular extension (e.g. instanceof) would work reliably across browsers
  4. Additions into the global namespace are more difficult within some browser engineering teams' processes than others.
  5. This is non-zero amount of work, and would be comparatively low priority to implement.
  6. We've come this far, and this hasn't been a deal-breaker that we're aware of.
  7. WebGL would no longer be (one of?) the only remaining users of this deprecated annotation.
  8. We don't want to have to go through two different working groups in order to draft and extension and then add it to the global namespace. (but it sounds like we don't have to do this)
  9. There should be a reasonable mapping from the string we pass to getExtension and the name of the returned's prototype.

Tentative resolution is that I will switch the PR to WebGLExtension_EXT_frag_depth, so that it's easier to programatically construct the prototype names, and we will cautiously move forward, with the understanding that this is unlikely to be a priority, but that it's probably the right thing to do on the balance.

It'd be cool to get a confirmation that "8" shouldn't be a problem as we move forward with future extensions. @domenic @annevk ?

Also can we get quantification of "7", and whether this is a "technically useful" thing to drop, or just "probably the right thing to do". @saschanaz (e.g. does this help anyone out on the generator/bindings side? Webidl (spec) side?)

@annevk
Copy link

annevk commented Feb 25, 2022

Do you go through two working groups to add an interface today? I'm not sure I understand. But generally speaking interface additions are relatively uncoordinated and each standards group can make them. (Cross-review can still be good though and the more generic the name the more review you want to seek.)

whatwg/webidl#1072 suggests WebGL is indeed the holdout.

@domenic
Copy link
Contributor

domenic commented Feb 25, 2022

I agree I don't understand what two working groups are being referred to in 8, or why any working groups besides yourselves would be involved in drafting new interfaces (including extension interfaces).

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.

7 participants