revamp handling of onscreen stuff. no more callback plumbing, and can

plumb prerender successfully into dynamic stuff. just can't do as much
work directly in DrawMap.

docs/TODO_ux.md

@@ -2,9 +2,9 @@
 
 ## Performance
 
-- cache draw stuff
+- cache draw agents
-	- use a Drawable in DrawPed/DrawCar/DrawBike.
+	- dont use traits for things... or move code back to DrawMap after all.
-	- fix the process_objects callback nonsense
+	- actually use Drawables in the places
 
 ## Quick n easy
 

editor/src/render/map.rs

@@ -7,10 +7,9 @@ use crate::render::extra_shape::{DrawExtraShape, ExtraShapeID};
 use crate::render::intersection::DrawIntersection;
 use crate::render::lane::DrawLane;
 use crate::render::parcel::DrawParcel;
-use crate::render::pedestrian::DrawPedestrian;
 use crate::render::turn::DrawTurn;
-use crate::render::{draw_vehicle, Renderable};
+use crate::render::Renderable;
-use crate::state::{Flags, ShowObjects};
+use crate::state::Flags;
 use aabb_quadtree::QuadTree;
 use abstutil::Timer;
 use ezgui::Prerender;
@@ -19,17 +18,11 @@ use map_model::{
     AreaID, BuildingID, BusStopID, FindClosest, IntersectionID, Lane, LaneID, Map, ParcelID,
     RoadID, Traversable, Turn, TurnID, LANE_THICKNESS,
 };
-use sim::{GetDrawAgents, Tick};
+use sim::Tick;
 use std::borrow::Borrow;
 use std::cell::RefCell;
 use std::collections::HashMap;
 
