Skip to content

port fix for icon-text-fit combined with variable text placement - #8737

Closed
ansis wants to merge 1 commit into
masterfrom
port-icon-text-fix-changes
Closed

ansis wants to merge 1 commit into
masterfrom
port-icon-text-fix-changes

Conversation

@ansis

@ansis ansis commented Sep 7, 2019

Copy link
Copy Markdown
Contributor

Addresses : #8583

This is still very much a WIP that doesn't work yet. mapbox/mapbox-gl-native#15367.

When I got to porting the changes in placement.cpp I tried adding the all at once because I didn't see a clear way of doing it incrementally. But that may have not been the best plan since it doesn't work and I'm not sure if it's close or if there are many large bugs remaining.

The next steps for working on this could be to try run yarn run test-render icon-text-fit and debug the crashes and problems one by one as they appear. But it may actually be better to do a careful readthrough of the original PR and make sure everything is included. Finally, if that doesn't work, it might be easiest to start with a fresh branch and gradually cherry-pick changes from this branch over or use it as a guide.

@kkaefer

kkaefer commented Sep 25, 2019

Copy link
Copy Markdown
Member

@ansis what's left to do for this PR? A few parts were already merged separately in #8755 and #8741. #8803 also has a related fix.

@ansis

ansis commented Sep 25, 2019

Copy link
Copy Markdown
Contributor Author

As far as I can tell all that is remaining is #8806. I'll close this pr and open a new one for that part.

@ansis ansis closed this Sep 25, 2019
@kkaefer
kkaefer deleted the port-icon-text-fix-changes branch September 26, 2019 07:47
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.

2 participants