[core] increase padding in CollisionIndex for MapMode::Tile

[ansis]

When rendering image tiles using MapMode::Tile we need to avoid clipped labels at tile boundaries. Clipped labels happen when one tile decides a label can be shown and the other tile decides that it collides with something and needs to be hidden.

When rendering overscaled image tiles (rendering z17 image tiles for a vector tile source that only goes up to z16) we should use all the data from the tile instead of just the data that fits within the image tile.

This increases the padding to 1024 for MapMode::Tile which should be enough to completely include all the data two zoom levels past the data source's max zoom level. Beyond that, 1024 should be enough of a padding to make clipped labels unlikely.

An alternative would be to calculate the max zoom levels of all the sources used and dynamically set the padding on all sides but increasing the constant seems as good and less complex.

[springmeyer]

Thanks for putting this up as a PR @ansis. As a prerequisite to reviewing, on my end, I just tried to build locally to check the test coverage. But 1) I'm not seeing a way to see test coverage locally - is there an established best practice here? and 2) I hit #15881 so currently I'm blocked on running the unit tests: is there an existing test for this behavior or do we need to add one?

[pozdnyakov]

@ansis is it fine to be merged?

[springmeyer]

I'm not seeing a way to see test coverage locally - is there an established best practice here?

@pozdnyakov do you know the answer to this? I've finally gotten my local build working, so unless there is already a way to do this, I'm planning on trying to get local coverage generation working.

The reason I'm interested in local coverage is for branch coverage details. It seems essential to me that we ensure there is there is test coverage for both mapMode == MapMode::Tile and mapMode != MapMode::Tile in the code changed in this PR. Currently the codecov.io based coverage reporting does not display branch information. So it is not possible for me to verify that both of the scenarios at https://codecov.io/gh/mapbox/mapbox-gl-native/src/7665aa8073b77346bcb50c8299d854f663259ba7/src/mbgl/text/collision_index.cpp#L31 are covered by tests (without testing locally).

So, I do not think this is not ready to merge yet. Before merging we should have confidence that the logic in this PR is tested under both MapMode::Tile and non-tiled modes.

[pozdnyakov]

@springmeyer sorry for missing your question, IMO a render test could suffice for verifying this PR's intended behavior. Actually, render tests provide even better QA than unit tests. Our test runner supports tile mode (e.g. pls see mapbox/mapbox-gl-js#8899)

[springmeyer]

Thanks @pozdnyakov - sounds good then to merge this if it has a render test covering it. Does it?

Also, I never found time to run coverage locally, but I'll create a ticket documenting my setup if I ever get around to it.

[springmeyer]

@ansis does this have a render test covering it?

[springmeyer]

Bump @ansis @pozdnyakov @tmpsantos - does this code have render-test coverage or not?

[pozdnyakov]

Bump @ansis @pozdnyakov @tmpsantos - does this code have render-test coverage or not?

The existing text-variable-anchor/all-anchors-tile-map-mode and map-mode/tile render tests will run the modified code, but there is no a dedicated test that would fail without this change. The work on enabling the necessary infra for such test is ongoing: https://github.com/mapbox/mapbox-gl-native-team/issues/94#issuecomment-564550546