Revert "Optimize order rendering and border drawing" (#3761)

Antonio Scandurra created

Reverts zed-industries/zed#3759

It seems like this is not quite right yet. Reverting until we figure out
what's wrong.

Change summary

Cargo.lock                   |  12 +++
crates/gpui2/Cargo.toml      |   1 
crates/gpui2/src/geometry.rs |  21 ------
crates/gpui2/src/scene.rs    | 126 +++++++++++++++++++++++++++++++------
crates/gpui2/src/style.rs    |  60 +----------------
crates/gpui2/src/window.rs   |   1 
6 files changed, 120 insertions(+), 101 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -4060,6 +4060,7 @@ dependencies = [
  "parking",
  "parking_lot 0.11.2",
  "pathfinder_geometry",
+ "plane-split",
  "png",
  "postage",
  "rand 0.8.5",
@@ -6655,6 +6656,17 @@ version = "0.3.27"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "26072860ba924cbfa98ea39c8c19b4dd6a4a25423dbdf219c1eca91aa0cf6964"
 
+[[package]]
+name = "plane-split"
+version = "0.18.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8c1f7d82649829ecdef8e258790b0587acf0a8403f0ce963473d8e918acc1643"
+dependencies = [
+ "euclid",
+ "log",
+ "smallvec",
+]
+
 [[package]]
 name = "plist"
 version = "1.5.0"

crates/gpui2/Cargo.toml 🔗

@@ -56,6 +56,7 @@ uuid = { version = "1.1.2", features = ["v4"] }
 waker-fn = "1.1.0"
 slotmap = "1.0.6"
 schemars.workspace = true
+plane-split = "0.18.0"
 bitflags = "2.4.0"
 
 [dev-dependencies]

crates/gpui2/src/geometry.rs 🔗

@@ -1590,15 +1590,6 @@ impl Edges<Pixels> {
             left: self.left.scale(factor),
         }
     }
-
-    /// Returns the maximum value of any edge.
-    ///
-    /// # Returns
-    ///
-    /// The maximum `Pixels` value among all four edges.
-    pub fn max(&self) -> Pixels {
-        self.top.max(self.right).max(self.bottom).max(self.left)
-    }
 }
 
 impl Into<Edges<Pixels>> for f32 {
@@ -1749,18 +1740,6 @@ impl Corners<Pixels> {
             bottom_left: self.bottom_left.scale(factor),
         }
     }
-
-    /// Returns the maximum value of any corner.
-    ///
-    /// # Returns
-    ///
-    /// The maximum `Pixels` value among all four corners.
-    pub fn max(&self) -> Pixels {
-        self.top_left
-            .max(self.top_right)
-            .max(self.bottom_right)
-            .max(self.bottom_left)
-    }
 }
 
 impl<T: Clone + Default + Debug> Corners<T> {

crates/gpui2/src/scene.rs 🔗

@@ -3,6 +3,8 @@ use crate::{
     ScaledPixels, StackingOrder,
 };
 use collections::BTreeMap;
+use etagere::euclid::{Point3D, Vector3D};
+use plane_split::{BspSplitter, Polygon as BspPolygon};
 use std::{fmt::Debug, iter::Peekable, mem, slice};
 
 // Exported to metal
@@ -17,6 +19,7 @@ pub type DrawOrder = u32;
 pub(crate) struct SceneBuilder {
     last_order: Option<(StackingOrder, LayerId)>,
     layers_by_order: BTreeMap<StackingOrder, LayerId>,
+    splitter: BspSplitter<(PrimitiveKind, usize)>,
     shadows: Vec<Shadow>,
     quads: Vec<Quad>,
     paths: Vec<Path<ScaledPixels>>,
@@ -31,6 +34,7 @@ impl Default for SceneBuilder {
         SceneBuilder {
             last_order: None,
             layers_by_order: BTreeMap::new(),
+            splitter: BspSplitter::new(),
             shadows: Vec::new(),
             quads: Vec::new(),
             paths: Vec::new(),
@@ -44,47 +48,103 @@ impl Default for SceneBuilder {
 
 impl SceneBuilder {
     pub fn build(&mut self) -> Scene {
-        let mut orders = vec![0; self.layers_by_order.len()];
+        // Map each layer id to a float between 0. and 1., with 1. closer to the viewer.
+        let mut layer_z_values = vec![0.; self.layers_by_order.len()];
         for (ix, layer_id) in self.layers_by_order.values().enumerate() {
-            orders[*layer_id as usize] = ix as u32;
+            layer_z_values[*layer_id as usize] = ix as f32 / self.layers_by_order.len() as f32;
         }
         self.layers_by_order.clear();
         self.last_order = None;
 
-        for shadow in &mut self.shadows {
-            shadow.order = orders[shadow.order as usize];
+        // Add all primitives to the BSP splitter to determine draw order
+        self.splitter.reset();
+
+        for (ix, shadow) in self.shadows.iter().enumerate() {
+            let z = layer_z_values[shadow.order as LayerId as usize];
+            self.splitter
+                .add(shadow.bounds.to_bsp_polygon(z, (PrimitiveKind::Shadow, ix)));
+        }
+
+        for (ix, quad) in self.quads.iter().enumerate() {
+            let z = layer_z_values[quad.order as LayerId as usize];
+            self.splitter
+                .add(quad.bounds.to_bsp_polygon(z, (PrimitiveKind::Quad, ix)));
         }
-        self.shadows.sort_by_key(|shadow| shadow.order);
 
-        for quad in &mut self.quads {
-            quad.order = orders[quad.order as usize];
+        for (ix, path) in self.paths.iter().enumerate() {
+            let z = layer_z_values[path.order as LayerId as usize];
+            self.splitter
+                .add(path.bounds.to_bsp_polygon(z, (PrimitiveKind::Path, ix)));
         }
-        self.quads.sort_by_key(|quad| quad.order);
 
-        for path in &mut self.paths {
-            path.order = orders[path.order as usize];
+        for (ix, underline) in self.underlines.iter().enumerate() {
+            let z = layer_z_values[underline.order as LayerId as usize];
+            self.splitter.add(
+                underline
+                    .bounds
+                    .to_bsp_polygon(z, (PrimitiveKind::Underline, ix)),
+            );
         }
-        self.paths.sort_by_key(|path| path.order);
 
-        for underline in &mut self.underlines {
-            underline.order = orders[underline.order as usize];
+        for (ix, monochrome_sprite) in self.monochrome_sprites.iter().enumerate() {
+            let z = layer_z_values[monochrome_sprite.order as LayerId as usize];
+            self.splitter.add(
+                monochrome_sprite
+                    .bounds
+                    .to_bsp_polygon(z, (PrimitiveKind::MonochromeSprite, ix)),
+            );
         }
-        self.underlines.sort_by_key(|underline| underline.order);
 
-        for monochrome_sprite in &mut self.monochrome_sprites {
-            monochrome_sprite.order = orders[monochrome_sprite.order as usize];
+        for (ix, polychrome_sprite) in self.polychrome_sprites.iter().enumerate() {
+            let z = layer_z_values[polychrome_sprite.order as LayerId as usize];
+            self.splitter.add(
+                polychrome_sprite
+                    .bounds
+                    .to_bsp_polygon(z, (PrimitiveKind::PolychromeSprite, ix)),
+            );
         }
-        self.monochrome_sprites.sort_by_key(|sprite| sprite.order);
 
-        for polychrome_sprite in &mut self.polychrome_sprites {
-            polychrome_sprite.order = orders[polychrome_sprite.order as usize];
+        for (ix, surface) in self.surfaces.iter().enumerate() {
+            let z = layer_z_values[surface.order as LayerId as usize];
+            self.splitter.add(
+                surface
+                    .bounds
+                    .to_bsp_polygon(z, (PrimitiveKind::Surface, ix)),
+            );
         }
-        self.polychrome_sprites.sort_by_key(|sprite| sprite.order);
 
-        for surface in &mut self.surfaces {
-            surface.order = orders[surface.order as usize];
+        // Sort all polygons, then reassign the order field of each primitive to `draw_order`
+        // We need primitives to be repr(C), hence the weird reuse of the order field for two different types.
+        for (draw_order, polygon) in self
+            .splitter
+            .sort(Vector3D::new(0., 0., 1.))
+            .iter()
+            .enumerate()
+        {
+            match polygon.anchor {
+                (PrimitiveKind::Shadow, ix) => self.shadows[ix].order = draw_order as DrawOrder,
+                (PrimitiveKind::Quad, ix) => self.quads[ix].order = draw_order as DrawOrder,
+                (PrimitiveKind::Path, ix) => self.paths[ix].order = draw_order as DrawOrder,
+                (PrimitiveKind::Underline, ix) => {
+                    self.underlines[ix].order = draw_order as DrawOrder
+                }
+                (PrimitiveKind::MonochromeSprite, ix) => {
+                    self.monochrome_sprites[ix].order = draw_order as DrawOrder
+                }
+                (PrimitiveKind::PolychromeSprite, ix) => {
+                    self.polychrome_sprites[ix].order = draw_order as DrawOrder
+                }
+                (PrimitiveKind::Surface, ix) => self.surfaces[ix].order = draw_order as DrawOrder,
+            }
         }
-        self.surfaces.sort_by_key(|surface| surface.order);
+
+        self.shadows.sort_unstable();
+        self.quads.sort_unstable();
+        self.paths.sort_unstable();
+        self.underlines.sort_unstable();
+        self.monochrome_sprites.sort_unstable();
+        self.polychrome_sprites.sort_unstable();
+        self.surfaces.sort_unstable();
 
         Scene {
             shadows: mem::take(&mut self.shadows),
@@ -785,3 +845,23 @@ impl PathVertex<Pixels> {
 
 #[derive(Copy, Clone, Debug)]
 pub struct AtlasId(pub(crate) usize);
+
+impl Bounds<ScaledPixels> {
+    fn to_bsp_polygon<A: Copy>(&self, z: f32, anchor: A) -> BspPolygon<A> {
+        let upper_left = self.origin;
+        let upper_right = self.upper_right();
+        let lower_right = self.lower_right();
+        let lower_left = self.lower_left();
+
+        BspPolygon::from_points(
+            [
+                Point3D::new(upper_left.x.into(), upper_left.y.into(), z as f64),
+                Point3D::new(upper_right.x.into(), upper_right.y.into(), z as f64),
+                Point3D::new(lower_right.x.into(), lower_right.y.into(), z as f64),
+                Point3D::new(lower_left.x.into(), lower_left.y.into(), z as f64),
+            ],
+            anchor,
+        )
+        .expect("Polygon should not be empty")
+    }
+}

crates/gpui2/src/style.rs 🔗

@@ -402,65 +402,13 @@ impl Style {
 
         if self.is_border_visible() {
             cx.with_z_index(3, |cx| {
-                let corner_radii = self.corner_radii.to_pixels(bounds.size, rem_size);
-                let border_widths = self.border_widths.to_pixels(rem_size);
-                let max_border_width = border_widths.max();
-                let max_corner_radius = corner_radii.max();
-
-                let top_bounds = Bounds::from_corners(
-                    bounds.origin,
-                    bounds.upper_right()
-                        + point(Pixels::ZERO, max_border_width.max(max_corner_radius)),
-                );
-                let bottom_bounds = Bounds::from_corners(
-                    bounds.lower_left()
-                        - point(Pixels::ZERO, max_border_width.max(max_corner_radius)),
-                    bounds.lower_right(),
-                );
-                let left_bounds = Bounds::from_corners(
-                    top_bounds.lower_left(),
-                    bottom_bounds.origin + point(max_border_width, Pixels::ZERO),
-                );
-                let right_bounds = Bounds::from_corners(
-                    top_bounds.lower_right() - point(max_border_width, Pixels::ZERO),
-                    bottom_bounds.upper_right(),
-                );
-
-                let quad = quad(
+                cx.paint_quad(quad(
                     bounds,
-                    corner_radii,
+                    self.corner_radii.to_pixels(bounds.size, rem_size),
                     Hsla::transparent_black(),
-                    border_widths,
+                    self.border_widths.to_pixels(rem_size),
                     self.border_color.unwrap_or_default(),
-                );
-
-                cx.with_content_mask(Some(ContentMask { bounds: top_bounds }), |cx| {
-                    cx.paint_quad(quad.clone());
-                });
-                cx.with_content_mask(
-                    Some(ContentMask {
-                        bounds: right_bounds,
-                    }),
-                    |cx| {
-                        cx.paint_quad(quad.clone());
-                    },
-                );
-                cx.with_content_mask(
-                    Some(ContentMask {
-                        bounds: bottom_bounds,
-                    }),
-                    |cx| {
-                        cx.paint_quad(quad.clone());
-                    },
-                );
-                cx.with_content_mask(
-                    Some(ContentMask {
-                        bounds: left_bounds,
-                    }),
-                    |cx| {
-                        cx.paint_quad(quad);
-                    },
-                );
+                ));
             });
         }
 

crates/gpui2/src/window.rs 🔗

@@ -3074,7 +3074,6 @@ impl From<(&'static str, u64)> for ElementId {
 }
 
 /// A rectangle, to be rendered on the screen by GPUI at the given position and size.
-#[derive(Clone)]
 pub struct PaintQuad {
     bounds: Bounds<Pixels>,
     corner_radii: Corners<Pixels>,