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

fix: PolylineLayer throws exception: "The west longitude can't be smaller than the east longitude" #1879

Conversation

josxha
Copy link
Contributor

@josxha josxha commented May 9, 2024

Some linestrings caused the PolylineLayer to throw an assertion exception.

Assertion failed: file:///Users/matthiaskoch/.pub-cache/git/flutter_map-930e8b60e125e630e3c85ab373c0ebb8e9b08d70/lib/src/geo/latlng_bounds.dart:82:16
east >= west
"The west longitude can't be smaller than the east longitude"

packages/flutter_map/src/geo/latlng_bounds.dart 82:28                             unsafe
packages/flutter_map/src/layer/polyline_layer/polyline_layer.dart 171:41          [_aggressivelyCullPolylines]
packages/flutter_map/src/layer/polyline_layer/polyline_layer.dart 138:11          build

This bug was reported by borg_1of3 on discord: https://discord.com/channels/951867686378409984/1235305884620689558/1235305884620689558

The bug sneaked into the code base in #1834.

@josxha josxha added the bug This issue reports broken functionality or another error label May 9, 2024
@josxha josxha added this to the v7.0 milestone May 9, 2024
@josxha josxha self-assigned this May 9, 2024
@josxha josxha changed the title fix: PolylineLayer throws exception for fix: PolylineLayer throws exception for some Polylines May 9, 2024
@josxha josxha changed the title fix: PolylineLayer throws exception for some Polylines fix: PolylineLayer throws exception for some Polylines May 9, 2024
@josxha josxha changed the title fix: PolylineLayer throws exception for some Polylines fix: PolylineLayer throws exception: "The west longitude can't be smaller than the east longitude" May 9, 2024
Copy link
Collaborator

@mootw mootw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@monsieurtanuki
Copy link
Contributor

My 2 cents: inlatlng_bounds.dart we see several times things like

    double minX = 180;
    double maxX = -180;
    double minY = 90;
    double maxY = -90;

As a refactoring, we may be better off making those values static const double and reuse those values elsewhere.

@fmt-Println-MKO
Copy link

thanks for the quick fix :)

@josxha
Copy link
Contributor Author

josxha commented May 10, 2024

@mootw thanks for the review and @monsieurtanuki thanks for the comment. I added the requested refactoring. Does anyone want to take another look before we merge?

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Should also make it easier for external uses (for example where a Polygon is painted over the entire world, and a hole created at the desired location).

@JaffaKetchup JaffaKetchup merged commit f68190c into fleaflet:master May 15, 2024
7 checks passed
@josxha josxha deleted the fix/PolylineLayer-throws-assertions-for-some-polylines branch May 15, 2024 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue reports broken functionality or another error
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants