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

Rename SymbolBucket::addFeatures and SymbolBucket::placeFeatures #2884

Closed
jfirebaugh opened this issue Oct 30, 2015 · 4 comments · Fixed by #6298
Closed

Rename SymbolBucket::addFeatures and SymbolBucket::placeFeatures #2884

jfirebaugh opened this issue Oct 30, 2015 · 4 comments · Fixed by #6298
Assignees

Comments

@jfirebaugh
Copy link
Contributor

From #2873 (comment):

There are actually three steps:

  • parseFeatures: determines the text we need to display by replacing tokens and applying text transformations, and the glyphs we need
  • addFeatures: determines the potential position of labels
  • placeFeatures: actually places them and adds them to a buffer

placeFeatures is currently called as the last step in addFeatures, but there's no good reason to; we could separate that out and call the addFeatures without placing as soon as we have the symbols. Then we would have less work to do later. However, it means we'd have to introduce a third step beyond pending.

@jfirebaugh
Copy link
Contributor Author

Also @kkaefer says "addFeatures is poorly named" -- we should rename it to something better.

@lucaswoj
Copy link
Contributor

I agree that we should develop some better terminology around this API. How about

  • resolveText
  • calculateTextPlacement
  • placeFeatures

@kkaefer
Copy link
Contributor

kkaefer commented Nov 2, 2015

text => symbol, but otherwise good

@ansis
Copy link
Contributor

ansis commented Jan 27, 2016

they were separated in #3543 but not renamed

@jfirebaugh jfirebaugh changed the title Separate SymbolBucket::addFeatures and SymbolBucket::placeFeatures Rename SymbolBucket::addFeatures and SymbolBucket::placeFeatures Sep 9, 2016
@jfirebaugh jfirebaugh self-assigned this Sep 12, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants