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

Short-circuit the rendering of fully transparent symbol tiles. #1757

Conversation

AdamS108
Copy link
Contributor

@AdamS108 AdamS108 commented Oct 20, 2022

When there are large clusters of colliding symbolic icons and labels, many of them are culled by being set fully transparent but their geometry is still rendered. In some cases, entire tiles full of symbols can be made transparent but are still rendered. This change looks for those fully transparent symbol tiles and avoids rendering them at all which can greatly reduce GL context commands (including draws). For example, for Bing Maps in downtown Seattle at LOD 17 the total number of rendering commands can be reduced by 20-25% with this change.

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.
  • (n/a) Include before/after visuals or gifs if this PR includes visual changes.
  • (n/a) Write tests for all new functionality.
  • (n/a) Document any changes to public APIs.
  • Post benchmark scores.
    benchmark result
  • Manually test the debug page.
  • Add an entry to CHANGELOG.md under the ## main section.

Adam Szofran added 3 commits October 20, 2022 10:51
When there are large clusters of colliding symbolic icons and labels, many of them are culled by being set fully transparent but their geometry is still rendered. In some cases, entire tiles full of symbols can be made transparent but are still rendered. This change looks for those fully transparent symbol tiles and avoids rendering them at all which can greatly reduce GL context commands (including draws). For example, for Bing Maps in downtown Seattle at LOD 17 the total number of rendering commands can be reduced by 20-25% with this change.
@HarelM
Copy link
Collaborator

HarelM commented Oct 20, 2022

How would this change affect query render features?

@github-actions
Copy link
Contributor

github-actions bot commented Oct 20, 2022

Bundle size report:

Size Change: +57 B
Total Size Before: 206 kB
Total Size After: 206 kB

Output file Before After Change
maplibre-gl.js 197 kB 197 kB +57 B
maplibre-gl.css 9.09 kB 9.09 kB 0 B
ℹ️ View Details
Source file Before After Change
node_modules/murmurhash-js/murmurhash3_gc.js 345 B 383 B +38 B
src/symbol/placement.ts 5.04 kB 5.08 kB +34 B
node_modules/quickselect/quickselect.js 366 B 385 B +19 B
src/data/bucket/fill_attributes.ts 98 B 112 B +14 B
src/render/draw_symbol.ts 2.6 kB 2.61 kB +14 B
src/style/style_layer/heatmap_style_layer_properties.g.ts 131 B 141 B +10 B
src/render/program/terrain_program.ts 701 B 711 B +10 B
src/data/bucket/symbol_bucket.ts 4.26 kB 4.27 kB +9 B
node_modules/gl-matrix/esm/quat.js 146 B 154 B +8 B
src/render/program/fill_program.ts 562 B 570 B +8 B
src/style-spec/expression/index.ts 1.66 kB 1.66 kB +7 B
src/style/style_layer/hillshade_style_layer_properties.g.ts 157 B 164 B +7 B
src/util/util.ts 1.96 kB 1.97 kB +5 B
node_modules/@mapbox/vector-tile/lib/vectortilefeature.js 1.01 kB 1.01 kB +5 B
src/gl/index_buffer.ts 347 B 352 B +5 B
src/util/is_char_in_unicode_block.ts 876 B 880 B +4 B
src/style/evaluation_parameters.ts 387 B 391 B +4 B
src/style/style_layer/fill_extrusion_style_layer.ts 930 B 934 B +4 B
src/style/style_layer/line_style_layer_properties.g.ts 273 B 277 B +4 B
src/render/image_atlas.ts 830 B 834 B +4 B
src/util/web_worker.ts 39 B 43 B +4 B
src/data/pos_attributes.ts 81 B 85 B +4 B
src/render/program/hillshade_program.ts 884 B 888 B +4 B
src/render/program/symbol_program.ts 1.29 kB 1.29 kB +4 B
src/render/painter.ts 3.75 kB 3.76 kB +4 B
src/render/draw_custom.ts 338 B 342 B +4 B
src/style/style_layer/circle_style_layer_properties.g.ts 227 B 230 B +3 B
node_modules/gl-matrix/esm/vec3.js 847 B 850 B +3 B
src/data/bucket/fill_bucket.ts 1.09 kB 1.1 kB +3 B
src/symbol/one_em.ts 29 B 32 B +3 B
src/render/program/collision_program.ts 718 B 721 B +3 B
src/render/draw_background.ts 577 B 580 B +3 B
src/ui/control/geolocate_control.ts 2.24 kB 2.24 kB +3 B
src/style-spec/expression/definitions/number_format.ts 527 B 529 B +2 B
src/style-spec/expression/definitions/format.ts 648 B 650 B +2 B
src/style-spec/validate/validate_color.ts 140 B 142 B +2 B
src/style-spec/validate_style.min.ts 291 B 293 B +2 B
src/data/feature_position_map.ts 612 B 614 B +2 B
node_modules/gl-matrix/esm/mat4.js 2.69 kB 2.69 kB +2 B
src/util/verticalize_punctuation.ts 586 B 588 B +2 B
src/source/tile_bounds.ts 315 B 317 B +2 B
src/util/global_worker_pool.ts 319 B 321 B +2 B
src/shaders/symbol_text_and_icon.vertex.glsl.g.ts 1.01 kB 1.01 kB +2 B
src/gl/context.ts 1.28 kB 1.28 kB +2 B
src/ui/handler/tap_recognizer.ts 535 B 537 B +2 B
src/data/pos3d_attributes.ts 86 B 88 B +2 B
src/ui/control/scale_control.ts 733 B 735 B +2 B
src/ui/control/terrain_control.ts 418 B 420 B +2 B
src/style-spec/validate/validate_padding.ts 241 B 242 B +1 B
src/util/transferable_grid_index.ts 1.02 kB 1.02 kB +1 B
src/data/array_types.g.ts 2.82 kB 2.82 kB +1 B
src/data/program_configuration.ts 2.61 kB 2.62 kB +1 B
src/data/extent.ts 31 B 32 B +1 B
src/data/bucket/fill_extrusion_bucket.ts 1.43 kB 1.44 kB +1 B
src/data/bucket/line_attributes.ts 118 B 119 B +1 B
node_modules/ieee754/index.js 565 B 566 B +1 B
src/symbol/check_max_angle.ts 286 B 287 B +1 B
src/symbol/collision_feature.ts 386 B 387 B +1 B
src/style/style_layer/symbol_style_layer.ts 1.17 kB 1.17 kB +1 B
src/render/texture.ts 679 B 680 B +1 B
src/style/load_glyph_range.ts 222 B 223 B +1 B
src/style/light.ts 559 B 560 B +1 B
src/util/dispatcher.ts 358 B 359 B +1 B
src/source/vector_tile_source.ts 1.1 kB 1.1 kB +1 B
src/util/offscreen_canvas_supported.ts 155 B 156 B +1 B
src/source/geojson_source.ts 1.32 kB 1.32 kB +1 B
src/symbol/grid_index.ts 1.35 kB 1.35 kB +1 B
src/style-spec/empty.ts 155 B 156 B +1 B
src/render/program.ts 1.15 kB 1.15 kB +1 B
src/render/program/circle_program.ts 456 B 457 B +1 B
src/gl/depth_mode.ts 135 B 136 B +1 B
src/util/primitives.ts 728 B 729 B +1 B
src/geo/transform.ts 4.55 kB 4.55 kB +1 B
src/ui/handler/keyboard.ts 569 B 570 B +1 B
src/ui/handler/click_zoom.ts 240 B 241 B +1 B
src/ui/handler/shim/drag_rotate.ts 178 B 179 B +1 B
src/source/terrain_source_cache.ts 876 B 877 B +1 B
src/gl/render_pool.ts 583 B 584 B +1 B
src/ui/control/navigation_control.ts 1.58 kB 1.58 kB +1 B
src/ui/marker.ts 2.87 kB 2.87 kB +1 B
src/ui/control/fullscreen_control.ts 783 B 784 B +1 B
src/style-spec/expression/definitions/comparison.ts 838 B 837 B -1 B
src/style-spec/validate/validate.ts 418 B 417 B -1 B
src/style-spec/validate/validate_boolean.ts 123 B 122 B -1 B
src/util/script_detection.ts 1.65 kB 1.65 kB -1 B
node_modules/@mapbox/vector-tile/lib/vectortilelayer.js 433 B 432 B -1 B
src/style/style_image.ts 126 B 125 B -1 B
src/render/glyph_manager.ts 947 B 946 B -1 B
src/render/line_atlas.ts 984 B 983 B -1 B
src/source/raster_tile_source.ts 912 B 911 B -1 B
src/data/raster_bounds_attributes.ts 97 B 96 B -1 B
src/source/source_cache.ts 3.99 kB 3.99 kB -1 B
src/style-spec/diff.ts 1.54 kB 1.54 kB -1 B
src/symbol/path_interpolator.ts 311 B 310 B -1 B
src/style/load_sprite.ts 410 B 409 B -1 B
src/shaders/symbol_text_and_icon.fragment.glsl.g.ts 597 B 596 B -1 B
src/shaders/terrain_depth.fragment.glsl.g.ts 197 B 196 B -1 B
src/render/vertex_array_object.ts 581 B 580 B -1 B
src/gl/value.ts 1.09 kB 1.09 kB -1 B
src/gl/cull_face_mode.ts 154 B 153 B -1 B
src/geo/edge_insets.ts 431 B 430 B -1 B
src/util/throttle.ts 143 B 142 B -1 B
src/ui/handler/mouse.ts 554 B 553 B -1 B
src/ui/handler/shim/touch_zoom_rotate.ts 280 B 279 B -1 B
src/ui/handler_manager.ts 2.46 kB 2.46 kB -1 B
src/ui/map.ts 7.16 kB 7.16 kB -1 B
src/style-spec/expression/types.ts 514 B 512 B -2 B
src/data/segment.ts 451 B 449 B -2 B
src/render/uniform_binding.ts 645 B 643 B -2 B
node_modules/gl-matrix/esm/common.js 182 B 180 B -2 B
src/data/bucket/line_attributes_ext.ts 104 B 102 B -2 B
src/style/style_layer/line_style_layer.ts 817 B 815 B -2 B
src/data/bucket/symbol_attributes.ts 767 B 765 B -2 B
src/symbol/anchor.ts 169 B 167 B -2 B
node_modules/@mapbox/tiny-sdf/index.js 1.1 kB 1.1 kB -2 B
src/source/query_features.ts 1.21 kB 1.21 kB -2 B
src/util/worker_pool.ts 419 B 417 B -2 B
src/shaders/terrain.fragment.glsl.g.ts 112 B 110 B -2 B
src/render/program/fill_extrusion_program.ts 692 B 690 B -2 B
src/gl/color_mode.ts 174 B 172 B -2 B
src/gl/stencil_mode.ts 150 B 148 B -2 B
node_modules/gl-matrix/esm/mat2.js 204 B 202 B -2 B
src/ui/handler/tap_drag_zoom.ts 483 B 481 B -2 B
src/render/terrain.ts 2.08 kB 2.08 kB -2 B
src/data/bucket/circle_attributes.ts 95 B 92 B -3 B
node_modules/gl-matrix/esm/vec4.js 396 B 393 B -3 B
src/util/color_ramp.ts 436 B 433 B -3 B
src/data/bucket/pattern_bucket_features.ts 327 B 324 B -3 B
src/data/bucket/line_bucket.ts 2.42 kB 2.41 kB -3 B
src/shaders/terrain.vertex.glsl.g.ts 221 B 218 B -3 B
src/ui/default_locale.ts 313 B 310 B -3 B
src/data/bucket/circle_bucket.ts 972 B 968 B -4 B
src/style/query_utils.ts 487 B 483 B -4 B
src/style/style_layer/fill_style_layer_properties.g.ts 196 B 192 B -4 B
src/symbol/shaping.ts 3.62 kB 3.62 kB -4 B
src/render/draw_collision_debug.ts 1.14 kB 1.14 kB -4 B
src/style/style_layer/fill_extrusion_style_layer_properties.g.ts 187 B 182 B -5 B
src/data/bucket/heatmap_bucket.ts 85 B 78 B -7 B
src/util/image.ts 723 B 716 B -7 B
src/style-spec/expression/definitions/index.ts 1.63 kB 1.62 kB -8 B
src/style/properties.ts 1.91 kB 1.9 kB -8 B
src/render/draw_raster.ts 1.05 kB 1.04 kB -8 B
src/index.ts 883 B 875 B -8 B
src/data/bucket/fill_extrusion_attributes.ts 134 B 125 B -9 B
node_modules/pbf/index.js 2.82 kB 2.81 kB -9 B
src/render/draw_terrain.ts 1.75 kB 1.74 kB -9 B
src/render/draw_debug.ts 1.37 kB 1.36 kB -10 B
node_modules/gl-matrix/esm/vec2.js 291 B 280 B -11 B
src/util/classify_rings.ts 258 B 246 B -12 B
node_modules/earcut/src/earcut.js 2.72 kB 2.7 kB -20 B
node_modules/murmurhash-js/murmurhash2_gc.js 265 B 244 B -21 B
src/data/bucket/pattern_attributes.ts 140 B 111 B -29 B
src/render/program/program_uniforms.ts 952 B 922 B -30 B

