CC @michaelkirk and @BudgieInWA as FYI

Background

Objects in the geom library perform some validity checks upfront:

• a Line can't be zero-length
• a PolyLine can't have any duplicate adjacent points (aka, no internal zero-length line segments)
• a PolyLine can't double back on itself and repeat points
• a Ring is like a PolyLine, but the first and last point must match
• It's unimplemented, but I've wanted to have even stronger checks for PolyLines and Rings -- the polylines should never cross themselves / self-intersect.

The definition of "zero length" depends on this rounding behavior. Everything in geom is meant to logically use fixed-precision arithmetic, so that serialization is idempotent. (Aka, if we calculate some geometry, serialize it, and deserialize it, the result in-memory should be exactly the same as the original thing, so we don't have subtle differences add up later in the traffic simulation. Worth noting that a-b-street/abstreet#689 means something here has been broken for ages, though!)

The intention behind upfront checks is to fail-fast, instead of let problems propagate and show up later, making debugging harder.

Problem

So so many bugs have been crashes from too-small lines somewhere. a-b-street/abstreet#1005 and a-b-street/abstreet#1051 are recent examples. And over in a GTFS viewer project, I recently switched over to a variation of PolyLine thickening that ignores these types of problems (https://github.com/dabreegster/bus_spotting/blob/7c3a0e8cf716e49d4291b3aa2930c64a90938620/ui/src/multiday/viewer.rs#L349).

In practice to resolve problems like this, the crash is totally unhelpful. I wind up disabling checks to just get something showing in the UI, so I can try to see what's going on. Sometimes there's no visible problem at all! (a-b-street/abstreet#1051 (comment) and the GTFS viewer case)

Past approach: double down and fix the root cause

The point of loudly crashing upfront is to force us to deal with whatever root cause. Sometimes that's been useful -- in a-b-street/abstreet#860, it made it obvious that we shifting a polyline multiple times is more dangerous than shifting once from some relative point. (But I think to debug that, I probably disabled all the assertions temporarily...)

Geometry comes from raw input (OSM, GTFS) and often has various problems. We generally try to smooth or dedupe points coming in. Lots of other geometry is derived (like tracing around the block for the LTN tool).

To fix some of the current round of bugs, we could keep trying to find the root cause. But for these specific cases, I tried a bit and couldn't. So, I'm tired of this approach; I don't think it's been helpful.

Short-term proposal: Remove validity checks

When we construct a Line with points too close to each other, just spam a warning to STDOUT or don't even care at all. This would "paper over" many of the current problems. There will be some downstream consequences -- like, what's the angle of a zero-length line segment? But I think most callers don't use angle and care. For example, Ring::get_shorter_slice_between winds up internally iterating over line segments just to check length, and angle doesn't matter at all.

Long-term proposal: Rewrite Line, PolyLine, and Ring APIs

#2 was about cleaning up many of the Polygon methods and always constructing something with valid Rings. I think it's time to attempt a big rewrite of the line-based stuff too, and just use georust wherever possible. PolyLine::dist_along_of_point can be replaced by LineLocatePoint, which isn't so obnoxiously picky about points that're very slightly past the start or end.

georust doesn't have everything we need today (projecting polylines left/right or slicing/clipping them to a [start, end] distance), but the ideal direction forward should be to contribute those algorithms there directly.

I wanted to test out some new georust/geo features in the context of abstreet, and consequently I've been spelunking in abstreet::geom.

I ran into some inconsistencies while trying to roundtrip some abstreet::geom::Polygyon to/from geo::Polygon - it resulted in some corrupt rendering like this:

My unverified hunch is that this some problem with one or more of:

• a mismatch in the points array and the precomputed triangulation indices
• an issue with the vertex uploading assuming closed vs open
• an issue with earcutr assuming closed vs open

While digging into that, I noticed that some (but not all) of the abstreet:geom::Polygon.points are closed. Do you have any strong feelings on whether abstreet::geom::Polygon.points should be closed?

Full disclosure is that more broadly I'm interested in making abstreet/geom more semantically consistent with georust/geo, in hopes of make future inter-op easier, but I'm also aware that these are the kinds of changes with no immediate benefit that could have a long tail of bugs in a system which is more or less currently working.