search

iTwin / itwinjs-core

Monorepo for iTwin.js Library
https://www.itwinjs.org
MIT License
582 stars 210 forks source link

Change compatibility API to use WebGL2 for iTwin.js 4.0 #5111

Open markschlosseratbentley opened 1 year ago

markschlosseratbentley commented 1 year ago

Related to this work item: https://github.com/iTwin/itwinjs-core/issues/5018

Related to this PR: https://github.com/iTwin/itwinjs-core/pull/4999

That PR didn't modify the webgl-compatibility APIs, but we need to. Looking at the isTextureRenderable() path, we might as well remove the rest of the WebGL1-only stuff while we are at it.

pmconne commented 1 year ago

What would remain if you removed all of the WebGL-1-specific stuff? Bear in mind the compatibility checker is going to need to continue to be able to report compatibility for 3.x too.

markschlosseratbentley commented 1 year ago

What would remain if you removed all of the WebGL-1-specific stuff? Bear in mind the compatibility checker is going to need to continue to be able to report compatibility for 3.x too.

In the totally-public-facing WebGLFeature enum, there are some items that look like they would get reported still -- float rendering, antialiasing stuff, shadow maps. But a lot will go away.

We also use Capabilities for other things in the system, though, like checking if we can render to floats, which is what started me down the path of thinking about more broadly removing WebGL1 from this part of the code - since WebGL2 no longer requires WebGL1's quirky ways of checking that.

Two possibilities I see for the compatibility checker itself:

1) I mainly envisioned that the compatibility checker will use a 3.x version of iTwin.js to report compatibility for 3.x apps. A small change could be backported to the 3.x release of webgl-compatibility so it can know whether or not 4.x will be supported. Then the compatibility checker can be upgraded to use 4.x versions of the compatibility libraries once 3.x support is eventually dropped.

2) Otherwise, if we want the checker to be on the very latest iTwin.js (4.x) and be able to report both situations (3.x and 4.x), then we will need to disentangle some of this code. Some code that the internal system checks in Capabilities.ts needs to get updated properly to take into consideration that WebGL1 is gone, while it also can report stuff about WebGL1 still for the purposes of the compatibility checker letting people know about 3.x compatibility.

I am not sure which is better, but I feel option 1 would result in clearer code in 4.x -- allowing us to drop WebGL1 concepts everywhere. Suggestions welcome.

pmconne commented 1 year ago

I see this is "in progress" - what is its urgency relative to other priorities?

4999 already removed from core-frontend any usage of the WebGL-1-specific aspects of webgl-compatibility and changed IModelApp.queryRenderCompatibility to report "incompatible" if WebGL 2 is not supported; if WebGL 2 is supported, none of the WebGL-1-specific limitations will be reported as missing.

Is Safari still stupid about floating point textures with a WebGL 2 context?

markschlosseratbentley commented 1 year ago

This mainly stems from https://github.com/iTwin/itwinjs-core/issues/5018 which work began on investigating - you're right, lower priority. Moving back to TODO. There are a lot of moving pieces here.

Safari still does have some issues on certain iOS devices with full floating point, that is true, at least last I checked.