Automatic label anchor (rebased) #842
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hjanetzek
force-pushed
the
karim/rb-automatic-label-anchor
branch
3 times, most recently
from
52bbcee to
c4f98e2
Compare
karimnaaji
force-pushed
the
karim/rb-automatic-label-anchor
branch
2 times, most recently
from
de15719 to
556dd49
Compare
hjanetzek
force-pushed
the
karim/rb-automatic-label-anchor
branch
from
f8e03b6 to
ae36dc7
Compare
hjanetzek
force-pushed
the
karim/rb-automatic-label-anchor
branch
from
e4db674 to
166c636
Compare
karimnaaji
force-pushed
the
karim/rb-automatic-label-anchor
branch
2 times, most recently
from
656690f to
95c1201
Compare
|
Both @hjanetzek and me are happy with the state of this branch, @tallytalwar can you give a quick review on this. |
karimnaaji
force-pushed
the
karim/rb-automatic-label-anchor
branch
from
39eddae to
fd7bcad
Compare
|
Will get to reviewing this soon. |
hjanetzek
force-pushed
the
karim/rb-automatic-label-anchor
branch
2 times, most recently
from
49b58e3 to
a0da5ae
Compare
- So anchor fallback state would be able to switch between different text alignments
- rename wait_occ to none state
squash: remove logs squash: fix post rebase
hjanetzek
force-pushed
the
karim/rb-automatic-label-anchor
branch
from
a0da5ae to
3a0e912
Compare
squash: remove unused include
squashed: - parse label anchor style string only once - fix packing of active label quads - fixup: missing alignment from first anchor - use direct mapping between Align enum and m_textRangeIndex position - fixup: uninitialized variables
label could have been hidden for a few frames (trying all fallbacks). fading it out does not make sense then
squashed - cleanup: use initializer-list (required gcc >= 5)
- m_occludedLastFrame was necessary for _onlyTransitions pass which did not keep the m_occluded state of the previous frame (fixed with labels.cpp:83)
hjanetzek
force-pushed
the
karim/rb-automatic-label-anchor
branch
from
3a0e912 to
bb67c0e
Compare
|
There are a bunch of conceptual changes here, can we document these in the PR description please? Will help reviewing this and also for debugging in future. |
|
@tallytalwar added some notes to the PR description. |
|
@karimnaaji thinking about the fallback logic again: why not try the fallbacks immediately instead of waiting for the next frame? I'll see if this works. Edit: I guess this was just not possible before the changes of tile-label-placement got merged which introduced incremental collision testing. |
|
Good to go. |
|
Continuing at #893 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rebase of #806
anchor: top,[right, bottom, left, top]Optimizations
Don't use multiple sets of quads (for each anchor), follow up work going in Store width offsets instead of quad ranges for automatic anchors #862Bugs
- [ ] label offset does not work (check bubble-wrap with debug_all_labels)Notes:
Label now stores a set of Anchors which are defined by the anchor-style parameter
https://github.com/tangrams/tangram-es/pull/842/files#diff-7d439434d5d36de1c37a6cb2f01b81aaR37.
When a label got occluded (or couldn't be placed when its tile becomes active) the label state changes from
visible/(none) -> fallbackand the next anchor position is tried on the next frame until all positions have been tried or the label could be placed.https://github.com/tangrams/tangram-es/pull/842/files#diff-d66207e00a5f8416ddf6f8aea50052dcR284
A TextLabel now stores quads for left/right/center alignments which are chosen to match the currently active Anchor. https://github.com/tangrams/tangram-es/pull/842/files#diff-1fd6c9d719003eb34d010a7063a941e7R38, https://github.com/tangrams/tangram-es/pull/842/files#diff-4cb2204fedb2ba4ea43314c75ab033f0R204