clip-rects in blurred shadows incorrectly handled

[Gankra]

Here's a modified version of shadow-clip-rect.yaml where I've truncated the clip on the right-side, added a blur, and expanded the bounds in all other directions.

Although only the elements have clip-rects, the blur of the shadow ends up being visually clipped as well. In addition, if I shrink the clip further on one of the elements, nothing happens at all (it doesn't even get clipped more!). If I expand the clip further then everything gets revealed more. This suggests to me that the shadow is somehow getting clipped to the union of all contained cliprects.

---
root:
  items:
        -
          type: "shadow"
          bounds: [10, 14, 215, 45]
          blur-radius: 3
          offset: [0, 0]
          color: black
        -
          type: line
          clip-rect: [10, 14, 119, 40]
          baseline: 45
          start: 14
          end: 210
          width: 3
          orientation: horizontal
          color: [0,0,0,0]
          style: solid
        -
          bounds: [10, 14, 215, 45]
          clip-rect: [10, 14, 119, 40]
          glyphs: [55, 75, 76, 86, 3, 76, 86, 3, 87, 75, 72, 3, 69, 72, 86, 87]
          offsets: [16, 43, 35.533333, 43, 51.533333, 43, 60.4, 43, 72.833336, 43, 80.833336, 43, 89.7, 43, 102.13333, 43, 110.13333, 43, 119, 43, 135, 43, 149.2, 43, 157.2, 43, 173.2, 43, 187.4, 43, 196.26666, 43]
          size: 18
          color: [0,0,0,0]
          font: "VeraBd.ttf"
        -
          type: line
          clip-rect: [10, 14, 119, 40]
          baseline: 32
          start: 14
          end: 210
          width: 3
          orientation: horizontal
          color: [0,0,0,0]
          style: solid
        -
          type: "pop-all-shadows"

[Gankra]

More dramatic example. Top: actual text, middle blurred shadow, bottom unblurred shadow.

---
root:
  items:
        -
          type: "shadow"
          bounds: [10, 14, 220, 200]
          blur-radius: 0
          offset: [0, 50]
          color: black
        -
          type: "shadow"
          bounds: [10, 14, 220, 100]
          blur-radius: 3
          offset: [0, 25]
          color: black
        -
          type: line
          clip-rect: [10, 14, 122, 40]
          baseline: 45
          start: 14
          end: 210
          width: 3
          orientation: horizontal
          color: red
          style: solid
        -
          bounds: [10, 14, 215, 45]
          clip-rect: [10, 14, 160, 40]
          glyphs: [55, 75, 76, 86, 3, 76, 86, 3, 87, 75, 72, 3, 69, 72, 86, 87]
          offsets: [16, 43, 35.533333, 43, 51.533333, 43, 60.4, 43, 72.833336, 43, 80.833336, 43, 89.7, 43, 102.13333, 43, 110.13333, 43, 119, 43, 135, 43, 149.2, 43, 157.2, 43, 173.2, 43, 187.4, 43, 196.26666, 43]
          size: 18
          color: red
          font: "VeraBd.ttf"
        -
          type: line
          clip-rect: [10, 14, 80, 40]
          baseline: 32
          start: 14
          end: 210
          width: 3
          orientation: horizontal
          color: red
          style: solid
        -
          type: "pop-all-shadows"

[Gankra]

@demo99, does this looks like it's just https://bugzilla.mozilla.org/show_bug.cgi?id=1416535 ?

[demo99]

I guess it's a different issue since the gecko bug 1416535 is for fixing the box shadow's clipping. The text shadow is another code path.

[Gankra]

Figured out why the clip seems to be unioned: it is, when computing the bounding box of the picture. The real question is why when drawing the sub-primitives we're ignoring(?) their local_clip_rect.

[mrobinson]

Here's a simplified case, which I think shows the problem pretty well:

---
root:
  items:
    -
      type: "shadow"
      bounds: [0, 0, 1000, 1000]
      blur-radius: 15
      offset: [0, 0]
      color: black
    -
      bounds: [100, 50, 300, 300]
      clip-rect: [100, 50, 50, 50]
      glyphs: [55]
      offsets: [100, 150]
      size: 100
      color: [255, 0, 0, 0.5]
      font: "VeraBd.ttf"
    -
      type: "pop-all-shadows"

Increasing the blur radius increases the amount of shadowed T visible. I will investigate a bit more.

[mrobinson]

The issue here seems to be that normal text is drawn with the shader from ps_text_run.glsl and text to be used in the blurred shadow is drawn to the cache so with cs_text_run.glsl. cs_txt_run.glsl doesn't take into account the item's local clip rect at all, so either the task_rect or the shader itself needs to take into account the local clip rect of the item.

[Gankra]

before i went on leave, this was what i had locally (this is actually two seperate bugs: lines aren't clipped properly, and text isn't clipped properly. the change to brush.glsl works and fixes lines, but i didn't finish getting the text fix working -- or i re-broke it and got confused at least):

diff --git a/webrender/res/brush.glsl b/webrender/res/brush.glsl
index c06a7e6e..e11268c2 100644
--- a/webrender/res/brush.glsl
+++ b/webrender/res/brush.glsl
@@ -87,11 +87,12 @@ void main(void) {
 
     if (pic_task.pic_kind_and_raster_mode > 0.0) {
         local_pos = local_segment_rect.p0 + aPosition.xy * local_segment_rect.size;
+        vec2 clamped_local_pos = clamp_rect(local_pos, brush_prim.local_clip_rect);
 
         // Right now - pictures only support local positions. In the future, this
         // will be expanded to support transform picture types (the common kind).
         device_pos = pic_task.common_data.task_rect.p0 +
-                     uDevicePixelRatio * (local_pos - pic_task.content_origin);
+                     uDevicePixelRatio * (clamped_local_pos - pic_task.content_origin);
 
 #ifdef WR_FEATURE_ALPHA_PASS
         write_clip(
diff --git a/webrender/res/cs_text_run.glsl b/webrender/res/cs_text_run.glsl
index 796fc789..8692bf60 100644
--- a/webrender/res/cs_text_run.glsl
+++ b/webrender/res/cs_text_run.glsl
@@ -32,9 +32,14 @@ void main(void) {
     // the glyph offset, relative to its primitive bounding rect.
     vec2 glyph_size = res.uv_rect.zw - res.uv_rect.xy;
     vec2 glyph_pos = res.offset + glyph_size * aPosition.xy;
-    vec2 local_pos = prim.task.common_data.task_rect.p0 + glyph_pos * res.scale +
-                     uDevicePixelRatio * (glyph.offset - prim.task.content_origin);
-    gl_Position = uTransform * vec4(local_pos, 0.0, 1.0);
+    RectWithSize device_rect = prim.local_clip_rect;
+    device_rect.p0 *= uDevicePixelRatio;
+    device_rect.size *= uDevicePixelRatio;
+    vec2 rel_device_pos = glyph_pos * res.scale + uDevicePixelRatio * glyph.offset;
+    vec2 clamped_rel_device_pos = clamp_rect(rel_device_pos, device_rect);
+    vec2 device_pos = clamped_rel_device_pos + prim.task.common_data.task_rect.p0
+                        - uDevicePixelRatio * prim.task.content_origin;
+    gl_Position = uTransform * vec4(device_pos, 0.0, 1.0);
 
     vec2 texture_size = vec2(textureSize(sColor0, 0));
     vec2 st0 = res.uv_rect.xy / texture_size;

[Gankra]

iirc @glennw was fine with fixing the issues in these places because we only care about supporting this for clip rects (and not complex clips).

[mrobinson]

Fixed via #2500.