Fix label default repeat groups

[bcamper]

This fixes an issue with label repeat group logic. The intended behavior is for labels to apply repeat rules within their unique scene layer group. For example, labels for a major and minor road with the same name will be allowed to repeat near each other. This was intended to catch uncommon but still existent cases where labels of different feature classes are desirable to show.

However, now that the bug is fixed, I notice that there are some suboptimal cases in Mapzen styles where extra labels are being shown nearby to each other and a bit crowded -- often 2 but sometimes even 3 -- because they are different road types, including ramps, etc. See the extra Market St and Park Row labels below:

This can be addressed in the style by putting all of the road labels into one repeat_group, or at least into fewer, to consolidate them. That's what I've done in the default demo here as an example.

This leads me to wonder if we should consider changing the default instead. A couple proposals:

• Use a global namespace for repeat_group by default (this is what the bug was doing!)
• Use only the top-level scene layer name instead (e.g. roads, instead of getting as granular as each sub-layer). This would still allow cases like roads and parks with the same name to repeat.

In either case, the default could still be overridden with a custom repeat_group.

@nvkelso please review the above and give your thoughts on restoring the original behavior vs. using a new default.

[nvkelso]

Generally speaking, this makes the most sense to me as a default:

Use only the top-level scene layer name instead (e.g. roads, instead of getting as granular as each sub-layer). This would still allow cases like roads and parks with the same name to repeat.

I'll need to review the examples a bit more... @bcamper is there a dev URL I can point at to play with this new code against the Mapzen scenes?

[bcamper]

@nvkelso there is a preview build at: https://line-label-repeats--tangram.netlify.com/dist/tangram.debug.js

And more generally, thanks to Netlify, you can always get a live demo of any branch at: https://branch-name--tangram.netlify.com/

And library build at: https://branch-name--tangram.netlify.com/dist/tangram.debug.js

🙌

Agree that top-level layer is a good middle-ground default.

[bcamper]

@nvkelso will you be able to take a look at this soon? I'd like to get this fixed -- and any behavioral changes made -- for upcoming v0.17.0 release. I think this is now the only thing outstanding. Let me know if you need more assistance!

[nvkelso]

👀

[nvkelso]

Looking at this location in San Francisco, I see a regression in map labels – more are shown than before in a duplicative way. See Sanchez, Dubose, Market, and Scott labels for regressions. Others change a little bit and are "okay".

Clarification: The new branch looks fine for default map... but not when adding in the bike map overlay when it looks worse. Perhaps turning that on exposes a bug in the "top-level" layer logic? The bike map overlay does modify the priority of labels, so maybe the interplay between these changed in this branch? Like the priority is now honored but not dedup'd?

Because all the bike and road labels are grouped inside the top-level roads layer I expected there to be almost no difference between the 2 renders. The roads layer doesn't set a custom repeat group, except for shields.

[nvkelso]

I do like the overall balance of labels better in the new branch – less minor road labels that cross major streets. But often there's duplicate major street labels introduced in that process. I think it's okay to knock some of the minor labels out, but not to make room for duplicate labels).

Here's zoom 15 in SF, with circle annotations this time!

[nvkelso]

Zoom 14 is also arguably better overall balance, but with the dup's added in. Fell and Oak are noticeably better in the middle of the image (though Oak is newly duplicated).

[bcamper]

OK what you're seeing makes sense, because this branch simply "fixes" the behavior to be the original intent, where sub-layers repeat independently, hence additional duplicates that are from different road layers. I think this just confirms that this is probably not the actual behavior we want.

I will update this branch to implement the suggestion to default the repeat logic to the top-level style layer, and then let's compare again.

[bcamper]

Alright, this branch is updated so that the repeat_group defaults to the top-level layer in layers (e.g. all roads under roads). I think this is good (and simpler logic and code!) and basically will have the same results we had previously, in the vast majority of cases. The bug would have done weird things like prevent a park with the same name as a road from showing up (because they were being lumped into one big repeat group). But this top-level-layer logic won't have the problem with dupe road labels from different road types. (No doubt there are other things we can do to improve road labelling... but let's leave that out of scope for this issue :)

@nvkelso feel free to double-check if inclined; from my Differ tests, all screens are restored to the same labels as pre-this-branch (allowing that there is some randomness in which labels "win" on page load).

[nvkelso]

New setup looks good, ship it!