-
-
Notifications
You must be signed in to change notification settings - Fork 743
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
Dual-Stack WebGL Runtime with WebGL2 to WebGL1 Fallback #5198
Conversation
Cc: @ibesora - make sure you don't do the same work as this PR does... |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5198 +/- ##
=======================================
Coverage 91.82% 91.82%
=======================================
Files 281 281
Lines 38786 38807 +21
Branches 6760 6764 +4
=======================================
+ Hits 35614 35635 +21
Misses 3044 3044
Partials 128 128 ☔ View full report in Codecov by Sentry. |
// Expand #pragmas to #ifdefs. | ||
|
||
function compile(fragmentSource: string, vertexSource: string): PreparedShader { | ||
/** Expand #pragmas to #ifdefs, extract attributes and uniforms */ | ||
function prepare(fragmentSource: string, vertexSource: string): PreparedShader { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a small rename and proper jsdoc comment on the function.
// Expand #pragmas to #ifdefs. | ||
|
||
function compile(fragmentSource: string, vertexSource: string): PreparedShader { | ||
/** Expand #pragmas to #ifdefs, extract attributes and uniforms */ | ||
function prepare(fragmentSource: string, vertexSource: string): PreparedShader { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a small rename and proper jsdoc comment on the function.
varying ${precision} ${type} ${name}; | ||
in ${precision} ${type} ${name}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now that we assume original shaders to be in WebGL2, we should update our attribute and uniform extraction/scanning code.
attribute ${precision} ${attrType} a_${name}; | ||
varying ${precision} ${type} ${name}; | ||
in ${precision} ${attrType} a_${name}; | ||
out ${precision} ${type} ${name}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ same as above
attribute ${precision} ${attrType} a_${name}; | ||
in ${precision} ${attrType} a_${name}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^ same as above
// WebGL1 Compat -- Start | ||
|
||
if (type === 'fragment') { | ||
code = code | ||
.replace(/\bin\s/g, 'varying ') // For fragment shaders, replace "in " with "varying " | ||
.replace('out highp vec4 fragColor;', ''); | ||
} | ||
|
||
if (type === 'vertex') { | ||
code = code | ||
.replace(/\bin\s/g, 'attribute ') // For vertex shaders, replace "in " with "attribute " | ||
.replace(/\bout\s/g, 'varying '); // For vertex shaders, replace "out " with "varying " | ||
} | ||
|
||
code = code | ||
.replace(/fragColor/g, 'gl_FragColor') | ||
.replace(/texture\(/g, 'texture2D('); | ||
|
||
// WebGL1 Compat -- End |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this behavior is moved entirely into transpileToWebGL1()
function in shaders.ts
to be executed at runtime (also see program.ts
changes).
if (isWebGL2(gl)) { | ||
defines.unshift('#version 300 es'); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a WebGL2 requirement in every shader source code, if we're on WebGL2.
I've added a few minor comments, looks good otherwise, thanks!! |
thank you for the code review @HarelM, I tried to address your feedback and I think it should be good to go now 🚀 |
@0xFA11 is it correctly understood that we still need, in addition to this PR, a proper abstraction of webgl1 and webgl2 as separate pipelines in order to better leverage webgl2? I mean, it must put certain limits on the set of webgl2 features that can be used, if there's a requirement to be convert it all back to webgl1 at runtime. |
You can use any webgl2 and assume your customers all have webgl2 or write webgl1 compatible code if they don't. |
Simply this:
But I'd like to add a few more senteces anyway. First of all, just to re-state, we don't need anything else to leverage WebGL2 features after this PR. So the basic idea behind this PR (and previous WebGL2 Shaders fix) was that, we should allow and choose WebGL2 shaders (with 300 es) over WebGL1 with a fallback if the canvas context was not initialized with WebGL2RenderingContext. Currently, all built-in shaders in maplibre-gl-js are written with simple WebGL1 compatible features. We do not have any WebGL2 specific shaders or features in the codebase. For instance, we wanted to use pixel derivatives in order to draw outlines on buildings edges (custom fill-extrusion) and wanted to make them super sharp, we needed WebGL2 shaders (version 300 es) for that. I would not suggest writing shaders with WebGL2 exclusive features for built-in shaders unless we completely drop WebGL1 — because it'd be a nightmare and arguably unreliable to port or emulate WebGL2 features on a WebGL1 stack. This PR, at least, pushed WebGL1 fallback to happen at runtime only when necessary, which means that we could simply ignore and drop support for WebGL1 in our custom projects without asking maplibre-gl-js to also drop WebGL1 support. Previously, this was not trivial because maplibre-gl-js was downgrading (transpiling) shaders to WebGL1 anyway and we needed to modify maplibre-gl-js slightly to run WebGL2 with our WebGL2-only shaders. (FWIW, there are several WebGL2 features which were available on WebGL1 with extensions over time but they're now just available by default without extensions in WebGL2, that's great too)
In general, we could implement 2 separate rendering pipelines for WebGL1 and WebGL2. It sounds attractive in theory but in practice, I see very little value in maintaining a full-fledged, separate pipeline just for WebGL1, especially since WebGL2 is mostly backwards compatible with WebGL1 anyway. Some of this line of thought was discussed in #3947 and @HarelM didn't like the idea of dropping WebGL1 entirely in the end. For us and our products, we wanted to write our custom shaders in WebGL2 environment with WebGL2 shaders, without needing to bring in other additional renderers on top (like deck.gl etc.). A great next step evolution for us would be WebGPU (because it's now also enabled by default on iOS 18.2 onwards) but that needs an entire separate discussion anyway. |
@0xFA11 , thanks for clarifying, I definitely think this is a good step, and I really appreciate some much-needed progress in this area after it has been a bit stuck for two years. My understanding is that the key motivation for this change is: A project can with this change decide to sunset webgl1, and leverage some webgl2-exclusive features that weren't possible before. |
That's correct @birkskyum and I can name a few: pixel derivatives, all vertex data in one buffer rather than chunks with 16-bit limit (e.g. for fill-extrusion vertices), MRT (Multiple Render Targets). There is simply no way to write WebGL1 compatible shaders for these things and now, we can write them side-by-side with built-in shaders and we simply do not have to support WebGL1 at all. |
Awesome! I think I know a webgl2-only project that'll be excited to learn of this! |
Launch Checklist
As a follow-up to #3947, this PR moves WebGL2 to WebGL1 shader conversion to happen at runtime depending on the initialized WebGL context from the canvas. (This PR also builds on prior work started here #2656 originally but implements it as an actual fallback rather than forced WebGL2 to WebGL1 conversion).
Put more simply, if canvas is running in a WebGL2 environment, we do not convert original vertex and fragment shaders from WebGL2 to WebGL1 — otherwise, if canvas is running in a non-WebGL2 environment (WebGL1 assumed), we convert vertex and fragment shader sources from WebGL2 syntax to WebGL1 syntax at runtime.
On browsers with no WebGL2 support (which are <2% globally), shader conversion will happen but for the vast majority of the user base (>98% globally), we will still be able to offer WebGL2 environment and capabilities within maplibre-gl-js.
This dual-stack approach will hopefully allow more sophisticated and modern shaders written directly in maplibre stack.
Include before/after visuals or gifs if this PR includes visual changes.Document any changes to public APIs.Post benchmark scores.CHANGELOG.md
under the## main
section.