feat: add rasterPixelAlignment map option - #8206
burmatov-step wants to merge 2 commits into
Conversation
|
Thanks for taking the time to open this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8206 +/- ##
==========================================
- Coverage 93.94% 93.94% -0.01%
==========================================
Files 290 290
Lines 24917 24922 +5
Branches 6576 6578 +2
==========================================
+ Hits 23408 23412 +4
- Misses 1509 1510 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Sorry, that was too much text. I've rewritten the description. |
|
Can you share a video? |
|
Video attached above. Yes, I'd call it a bug. The snapping is intentional; the fact that it applies to one family of I picked the flag because it's the one version that provably can't regress anyone — the default |
|
Thanks for the video, now I think I understand the bug better. |
|
I would like to see more analysis on the commit history to understand if there's reasons we're missing as to why it's been done like this. |
|
Any updates on this? |
|
In my opinion, these AI assisted PRs without a proper analysis of the change, should just be closed, or converted to an issue for further discussion. Anything can trivially be proposed, so the change should have analysis of why the change should be made, which this PR is lacking. From my own analysis, 2e97715 (mapbox/mapbox-gl-js#6150) was the change that made the raster alignment only occur after the map becomes idle, to prevent visible jitter due to alignments during an easing transition as before that change it would continually align during the map movements. The continual alignment was done to address mapbox/mapbox-gl-js#6026 to address blurry raster tiles at integer zoom levels. This proposed change would allow users to avoid the jitter on idle in exchange for accepting blurry rasters at all times. https://jsbin.com/pokutuzuyi/edit?html,output demonstrates this
left is with the pixel alignment, right is without. While I can see why some users might choose to accept blur for less jitter, they are still both bad options. I'd like to see wider views on if this is something worth having a dedicated option for. |
|
Let's leave a side the discussion around the PR and the AI usage for a minute. The problem statement is clear to me, there's a jitter at the end of the movement, which originally was designed this way. Changing the default, as you mentioned, is probably not the right solution, so a flag to allow controlling it, is the only viable option, as far as I can suggest. @burmatov-step can you define the problem this PR is aimed to solve? Did you take into consideration the downside of this flag? Is the flag an accepted solution from your perspective? |

video_maplibre.mp4
What I ran into. In my own projects I use raster tiles as the basemap, with markers and vector layers (lines) on top. Every time the camera stops moving — end of a pan, end of a
flyTo, end of drag inertia — the basemap and the overlays jump relative to each other by a fraction of a pixel. What you see is the markers and the lines twitching against the map. Short slow drags show it best; inertia makes it happen twice per gesture.Why it happens. Raster layers are drawn with a pixel-grid-aligned projection matrix, but only while the map is idle. Every other layer type always uses the unaligned one.
What this PR does. Adds a
rasterPixelAlignmentmap option, defaulttrue, which is exactly today's behavior. Set it tofalseand raster, hillshade and color-relief layers never snap to the pixel grid, so nothing shifts relative to anything else.claude-opus-5[1m]) via Claude Code to find the code path and draft the tests. I verified all of it and ran the unit, build and render suites locally.