maplibre-gl-js
maplibre-gl-js copied to clipboard
Fix flicker on map resize
Fixes #2971 and #4158. Flicker on resize was introduced in #2157, released in v3.0.0, when map resizing was switched to using resizeObserver from using window resize events.
The fix is to immediately render the map upon resizing. The lead was this thread.
Caveat: I'm not sure what to pass for the paintStartTimeStamp for map._render(). I set it to Date.now(). 0 also works fine.
Before and after gifs (half speed):
- [x] Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
- [x] Briefly describe the changes in this PR.
- [x] Link to related issues.
- [x] Include before/after visuals or gifs if this PR includes visual changes.
- [ ] Post benchmark scores.
- [x] Add an entry to
CHANGELOG.mdunder the## mainsection.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 87.73%. Comparing base (
8818fe3) to head (f2ee69c). Report is 545 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #4429 +/- ##
==========================================
- Coverage 87.88% 87.73% -0.16%
==========================================
Files 246 246
Lines 33450 33451 +1
Branches 2208 2217 +9
==========================================
- Hits 29399 29347 -52
- Misses 3050 3101 +51
- Partials 1001 1003 +2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Looks good, can you fix the failing tests, and add another test if needed please?
Not sure how to debug the failing test yet. I can confirm that jest --coverage --selectProjects unit returns a nonzero exit code when I run it with the changes (echo $? prints 1), but it doesn't print any error output on my machine. The CI job produced a type error in painter.ts, but I don't get any errors for npm run lint.
npm run test-unit --reportes=default See more options here: https://github.com/maplibre/maplibre-gl-js/tree/main/test#running-tests
Any updates on this?
It seems like merging main fixed the test... Trying again to make sure it's not a fluke.
I'm not sure I understand how this solves the problem.
When calling _update, which is done in the resize event, the _update method should add a requestAnimtionFrame with a call to _render.
So, I'm not sure I understand how forcing a call to render solves this issue...
Also, as stated before, please add a test for this bug.
@HarelM I think it’s a timing problem. _update() calls triggerRepaint(), which calls _render(), but in testing this I noticed that since the _render() call is in this browser.frameAsync callback, it happens after about a ~10ms delay, compared to if it’s called directly in the resize handler, like in this commit. When I add an explicit 10ms delay before the new _render() call in the resize handler, the flicker comes back. Perhaps this points to a simpler solution.
Maybe #4535 can help here too?
This needs a test nevertheless, but if #4535 solves this, it will be a better solution from my point of view.
Maybe #4535 can help here too?
Unfortunately thats not the case. I can reproduce the flicker in that branch.
Closing in favor of:
- #4632
If the linked PR is not enough, please comment on it or comment in the linked closed issue.