Skip to content

Commit

Permalink
Revert 2895759 and
Browse files Browse the repository at this point in the history
11f37f1 (#1061).

This is breaking more things in the map importer pipeline. Need to
rethink this change and do it more cautiously.
  • Loading branch information
dabreegster committed Feb 13, 2023
1 parent 2895759 commit 9abe271
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 17 deletions.
6 changes: 2 additions & 4 deletions geom/src/line.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@ use crate::{Angle, Distance, PolyLine, Polygon, Pt2D, EPSILON_DIST};
pub struct Line(Pt2D, Pt2D);

impl Line {
// TODO Merge this with must_new and change all callers
/// Creates a line segment between two points. Never fails. If the two points are extremely
/// close together, behavior may be unusual later.
/// Creates a line segment between two points, which must not be the same
pub fn new(pt1: Pt2D, pt2: Pt2D) -> Result<Line> {
if pt1.dist_to(pt2) <= EPSILON_DIST {
println!("Warning: Line from {:?} to {:?} is small", pt1, pt2);
bail!("Line from {:?} to {:?} too small", pt1, pt2);
}
Ok(Line(pt1, pt2))
}
Expand Down
2 changes: 1 addition & 1 deletion geom/src/polygon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl Polygon {

/// Top-left at the origin. Doesn't take Distance, because this is usually pixels, actually.
pub fn maybe_rectangle(width: f64, height: f64) -> Result<Self> {
Ring::strict_new(vec![
Ring::new(vec![
Pt2D::new(0.0, 0.0),
Pt2D::new(width, 0.0),
Pt2D::new(width, height),
Expand Down
22 changes: 17 additions & 5 deletions geom/src/polyline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::{BTreeMap, HashSet};
use std::fmt;

use anyhow::{Context, Result};
use geo::ClosestPoint;
use geo::prelude::ClosestPoint;
use serde::{Deserialize, Serialize};

use crate::{
Expand Down Expand Up @@ -40,8 +40,8 @@ impl PolyLine {
});

if pts.windows(2).any(|pair| pair[0] == pair[1]) {
println!(
"Warning: PL with total length {} and {} pts has ~dupe adjacent pts",
bail!(
"PL with total length {} and {} pts has ~dupe adjacent pts",
length,
pts.len(),
);
Expand All @@ -67,7 +67,6 @@ impl PolyLine {
}

/// Doesn't check for duplicates. Use at your own risk.
// TODO Merge all of the constructors
pub fn unchecked_new(pts: Vec<Pt2D>) -> PolyLine {
assert!(pts.len() >= 2);
let length = pts.windows(2).fold(Distance::ZERO, |so_far, pair| {
Expand Down Expand Up @@ -529,6 +528,14 @@ impl PolyLine {
Polygon::pretessellated(vec![ring], tessellation)
}

/// This produces a `Polygon` with a possibly invalid `Ring`. It shouldn't crash. Use sparingly.
pub fn unsafe_make_polygons(&self, width: Distance) -> Polygon {
let tessellation = self.thicken_tessellation(width);
let ring = Ring::unsafe_deduping_new(tessellation.points.clone())
.expect("PolyLine::make_polygons() failed");
Polygon::pretessellated(vec![ring], tessellation)
}

/// Just produces a Tessellation
pub fn thicken_tessellation(&self, width: Distance) -> Tessellation {
// TODO Don't use the angle corrections yet -- they seem to do weird things.
Expand Down Expand Up @@ -998,7 +1005,12 @@ impl PolyLine {
}

pub(crate) fn to_geo(&self) -> geo::LineString {
geo::LineString::from(self)
let pts: Vec<geo::Point> = self
.pts
.iter()
.map(|pt| geo::Point::new(pt.x(), pt.y()))
.collect();
pts.into()
}

/// Walk along the PolyLine, starting `buffer_ends` from the start and ending `buffer_ends`
Expand Down
31 changes: 24 additions & 7 deletions geom/src/ring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ impl Ring {
}

if let Some(pair) = pts.windows(2).find(|pair| pair[0] == pair[1]) {
println!(
"Warning: Ring has duplicate adjacent points near {}",
pair[0]
);
bail!("Ring has duplicate adjacent points near {}", pair[0]);
}

let result = Ring { pts };
Expand All @@ -43,12 +40,32 @@ impl Ring {
Ok(result)
}

pub fn strict_new(pts: Vec<Pt2D>) -> Result<Self> {
// Enforce no duplicate adjacent points
/// Use with caution. Ignores duplicate points
pub fn unsafe_deduping_new(mut pts: Vec<Pt2D>) -> Result<Ring> {
pts.dedup();
if pts.len() < 3 {
bail!("Can't make a ring with < 3 points");
}
if pts[0] != *pts.last().unwrap() {
bail!("Can't make a ring with mismatching first/last points");
}

if let Some(pair) = pts.windows(2).find(|pair| pair[0] == pair[1]) {
bail!("Ring has duplicate adjacent points near {}", pair[0]);
}
Self::new(pts)

let result = Ring { pts };

let mut seen_pts = HashSet::new();
for pt in result.pts.iter().skip(1) {
if seen_pts.contains(&pt.to_hashable()) {
// Just spam logs instead of crashing
println!("Ring has repeat non-adjacent points near {}", pt);
}
seen_pts.insert(pt.to_hashable());
}

Ok(result)
}

pub fn must_new(pts: Vec<Pt2D>) -> Ring {
Expand Down

0 comments on commit 9abe271

Please sign in to comment.