Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collision boxes drift toward angled geometry despite text-offset #2171

Closed
1ec5 opened this issue Feb 12, 2023 · 4 comments
Closed

Collision boxes drift toward angled geometry despite text-offset #2171

1ec5 opened this issue Feb 12, 2023 · 4 comments
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed

Comments

@1ec5
Copy link
Contributor

1ec5 commented Feb 12, 2023

maplibre-gl-js version: 3.0.0-pre.4 (but this issue has been reported since at least 2017)

browser: Firefox 109 on macOS

Steps to Trigger Behavior

  1. Add two symbol layers to the style that each have a text-field and symbol-placement set to line.
  2. Set the Y component of each layer’s text-offset to a different value to spread the layers apart.

Link to Demonstration

In this test case, there are two labels that say “Lorem ipsum dolor sit amet”. Each is rendered by a symbol layer that is identical except for a different text-offset. text-ignore-placement remains disabled:

https://jsbin.com/zuxuyap/edit?js,output

showCollisionBoxes is enabled: the blue circles are the collision boxes of the label that GL JS decided to show, while the red circles are the collision boxes of the label that GL JS decided to hide.

Expected Behavior

Both labels should be visible. The collision boxes should approximately align with the rendered glyphs.

Actual Behavior

The second layer’s label collides out the first layer’s label, even though there’s plenty of room to show both. Both labels’ collision boxes drift toward the angle in the geometry instead of staying consistently offset:

Lorem ipsum

Impact

Typically, you’d style an international border by labeling a different country on either side of the border. This requires text-offset. However, this issue means that, on a sufficiently curved border, only one of the two labels will ever appear. For a practical example in a real, deployed style, see osm-americana/openstreetmap-americana#753 (comment).

Depending on the style, it may be possible to partially work around this issue by setting text-ignore-placement to true, disabling collision detection. However, the issue still reproduces, just less frequently. Moreover, in some styles, this may cause other layers’ labels to overlap, making them unreadable.

This issue was previously reported and confirmed in mapbox/mapbox-gl-js#4798 (comment).

@HarelM
Copy link
Collaborator

HarelM commented Feb 12, 2023

Isn't this basically the same as #2170?

@1ec5
Copy link
Contributor Author

1ec5 commented Feb 12, 2023

I figured it’s a different issue that happens to affect the same property. If you disable text-ignore-placement, #2170 still reproduces, but nothing gets collided anymore. Also, some of the issues linked from mapbox/mapbox-gl-js#4798 indicate that this issue also affects mouse events and icon-offset, which are unrelated to the glyph placement code. However, it’s possible that the fix would lie nearby.

@HarelM
Copy link
Collaborator

HarelM commented Feb 12, 2023

I see, thanks for the clarification.

@HarelM HarelM added bug Something isn't working PR is more than welcomed Extra attention is needed labels Feb 12, 2023
ChrisLoer added a commit to felt/maplibre-gl-js that referenced this issue Mar 10, 2023
ChrisLoer added a commit to felt/maplibre-gl-js that referenced this issue Mar 13, 2023
HarelM pushed a commit that referenced this issue Mar 14, 2023
* Add render tests for text-offset with curved lines.

* Improvements to text-offset for symbol-placement: line.
Instead of offsetting glyphs directly from line (which causes gaps or overlap at line vertices), create an offset line to place glyphs along.

* Updated render tests with new behavior for text-offset with line labels.

* Add CHANGELOG entry.

* Add ProjectionCache typing

* Add line intersection unit test.

* Add render test for collision issue #2171

* Add more line intersection tests.

* Move findLineIntersection into util

* Factor out projectVertexToViewport and findOffsetIntersectionPoint
Add comments
Rename variables with preference for avoiding abbreviation

* Factor out normal calculation into a function.

* Unit tests for line offset calculation.

* Add jsdoc style type comments.

* Reduce indentation.
@HarelM
Copy link
Collaborator

HarelM commented Mar 14, 2023

Fixed by #2260

@HarelM HarelM closed this as completed Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working PR is more than welcomed Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants