Fix: use correct location for mouse events of line layer with line-offset (#1108)

[jkoelewijn]

Changelog category

bug

Changelog

Use correct location for mouse events of line layer with line-offset (#1108).

Technical description

The offsetLine function uses a ring array that contains instances of the class Point from @mapbox/point-geometry. Methods of the Point class that are prefixed with _ adjust the instance of the Point class in place, i.e. no new instance is created and the current instance is mutated as a side effect and returned. This goes wrong when the zero variable is reused.

In the solution of this pull request the zero variable is removed and new instances of the Point class are created when needed.

Backport from Mapbox project?

This fixes #1108 using a similar solution as in jack9ye/mapbox-gl-js@ad2df2d, which is referred to in mapbox/mapbox-gl-js#9419. The code is different, but the solution is very similar.

I guess that does not count as a backport, but I'm not totally sure. Please confirm.

Update: I checked what Mapbox GL JS version jack9ye uses for his proposed solution and unfortunately that is v2.2.0. However, all the code that is visible in jack9ye/mapbox-gl-js@ad2df2d has not changed since v1.14.0-dev. Furthermore, the solution of this pull request will typically create 2 new Point objects per ring and the solution of jack9ye/mapbox-gl-js@ad2df2d will create 3.

Tasks

• 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.
• Include before/after visuals or gifs if this PR includes visual changes.
  • No visual change, only a change in behavior.
• Write tests for all new functionality.
• Document any changes to public APIs.
  • No change to public APIs.
• Post benchmark scores.
• Manually test the debug page.
• Suggest a changelog category: bug/feature/docs/etc. or "skip changelog".
• Add an entry inside this element for inclusion in the maplibre-gl-js changelog.

[HarelM]

Can you create a test that fails before this fix and passes afterwards?
Also a changelog entry is needed.
P.S. why would creating a new object inside the loop fixes this issue? The zero point is updated somewhere along the process?

[github-actions]

Bundle size report:

Size Change: +5 B
Total Size Before: 194 kB
Total Size After: 194 kB

Output file	Before	After	Change
maplibre-gl.js	185 kB	185 kB	+5 B
maplibre-gl.css	9.25 kB	9.25 kB	0 B

ℹ️ View Details

Source file	Before	After	Change
src/style/query_utils.ts	260 B	487 B	+227 B
node_modules/earcut/src/earcut.js	2.63 kB	2.64 kB	+11 B
src/data/bucket/fill_extrusion_attributes.ts	111 B	120 B	+9 B
node_modules/pbf/index.js	2.8 kB	2.81 kB	+9 B
src/style/style_layer/hillshade_style_layer_properties.g.ts	157 B	164 B	+7 B
src/style-spec/expression/definitions/match.ts	906 B	912 B	+6 B
src/symbol/shaping.ts	3.62 kB	3.63 kB	+6 B
src/style-spec/expression/definitions/number_format.ts	600 B	605 B	+5 B
src/style-spec/expression/index.ts	1.6 kB	1.6 kB	+4 B
src/style/evaluation_parameters.ts	387 B	391 B	+4 B
src/data/bucket/heatmap_bucket.ts	78 B	82 B	+4 B
src/style-spec/validate/validate_enum.ts	207 B	210 B	+3 B
src/style/style_layer/fill_extrusion_style_layer.ts	919 B	922 B	+3 B
src/style/style_layer/symbol_style_layer_properties.g.ts	650 B	653 B	+3 B
src/util/util.ts	1.87 kB	1.87 kB	+2 B
src/style-spec/validate/validate_function.ts	1.14 kB	1.14 kB	+2 B
src/style-spec/validate/validate_property.ts	545 B	547 B	+2 B
src/style-spec/validate/validate_image.ts	60 B	62 B	+2 B
src/util/transferable_grid_index.ts	1.01 kB	1.02 kB	+2 B
src/style/zoom_history.ts	179 B	181 B	+2 B
src/data/segment.ts	451 B	453 B	+2 B
src/data/extent.ts	32 B	34 B	+2 B
src/util/classify_rings.ts	244 B	246 B	+2 B
src/style/style_layer/line_style_layer_properties.g.ts	273 B	275 B	+2 B
src/symbol/one_em.ts	30 B	32 B	+2 B
node_modules/potpack/index.mjs	351 B	353 B	+2 B
src/symbol/mergelines.ts	401 B	403 B	+2 B
src/style/create_style_layer.ts	323 B	325 B	+2 B
src/style-spec/expression/definitions/slice.ts	491 B	492 B	+1 B
src/style-spec/feature_filter/index.ts	892 B	893 B	+1 B
src/style-spec/validate/validate_source.ts	614 B	615 B	+1 B
src/style-spec/validate/validate.ts	384 B	385 B	+1 B
src/style-spec/validate/validate_glyphs_url.ts	168 B	169 B	+1 B
src/data/evaluation_feature.ts	96 B	97 B	+1 B
src/style/style_layer/fill_style_layer_properties.g.ts	193 B	194 B	+1 B
src/symbol/symbol_size.ts	680 B	681 B	+1 B
src/symbol/check_max_angle.ts	288 B	289 B	+1 B
src/style/style_layer/fill_style_layer.ts	276 B	277 B	+1 B
src/style-spec/validate/validate_filter.ts	620 B	619 B	-1 B
src/shaders/encode_attribute.ts	81 B	80 B	-1 B
node_modules/gl-matrix/esm/mat4.js	2.2 kB	2.2 kB	-1 B
src/data/bucket/line_attributes.ts	120 B	119 B	-1 B
src/data/bucket/line_attributes_ext.ts	103 B	102 B	-1 B
src/data/bucket/symbol_attributes.ts	767 B	766 B	-1 B
node_modules/ieee754/index.js	566 B	565 B	-1 B
src/symbol/clip_line.ts	302 B	301 B	-1 B
src/util/find_pole_of_inaccessibility.ts	686 B	685 B	-1 B
src/style-spec/expression/definitions/index_of.ts	549 B	547 B	-2 B
src/style-spec/expression/definitions/comparison.ts	873 B	871 B	-2 B
src/style-spec/expression/definitions/length.ts	372 B	370 B	-2 B
src/style-spec/validate/validate_paint_property.ts	56 B	54 B	-2 B
src/style-spec/validate/validate_light.ts	291 B	289 B	-2 B
src/util/web_worker_transfer.ts	952 B	950 B	-2 B
src/data/program_configuration.ts	2.61 kB	2.61 kB	-2 B
src/style/style_layer/circle_style_layer.ts	529 B	527 B	-2 B
src/data/bucket/pattern_bucket_features.ts	325 B	323 B	-2 B
src/symbol/anchor.ts	172 B	170 B	-2 B
src/symbol/collision_feature.ts	379 B	377 B	-2 B
src/data/bucket/symbol_bucket.ts	4.26 kB	4.26 kB	-2 B
src/style/format_section_override.ts	309 B	307 B	-2 B
src/style-spec/validate/validate_layer.ts	865 B	862 B	-3 B
src/util/intersection_tests.ts	941 B	938 B	-3 B
src/util/color_ramp.ts	321 B	318 B	-3 B
src/style/parse_glyph_pbf.ts	337 B	334 B	-3 B
src/style-spec/expression/definitions/index.ts	1.63 kB	1.63 kB	-4 B
src/style/style_layer/heatmap_style_layer_properties.g.ts	141 B	137 B	-4 B
src/data/bucket/line_bucket.ts	2.41 kB	2.41 kB	-4 B
src/util/verticalize_punctuation.ts	589 B	585 B	-4 B
src/render/image_atlas.ts	835 B	831 B	-4 B
src/util/script_detection.ts	1.65 kB	1.64 kB	-5 B
node_modules/@mapbox/vector-tile/lib/vectortilefeature.js	1.01 kB	1.01 kB	-5 B
src/style-spec/validate/validate_expression.ts	457 B	451 B	-6 B
src/util/is_char_in_unicode_block.ts	885 B	876 B	-9 B
src/style/properties.ts	1.88 kB	1.86 kB	-13 B
src/data/bucket/fill_attributes.ts	118 B	92 B	-26 B
src/style/style_layer/line_style_layer.ts	962 B	820 B	-142 B

[jkoelewijn]

@HarelM, this pull request should be improved now. I'm not sure how to post the benchmark scores though...

Update: I found out how to run the benchmark: #666 (comment).

[HarelM]

Also the first checkbox is not ticked...

[HarelM]

If the code is different then this is not a backport.

[HarelM]

Changelog file entry is missing, can you please add?