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

Render tests for icon-text-fit combined with text-variable-anchor and text-writing-mode #8625

Merged
merged 4 commits into from
Aug 14, 2019

Conversation

alexshalamov
Copy link
Contributor

@alexshalamov alexshalamov commented Aug 13, 2019

Render tests required for mapbox/mapbox-gl-native#15367 and #8583

Copy link
Contributor

@ansis ansis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

I added ignores for -gl-js so that these can be merged.

@alexshalamov
Copy link
Contributor Author

@ansis what do you think about updated tests with 1px border diff? Also, since the collision box for a icon-text-fit icon has proper size (bigger), it affects symbols with line placement (#4861), therefore, I've updated one line placement test.

If you think these changes are acceptable and I can proceed with mapbox/mapbox-gl-native#15367, we can land new render tests and I will continue to work with native PR tomorrow.

@ansis
Copy link
Contributor

ansis commented Aug 14, 2019

I checked with Saman and he thinks this should be ok from the map design side! Seems ok to me as well.

@alexshalamov alexshalamov marked this pull request as ready for review August 14, 2019 19:35
alexshalamov and others added 4 commits August 14, 2019 22:39
In quads.js (quads.cpp) 1px quad padding is added before quad is
resized for icon-text-fit property. If 1px padding is added after
icon is stretched over the text box, we get 1px border. This change
addresses 1px rendering difference and updates failing render tests.
@alexshalamov alexshalamov force-pushed the alexshalamov_icon_text_fit_tests branch from bc31ba1 to d93fcd0 Compare August 14, 2019 19:41
@alexshalamov alexshalamov merged commit a61a887 into master Aug 14, 2019
@alexshalamov alexshalamov deleted the alexshalamov_icon_text_fit_tests branch August 14, 2019 20:59
@alexshalamov alexshalamov self-assigned this Aug 14, 2019
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.

3 participants