Skip to content
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.

Sign up for GitHub

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

Jump to bottom

only align raster sources to pixel grid when map is idle to prevent shaking #7426

Merged
merged 3 commits into from
Dec 11, 2018
Merged

only align raster sources to pixel grid when map is idle to prevent shaking #7426

merged 3 commits into from
Dec 11, 2018

Conversation

mollymerp
Copy link
Contributor

@mollymerp mollymerp commented Oct 16, 2018 •
edited
Loading

fix #6150

one note: map.isZooming() was never returning true inside the map._render() function when using the ScrollZoomHandler because that handler returns to active: false before render is called, whereas zoomend is on a timeout so it is only called once if many scroll frames are triggered in succession. I mutated the map._zooming state to account for that, but I'm open to other ideas on how to handle it. removed this change for a separate PR

Any ideas on how to test this? 💭

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • post benchmark scores
  • manually test the debug page
  • tagged @mapbox/studio and/or @mapbox/maps-design if this PR includes style spec changes

@ansis
Copy link
Contributor

ansis commented Oct 16, 2018

In addition to only aligning when the map is moving should we only align the raster matrix when at integer zooms and quarter rotations?

@mollymerp
Copy link
Contributor Author

mollymerp commented Oct 16, 2018 •
edited
Loading

@ansis I don't think so – prior to the pixel-grid alignment change, raster maps looked blurry at non-integer ZLs and that is why we added the alignment in the first place. nvm! I misread the original PR. I think you're right 👍

@mollymerp
Copy link
Contributor Author

@ansis @kkaefer can you a take a look at this again?

kkaefer
kkaefer reviewed Nov 19, 2018
View reviewed changes
src/render/draw_raster.js Outdated Show resolved Hide resolved
@mollymerp
Copy link
Contributor Author

@kkaefer

Another behavior that I'm observing at least in Firefox is that moving isn't reset to false correctly when the mouse is released. Another possibility is that it is reset correctly, but no render is triggered when its state changes, leaving the map in a blurry state when it is rotated.

I can't reproduce this on FF with my trackpad (raster layer always returns to render an aligned frame after animation completes) – are you using a physical mouse?

@kkaefer
Copy link
Contributor

kkaefer commented Nov 20, 2018

@mollymerp: Yes, I'm using a physical mouse. This issue only manifests itself when the browser window has a pixel width that is odd. It's also not happening on every pan, but frequently enough to see it at least once among 10 pans.

@mollymerp
Copy link
Contributor Author

@kkaefer @ChrisLoer can one of you take another look here?

@mollymerp mollymerp merged commit 2e97715 into master Dec 11, 2018
@mollymerp mollymerp deleted the shaky-shake branch December 11, 2018 22:33
ansis pushed a commit that referenced this pull request Dec 12, 2018
@mollymerp @ansis
only align raster sources to pixel grid when map is idle to prevent s…
ebd9132 
…haking (#7426)

* only align raster sources to pixel grid when map is idle to prevent shaking

* remove integer zoom condition per kk's comment

* ensure map renders once more on moveend
ansis added a commit that referenced this pull request Dec 12, 2018
@ansis
only align raster sources to pixel grid when map is idle to prevent s…
ca638dc 
…haking (#7426) (#7694)

* only align raster sources to pixel grid when map is idle to prevent shaking

* remove integer zoom condition per kk's comment

* ensure map renders once more on moveend
@peterqliu peterqliu mentioned this pull request Jan 14, 2019
Closed
katydecorah pushed a commit that referenced this pull request Jan 23, 2019
Merge branch 'publisher-production' into docs-redesign
bca1d4f 
* publisher-production: (115 commits)
  fix inter-documentation link (#7791)
  [docs] use docs subdomain in examples (#7789)
  Bump Publisher
  First shot at new-domain staging
  [docs] Update page shell (#7760)
  updates API docs links to new url structure (#7757)
  add mapbox-gl-utils to plugins (#7752)
  [docs] Use docs-page-shell (#7742)
  Fixes bugs in documentation (#7741)
  Add worldviews example (#7720)
  Update compatibility matrix for `fill-extrusion-vertical-gradient` for ios & macos. (#7712)
  updates mapbox-gl-directions version in example (#7719)
  v0.52.0
  cherry-pick color state fix to release branch (#7715)
  Update location of drone video used in examples.
  v0.52.0-beta.2
  Cache hillshade textures based on texture size, not tile size. (#7695)
  avoid incomplete webp support in Edge 18 (#7687) (#7692)
  only align raster sources to pixel grid when map is idle to prevent shaking (#7426) (#7694)
  Flattens nested single element all expressions when converting to expressions (#7679)
  ...
katydecorah pushed a commit that referenced this pull request Jan 23, 2019
Merge branch 'publisher-production' into docs-redesign
757ffe2 
* publisher-production: (115 commits)
  fix inter-documentation link (#7791)
  [docs] use docs subdomain in examples (#7789)
  Bump Publisher
  First shot at new-domain staging
  [docs] Update page shell (#7760)
  updates API docs links to new url structure (#7757)
  add mapbox-gl-utils to plugins (#7752)
  [docs] Use docs-page-shell (#7742)
  Fixes bugs in documentation (#7741)
  Add worldviews example (#7720)
  Update compatibility matrix for `fill-extrusion-vertical-gradient` for ios & macos. (#7712)
  updates mapbox-gl-directions version in example (#7719)
  v0.52.0
  cherry-pick color state fix to release branch (#7715)
  Update location of drone video used in examples.
  v0.52.0-beta.2
  Cache hillshade textures based on texture size, not tile size. (#7695)
  avoid incomplete webp support in Edge 18 (#7687) (#7692)
  only align raster sources to pixel grid when map is idle to prevent shaking (#7426) (#7694)
  Flattens nested single element all expressions when converting to expressions (#7679)
  ...
katydecorah pushed a commit that referenced this pull request Jan 25, 2019
@colleenmcginnis @katydecorah
[docs] redesign to use dr-ui components (#7530)
8bdd869 
* initial layout

* clean up style spec layout

* clean up examples page

* fix links in style spec toc

* split style spec

* add examples landing page

* update plugins page layout

* format roadmap page

* fix icon positioning

* delete generated bench files

* address @mollymerp's feedback

* update dr ui version

* clean up console errors

* refactor page shell

* use docs-page-shell; update dr-ui and assembly; remove ProductDropdown

* fix spacing; add borderRadius

* various spacing, sizing

* remove additional space between h2  h3 siblings

* runs eslint --fix on component files

* remove unused lines; linting fixes

* navigation style updates: make link `link--gray` indent nested items

* add spacing to code blocks

* remove `color-gray` from non link elements

* api item + member styling

* remove base classes from /plugins; fix heading spacing

* use mr-ui copy-button in copyable.js

* run eslint --fix on docs/pages, docs/util

* linter fixes

* heading spacing for mobile

* rework copyable to use codesnippet

* add hover class to simple-map card

* make examples header consistent with other headers

* turn off h2 borders for the api section as they clash with the instance members toggle sections; max height and pretty scroll for code examples

* remove unused css

* adds hover state

* replace color-gray-light with color-gray; it's too light on white backgrounds

* remove padding conditional

* rm unused file

* break style-spec into components

* make tocnote link--gray

* adds back top padding to ApiItemMember

* set borderRadius on table rows

* readjust border on ApiItemMember

* adds txt-break-word to overflowing code blocks

* adds style spec to header

* light header adjustments

* style error note

* Merge branch 'publisher-production' into docs-redesign

* publisher-production: (115 commits)
  fix inter-documentation link (#7791)
  [docs] use docs subdomain in examples (#7789)
  Bump Publisher
  First shot at new-domain staging
  [docs] Update page shell (#7760)
  updates API docs links to new url structure (#7757)
  add mapbox-gl-utils to plugins (#7752)
  [docs] Use docs-page-shell (#7742)
  Fixes bugs in documentation (#7741)
  Add worldviews example (#7720)
  Update compatibility matrix for `fill-extrusion-vertical-gradient` for ios & macos. (#7712)
  updates mapbox-gl-directions version in example (#7719)
  v0.52.0
  cherry-pick color state fix to release branch (#7715)
  Update location of drone video used in examples.
  v0.52.0-beta.2
  Cache hillshade textures based on texture size, not tile size. (#7695)
  avoid incomplete webp support in Edge 18 (#7687) (#7692)
  only align raster sources to pixel grid when map is idle to prevent shaking (#7426) (#7694)
  Flattens nested single element all expressions when converting to expressions (#7679)
  ...

* update batfish, react, and react-dom

* rm unused module

* update mr-ui and github-slugger

* adds version var to OverviewHeader

* mb-pages -> publisher-production

* fix bool on iframe

* clean up page shell to fix build errors

* update/add pathname; add descriptions

* make toggles buttons for acessbility; increase hit box; make icon larger; do not change location on click

* use push state to change hash on click, but not to move page

* update dr-ui

* adds interactiveClass and sideBarColSize

* smaller sidebar on spec

* adds addtional padding to h3s on api page to clear the sticker

* make hit box larger for nav items on sidebard

* adjust topbarstick columns when `sidebarColSize` is set

* adds make-table-scroll plugin; make non markdown table scroll

* remove extra docs-content ids

* make table scroll; wrap code

* reorder tabs

* adds back to top button long pages

* update dr-ui

* sets unStickWidth on TopbarSticker

* Update batfish.config.js

* Update yarn.lock

* adds unit tests for examples

* fix tests

* add missing images

* adds test to make sure every example as a thumbnail image

* compress images

* resize thumbnail images to 1200 x 500

* compress again

* add a little wiggle room for image size

* updates documentation for tags and adds information about example thumbnail images
@asheemmamoowala asheemmamoowala mentioned this pull request Jan 31, 2019
Closed
pirxpilot pushed a commit to pirxpilot/mapbox-gl-js that referenced this pull request Jun 14, 2019
@mollymerp @pirxpilot
only align raster sources to pixel grid when map is idle to prevent s…
f705ae9 
…haking (mapbox#7426)

* only align raster sources to pixel grid when map is idle to prevent shaking

* remove integer zoom condition per kk's comment

* ensure map renders once more on moveend
pirxpilot pushed a commit to pirxpilot/mapbox-gl-js that referenced this pull request Jun 18, 2019
@mollymerp @pirxpilot
only align raster sources to pixel grid when map is idle to prevent s…
7b4d20a 
…haking (mapbox#7426)

* only align raster sources to pixel grid when map is idle to prevent shaking

* remove integer zoom condition per kk's comment

* ensure map renders once more on moveend
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kkaefer kkaefer kkaefer left review comments

@ChrisLoer ChrisLoer ChrisLoer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tiny map shake while zooming in on raster tiles
4 participants
@mollymerp @ansis @kkaefer @ChrisLoer