From 2c46228d06374122a8ae710b591078a771d43a81 Mon Sep 17 00:00:00 2001
From: Arman Uguray <armansito@google.com>
Date: Wed, 28 Jun 2023 17:58:01 -0700
Subject: [PATCH] [binning] Correctly handle disjoint bounding-box
 intersections

When the bounding boxes of a path and its clip are disjoint (i.e. they
do not intersect) the result of their intersection is a negative
rectangle.

When calculating the intersection of the bboxes, the binning stage
ensures that the bbox is non-negative. It then normalizes the
coordinates to bin dimensions and rounds the top-left corner down
to the neareast integer and the bottom-right corner up.

However this rounding causes zero-area bounding boxes to have a non-zero
area and sends the bottom-right corner to the placed in the next bin.
This causes fully clipped out draw objects to be included in binning,
with an incorrect clip bounding box that causes them to be erroneously
drawn with partial clipping.

`binning` now takes care around this logic to make sure that a zero-area
intersected-bbox gets skipped and clipped out. `tile_alloc`, also takes
care in its logic.

Fixes #286 and #333
---
 shader/binning.wgsl    | 19 ++++++++++++-------
 shader/tile_alloc.wgsl | 13 +++++++++----
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/shader/binning.wgsl b/shader/binning.wgsl
index 2672bb8a..c46026dc 100644
--- a/shader/binning.wgsl
+++ b/shader/binning.wgsl
@@ -86,15 +86,20 @@ fn main(
 
         let path_bbox = path_bbox_buf[draw_monoid.path_ix];
         let pb = vec4<f32>(vec4(path_bbox.x0, path_bbox.y0, path_bbox.x1, path_bbox.y1));
-        let bbox_raw = bbox_intersect(clip_bbox, pb);
-        // TODO(naga): clunky expression a workaround for broken lhs swizzle
-        let bbox = vec4(bbox_raw.xy, max(bbox_raw.xy, bbox_raw.zw));
+        let bbox = bbox_intersect(clip_bbox, pb);
 
         intersected_bbox[element_ix] = bbox;
-        x0 = i32(floor(bbox.x * SX));
-        y0 = i32(floor(bbox.y * SY));
-        x1 = i32(ceil(bbox.z * SX));
-        y1 = i32(ceil(bbox.w * SY));
+
+        // `bbox_intersect` can result in a zero or negative area intersection if the path bbox lies
+        // outside the clip bbox. If that is the case, Don't round up the bottom-right corner of the
+        // and leave the coordinates at 0. This way the path will get clipped out and won't get
+        // assigned to a bin.
+        if bbox.x < bbox.z && bbox.y < bbox.w {
+            x0 = i32(floor(bbox.x * SX));
+            y0 = i32(floor(bbox.y * SY));
+            x1 = i32(ceil(bbox.z * SX));
+            y1 = i32(ceil(bbox.w * SY));
+        }
     }
     let width_in_bins = i32((config.width_in_tiles + N_TILE_X - 1u) / N_TILE_X);
     let height_in_bins = i32((config.height_in_tiles + N_TILE_Y - 1u) / N_TILE_Y);
diff --git a/shader/tile_alloc.wgsl b/shader/tile_alloc.wgsl
index 8ec92176..1f7a4bcc 100644
--- a/shader/tile_alloc.wgsl
+++ b/shader/tile_alloc.wgsl
@@ -68,10 +68,15 @@ fn main(
     var y1 = 0;
     if drawtag != DRAWTAG_NOP && drawtag != DRAWTAG_END_CLIP {
         let bbox = draw_bboxes[drawobj_ix];
-        x0 = i32(floor(bbox.x * SX));
-        y0 = i32(floor(bbox.y * SY));
-        x1 = i32(ceil(bbox.z * SX));
-        y1 = i32(ceil(bbox.w * SY));
+
+        // Don't round up the bottom-right corner of the bbox if the area is zero and leave the
+        // coordinates at 0. This will make `tile_count` zero as the shape is clipped out.
+        if bbox.x < bbox.z && bbox.y < bbox.w {
+            x0 = i32(floor(bbox.x * SX));
+            y0 = i32(floor(bbox.y * SY));
+            x1 = i32(ceil(bbox.z * SX));
+            y1 = i32(ceil(bbox.w * SY));
+        }
     }
     let ux0 = u32(clamp(x0, 0, i32(config.width_in_tiles)));
     let uy0 = u32(clamp(y0, 0, i32(config.height_in_tiles)));