@AdamS108
Copy link
Contributor Author

AdamS108 commented Oct 20, 2022

How would this change affect query render features?

@HarelM: That sounds like a great question... about which I have no clue! So, I'll see if I can find an answer. Thanks.

Edit: @HarelM is your question about the impact of this PR on the test-query tests and the two query related benchmarks? Or something else? Thanks.

@AdamS108
Copy link
Contributor Author

How would this change affect query render features?

@HarelM is your question about the impact of this PR on the test-query tests and the two query related benchmarks? Or something else? Thanks.

FWIW, the test-query tests all pass and the two query related benchmarks don't show any significant difference.

@AdamS108 AdamS108 marked this pull request as ready for review October 20, 2022 22:09
@HarelM
Copy link
Collaborator

HarelM commented Oct 21, 2022

The tests may pass if there isn't a test there to simulate this "extreme" condition.
The question here, due to the change in rendering etc, if this somehow affect the logic related to querying these hidden-now-removed features.
I'm not sure I know how to test this though...

@AdamS108
Copy link
Contributor Author

@HarelM are you referring to the queryRenderedFeatures API specifically? Thanks.

@HarelM
Copy link
Collaborator

HarelM commented Oct 21, 2022

Yes, exactly this API.

@AdamS108
Copy link
Contributor Author

