avoid incomplete webp support in Edge 18

[ansis]

Edge 18 supports WebP but not uploading WebP images to gl textures. Avoid this bug by testing support for this before enabling WebP loading.

fix #7671

The one downside of this approach is that once per page load we create a gl context and throw it away. I haven't seen this be a problem though. We could wait until a map is initialized and use that context but that would be more complex.

Launch Checklist

• briefly describe the changes in this PR
• write tests for all new functionality
• manually test the debug page

We can't add automated testing because we don't do any of that in Edge. Manually testing the satellite map example does test this.

[asheemmamoowala]

@ansis This approach makes every browser pay the price for the check that only the Edge browser really needs. To mitigate the cost on document load (that would affect TTFR ever so slightly), could this supportsWebp be exported as a function and the support value cached after the first call?

It would also mean that if there are any issues with webgl context creation, they do not surface in this code.

[ansis]

I reworked the implementation to avoid creating a context and instead using the one already created for a map. That is slightly better but the implementation is more complex in order to properly deal with asynchness.

@ansis This approach makes every browser pay the price for the check that only the Edge browser really needs.

Yep, the goal is to avoid user agent sniffing and instead check for the capability.

could this supportsWebp be exported as a function and the support value cached after the first call?

In the first implementation the function gets run as soon as the mapbox-gl-js script gets loaded (once per page load). In the new implementation it gets run on map creation (to avoid creating it's own context). Is this what you meant?

It would also mean that if there are any issues with webgl context creation, they do not surface in this code.

Yep, this needs to change! I've changed it in the latest commit.

[ansis]

@ChrisLoer would it be possible to get a gut review from you? Does e88ecae or the current HEAD of the branch seem like a better way to check for webp support?

[asheemmamoowala]

In the first implementation the function gets run as soon as the mapbox-gl-js script gets loaded (once per page load). In the new implementation it gets run on map creation (to avoid creating it's own context). Is this what you meant?

I was thinking more that it would be a function that is invoked when needed by the usage in mapbox.js#normalizeTileURL. This method, from what I can tell is called from loadTIle on the main thread before passing to a worker. So there shouldn't be any synchronization issue with the way we use it.

[ChrisLoer]

Does e88ecae or the current HEAD of the branch seem like a better way to check for webp support?

I don't have a strong opinion. Can you write a tiny benchmark to get an idea of the cost of the create context and texture upload check? My guess would be it's pretty trivial, in which case we might as well go with the e88ecae approach just because it's simpler. But the "do it with the gl context provided by setupPainter" approach doesn't seem too bad to me if there's any measurable performance win. Is there some tiny chance that while webpImgTest is loading, the GL context could be lost/restored, and then the test would fail from having stored the old context?

[ansis]

@asheemmamoowala I think I like keeping the test method separate from the way of accessing the result because otherwise we'd have to pass a gl context through to normalizeTileURL which seems a bit more invasive.

@ChrisLoer It looks like context creation is measurable but not that expensive. So keeping this approach? I added a context loss check to avoid that bug.

[ChrisLoer]

🍏