Skip to content

Commit

Permalink
Get rid of geom::Polygon union operations. They always produced inval…
Browse files Browse the repository at this point in the history
…id (#981)

polygons. #951
  • Loading branch information
dabreegster committed Aug 30, 2022
1 parent 0e890a6 commit daf3ed3
Show file tree
Hide file tree
Showing 26 changed files with 223 additions and 164 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions apps/game/src/info/intersection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,11 @@ fn current_demand_body(ctx: &mut EventCtx, app: &App, id: IntersectionID) -> Wid
batch,
tooltips,
Box::new(|arrow| {
let mut list = vec![(Color::hex("#EE702E"), arrow.clone())];
let mut batch = GeomBatch::from(vec![(Color::hex("#EE702E"), arrow.clone())]);
if let Ok(p) = arrow.to_outline(Distance::meters(1.0)) {
list.push((Color::WHITE, p));
batch.push(Color::WHITE, p);
}
GeomBatch::from(list)
batch
}),
),
])
Expand Down
20 changes: 12 additions & 8 deletions apps/game/src/ungap/trip/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,14 @@ impl TripPlanner {

let main_route = RouteDetails::main_route(ctx, app, self.waypoints.get_waypoints());
self.main_route = main_route.details;
world
.add(ID::MainRoute)
.hitbox(main_route.hitbox)
.zorder(1)
.draw(main_route.draw)
.build(ctx);
if self.waypoints.get_waypoints().len() > 1 {
world
.add(ID::MainRoute)
.hitboxes(main_route.hitboxes)
.zorder(1)
.draw(main_route.draw)
.build(ctx);
}

self.files.autosave(app);
// This doesn't depend on the alt routes, so just do it here
Expand All @@ -107,7 +109,9 @@ impl TripPlanner {
avoid_stressful_roads: true,
},
] {
if app.session.routing_preferences == preferences {
if app.session.routing_preferences == preferences
|| self.waypoints.get_waypoints().len() < 2
{
continue;
}
let mut alt = RouteDetails::alt_route(
Expand All @@ -124,7 +128,7 @@ impl TripPlanner {
self.alt_routes.push(alt.details);
world
.add(ID::AltRoute(self.alt_routes.len() - 1))
.hitbox(alt.hitbox)
.hitboxes(alt.hitboxes)
.zorder(0)
.draw(alt.draw)
.hover_alpha(0.8)
Expand Down
13 changes: 4 additions & 9 deletions apps/game/src/ungap/trip/results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub struct BuiltRoute {
pub details: RouteDetails,
pub details_widget: Widget,
pub draw: ToggleZoomedBuilder,
pub hitbox: Polygon,
pub hitboxes: Vec<Polygon>,
pub tooltip_for_alt: Option<Text>,
}

Expand Down Expand Up @@ -99,7 +99,7 @@ impl RouteDetails {
preferences: RoutingPreferences,
) -> BuiltRoute {
let mut draw_route = ToggleZoomed::builder();
let mut hitbox_pieces = Vec::new();
let mut hitboxes = Vec::new();
let mut draw_high_stress = GeomBatch::new();
let mut draw_traffic_signals = GeomBatch::new();
let mut draw_unprotected_turns = GeomBatch::new();
Expand Down Expand Up @@ -176,7 +176,7 @@ impl RouteDetails {
.zoomed
.push(route_color.alpha(0.5), shape.clone());

hitbox_pieces.push(shape);
hitboxes.push(shape);

if let Some(color) = outline_color {
if let Some(outline) =
Expand Down Expand Up @@ -229,12 +229,7 @@ impl RouteDetails {
},
details_widget,
draw: draw_route,
hitbox: if hitbox_pieces.is_empty() {
// Dummy tiny hitbox
Polygon::rectangle(0.0001, 0.0001)
} else {
Polygon::union_all(hitbox_pieces)
},
hitboxes,
tooltip_for_alt: None,
}
}
Expand Down
4 changes: 2 additions & 2 deletions apps/ltn/src/connectivity.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use geom::{ArrowCap, Distance, PolyLine, Polygon};
use geom::{ArrowCap, Distance, PolyLine};
use street_network::Direction;
use widgetry::mapspace::{DummyID, World};
use widgetry::tools::PopupMsg;
Expand Down Expand Up @@ -284,7 +284,7 @@ fn setup_editing(

highlight_cell
.add_unnamed()
.hitbox(Polygon::union_all(polygons.clone()))
.hitboxes(polygons.clone())
// Don't draw cells by default
.drawn_in_master_batch()
.draw_hovered(batch)
Expand Down
4 changes: 2 additions & 2 deletions apps/ltn/src/draw_cells.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ impl RenderCells {
pub fn draw_island_outlines(&self) -> GeomBatch {
let neighbourhood_boundary = self
.boundary_polygon
.to_outline(Distance::meters(25.0))
.ok();
.get_outer_ring()
.map(|r| r.to_outline(Distance::meters(25.0)));

let mut batch = GeomBatch::new();
for (cell_color, polygons) in self.colors.iter().zip(self.polygons_per_cell.iter()) {
Expand Down
14 changes: 9 additions & 5 deletions apps/ltn/src/edit/shortcuts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,16 @@ pub fn make_world(
if let Some(ref focus) = focus {
let mut draw_path = GeomBatch::new();
let path = &focus.paths[focus.current_idx];
let poly = path
.trace_v2(&app.map)
.unwrap_or_else(|_| path.trace_all_polygons(&app.map));
let color = app.cs.good_to_bad_red.0.last().unwrap().alpha(0.8);

let color = *app.cs.good_to_bad_red.0.last().unwrap();
draw_path.push(color.alpha(0.8), poly);
match path.trace_v2(&app.map) {
Ok(poly) => {
draw_path.push(color, poly);
}
Err(_) => {
draw_path.extend(color, path.trace_all_polygons(&app.map));
}
}

let first_pt = path.get_req().start.pt(&app.map);
let last_pt = path.get_req().end.pt(&app.map);
Expand Down
11 changes: 11 additions & 0 deletions geom/src/bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ impl Bounds {
b
}

/// Create a boundary covering some polygons.
pub fn from_polygons(polygons: &[Polygon]) -> Bounds {
let mut b = Bounds::new();
for poly in polygons {
for pt in poly.points() {
b.update(*pt);
}
}
b
}

/// Update the boundary to include this point.
pub fn update(&mut self, pt: Pt2D) {
self.min_x = self.min_x.min(pt.x());
Expand Down
44 changes: 13 additions & 31 deletions geom/src/polygon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,30 +340,6 @@ impl Polygon {
Polygon::maybe_rounded_rectangle(w, h, r).unwrap_or_else(|| Polygon::rectangle(w, h))
}

// TODO Result won't be a nice Ring
pub fn union(self, other: Polygon) -> Polygon {
let mut points = self.points;
let mut indices = self.indices;
let offset = points.len() as u16;
points.extend(other.points);
for idx in other.indices {
indices.push(offset + idx);
}
Polygon {
points,
indices,
rings: None,
}
}

pub fn union_all(mut list: Vec<Polygon>) -> Polygon {
let mut result = list.pop().unwrap();
for p in list {
result = result.union(p);
}
result
}

/// Union all of the polygons into one geo::MultiPolygon
pub fn union_all_into_multipolygon(mut list: Vec<Polygon>) -> geo::MultiPolygon {
// TODO Not sure why this happened, or if this is really valid to construct...
Expand Down Expand Up @@ -414,16 +390,22 @@ impl Polygon {
self.to_geo().intersects(&pl.to_geo())
}

/// Creates the outline around the polygon, with the thickness half straddling the polygon and
/// half of it just outside. Only works for polygons that're formed from rings. Those made from
/// PolyLines won't work, for example.
pub fn to_outline(&self, thickness: Distance) -> Result<Polygon> {
/// Creates the outline around the polygon (both the exterior and holes), with the thickness
/// half straddling the polygon and half of it just outside. Only works for polygons that're
/// formed from rings.
///
/// Returns a `Tessellation` that may union together the outline from the exterior and multiple
/// holes. Callers that need a `Polygon` must call `to_outline` on the individual `Rings`.
pub fn to_outline(&self, thickness: Distance) -> Result<Tessellation> {
if let Some(ref rings) = self.rings {
Ok(Polygon::union_all(
rings.iter().map(|r| r.to_outline(thickness)).collect(),
Ok(Tessellation::union_all(
rings
.iter()
.map(|r| Tessellation::from(r.to_outline(thickness)))
.collect(),
))
} else {
Ring::new(self.points.clone()).map(|r| r.to_outline(thickness))
Ring::new(self.points.clone()).map(|r| Tessellation::from(r.to_outline(thickness)))
}
}

Expand Down
21 changes: 20 additions & 1 deletion geom/src/tessellation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ impl Tessellation {
Bounds::from(&self.points)
}

fn center(&self) -> Pt2D {
pub fn center(&self) -> Pt2D {
self.get_bounds().center()
}

Expand Down Expand Up @@ -135,6 +135,25 @@ impl Tessellation {
);
}
}

pub fn union(self, other: Self) -> Self {
let mut points = self.points;
let mut indices = self.indices;
let offset = points.len() as u16;
points.extend(other.points);
for idx in other.indices {
indices.push(offset + idx);
}
Self { points, indices }
}

pub fn union_all(mut list: Vec<Self>) -> Self {
let mut result = list.pop().unwrap();
for p in list {
result = result.union(p);
}
result
}
}

pub fn downsize(input: Vec<usize>) -> Vec<u16> {
Expand Down
10 changes: 7 additions & 3 deletions map_gui/src/render/area.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use geom::Polygon;
use geom::{Pt2D, Tessellation};
use map_model::{Area, AreaID, AreaType, Map};
use widgetry::{Color, EventCtx, Fill, GeomBatch, GfxCtx, Line, Text};

Expand Down Expand Up @@ -54,8 +54,12 @@ impl Renderable for DrawArea {

fn draw(&self, _: &mut GfxCtx, _: &dyn AppLike, _: &DrawOptions) {}

fn get_outline(&self, map: &Map) -> Polygon {
fn get_outline(&self, map: &Map) -> Tessellation {
// Since areas are so big, don't just draw the outline
map.get_a(self.id).polygon.clone()
Tessellation::from(map.get_a(self.id).polygon.clone())
}

fn contains_pt(&self, pt: Pt2D, map: &Map) -> bool {
map.get_a(self.id).polygon.contains_pt(pt)
}
}
12 changes: 7 additions & 5 deletions map_gui/src/render/bike.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use geom::{ArrowCap, Circle, Distance, Line, PolyLine, Polygon, Pt2D};
use geom::{ArrowCap, Circle, Distance, Line, PolyLine, Pt2D, Tessellation};
use map_model::Map;
use sim::{CarID, DrawCarInput, Intent, Sim};
use widgetry::{Drawable, GeomBatch, GfxCtx, Prerender};
Expand Down Expand Up @@ -139,10 +139,12 @@ impl Renderable for DrawBike {
g.redraw(&self.draw_default);
}

fn get_outline(&self, _: &Map) -> Polygon {
Circle::new(self.body_circle.center, Distance::meters(2.0))
.to_outline(OUTLINE_THICKNESS)
.unwrap()
fn get_outline(&self, _: &Map) -> Tessellation {
Tessellation::from(
Circle::new(self.body_circle.center, Distance::meters(2.0))
.to_outline(OUTLINE_THICKNESS)
.unwrap(),
)
}

fn contains_pt(&self, pt: Pt2D, _: &Map) -> bool {
Expand Down
6 changes: 3 additions & 3 deletions map_gui/src/render/building.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::cell::RefCell;

use geom::{Angle, Distance, Line, Polygon, Pt2D, Ring};
use geom::{Angle, Distance, Line, Polygon, Pt2D, Ring, Tessellation};
use map_model::{Building, BuildingID, Map, OffstreetParking};
use widgetry::{Color, Drawable, EventCtx, GeomBatch, GfxCtx, Line, Text};

Expand Down Expand Up @@ -237,12 +237,12 @@ impl Renderable for DrawBuilding {
0
}

fn get_outline(&self, map: &Map) -> Polygon {
fn get_outline(&self, map: &Map) -> Tessellation {
let b = map.get_b(self.id);
if let Ok(p) = b.polygon.to_outline(OUTLINE_THICKNESS) {
p
} else {
b.polygon.clone()
Tessellation::from(b.polygon.clone())
}
}

Expand Down
Loading

0 comments on commit daf3ed3

Please sign in to comment.