AdamS108 commented Oct 21, 2022

This code in this PR waits until nearly the last moment to avoid rendering the fully transparent symbol tiles. It doesn't keep them from being loaded or stored or considered by render queries as far as I can tell.

Up to the point of rendering in drawLayerSymbols the only thing this PR does is add some additional state to the symbol tiles to help track the aggregate transparency of the symbols in the tile.

It looks like there might be one possible place where side-effects could come into play. The early out from drawLayerSymbols happens before it calls updateLineLabels. If that call affects the results of queryRenderedFeatures then perhaps I can move the early out after updateLineLabels to preserve the side-effect.

@HarelM
Copy link
Collaborator

HarelM commented Oct 22, 2022

I would also like to see tests that are making sure this behavior is as expected - both the old behavior and the new one.

@AdamS108
Copy link
Contributor Author

Thanks. Will try to author some unit tests to determine the impact of this PR on queryRenderedFeatures, if any.

@AdamS108
Copy link
Contributor Author

FWIW, I instrumented the LayerSymbolWithSortKey benchmark to call queryRenderedFeatures on every interation and it gets the same result with or without the changes in this PR. I know hacking the benchmark isn't enough; I need to write a unit test that shows the same. But at least it shows that probably no harm has been done.

@AdamS108
Copy link
Contributor Author

@HarelM, I added a new map unit test that confirms that the result of queryRenderedFeatures() is unaffected by the render optimization in this PR. The test compares the number of features returned by queryRenderedFeatures() with and without the optimization enabled and it gets the same result both times.

src/render/draw_symbol.ts Outdated Show resolved Hide resolved
@HarelM
Copy link
Collaborator

HarelM commented Nov 11, 2022

Thanks for taking the time to review my comments!
I would focus on writing a unit test around the method you changed and render test to check the functionally.

@HarelM
Copy link
Collaborator

HarelM commented Nov 11, 2022

Also there are also the query tests which can be used here as well I guess.

@AdamS108
Copy link
Contributor Author

AdamS108 commented Nov 15, 2022

@HarelM, I removed the test I added to map.test.ts and recreated it (in spirit, at least) as a query test. I also added a simple test to draw_symbol.test.ts to confirm that no rendering occurs for symbol buffers without the hasVisibleVertices flag set.

@HarelM
Copy link
Collaborator

HarelM commented Nov 17, 2022

Last comment before I merge...

@AdamS108
Copy link
Contributor Author

Last comment before I merge...

All good now @HarelM ?

@HarelM
Copy link
Collaborator

HarelM commented Nov 18, 2022

Yes, Thanks!
I saw your initial comment about checking the flag but Github doesn't notify when you edit a comment, so I didn't have the last amswer.
Thanks for the ping!

@HarelM HarelM merged commit 1a4d81d into maplibre:main Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants