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

"icon-allow-overlap: true, text-allow-overlap: true, text-optional: false" displays icons when it shouldn't #12483

Closed
RomainQuidet opened this issue Jul 25, 2018 · 11 comments
Assignees
Labels
bug Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS rendering

Comments

@RomainQuidet
Copy link
Contributor

RomainQuidet commented Jul 25, 2018

Hi,
We are using our own style and since mapbox iOS 4.x, our road shields rendering is incorrect (see video attached). Previous SDK 3.x were rendering the shields without flickering.

I also paste our shields style layer.
We are using icon-allow-overlap = true in the styling. When I remove this option, the rendering is not flickering anymore.
It seems to be a collision detection issue or a rendering priority vs collision issue.

shields_tests

Here is the style used:
{ "filter": [ "==", "shield_type", "4" ], "id": "shield_regional_label", "layout": { "icon-allow-overlap": true, "icon-image": "eu_shield_yellow_3.png", "icon-offset": [ 0, -0.7 ], "icon-padding": { "stops": [ [ 9.9, 80 ], [ 10.9, 70 ], [ 11.9, 60 ], [ 12.9, 50 ], [ 13.9, 30 ], [ 14.9, 2 ] ] }, "icon-rotation-alignment": "viewport", "symbol-avoid-edges": true, "symbol-placement": "line", "symbol-spacing": 350, "text-field": "{shield_text}", "text-font": [ "DejaVu Sans Condensed" ], "text-rotation-alignment": "viewport", "text-size": 9 }, "minzoom": 9, "paint": { "icon-opacity": 0.8, "text-color": "#000000" }, "source": "standard", "source-layer": "road_network", "type": "symbol" }

Steps to reproduce

  1. Load style with a layer using
  2. zoom in / out

Expected behavior

The shields are rendered without flickering

Actual behavior

the shields are flickering

Configuration

iOS/macOS versions: 4.x
Xcode version: 9.x

@fabian-guerra fabian-guerra added bug iOS Mapbox Maps SDK for iOS Core The cross-platform C++ core, aka mbgl rendering labels Jul 25, 2018
@fabian-guerra
Copy link
Contributor

/cc @ChrisLoer

@ChrisLoer
Copy link
Contributor

Hi @RomainQuidet can you include an equivalent screen capture from the 3.x SDK? I'm having trouble visualizing what the expected behavior is (and also which zoom levels are being crossed). There's a moment in the zoom-in animation where a bunch of the shield icons appear overlapping each other, but then fade out. That could be cross-fading happening because of crossing to a new integer zoom level, or that could be collision detection, I'm not sure. If you stayed at that zoom level where they show up, you'd expect them to all keep showing, but only having text labels for some of them.

I don't see text-optional: true in your style snippet, so I think that means once placement happens and the text gets collided out (since it doesn't have text-allow-overlap: true), it also knocks the icons out.

@RomainQuidet
Copy link
Contributor Author

Hi @ChrisLoer, thanks for your quick reply!
The incorrect behavior is the yellow shields showing empty and then disappearing (flashing).
This behavior is not visible with the same style on 3.x SDKs.

I'll make a video with the 3.x SDK, and try your suggestions about the text options.

@RomainQuidet
Copy link
Contributor Author

Same style with 3.x SDK:

shields_tests_v3

@RomainQuidet
Copy link
Contributor Author

I don't see text-optional: true in your style snippet, so I think that means once placement happens and the text gets collided out (since it doesn't have text-allow-overlap: true), it also knocks the icons out.

We don't want the text in shields to be optional. That's why we did not add those options.
It seems that the 3.x SDK tests text collisions before drawing the icon, but the 4.x SDK does draw the icon then test text collisions and removes the icons...

@RomainQuidet
Copy link
Contributor Author

@ChrisLoer For information, our web team falls in the same issue.
They were using JS sdk 0.43, all was working great. Since they tried 0.46, they found out the same issue, plus another wrong display of town label on dezoom.

Here is the video

test_web_0 46

@ChrisLoer
Copy link
Contributor

