Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] avoid edges for labels that use text-variable-anchors #15854

Merged
merged 1 commit into from
Oct 24, 2019

Conversation

pozdnyakov
Copy link
Contributor

@pozdnyakov pozdnyakov commented Oct 24, 2019

to prevent clipped labels in rendered image tiles.

Fixes https://github.com/mapbox/mapbox-gl-native-team/issues/90

to prevent clipped labels in rendered image tiles.
@pozdnyakov pozdnyakov added the Core The cross-platform C++ core, aka mbgl label Oct 24, 2019
@pozdnyakov pozdnyakov merged commit 4fb75ba into master Oct 24, 2019
@pozdnyakov pozdnyakov deleted the mikhail_variable-text-avoid-edges branch October 24, 2019 12:39
@springmeyer
Copy link
Contributor

This is missing a test. Can a test be added to the C++ test runner that ensures this is working as expected?

/cc @chloekraw

@springmeyer
Copy link
Contributor

Coverage reporting indicates that this code is not covered by any tests currently:

Screen Shot 2019-10-24 at 7 33 08 AM

https://codecov.io/gh/mapbox/mapbox-gl-native/compare/744102d95524f6ae3b451f9cfd726651008fcd6b...f8550b81b023d0fcb8b61ed9c2bdf794195b3dae/src/src/mbgl/text/placement.cpp#L166

/cc @mapbox/static-apis

@pozdnyakov
Copy link
Contributor Author

This is missing a test. Can a test be added to the C++ test runner that ensures this is working as expected?

Working on it atm, the test will be added to the gl-js repo shortly

@springmeyer
Copy link
Contributor

@pozdnyakov I'm noticing that the code in this PR is still uncovered in latest master: https://codecov.io/gh/mapbox/mapbox-gl-native/src/128da903c19c860e89be36bf67bd02cfa408a525/src/mbgl/text/placement.cpp#L167

Is that because mapbox-gl-native still needs to be updated to the latest mapbox-gl-js submodule version?

@pozdnyakov
Copy link
Contributor Author

Is that because mapbox-gl-native still needs to be updated to the latest mapbox-gl-js submodule version?

Right, #15865 should fix it.

@springmeyer
Copy link
Contributor

Oddly it doesn't. But maybe therefore I have the wrong expectations. I've been assuming that these lines would be covered:

Screen Shot 2019-10-26 at 8 32 48 AM

But they still appear not to be with 8aaffd7 given: https://codecov.io/gh/mapbox/mapbox-gl-native/src/8aaffd71108117120652f45dba199d9c1dff2307/src/mbgl/text/placement.cpp#L166

@tmpsantos
Copy link
Contributor

@springmeyer I think it is because we do not collect coverage from render tests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Core The cross-platform C++ core, aka mbgl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants