-
-
Notifications
You must be signed in to change notification settings - Fork 735
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
fix: Convert WebGL1 shaders to WebGL2 #5166
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5166 +/- ##
=======================================
Coverage 91.84% 91.84%
=======================================
Files 278 278
Lines 38336 38336
Branches 6714 6713 -1
=======================================
Hits 35211 35211
+ Misses 2996 2995 -1
- Partials 129 130 +1 ☔ View full report in Codecov by Sentry. |
b7f6305
to
8def403
Compare
Is there a lint rule we can add to make sure this won't happen in the future? |
unfortunately not at the moment but I'm planning to introduce webgl2 specific integration test on puppeteer in the future when we come back with webgl2 option to test against webgl2 shader compatibility. |
I thought about a test as well, but I wasn't sure if all the types of shaders were needed to make sure all the shaders are correctly written, which can increase the CI dramatically, which I would like to avoid. |
1 single additional puppeteer test case just for webgl2 sanity check wouldn't be too bad, would it? :) |
It wouldn't hurt 😁 |
Let me know if you want to add it here, if not I'll merge this. THANKS! |
I think we can merge this, I wouldn't add it here because currently there is no way to enforce WebGL2 only shaders, we will follow-up on our discussion over #3947 this week. |
Launch Checklist
We are already relying on WebGL2 to WebGL1 conversion at build time (
generate-shaders.ts
).However, some shaders are written in WebGL1 syntax but they should've been in WebGL2.
Removing WebGL1 Compat code in
glslToTs
function easily breaks maplibre because these shaders are not WebGL2 friendly atm.Link to related issues.Include before/after visuals or gifs if this PR includes visual changes.Write tests for all new functionality.Document any changes to public APIs.Post benchmark scores.CHANGELOG.md
under the## main
section.