From 23cf7c8d0c15d9c503d7fa57261c85ecbbe5622c Mon Sep 17 00:00:00 2001
From: Dustin Carlino <dabreegster@gmail.com>
Date: Sat, 26 Feb 2022 20:24:32 +0000
Subject: [PATCH] Detect incorrectly merged perimeters MUCH faster. #841
 [rebuild] [release]

---
 map_model/src/objects/block.rs | 29 +++++++++++++++++++++++------
 map_model/src/objects/lane.rs  |  1 +
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/map_model/src/objects/block.rs b/map_model/src/objects/block.rs
index 4fa1f59ddf..f88e8346a1 100644
--- a/map_model/src/objects/block.rs
+++ b/map_model/src/objects/block.rs
@@ -264,17 +264,34 @@ impl Perimeter {
         // Make sure we didn't wind up with any internal dead-ends
         self.collapse_deadends();
 
-        // TODO This is an expensive sanity check needed for
-        // https://github.com/a-b-street/abstreet/issues/841
-        if LOSSLESS_BLOCKFINDING && self.clone().to_block(map).is_err() {
-            *self = orig_self;
-            *other = orig_other;
-            return false;
+        // TODO Something in this method is buggy and produces invalid merges. Use a lightweight
+        // detection and bail out for now. https://github.com/a-b-street/abstreet/issues/841
+        if LOSSLESS_BLOCKFINDING {
+            if let Err(err) = self.check_continuity(map) {
+                error!(
+                    "A merged perimeter couldn't be blockified: {}. {:?}",
+                    err, self
+                );
+                *self = orig_self;
+                *other = orig_other;
+                return false;
+            }
         }
 
         true
     }
 
+    fn check_continuity(&self, map: &Map) -> Result<()> {
+        for pair in self.roads.windows(2) {
+            let r1 = map.get_r(pair[0].road);
+            let r2 = map.get_r(pair[1].road);
+            if r1.common_endpoint(r2) == CommonEndpoint::None {
+                bail!("Part of the perimeter goes from {:?} to {:?}, but they don't share a common endpoint", pair[0], pair[1]);
+            }
+        }
+        Ok(())
+    }
+
     /// Try to merge all given perimeters. If successful, only one perimeter will be returned.
     /// Perimeters are never "destroyed" -- if not merged, they'll appear in the results. If
     /// `stepwise_debug` is true, returns after performing just one merge.
diff --git a/map_model/src/objects/lane.rs b/map_model/src/objects/lane.rs
index 57fda2007e..0f95e2698e 100644
--- a/map_model/src/objects/lane.rs
+++ b/map_model/src/objects/lane.rs
@@ -303,6 +303,7 @@ impl Lane {
     }
 }
 
+#[derive(PartialEq)]
 pub enum CommonEndpoint {
     /// Two lanes/roads share one endpoint
     One(IntersectionID),