-
-
Notifications
You must be signed in to change notification settings - Fork 335
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
Bug/stencil tiles #1871
Bug/stencil tiles #1871
Conversation
…her than just the tiles for which drawables are generated. Simplify tile ID handling. Remove calls setting default stencil options.
Bloaty Results 🐋Compared to main
Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1871-compared-to-main.txtCompared to d387090 (legacy)
Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1871-compared-to-legacy.txt |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1871 +/- ##
==========================================
+ Coverage 85.82% 85.84% +0.01%
==========================================
Files 568 568
Lines 27979 27973 -6
==========================================
Hits 24013 24013
+ Misses 3966 3960 -6 ☔ View full report in Codecov by Sentry. |
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.
Great! Not seeing either issue with this PR.
Fixes incorrect ordering of tiles rendered to the stencil buffer, causing low-zoom tiles to be rendered over high-zoom tiles in some cases, particularly between the completion of loading prefetch tiles (see
setPrefetchZoomDelta
) and their removal.This seems to be the root cause of #1845. ... and #1846
Also fixes incorrect use of
renderTiles
in the background layer and depth write options on fill drawables.https://osmus.slack.com/archives/C04SSR0S78B/p1700070024256889?thread_ts=1699920382.735459&cid=C04SSR0S78B