Improve comments on shader code for dashed borders (#28341)

Michael Sloan created

Improvements from going over the code with @as-cii 

Release Notes:

- N/A

Change summary

crates/gpui/src/platform/blade/shaders.wgsl | 49 +++++++++++++++-------
crates/gpui/src/platform/mac/shaders.metal  | 47 +++++++++++++++-------
2 files changed, 64 insertions(+), 32 deletions(-)

Detailed changes

crates/gpui/src/platform/blade/shaders.wgsl 🔗

@@ -513,9 +513,8 @@ fn fs_quad(input: QuadVarying) -> @location(0) vec4<f32> {
     let point = input.position.xy - quad.bounds.origin;
     let center_to_point = point - half_size;
 
-    // Signed distance field threshold for inclusion of pixels. Use of 0.5
-    // instead of 1.0 causes the width of rounded borders to appear more
-    // consistent with straight borders.
+    // Signed distance field threshold for inclusion of pixels. 0.5 is the
+    // minimum distance between the center of the pixel and the edge.
     let antialias_threshold = 0.5;
 
     // Radius of the nearest corner
@@ -612,24 +611,29 @@ fn fs_quad(input: QuadVarying) -> @location(0) vec4<f32> {
 
         // Dashed border logic when border_style == 1
         if (quad.border_style == 1) {
-            // Position in "dash space", where each dash period has length 1
+            // Position along the perimeter in "dash space", where each dash
+            // period has length 1
             var t = 0.0;
 
             // Total number of dash periods, so that the dash spacing can be
             // adjusted to evenly divide it
             var max_t = 0.0;
 
-            // Since border width affects the dash size, the density of dashes
-            // varies, and this is indicated by dash_velocity. It has units
-            // (dash period / pixel). So a dash velocity of (1 / 10) is 1 dash
-            // every 10 pixels.
-            var dash_velocity = 0.0;
-
+            // Border width is proportional to dash size. This is the behavior
+            // used by browsers, but also avoids dashes from different segments
+            // overlapping when dash size is smaller than the border width.
+            //
             // Dash pattern: (2 * border width) dash, (1 * border width) gap
             let dash_length_per_width = 2.0;
             let dash_gap_per_width = 1.0;
             let dash_period_per_width = dash_length_per_width + dash_gap_per_width;
 
+            // Since the dash size is determined by border width, the density of
+            // dashes varies. Multiplying a pixel distance by this returns a
+            // position in dash space - it has units (dash period / pixels). So
+            // a dash velocity of (1 / 10) is 1 dash every 10 pixels.
+            var dash_velocity = 0.0;
+
             // Dividing this by the border width gives the dash velocity
             let dv_numerator = 1.0 / dash_period_per_width;
 
@@ -645,8 +649,8 @@ fn fs_quad(input: QuadVarying) -> @location(0) vec4<f32> {
                 t = select(point.y, point.x, is_horizontal) * dash_velocity;
                 max_t = select(size.y, size.x, is_horizontal) * dash_velocity;
             } else {
-                // When corners are rounded, the dashes are laid out around the
-                // whole perimeter.
+                // When corners are rounded, the dashes are laid out clockwise
+                // around the whole perimeter.
 
                 let r_tr = quad.corner_radii.top_right;
                 let r_br = quad.corner_radii.bottom_right;
@@ -694,23 +698,34 @@ fn fs_quad(input: QuadVarying) -> @location(0) vec4<f32> {
                 if (is_near_rounded_corner) {
                     let radians = atan2(corner_center_to_point.y,
                                         corner_center_to_point.x);
-                    let corner_t = radians * corner_radius;
+                    let corner_t = radians * corner_radius * dash_velocity;
 
                     if (center_to_point.x >= 0.0) {
                         if (center_to_point.y < 0.0) {
                             dash_velocity = corner_dash_velocity_tr;
-                            t = upto_r - corner_t * dash_velocity;
+                            // Subtracted because radians is pi/2 to 0 when
+                            // going clockwise around the top right corner,
+                            // since the y axis has been flipped
+                            t = upto_r - corner_t;
                         } else {
                             dash_velocity = corner_dash_velocity_br;
-                            t = upto_br + corner_t * dash_velocity;
+                            // Added because radians is 0 to pi/2 when going
+                            // clockwise around the bottom-right corner
+                            t = upto_br + corner_t;
                         }
                     } else {
                         if (center_to_point.y >= 0.0) {
                             dash_velocity = corner_dash_velocity_bl;
-                            t = upto_l - corner_t * dash_velocity;
+                            // Subtracted because radians is pi/2 to 0 when
+                            // going clockwise around the bottom-left corner,
+                            // since the x axis has been flipped
+                            t = upto_l - corner_t;
                         } else {
                             dash_velocity = corner_dash_velocity_tl;
-                            t = upto_tl + corner_t * dash_velocity;
+                            // Added because radians is 0 to pi/2 when going
+                            // clockwise around the top-left corner, since both
+                            // axis were flipped
+                            t = upto_tl + corner_t;
                         }
                     }
                 } else {

crates/gpui/src/platform/mac/shaders.metal 🔗

@@ -121,7 +121,8 @@ fragment float4 quad_fragment(QuadFragmentInput input [[stage_in]],
   float2 point = input.position.xy - float2(quad.bounds.origin.x, quad.bounds.origin.y);
   float2 center_to_point = point - half_size;
 
-  // Signed distance field threshold for inclusion of pixels
+  // Signed distance field threshold for inclusion of pixels. 0.5 is the
+  // minimum distance between the center of the pixel and the edge.
   const float antialias_threshold = 0.5;
 
   // Radius of the nearest corner
@@ -211,24 +212,29 @@ fragment float4 quad_fragment(QuadFragmentInput input [[stage_in]],
 
     // Dashed border logic when border_style == 1
     if (quad.border_style == 1) {
-      // Position in "dash space", where each dash period has length 1
+      // Position along the perimeter in "dash space", where each dash
+      // period has length 1
       float t = 0.0;
 
       // Total number of dash periods, so that the dash spacing can be
       // adjusted to evenly divide it
       float max_t = 0.0;
 
-      // Since border width affects the dash size, the density of dashes
-      // varies, and this is indicated by dash_velocity. It has units
-      // (dash period / pixel). So a dash velocity of (1 / 10) is 1 dash
-      // every 10 pixels.
-      float dash_velocity = 0.0;
-
+      // Border width is proportional to dash size. This is the behavior
+      // used by browsers, but also avoids dashes from different segments
+      // overlapping when dash size is smaller than the border width.
+      //
       // Dash pattern: (2 * border width) dash, (1 * border width) gap
       const float dash_length_per_width = 2.0;
       const float dash_gap_per_width = 1.0;
       const float dash_period_per_width = dash_length_per_width + dash_gap_per_width;
 
+      // Since the dash size is determined by border width, the density of
+      // dashes varies. Multiplying a pixel distance by this returns a
+      // position in dash space - it has units (dash period / pixels). So
+      // a dash velocity of (1 / 10) is 1 dash every 10 pixels.
+      float dash_velocity = 0.0;
+
       // Dividing this by the border width gives the dash velocity
       const float dv_numerator = 1.0 / dash_period_per_width;
 
@@ -244,8 +250,8 @@ fragment float4 quad_fragment(QuadFragmentInput input [[stage_in]],
         max_t = is_horizontal ? size.x : size.y;
         max_t *= dash_velocity;
       } else {
-        // When corners are rounded, the dashes are laid out around the
-        // whole perimeter.
+        // When corners are rounded, the dashes are laid out clockwise
+        // around the whole perimeter.
 
         float r_tr = quad.corner_radii.top_right;
         float r_br = quad.corner_radii.bottom_right;
@@ -292,23 +298,34 @@ fragment float4 quad_fragment(QuadFragmentInput input [[stage_in]],
 
         if (is_near_rounded_corner) {
           float radians = atan2(corner_center_to_point.y, corner_center_to_point.x);
-          float corner_t = radians * corner_radius;
+          float corner_t = radians * corner_radius * dash_velocity;
 
           if (center_to_point.x >= 0.0) {
             if (center_to_point.y < 0.0) {
               dash_velocity = corner_dash_velocity_tr;
-              t = upto_r - corner_t * dash_velocity;
+              // Subtracted because radians is pi/2 to 0 when
+              // going clockwise around the top right corner,
+              // since the y axis has been flipped
+              t = upto_r - corner_t;
             } else {
               dash_velocity = corner_dash_velocity_br;
-              t = upto_br + corner_t * dash_velocity;
+              // Added because radians is 0 to pi/2 when going
+              // clockwise around the bottom-right corner
+              t = upto_br + corner_t;
             }
           } else {
             if (center_to_point.y >= 0.0) {
               dash_velocity = corner_dash_velocity_bl;
-              t = upto_l - corner_t * dash_velocity;
+              // Subtracted because radians is pi/1 to 0 when
+              // going clockwise around the bottom-left corner,
+              // since the x axis has been flipped
+              t = upto_l - corner_t;
             } else {
               dash_velocity = corner_dash_velocity_tl;
-              t = upto_tl + corner_t * dash_velocity;
+              // Added because radians is 0 to pi/2 when going
+              // clockwise around the top-left corner, since both
+              // axis were flipped
+              t = upto_tl + corner_t;
             }
           }
         } else {