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

Use frustum culling in OSM tile traversal #4593

Merged
merged 2 commits into from
May 27, 2020
Merged

Conversation

Pessimistress
Copy link
Collaborator

For #4482

Change List

  • Use frustum culling in OSM tile traversal
  • Show tile boundary in website demo

@coveralls
Copy link

coveralls commented May 15, 2020

Coverage Status

Coverage increased (+2.5%) to 81.5% when pulling 6cc65ae on x/2d-tile-culling into 1b902ae on master.

@Pessimistress Pessimistress requested a review from kylebarron May 18, 2020 00:53
const elevationMax = (zRange && zRange[1] * unitsPerMeter) || 0;

// Always load at the current zoom level if pitch is small
const minZ = viewport.pitch <= 60 ? maxZ : 0;
Copy link
Collaborator

@kylebarron kylebarron May 18, 2020

Choose a reason for hiding this comment

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

Is this necessary to keep in sync with basemaps, e.g. Mapbox GL? I could envision cases where a user might want to relax this to load fewer tiles at moderate pitch, e.g. 40-60 degrees. But I guess relaxing this by default would be a backwards incompatible change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is also what mapbox-gl is doing. Lowering the threshold to 40-50 saves 2-3 tile loads tops, and may have unexpected behavioral changes for existing users (60 is the default maxPitch)

Base automatically changed from x/get-bounds to master May 26, 2020 20:59
@Pessimistress Pessimistress merged commit 79e6564 into master May 27, 2020
@Pessimistress Pessimistress deleted the x/2d-tile-culling branch May 27, 2020 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants