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.

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

Add globe view support to heatmap shaders #11120

Merged
merged 1 commit into from
Oct 14, 2021

Conversation

mpulkki-mapbox
Copy link
Contributor

Update heatmap shaders to support the globe view. Additional vertex attributes and shader uniforms are used conditionally only if the globe view is active. No changes in memory usage nor in complexity in default mercator projection.

@mpulkki-mapbox mpulkki-mapbox added the skip changelog Used for PRs that do not need a changelog entry label Oct 14, 2021
Copy link
Contributor

@karimnaaji karimnaaji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like something is introducing a screen dependent resolution bug. I don't have any issue from the native branch on my 1x screen on ubuntu linux, but I do see an issue on macOS with retina display:
Screen Shot 2021-10-14 at 11 32 55 AM

Is there any step in the vertex projection steps that should depend on parameters.pixelRatio in the native branch?

src/shaders/heatmap.vertex.glsl Show resolved Hide resolved
@mpulkki-mapbox
Copy link
Contributor Author

Seems like something is introducing a screen dependent resolution bug. I don't have any issue from the native branch on my 1x screen on ubuntu linux, but I do see an issue on macOS with retina display: Screen Shot 2021-10-14 at 11 32 55 AM

Is there any step in the vertex projection steps that should depend on parameters.pixelRatio in the native branch?

This one is really interesting! Are you running the native branch with these shader changes? The image looks like you're using outdated versions of the shaders (ie. no globe support). Are you sure that metal shaders are up-to-date? I haven't included those in any of my development branches yet.

Copy link
Contributor

@karimnaaji karimnaaji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm yes that seems like it could be an out-of-sync issue as the shaders aren't all up to date, I did update the submodule with this branch and regen the shaders in the other PR (even on Metal) but still having the issue. I'll approve to unblock as the Linux build is working as expected.

@karimnaaji karimnaaji merged commit 6c1b30b into main Oct 14, 2021
@karimnaaji karimnaaji deleted the mpulkki-globe-heatmap-shaders branch October 14, 2021 19:42
katydecorah pushed a commit that referenced this pull request Oct 20, 2021
* main:
  Add touch pan blocker to gesture handling for touch devices (#11116)
  Address accessibility issues (#11064)
  add support for non-mercator projections (#11124)
  Image fallback expressions within paint properties (#11049)
  Replaces EPSG:4326 with OGC:CRS84 in GL JS `LngLat` doc (#11072)
  Add globe view support to heatmap shaders (#11120)
  Exclude flaky test (#11118)
  consistify YOUR_MAPBOX_ACCESS_TOKEN as placeholder string (#11113)
  Allow adding multiple layers to `map.on()` event handler (h/t @omerbn) (#11114)
  render-test-flakiness:clear worker storage (#11111)
  upgrade to supercluster v7.1.4, earcut v2.2.3, vt-pbf v3.1.3, geojson-rewind v0.5.1 (#11110)
  Added v1.13.2 changelog entry (#11108)
  One weird JSON.parse() trick (#11098)
  Fixed doc usage of map.getCenter (#11093)
  s̶y̶m̶b̶o̶l̶-̶c̶l̶i̶p̶ dynamic-filtering with `pitch` and `distance-from-camera` expressions (#10795)
  Update link to transpiling guide (#11096)
  Cherry pick 2.5.1 changelog (#11099)
  Fix an iOS15 issue where Safari tab bar interrupts panning (#11084)
  Fix conditional check for isFullscreen to accommodate Safari (#11086)
  Render tests for #11041 (#11070)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip changelog Used for PRs that do not need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants