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

Fix uncautht OverscaledTileID error appearing with terrain enabled #1871

Merged
merged 4 commits into from
Nov 25, 2022

Conversation

zbigniewmatysek-tomtom
Copy link
Contributor

Fixes #1650.

Context

Negative zoom levels were introduced by mapbox to handle displaying tiny maps. By negative zoom level they can scale down tiles. This PR is not questioning this design choice, just fixes issue with querying elevation caused by not handling negative zoom.

Problem

OverscaledTileID in getElevation() was constructed with negative zoomLevel (not allowed by tile spec) and thus threw uncaught error.

References:

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Write tests for all new functionality.
  • Manually test the debug page.
  • Add an entry to CHANGELOG.md under the ## main section.

@HarelM
Copy link
Collaborator

HarelM commented Nov 24, 2022

Can you elaborate how removing this variable solves the issue?
@prozessor13 can you take a look and see if this seems like a good fix?

@github-actions
Copy link
Contributor

github-actions bot commented Nov 24, 2022

Bundle size report:

Size Change: -7 B
Total Size Before: 206 kB
Total Size After: 206 kB

Output file Before After Change
maplibre-gl.js 197 kB 197 kB -7 B
maplibre-gl.css 9.09 kB 9.09 kB 0 B
ℹ️ View Details
Source file Before After Change
src/render/program/program_uniforms.ts 922 B 960 B +38 B
src/render/draw_debug.ts 1.36 kB 1.37 kB +17 B
src/render/draw_terrain.ts 1.74 kB 1.75 kB +9 B
src/render/draw_raster.ts 1.04 kB 1.05 kB +8 B
src/index.ts 875 B 883 B +8 B
src/render/draw_fill_extrusion.ts 788 B 795 B +7 B
src/render/painter.ts 3.76 kB 3.76 kB +5 B
src/shaders/terrain.vertex.glsl.g.ts 218 B 222 B +4 B
src/shaders/shaders.ts 1.48 kB 1.49 kB +4 B
src/ui/handler/handler_util.ts 173 B 177 B +4 B
src/source/query_features.ts 1.21 kB 1.22 kB +3 B
src/shaders/background.vertex.glsl.g.ts 103 B 106 B +3 B
src/shaders/clipping_mask.vertex.glsl.g.ts 103 B 106 B +3 B
src/shaders/heatmap.vertex.glsl.g.ts 366 B 369 B +3 B
src/shaders/fill_extrusion_pattern.vertex.glsl.g.ts 826 B 829 B +3 B
src/shaders/line_gradient.vertex.glsl.g.ts 736 B 739 B +3 B
src/render/program/collision_program.ts 721 B 724 B +3 B
src/render/draw_fill.ts 971 B 974 B +3 B
src/util/throttle.ts 142 B 145 B +3 B
src/ui/default_locale.ts 310 B 313 B +3 B
node_modules/@mapbox/tiny-sdf/index.js 1.1 kB 1.1 kB +2 B
src/source/raster_dem_tile_source.ts 969 B 971 B +2 B
src/source/image_source.ts 1.11 kB 1.11 kB +2 B
src/source/source_cache.ts 3.99 kB 3.99 kB +2 B
src/util/worker_pool.ts 417 B 419 B +2 B
src/symbol/cross_tile_symbol_index.ts 1.37 kB 1.37 kB +2 B
src/shaders/_prelude.fragment.glsl.g.ts 119 B 121 B +2 B
src/shaders/circle.fragment.glsl.g.ts 408 B 410 B +2 B
src/shaders/heatmap_texture.fragment.glsl.g.ts 205 B 207 B +2 B
src/shaders/collision_box.fragment.glsl.g.ts 146 B 148 B +2 B
src/shaders/collision_circle.vertex.glsl.g.ts 545 B 547 B +2 B
src/shaders/fill.fragment.glsl.g.ts 176 B 178 B +2 B
src/shaders/fill_outline.vertex.glsl.g.ts 216 B 218 B +2 B
src/shaders/fill_outline_pattern.vertex.glsl.g.ts 413 B 415 B +2 B
src/shaders/fill_pattern.fragment.glsl.g.ts 394 B 396 B +2 B
src/shaders/fill_extrusion.fragment.glsl.g.ts 117 B 119 B +2 B
src/shaders/hillshade.fragment.glsl.g.ts 553 B 555 B +2 B
src/shaders/line.vertex.glsl.g.ts 707 B 709 B +2 B
src/shaders/line_pattern.fragment.glsl.g.ts 705 B 707 B +2 B
src/shaders/raster.vertex.glsl.g.ts 200 B 202 B +2 B
src/shaders/symbol_sdf.fragment.glsl.g.ts 552 B 554 B +2 B
src/shaders/symbol_text_and_icon.fragment.glsl.g.ts 596 B 598 B +2 B
src/shaders/terrain.fragment.glsl.g.ts 110 B 112 B +2 B
src/render/program/clipping_mask_program.ts 104 B 106 B +2 B
src/gl/color_mode.ts 172 B 174 B +2 B
src/gl/stencil_mode.ts 148 B 150 B +2 B
src/render/draw_collision_debug.ts 1.14 kB 1.14 kB +2 B
src/render/draw_circle.ts 618 B 620 B +2 B
node_modules/gl-matrix/esm/mat2.js 202 B 204 B +2 B
src/ui/handler/map_event.ts 444 B 446 B +2 B
src/ui/handler_manager.ts 2.46 kB 2.46 kB +2 B
src/source/terrain_source_cache.ts 877 B 879 B +2 B
src/render/terrain.ts 2.08 kB 2.08 kB +2 B
src/ui/control/navigation_control.ts 1.58 kB 1.58 kB +2 B
src/style/style_image.ts 125 B 126 B +1 B
src/render/glyph_manager.ts 946 B 947 B +1 B
src/render/line_atlas.ts 983 B 984 B +1 B
src/source/raster_tile_source.ts 911 B 912 B +1 B
src/source/video_source.ts 875 B 876 B +1 B
src/style-spec/diff.ts 1.54 kB 1.54 kB +1 B
src/symbol/path_interpolator.ts 310 B 311 B +1 B
src/symbol/collision_index.ts 1.64 kB 1.64 kB +1 B
src/style/pauseable_placement.ts 597 B 598 B +1 B
src/style/load_sprite.ts 409 B 410 B +1 B
src/shaders/line_sdf.fragment.glsl.g.ts 459 B 460 B +1 B
src/render/vertex_array_object.ts 580 B 581 B +1 B
src/render/program/debug_program.ts 191 B 192 B +1 B
src/render/program/heatmap_program.ts 557 B 558 B +1 B
src/gl/value.ts 1.09 kB 1.09 kB +1 B
src/gl/cull_face_mode.ts 153 B 154 B +1 B
src/geo/edge_insets.ts 430 B 431 B +1 B
src/ui/handler/mouse.ts 553 B 554 B +1 B
src/ui/handler/touch_pan.ts 540 B 541 B +1 B
src/ui/handler/tap_drag_zoom.ts 481 B 482 B +1 B
src/ui/handler/shim/touch_zoom_rotate.ts 279 B 280 B +1 B
src/ui/camera.ts 3.4 kB 3.41 kB +1 B
src/ui/control/logo_control.ts 486 B 487 B +1 B
src/ui/map.ts 7.16 kB 7.16 kB +1 B
src/render/texture.ts 680 B 679 B -1 B
src/style/load_glyph_range.ts 223 B 222 B -1 B
src/style/light.ts 560 B 559 B -1 B
src/util/dispatcher.ts 359 B 358 B -1 B
src/source/vector_tile_source.ts 1.1 kB 1.1 kB -1 B
src/source/geojson_source.ts 1.32 kB 1.32 kB -1 B
src/source/tile_cache.ts 556 B 555 B -1 B
src/symbol/grid_index.ts 1.35 kB 1.35 kB -1 B
src/source/pixels_to_tile_units.ts 104 B 103 B -1 B
src/style-spec/empty.ts 156 B 155 B -1 B
src/shaders/_prelude.vertex.glsl.g.ts 957 B 956 B -1 B
src/shaders/background_pattern.vertex.glsl.g.ts 225 B 224 B -1 B
src/shaders/clipping_mask.fragment.glsl.g.ts 60 B 59 B -1 B
src/shaders/heatmap.fragment.glsl.g.ts 267 B 266 B -1 B
src/shaders/collision_box.vertex.glsl.g.ts 316 B 315 B -1 B
src/shaders/debug.fragment.glsl.g.ts 148 B 147 B -1 B
src/shaders/debug.vertex.glsl.g.ts 162 B 161 B -1 B
src/shaders/fill_extrusion.vertex.glsl.g.ts 623 B 622 B -1 B
src/shaders/hillshade_prepare.fragment.glsl.g.ts 501 B 500 B -1 B
src/shaders/line.fragment.glsl.g.ts 325 B 324 B -1 B
src/shaders/line_gradient.fragment.glsl.g.ts 351 B 350 B -1 B
src/shaders/line_sdf.vertex.glsl.g.ts 809 B 808 B -1 B
src/shaders/raster.fragment.glsl.g.ts 439 B 438 B -1 B
src/shaders/symbol_icon.vertex.glsl.g.ts 948 B 947 B -1 B
src/render/program/symbol_program.ts 1.29 kB 1.29 kB -1 B
src/gl/depth_mode.ts 136 B 135 B -1 B
src/render/draw_line.ts 1.05 kB 1.05 kB -1 B
src/util/primitives.ts 729 B 728 B -1 B
src/ui/handler_inertia.ts 819 B 818 B -1 B
src/ui/events.ts 356 B 355 B -1 B
src/ui/handler/keyboard.ts 570 B 569 B -1 B
src/ui/handler/scroll_zoom.ts 1.28 kB 1.28 kB -1 B
src/util/debug.ts 160 B 159 B -1 B
src/util/task_queue.ts 316 B 315 B -1 B
src/gl/render_pool.ts 584 B 583 B -1 B
src/render/render_to_texture.ts 1 kB 1 kB -1 B
src/ui/marker.ts 2.87 kB 2.87 kB -1 B
src/ui/control/fullscreen_control.ts 784 B 783 B -1 B
src/source/tile_bounds.ts 317 B 315 B -2 B
src/util/offscreen_canvas_supported.ts 156 B 154 B -2 B
src/util/global_worker_pool.ts 321 B 319 B -2 B
src/shaders/background.fragment.glsl.g.ts 139 B 137 B -2 B
src/shaders/circle.vertex.glsl.g.ts 559 B 557 B -2 B
src/shaders/heatmap_texture.vertex.glsl.g.ts 145 B 143 B -2 B
src/shaders/collision_circle.fragment.glsl.g.ts 260 B 258 B -2 B
src/shaders/fill_outline_pattern.fragment.glsl.g.ts 420 B 418 B -2 B
src/shaders/fill_pattern.vertex.glsl.g.ts 389 B 387 B -2 B
src/shaders/fill_extrusion_pattern.fragment.glsl.g.ts 417 B 415 B -2 B
src/shaders/hillshade_prepare.vertex.glsl.g.ts 192 B 190 B -2 B
src/shaders/line_pattern.vertex.glsl.g.ts 789 B 787 B -2 B
src/shaders/symbol_icon.fragment.glsl.g.ts 228 B 226 B -2 B
src/shaders/symbol_sdf.vertex.glsl.g.ts 1.01 kB 1.01 kB -2 B
src/shaders/symbol_text_and_icon.vertex.glsl.g.ts 1.01 kB 1.01 kB -2 B
src/shaders/terrain_coords.fragment.glsl.g.ts 170 B 168 B -2 B
src/gl/context.ts 1.28 kB 1.28 kB -2 B
src/render/draw_symbol.ts 2.61 kB 2.61 kB -2 B
src/ui/control/scale_control.ts 735 B 733 B -2 B
src/ui/control/terrain_control.ts 420 B 418 B -2 B
src/data/raster_bounds_attributes.ts 96 B 93 B -3 B
src/shaders/fill.vertex.glsl.g.ts 174 B 171 B -3 B
src/shaders/hillshade.vertex.glsl.g.ts 139 B 136 B -3 B
src/render/program/circle_program.ts 457 B 454 B -3 B
src/ui/control/geolocate_control.ts 2.24 kB 2.24 kB -3 B
src/util/web_worker.ts 43 B 39 B -4 B
src/render/program/raster_program.ts 563 B 559 B -4 B
src/gl/index_buffer.ts 352 B 348 B -4 B
src/render/draw_hillshade.ts 1.16 kB 1.15 kB -4 B
src/source/source.ts 356 B 351 B -5 B
src/data/pos_attributes.ts 85 B 80 B -5 B
src/render/program/fill_extrusion_program.ts 690 B 685 B -5 B
src/render/draw_custom.ts 342 B 337 B -5 B
src/ui/handler/tap_recognizer.ts 537 B 532 B -5 B
src/render/program/hillshade_program.ts 888 B 882 B -6 B
src/ui/hash.ts 941 B 935 B -6 B
src/data/pos3d_attributes.ts 88 B 82 B -6 B
src/geo/transform.ts 4.55 kB 4.54 kB -7 B
src/render/draw_background.ts 580 B 572 B -8 B
src/render/draw_heatmap.ts 1.05 kB 1.04 kB -11 B
src/render/program/terrain_program.ts 711 B 695 B -16 B

@zbigniewmatysek-tomtom
Copy link
Contributor Author

@HarelM
This line solves the issue:
this.tileZoom = Math.max(0, Math.floor(constrainedZoom));

I noticed zoomFraction is not used anywhere, so I removed it, but it's not related to the original issue.

@HarelM
Copy link
Collaborator

HarelM commented Nov 24, 2022

Ahh, then I focused on the wrong thing. Can you split the two changes in to separate PRs by any chance?

@zbigniewmatysek-tomtom
Copy link
Contributor Author

zbigniewmatysek-tomtom commented Nov 25, 2022

@HarelM
I would prefer to leave it - zoomFraction is based on tileZoom which I've changed, this variable doesn't make any sense nor is used. I played an archeologist and last time it had been used was around 9 years ago here and in another file that doesn't exist anymore.:

@HarelM HarelM merged commit 870c2a3 into maplibre:main Nov 25, 2022
@sebastianoscarlopez
Copy link
Contributor

Thanks, @zbigniewmatysek-tomtom, because of the fix and archeologist work 😄 and @HarelM, because of the review & merge

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.

map.setTerrain() throws error at max zoom
3 participants