[wip] Tile label placement #749

hjanetzek · 2016-05-24T11:27:36Z

Determine initial label placement during tile generation.

Positions tested during initial placements (discarded labels in purple)

All labels in final tiles

TODO:

Improve line-label candidate positions

karimnaaji · 2016-06-14T17:29:16Z

Starting to look over the details of this, looks really good so far!

karimnaaji · 2016-06-14T17:31:52Z

core/src/labels/labels.cpp

+
+    // Note: This causes non-deterministic placement, i.e. depending on
+    // navigation history.
+    if (l1->occludedLastFrame() != l2->occludedLastFrame()) {


Is this needed as a sorting rule? So we base the sorting only on things not related to label state.

It's not 'needed' to do this. It's a matter of preference of less flickering vs having always the same set of labels shown for a viewport - This is the behavior we had before so I kept it and added the Note.

karimnaaji · 2016-06-16T18:57:05Z

core/src/labels/labels.cpp

@@ -169,187 +174,135 @@ void Labels::skipTransitions(const std::vector<std::unique_ptr<Style>>& _styles,
    }
 }

-void Labels::checkRepeatGroups(std::vector<Label*>& _visibleSet) const {
+bool Labels::labelComparator(const LabelEntry& _a, const LabelEntry& _b) {


As the discussion we had in the meeting, we could use the depth (ground distance from camera, or perspective factor already available when projecting to screen) to sort labels within the same priority level.
Probably to consider out of this PR.

karimnaaji · 2016-07-12T22:18:43Z

core/src/labels/spriteLabel.cpp


-void SpriteLabel::align(glm::vec2& _screenPosition, const glm::vec2& _ap1, const glm::vec2& _ap2) {
+    if (m_occludedLastFrame) { dim += 2; }


Is that for debug?

This should reduce flickering. The idea was to increase the OBB of occluded labels so that they can only become active when there is some buffer to other labels and thereby are less likely to get occluded again immediately by minimal map movements. - Yes, needs a comment and the value should have a name.
ef535cb

I see, it's a good idea but I think this value should also be scaled by the pixel scale.

was exp: ignore out_of_screen labels - this caused way too many labels in isect2d buckets

- having it inlined should give the compiler some options to optimize - todo: test if compiler is smart enough to omit 2 divisions

matteblair · 2016-07-13T16:40:22Z

Seems like a good time to consider merging this - it's lingered for a long time now.

karimnaaji · 2016-07-13T17:17:21Z

@blair1618 yes we talked about merging this yesterday and I did a last review, it seems ready to go to me if @hjanetzek doesn't have any blockers on it.

hjanetzek · 2016-07-13T17:40:40Z

yeah, ready to merge.

♫just wait-ing for travis♫

matteblair · 2016-07-13T17:43:21Z

LGTM 👍

- major speedup compared to collecting all possible collisions first

- allows easier perf tracing for each phase

squashed: tweak: connect up to four line segments squash: line labels candidates

…iew)

- add Scene::m_pixelScale member

tallytalwar · 2016-07-18T18:12:07Z

core/src/labels/label.cpp

-                rot += M_PI;
-            } else {
-                std::swap(p1, p2);
+            if (_testVisibility && length < minLength) {


No need to test _testVisibility here, already done above.

The name _testVisibility may not be the best choice. It is used to return false and return early when the label is not renderable. In this case when the line segment is too short for the label.

Ohh I just mean, to do if (length < minLength). _testVisibility must be true here before of if (_testVisibility && clipped) check before.

hjanetzek force-pushed the tile-label-placement branch 4 times, most recently from 48106bd to 5945236 Compare May 27, 2016 12:47

hjanetzek force-pushed the tile-label-placement branch 9 times, most recently from b565b48 to 78dc184 Compare June 6, 2016 10:38

karimnaaji reviewed Jun 14, 2016
View reviewed changes

hjanetzek force-pushed the tile-label-placement branch from 4e091ed to e48a48f Compare June 16, 2016 08:32

hjanetzek mentioned this pull request Jun 16, 2016

Subdivide label line segments on overzoomed tile #637

Closed

karimnaaji reviewed Jun 16, 2016
View reviewed changes

hjanetzek force-pushed the tile-label-placement branch 2 times, most recently from e438ec3 to 483e2dd Compare June 23, 2016 15:32

karimnaaji mentioned this pull request Jun 24, 2016

[wip] Automatic label anchor #806

Closed

1 task

hjanetzek force-pushed the tile-label-placement branch 2 times, most recently from e40e1ce to 19bbcce Compare July 12, 2016 13:44

hjanetzek added the ready for review label Jul 12, 2016

karimnaaji self-assigned this Jul 12, 2016

karimnaaji reviewed Jul 12, 2016
View reviewed changes

label: no need to store aabb

b28e58b

hjanetzek force-pushed the tile-label-placement branch 2 times, most recently from 2e03461 to a3f7c6c Compare July 13, 2016 14:43

hjanetzek added 4 commits July 13, 2016 16:54

cleanup tests

8d7bd6a

was exp: ignore out_of_screen labels - this caused way too many labels in isect2d buckets

skip occluded out_of_screen labels

d962d54

smaller text/sprite vertices, simpler shader, less trigonometry

c1f5aa6

using worldToScreenSpace instead of world- to clip-, to screenspace

57d605b

- having it inlined should give the compiler some options to optimize - todo: test if compiler is smart enough to omit 2 divisions

hjanetzek force-pushed the tile-label-placement branch from a3f7c6c to d70dbc5 Compare July 13, 2016 15:08

hjanetzek and others added 14 commits July 13, 2016 20:15

screen-space labeling: use incremental collision testing

3ea60b4

- major speedup compared to collecting all possible collisions first

screen-space labeling: check repeatGroups before insert

21d16d7

fade-in non-occluded labels from fade-out state

166d61d

split-up updateLabelSet function

3aa941e

- allows easier perf tracing for each phase

improve line label candidate positions

3953ed2

squashed: tweak: connect up to four line segments squash: line labels candidates

keep screen position center at world center (less sliding in tilted v…

7a62ce3

…iew)

use correct tileSize in LabelCollider

bba5cd4

- add Scene::m_pixelScale member

fix: calc label screen offset

5ee993c

opt: use glm::length2

77367c9

fixup: labels debug-view

dc77881

Remove unused vertex attribute

aef392e

keep 4-byte vertex alignment

951a9c9

squash: name Label::activation_distance_threshold

129b631

scene: use moderate font size

51c484d

hjanetzek force-pushed the tile-label-placement branch from d70dbc5 to 51c484d Compare July 13, 2016 18:17

hjanetzek merged commit 14d8d52 into master Jul 14, 2016

karimnaaji deleted the tile-label-placement branch July 14, 2016 14:09

tallytalwar reviewed Jul 18, 2016
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wip] Tile label placement #749

[wip] Tile label placement #749

hjanetzek commented May 24, 2016 •

edited

Loading

karimnaaji commented Jun 14, 2016

karimnaaji Jun 14, 2016

hjanetzek Jun 14, 2016

karimnaaji Jun 16, 2016

karimnaaji Jul 12, 2016

hjanetzek Jul 13, 2016 •

edited

Loading

karimnaaji Jul 13, 2016

matteblair commented Jul 13, 2016

karimnaaji commented Jul 13, 2016

hjanetzek commented Jul 13, 2016

matteblair commented Jul 13, 2016

tallytalwar Jul 18, 2016

hjanetzek Jul 18, 2016

tallytalwar Jul 18, 2016


		void SpriteLabel::align(glm::vec2& _screenPosition, const glm::vec2& _ap1, const glm::vec2& _ap2) {
		if (m_occludedLastFrame) { dim += 2; }

[wip] Tile label placement #749

[wip] Tile label placement #749

Conversation

hjanetzek commented May 24, 2016 • edited Loading

karimnaaji commented Jun 14, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hjanetzek Jul 13, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matteblair commented Jul 13, 2016

karimnaaji commented Jul 13, 2016

hjanetzek commented Jul 13, 2016

matteblair commented Jul 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hjanetzek commented May 24, 2016 •

edited

Loading

hjanetzek Jul 13, 2016 •

edited

Loading