It seems that the 3.x SDK tests text collisions before drawing the icon, but the 4.x SDK does draw the icon then test text collisions and removes the icons...

Yeah that's true. In 3.x we did tile-based collision detection and we'd do all the collision detection for a tile before loading it. In 4.x we do global collision detection (see #10436), which sometimes requires showing tiles before collision detection has run on all their symbols. For symbols with *-allow-overlap: true, we show them even before collision detection runs because we "know" they're going to be able to show. But when we made the changes we just missed the case where only one of them allows overlap and the other one (text/icon) isn't optional. This is just a bug, thanks for helping us find it! The relevant code for us to change is here:

JointOpacityState defaultOpacityState(
bucket.layout.get<style::TextAllowOverlap>(),
bucket.layout.get<style::IconAllowOverlap>(),
true);

Until we fix the bug, does it work for you to just set icon-allow-overlap: false? I'm guessing you had icon-allow-overlap: true because you thought it looked OK for the edges of the shields to overlap a little bit, but since the size of the text and the size of the icon are pretty close to each other and you already have text-allow-overlap: false, I think it should be a pretty minimal change to your style?

cc @ansis

@ChrisLoer ChrisLoer changed the title Wrong shields rendering "icon-allow-overlap: true, text-allow-overlap: true, text-optional: false" displays icons when it shouldn't Jul 26, 2018
@RomainQuidet
Copy link
Contributor Author

Thanks again for this feedback.
I'm discovering our shields style as I did not design it (I'm in the mobile dev team).
In our style we have icon-allow-overlap: true and implicit text-allow-overlap: false. Of course the text is not optional in the shields.

In the documentation for icon-allow-overlap: true:

If true, the icon will be visible even if it collides with other previously drawn symbols

By symbols, it means icons AND texts ?
I think we choose icon overlap to true to be able to draw shields over road names for example. But we don't want shields to overlap each other.

If I set icon-allow-overlap: false and text-allow-overlap: false, I don't see anymore any shields.
If I set icon-allow-overlap: true and text-allow-overlap: true, I see too many shields, overlapping.

My confusion comes from the symbol construction, including an icon+text, what parameters are best to avoid this shields symbols to overlap each other, but still be draw even if there is road name below?

I'm also working with our server team for the styling.

@ChrisLoer
Copy link
Contributor

Yeah, these options can be confusing... 😕

By symbols, it means icons AND texts ?

For a single "symbol" in a symbol layer, the icon and the text are paired. The assumption is that they're meant to be shown together, so you don't have to worry about one of them colliding with the other.

If I set icon-allow-overlap: false and text-allow-overlap: false, I don't see anymore any shields.

I'm just guessing from the screenshots you attached, but this may be because the icons are bigger than their matching text. There was just enough room for the text to show, but not enough room for both the icon and the text to show. You can see what's happening with collision detection in GL JS by setting map.showCollisionBoxes = true, or in iOS by setting the debugMask to MGLMapDebugCollisionBoxesMask.

If my guess is right, you might be able to adjust the behavior by changing the icon-padding layout property, in order to make the icons appear smaller for collision purposes. The default is 2 pixels, you can try taking that down to 0 pixels.

what parameters are best to avoid this shields symbols to overlap each other, but still be draw even if there is road name below?

If you use *-allow-overlap, that means "overlap with everything", which means you'll have way too many of them. There is another option you could use on the road label layer, text-ignore-placement, which would mean "anything can overlap with this road label" -- but that would apply not just to road shields but to city names, etc.

The standard way to give "road shields" precedence over "road labels" is to place the road shield layer earlier in the style's layer order. That way the road shield gets placed first, and then when the road labels are being drawn, they'll be skipped if there's already a road shield taking their space.

@lilykaiser
Copy link

@ChrisLoer I'm assigning you since this seems to be related to your domain, symbols/collision - feel free to remove the assignment if you won't work on it.

@ChrisLoer
Copy link
Contributor

Fixed in #12521.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl iOS Mapbox Maps SDK for iOS rendering
Projects
None yet
Development

No branches or pull requests

4 participants