-#[derive(PartialEq)]
-pub enum RenderOrder {
-    BackToFront,
-    FrontToBack,
-}
-
 pub struct DrawMap {
     pub lanes: Vec<DrawLane>,
     pub intersections: Vec<DrawIntersection>,
@@ -40,7 +33,8 @@ pub struct DrawMap {
     pub bus_stops: HashMap<BusStopID, DrawBusStop>,
     pub areas: Vec<DrawArea>,
 
-    agents: RefCell<AgentCache>,
+    // TODO Move?
+    pub agents: RefCell<AgentCache>,
 
     quadtree: QuadTree<ID>,
 }
@@ -271,123 +265,23 @@ impl DrawMap {
         results
     }
 
-    // False from the callback means abort
+    // Unsorted, unexpanded, raw result.
-    pub fn handle_objects<T: ShowObjects, F: FnMut(Box<&Renderable>) -> bool>(
+    pub fn get_matching_objects(&self, bounds: Bounds) -> Vec<ID> {
-        &self,
+        let mut results: Vec<ID> = Vec::new();
-        screen_bounds: Bounds,
+        for &(id, _, _) in &self.quadtree.query(bounds.as_bbox()) {
-        map: &Map,
+            results.push(*id);
-        sim: &GetDrawAgents,
-        show_objs: &T,
-        order: RenderOrder,
-        mut callback: F,
-    ) {
-        // From background to foreground Z-order
-        let mut areas: Vec<Box<&Renderable>> = Vec::new();
-        let mut parcels: Vec<Box<&Renderable>> = Vec::new();
-        let mut lanes: Vec<Box<&Renderable>> = Vec::new();
-        let mut intersections: Vec<Box<&Renderable>> = Vec::new();
-        let mut buildings: Vec<Box<&Renderable>> = Vec::new();
-        let mut extra_shapes: Vec<Box<&Renderable>> = Vec::new();
-        let mut bus_stops: Vec<Box<&Renderable>> = Vec::new();
-        let mut turn_icons: Vec<Box<&Renderable>> = Vec::new();
-        // We can't immediately grab the Renderables; we need to do it all at once at the end.
-        let mut agents_on: Vec<Traversable> = Vec::new();
-
-        for &(id, _, _) in &self.quadtree.query(screen_bounds.as_bbox()) {
-            if show_objs.show(*id) {
-                match id {
-                    ID::Area(id) => areas.push(Box::new(self.get_a(*id))),
-                    ID::Parcel(id) => parcels.push(Box::new(self.get_p(*id))),
-                    ID::Lane(id) => {
-                        lanes.push(Box::new(self.get_l(*id)));
-                        if !show_objs.show_icons_for(map.get_l(*id).dst_i) {
-                            let on = Traversable::Lane(*id);
-                            if !self.agents.borrow().has(sim.tick(), on) {
-                                let mut list: Vec<Box<Renderable>> = Vec::new();
-                                for c in sim.get_draw_cars(on, map).into_iter() {
-                                    list.push(draw_vehicle(c, map));
-                                }
-                                for p in sim.get_draw_peds(on, map).into_iter() {
-                                    list.push(Box::new(DrawPedestrian::new(p, map)));
-                                }
-                                self.agents.borrow_mut().put(sim.tick(), on, list);
-                            }
-                            agents_on.push(on);
-                        }
-                    }
-                    ID::Intersection(id) => {
-                        intersections.push(Box::new(self.get_i(*id)));
-                        for t in &map.get_i(*id).turns {
-                            if show_objs.show_icons_for(*id) {
-                                turn_icons.push(Box::new(self.get_t(*t)));
-                            } else {
-                                let on = Traversable::Turn(*t);
-                                if !self.agents.borrow().has(sim.tick(), on) {
-                                    let mut list: Vec<Box<Renderable>> = Vec::new();
-                                    for c in sim.get_draw_cars(on, map).into_iter() {
-                                        list.push(draw_vehicle(c, map));
-                                    }
-                                    for p in sim.get_draw_peds(on, map).into_iter() {
-                                        list.push(Box::new(DrawPedestrian::new(p, map)));
-                                    }
-                                    self.agents.borrow_mut().put(sim.tick(), on, list);
-                                }
-                                agents_on.push(on);
-                            }
-                        }
-                    }
-                    // TODO front paths will get drawn over buildings, depending on quadtree order.
-                    // probably just need to make them go around other buildings instead of having
-                    // two passes through buildings.
-                    ID::Building(id) => buildings.push(Box::new(self.get_b(*id))),
-                    ID::ExtraShape(id) => extra_shapes.push(Box::new(self.get_es(*id))),
-                    ID::BusStop(id) => bus_stops.push(Box::new(self.get_bs(*id))),
-
-                    ID::Turn(_) | ID::Car(_) | ID::Pedestrian(_) | ID::Trip(_) => {
-                        panic!("{:?} shouldn't be in the quadtree", id)
-                    }
-                }
-            }
-        }
-
-        let mut borrows: Vec<Box<&Renderable>> = Vec::new();
-        borrows.extend(areas);
-        borrows.extend(parcels);
-        borrows.extend(lanes);
-        borrows.extend(intersections);
-        borrows.extend(buildings);
-        borrows.extend(extra_shapes);
-        borrows.extend(bus_stops);
-        borrows.extend(turn_icons);
-        let cache = self.agents.borrow();
-        for on in agents_on {
-            for obj in cache.get(on) {
-                borrows.push(obj);
-            }
-        }
-
-        // This is a stable sort.
-        borrows.sort_by_key(|r| r.get_zorder());
-
-        if order == RenderOrder::FrontToBack {
-            borrows.reverse();
-        }
-
-        for r in borrows {
-            if !callback(r) {
-                break;
-            }
         }
+        results
     }
 }
 
-struct AgentCache {
+pub struct AgentCache {
     tick: Option<Tick>,
     agents_per_on: HashMap<Traversable, Vec<Box<Renderable>>>,
 }
 
 impl AgentCache {
-    fn has(&self, tick: Tick, on: Traversable) -> bool {
+    pub fn has(&self, tick: Tick, on: Traversable) -> bool {
         if Some(tick) != self.tick {
             return false;
         }
@@ -395,14 +289,14 @@ impl AgentCache {
     }
 
     // Must call has() first.
-    fn get(&self, on: Traversable) -> Vec<Box<&Renderable>> {
+    pub fn get(&self, on: Traversable) -> Vec<Box<&Renderable>> {
         self.agents_per_on[&on]
             .iter()
             .map(|obj| Box::new(obj.borrow()))
             .collect()
     }
 
-    fn put(&mut self, tick: Tick, on: Traversable, agents: Vec<Box<Renderable>>) {
+    pub fn put(&mut self, tick: Tick, on: Traversable, agents: Vec<Box<Renderable>>) {
         if Some(tick) != self.tick {
             self.agents_per_on.clear();
             self.tick = Some(tick);

editor/src/render/mod.rs

@@ -11,6 +11,7 @@ mod parcel;
 mod pedestrian;
 mod turn;
 
+use crate::colors::ColorScheme;
 use crate::objects::{Ctx, ID};
 pub use crate::render::area::DrawArea;
 use crate::render::bike::DrawBike;
@@ -18,10 +19,10 @@ use crate::render::car::DrawCar;
 pub use crate::render::extra_shape::ExtraShapeID;
 pub use crate::render::intersection::{draw_signal_cycle, draw_signal_diagram};
 pub use crate::render::lane::DrawLane;
-pub use crate::render::map::{DrawMap, RenderOrder};
+pub use crate::render::map::DrawMap;
 pub use crate::render::pedestrian::DrawPedestrian;
 pub use crate::render::turn::{DrawCrosswalk, DrawTurn};
-use ezgui::{Color, GfxCtx};
+use ezgui::{Color, GfxCtx, Prerender};
 use geom::{Bounds, Distance, Pt2D};
 use map_model::Map;
 use sim::{DrawCarInput, VehicleType};
@@ -61,6 +62,15 @@ pub struct RenderOptions {
     pub show_all_detail: bool,
 }
 
+pub fn new_draw_vehicle(
+    input: DrawCarInput,
+    map: &Map,
+    _prerender: &Prerender,
+    _cs: &ColorScheme,
+) -> Box<Renderable> {
+    draw_vehicle(input, map)
+}
+
 pub fn draw_vehicle(input: DrawCarInput, map: &Map) -> Box<Renderable> {
     if input.vehicle_type == VehicleType::Bike {
         Box::new(DrawBike::new(input))

editor/src/render/pedestrian.rs

@@ -1,6 +1,7 @@
+use crate::colors::ColorScheme;
 use crate::objects::{Ctx, ID};
 use crate::render::{RenderOptions, Renderable};
-use ezgui::{Color, GfxCtx};
+use ezgui::{Color, GfxCtx, Prerender};
 use geom::{Bounds, Circle, Distance, Line, Pt2D};
 use map_model::Map;
 use sim::{DrawPedestrianInput, PedestrianID};
@@ -16,6 +17,15 @@ pub struct DrawPedestrian {
 }
 
 impl DrawPedestrian {
+    pub fn new_new(
+        input: DrawPedestrianInput,
+        map: &Map,
+        _prerender: &Prerender,
+        _cs: &ColorScheme,
+    ) -> DrawPedestrian {
+        DrawPedestrian::new(input, map)
+    }
+
     pub fn new(input: DrawPedestrianInput, map: &Map) -> DrawPedestrian {
         let turn_arrow = if let Some(t) = input.waiting_for_turn {
             // TODO this isn't quite right, but good enough for now

editor/src/state.rs

@@ -376,6 +376,7 @@ impl UIState for DefaultUIState {
     }
 }
 
+// TODO Doesn't need to be a trait anymore
 pub trait ShowObjects {
     fn show_icons_for(&self, id: IntersectionID) -> bool;
     fn show(&self, obj: ID) -> bool;

editor/src/ui.rs

@@ -1,15 +1,15 @@
 use abstutil;
 //use cpuprofiler;
 use crate::objects::{Ctx, RenderingHints, ID};
-use crate::render::{RenderOptions, RenderOrder, Renderable};
+use crate::render::{new_draw_vehicle, DrawPedestrian, RenderOptions, Renderable};
-use crate::state::UIState;
+use crate::state::{ShowObjects, UIState};
 use ezgui::{
-    Canvas, Color, EventCtx, EventLoopMode, Folder, GfxCtx, Key, ModalMenu, Text, TopMenu,
+    Canvas, Color, EventCtx, EventLoopMode, Folder, GfxCtx, Key, ModalMenu, Prerender, Text,
-    BOTTOM_LEFT, GUI,
+    TopMenu, BOTTOM_LEFT, GUI,
 };
 use geom::{Bounds, Circle, Distance};
 use kml;
-use map_model::{BuildingID, LaneID};
+use map_model::{BuildingID, LaneID, Traversable};
 use serde_derive::{Deserialize, Serialize};
 use sim::GetDrawAgents;
 use std::collections::HashSet;
@@ -237,27 +237,38 @@ impl<S: UIState> GUI<RenderingHints> for UI<S> {
     fn draw(&self, _: &mut GfxCtx, _: &RenderingHints) {}
 
     fn new_draw(&self, g: &mut GfxCtx, hints: &RenderingHints, screencap: bool) -> Option<String> {
+        let state = self.state.get_state();
+
         g.clear(
-            self.state
+            state
-                .get_state()
                 .cs
                 .get_def("map background", Color::rgb(242, 239, 233)),
         );
 
+        let (mut borrows, agents_on) =
+            self.get_renderables_back_to_front(g.get_screen_bounds(), &g.prerender());
+        let cache = state.primary.draw_map.agents.borrow();
+        for on in agents_on {
+            for obj in cache.get(on) {
+                borrows.push(obj);
+            }
+        }
+        // This is a stable sort.
+        borrows.sort_by_key(|r| r.get_zorder());
+
         let ctx = Ctx {
-            cs: &self.state.get_state().cs,
+            cs: &state.cs,
-            map: &self.state.get_state().primary.map,
+            map: &state.primary.map,
-            draw_map: &self.state.get_state().primary.draw_map,
+            draw_map: &state.primary.draw_map,
-            sim: &self.state.get_state().primary.sim,
+            sim: &state.primary.sim,
             hints: &hints,
         };
-
         let mut sample_intersection: Option<String> = None;
-        self.handle_objects(g.get_screen_bounds(), RenderOrder::BackToFront, |obj| {
+        for obj in borrows {
             let opts = RenderOptions {
-                color: self.state.get_state().color_obj(obj.get_id(), &ctx),
+                color: state.color_obj(obj.get_id(), &ctx),
-                debug_mode: self.state.get_state().layers.debug_mode.is_enabled(),
+                debug_mode: state.layers.debug_mode.is_enabled(),
-                is_selected: self.state.get_state().primary.current_selection == Some(obj.get_id()),
+                is_selected: state.primary.current_selection == Some(obj.get_id()),
                 // TODO If a ToggleableLayer is currently off, this won't affect it!
                 show_all_detail: screencap,
             };
@@ -268,9 +279,7 @@ impl<S: UIState> GUI<RenderingHints> for UI<S> {
                     sample_intersection = Some(format!("_i{}", id.0));
                 }
             }
-
+        }
-            true
-        });
 
         if !screencap {
             self.state.draw(g, &ctx);
@@ -344,56 +353,34 @@ impl<S: UIState> UI<S> {
     fn mouseover_something(&self, ctx: &EventCtx) -> Option<ID> {
         let pt = ctx.canvas.get_cursor_in_map_space()?;
 
-        let mut id: Option<ID> = None;
+        let (mut borrows, agents_on) = self.get_renderables_back_to_front(
-
-        self.handle_objects(
             Circle::new(pt, Distance::meters(3.0)).get_bounds(),
-            RenderOrder::FrontToBack,
+            ctx.prerender,
-            |obj| {
-                // Don't mouseover parcels.
-                // TODO Might get fancier rules in the future, so we can't mouseover irrelevant things
-                // in intersection editor mode, for example.
-                match obj.get_id() {
-                    ID::Parcel(_) => {}
-                    _ => {
-                        if obj.contains_pt(pt) {
-                            id = Some(obj.get_id());
-                            return false;
-                        }
-                    }
-                };
-                true
-            },
         );
-
+        let cache = self.state.get_state().primary.draw_map.agents.borrow();
-        id
+        for on in agents_on {
-    }
+            for obj in cache.get(on) {
-
+                borrows.push(obj);
-    fn handle_objects<F: FnMut(Box<&Renderable>) -> bool>(
-        &self,
-        bounds: Bounds,
-        order: RenderOrder,
-        callback: F,
-    ) {
-        let state = self.state.get_state();
-
-        let draw_agent_source: &GetDrawAgents = {
-            let tt = &state.primary_plugins.time_travel;
-            if tt.is_active() {
-                tt
-            } else {
-                &state.primary.sim
             }
-        };
+        }
+        // This is a stable sort.
+        borrows.sort_by_key(|r| r.get_zorder());
+        borrows.reverse();
 
-        state.primary.draw_map.handle_objects(
+        for obj in borrows {
-            bounds,
+            // Don't mouseover parcels.
-            &state.primary.map,
+            // TODO Might get fancier rules in the future, so we can't mouseover irrelevant things
-            draw_agent_source,
+            // in intersection editor mode, for example.
-            state,
+            match obj.get_id() {
-            order,
+                ID::Parcel(_) => {}
-            callback,
+                _ => {
-        )
+                    if obj.contains_pt(pt) {
+                        return Some(obj.get_id());
+                    }
+                }
+            };
+        }
+        None
     }
 
     fn save_editor_state(&self, canvas: &Canvas) {
@@ -407,6 +394,106 @@ impl<S: UIState> UI<S> {
         abstutil::write_json("../editor_state", &state).expect("Saving editor_state failed");
         info!("Saved editor_state");
     }
+
+    // TODO I guess this technically could go in DrawMap, but we have to pass lots of stuff again.
+    fn get_renderables_back_to_front(
+        &self,
+        bounds: Bounds,
+        prerender: &Prerender,
+    ) -> (Vec<Box<&Renderable>>, Vec<Traversable>) {
+        let state = self.state.get_state();
+        let map = &state.primary.map;
+        let draw_map = &state.primary.draw_map;
+
+        let mut areas: Vec<Box<&Renderable>> = Vec::new();
+        let mut parcels: Vec<Box<&Renderable>> = Vec::new();
+        let mut lanes: Vec<Box<&Renderable>> = Vec::new();
+        let mut intersections: Vec<Box<&Renderable>> = Vec::new();
+        let mut buildings: Vec<Box<&Renderable>> = Vec::new();
+        let mut extra_shapes: Vec<Box<&Renderable>> = Vec::new();
+        let mut bus_stops: Vec<Box<&Renderable>> = Vec::new();
+        let mut turn_icons: Vec<Box<&Renderable>> = Vec::new();
+        let mut agents_on: Vec<Traversable> = Vec::new();
+
+        for id in draw_map.get_matching_objects(bounds) {
+            if !state.show(id) {
+                continue;
+            }
+            match id {
+                ID::Area(id) => areas.push(Box::new(draw_map.get_a(id))),
+                ID::Parcel(id) => parcels.push(Box::new(draw_map.get_p(id))),
+                ID::Lane(id) => {
+                    lanes.push(Box::new(draw_map.get_l(id)));
+                    if !state.show_icons_for(map.get_l(id).dst_i) {
+                        agents_on.push(Traversable::Lane(id));
+                    }
+                }
+                ID::Intersection(id) => {
+                    intersections.push(Box::new(draw_map.get_i(id)));
+                    for t in &map.get_i(id).turns {
+                        if state.show_icons_for(id) {
+                            turn_icons.push(Box::new(draw_map.get_t(*t)));
+                        } else {
+                            agents_on.push(Traversable::Turn(*t));
+                        }
+                    }
+                }
+                // TODO front paths will get drawn over buildings, depending on quadtree order.
+                // probably just need to make them go around other buildings instead of having
+                // two passes through buildings.
+                ID::Building(id) => buildings.push(Box::new(draw_map.get_b(id))),
+                ID::ExtraShape(id) => extra_shapes.push(Box::new(draw_map.get_es(id))),
+                ID::BusStop(id) => bus_stops.push(Box::new(draw_map.get_bs(id))),
+
+                ID::Turn(_) | ID::Car(_) | ID::Pedestrian(_) | ID::Trip(_) => {
+                    panic!("{:?} shouldn't be in the quadtree", id)
+                }
+            }
+        }
+
+        // From background to foreground Z-order
+        let mut borrows: Vec<Box<&Renderable>> = Vec::new();
+        borrows.extend(areas);
+        borrows.extend(parcels);
+        borrows.extend(lanes);
+        borrows.extend(intersections);
+        borrows.extend(buildings);
+        borrows.extend(extra_shapes);
+        borrows.extend(bus_stops);
+        borrows.extend(turn_icons);
+
+        // Make sure agents are cached, but we can't actually return the references to them here,
+        // since the RefCell borrow can't outlive this function.
+        {
+            let sim: &GetDrawAgents = {
+                let tt = &state.primary_plugins.time_travel;
+                if tt.is_active() {
+                    tt
+                } else {
+                    &state.primary.sim
+                }
+            };
+            let tick = sim.tick();
+            let mut agents = draw_map.agents.borrow_mut();
+
+            for on in &agents_on {
+                if !agents.has(tick, *on) {
+                    let mut list: Vec<Box<Renderable>> = Vec::new();
+                    for c in sim.get_draw_cars(*on, map).into_iter() {
+                        list.push(new_draw_vehicle(c, map, prerender, &state.cs));
+                    }
+                    for p in sim.get_draw_peds(*on, map).into_iter() {
+                        list.push(Box::new(DrawPedestrian::new_new(
+                            p, map, prerender, &state.cs,
+                        )));
+                    }
+                    agents.put(tick, *on, list);
+                }
+            }
+        }
+
+        (borrows, agents_on)
+    }
 }
 
 #[derive(Serialize, Deserialize, Debug)]

ezgui/src/drawing.rs

@@ -56,6 +56,12 @@ impl<'a> GfxCtx<'a> {
         }
     }
 
+    pub fn prerender(&self) -> Prerender {
+        Prerender {
+            display: self.display,
+        }
+    }
+
     // Up to the caller to call unfork()!
     // TODO Canvas doesn't understand this change, so things like text drawing that use
     // map_to_screen will just be confusing.
@@ -120,19 +126,13 @@ impl<'a> GfxCtx<'a> {
     }
 
     pub fn draw_polygon(&mut self, color: Color, poly: &Polygon) {
-        let obj = Prerender {
+        let obj = self.prerender().upload_borrowed(vec![(color, poly)]);
-            display: self.display,
-        }
-        .upload_borrowed(vec![(color, poly)]);
         self.num_new_uploads += 1;
         self.redraw(&obj);
     }
 
     pub fn draw_polygon_batch(&mut self, list: Vec<(Color, &Polygon)>) {
-        let obj = Prerender {
+        let obj = self.prerender().upload_borrowed(list);
-            display: self.display,
-        }
-        .upload_borrowed(list);
         self.num_new_uploads += 1;
         self.redraw(&obj);
     }