Rewrote and documented `was_top_layer` logic

Julia and Antonio Scandurra created 2 years ago

Co-Authored-By: Antonio Scandurra <[e-mail]>

Change summary

crates/gpui/src/window.rs | 46 ++++++++++++++++++++++++++--------------
1 file changed, 30 insertions(+), 16 deletions(-)

Detailed changes

crates/gpui/src/window.rs

@@ -800,26 +800,39 @@ impl<'a> WindowContext<'a> {
     }
 
     /// Returns true if there is no opaque layer containing the given point
-    /// on top of the given level. Layers whose level is an extension of the
-    /// level are not considered to be on top of the level.
-    pub fn was_top_layer(&self, point: &Point<Pixels>, level: &StackingOrder) -> bool {
-        for (opaque_level, _, bounds) in self.window.rendered_frame.depth_map.iter() {
-            if level >= opaque_level {
-                break;
+    /// on top of the given level. Layers who are extensions of the queried layer
+    /// are not considered to be on top of queried layer.
+    pub fn was_top_layer(&self, point: &Point<Pixels>, layer: &StackingOrder) -> bool {
+        // Precondition: the depth map is ordered from topmost to bottomost.
+
+        for (opaque_layer, _, bounds) in self.window.rendered_frame.depth_map.iter() {
+            if layer >= opaque_layer {
+                // The queried layer is either above or is the same as the this opaque layer.
+                // Anything after this point is guaranteed to be below the queried layer.
+                return true;
             }
 
-            if bounds.contains(point) {
-                let starts_with = opaque_level
-                    .iter()
-                    .zip(level.iter())
-                    .all(|(a, b)| a.z_index == b.z_index)
-                    && opaque_level.len() >= level.len();
+            if !bounds.contains(point) {
+                // This opaque layer is above the queried layer but it doesn't contain
+                // the given position, so we can ignore it even if it's above.
+                continue;
+            }
 
-                if !starts_with {
-                    return false;
-                }
+            // At this point, we've established that this opaque layer is on top of the queried layer
+            // and contains the position:
+            // - If the opaque layer is an extension of the queried layer, we don't want
+            // to consider the opaque layer to be on top and so we ignore it.
+            // - Else, we will bail early and say that the queried layer wasn't the top one.
+            let opaque_layer_is_extension_of_queried_layer = opaque_layer.len() >= layer.len()
+                && opaque_layer
+                    .iter()
+                    .zip(layer.iter())
+                    .all(|(a, b)| a.z_index == b.z_index);
+            if !opaque_layer_is_extension_of_queried_layer {
+                return false;
             }
         }
+
         true
     }
 
@@ -832,6 +845,7 @@ impl<'a> WindowContext<'a> {
             if level >= opaque_level {
                 break;
             }
+
             if opaque_level
                 .first()
                 .map(|c| c.z_index == ACTIVE_DRAG_Z_INDEX)
@@ -840,7 +854,7 @@ impl<'a> WindowContext<'a> {
                 continue;
             }
 
-            if bounds.contains(point) && !opaque_level.starts_with(level) {
+            if bounds.contains(point) {
                 return false;
             }
         }