Webrender: Local clips on box shadows are ignored
https://bugzilla.mozilla.org/show_bug.cgi?id=1416535
When Gecko has a shadow on an inline element that is broken up over multiple lines, it wants to make it look as if there really was just a single shadow, but sliced up into pieces and distributed over the multiple lines. It does that by drawing the entire shadow for each of the pieces and clipping it to just the size of that piece.
That clipping is done using a local clip:
#[no_mangle]
pub extern "C" fn wr_dp_push_box_shadow(state: &mut WrState,
rect: LayoutRect,
clip: LayoutRect,
// ...
clip_mode: BoxShadowClipMode) {
let mut prim_info = LayoutPrimitiveInfo::with_clip_rect(rect, clip.into());
However, it looks like WebRender ignores that local clip, and the broken up inset box shadows end up being displayed completely. This can lead to surprisingly long underlines, for example.
Here's a wrench reftest that fails but which I'd expect to pass:
diff --git a/wrench/reftests/boxshadow/box-shadow-clip-ref.yaml b/wrench/reftests/boxshadow/box-shadow-clip-ref.yaml
new file mode 100644
index 00000000..7b181c80
--- /dev/null
+++ b/wrench/reftests/boxshadow/box-shadow-clip-ref.yaml
@@ -0,0 +1,40 @@
+---
+root:
+ items:
+ - type: stacking-context
+ bounds: [0, 0, 1000, 1000]
+ items:
+ - type: clip
+ bounds: [0, 0, 100, 200]
+ items:
+ - type: box-shadow
+ bounds: [ 20, 20, 200, 80 ]
+ color: blue
+ offset: [10, 20]
+ clip-mode: outset
+ - type: clip
+ bounds: [200, 0, 100, 200]
+ items:
+ - type: box-shadow
+ bounds: [ 220, 20, 200, 80 ]
+ color: red
+ offset: [10, 20]
+ blur-radius: 10
+ clip-mode: outset
+ - type: clip
+ bounds: [0, 200, 100, 200]
+ items:
+ - type: box-shadow
+ bounds: [ 20, 220, 200, 80 ]
+ color: green
+ offset: [10, 20]
+ clip-mode: inset
+ - type: clip
+ bounds: [200, 200, 100, 200]
+ items:
+ - type: box-shadow
+ bounds: [ 220, 220, 200, 80 ]
+ color: yellow
+ offset: [10, 20]
+ blur-radius: 10
+ clip-mode: inset
diff --git a/wrench/reftests/boxshadow/box-shadow-clip.yaml b/wrench/reftests/boxshadow/box-shadow-clip.yaml
new file mode 100644
index 00000000..fd5a419f
--- /dev/null
+++ b/wrench/reftests/boxshadow/box-shadow-clip.yaml
@@ -0,0 +1,33 @@
+---
+root:
+ items:
+ - type: stacking-context
+ bounds: [0, 0, 1000, 1000]
+ items:
+ - type: box-shadow
+ bounds: [ 20, 20, 200, 80 ]
+ clip-rect: [0, 0, 100, 200]
+ color: blue
+ offset: [10, 20]
+ clip-mode: outset
+ - type: box-shadow
+ bounds: [ 220, 20, 200, 80 ]
+ clip-rect: [200, 0, 100, 200]
+ color: red
+ offset: [10, 20]
+ blur-radius: 10
+ clip-mode: outset
+ - type: box-shadow
+ bounds: [ 20, 220, 200, 80 ]
+ clip-rect: [0, 200, 100, 200]
+ color: green
+ offset: [10, 20]
+ clip-mode: inset
+ - type: box-shadow
+ bounds: [ 220, 220, 200, 80 ]
+ clip-rect: [200, 200, 100, 200]
+ color: yellow
+ offset: [10, 20]
+ blur-radius: 10
+ clip-mode: inset
+
diff --git a/wrench/reftests/boxshadow/reftest.list b/wrench/reftests/boxshadow/reftest.list
index d08ca9b1..a1ad4d45 100644
--- a/wrench/reftests/boxshadow/reftest.list
+++ b/wrench/reftests/boxshadow/reftest.list
@@ -13,6 +13,7 @@ fuzzy(1,396) == inset-large-offset.yaml inset-large-offset-ref.png
fuzzy(1,6388) == rounding.yaml rounding-ref.yaml
== box-shadow-border-radii.yaml box-shadow-border-radii.png
== box-shadow-spread.yaml box-shadow-spread.png
+== box-shadow-clip.yaml box-shadow-clip-ref.yaml
== invalid.yaml invalid-ref.yaml
== inset-subpx.yaml inset-subpx.png
== inset-downscale.yaml inset-downscale.png
mstange
❤1
All 4 comments
@demo99 has started working on this bug.
mstange
on 17 Jan 2018
Here's Ethan's patch from his try push:
# HG changeset patch
# User Ethan Lin <[email protected]>
# Date 1516180055 -28800
# Node ID 57610bed06574df9d1856c8eb613ee4b8ad137bc
# Parent acc729d8b3d47505bdb19c340f411920a8500df5
fix box shadow clip
MozReview-Commit-ID: DrC0NMvGNd4
diff --git a/gfx/webrender/src/box_shadow.rs b/gfx/webrender/src/box_shadow.rs
--- a/gfx/webrender/src/box_shadow.rs
+++ b/gfx/webrender/src/box_shadow.rs
@@ -58,16 +58,17 @@ impl FrameBuilder {
mut blur_radius: f32,
spread_radius: f32,
border_radius: BorderRadius,
clip_mode: BoxShadowClipMode,
) {
if color.a == 0.0 {
return;
}
+ let local_clip = *prim_info.local_clip.clip_rect();
let (spread_amount, brush_clip_mode) = match clip_mode {
BoxShadowClipMode::Outset => {
(spread_radius, ClipMode::Clip)
}
BoxShadowClipMode::Inset => {
(-spread_radius, ClipMode::ClipOut)
}
@@ -92,39 +93,47 @@ impl FrameBuilder {
BoxShadowClipMode::Outset => {
// TODO(gw): Add a fast path for ClipOut + zero border radius!
clips.push(ClipSource::new_rounded_rect(
prim_info.rect,
border_radius,
ClipMode::ClipOut
));
+ let local_clip = local_clip.intersection(&shadow_rect);
+ if local_clip.is_none() {
+ return;
+ }
LayerPrimitiveInfo::with_clip(
shadow_rect,
LocalClip::RoundedRect(
- shadow_rect,
+ local_clip.unwrap(),
ComplexClipRegion::new(
shadow_rect,
shadow_radius,
ClipMode::Clip,
),
),
)
}
BoxShadowClipMode::Inset => {
clips.push(ClipSource::new_rounded_rect(
shadow_rect,
shadow_radius,
ClipMode::ClipOut
));
+ let local_clip = local_clip.intersection(&prim_info.rect);
+ if local_clip.is_none() {
+ return;
+ }
LayerPrimitiveInfo::with_clip(
prim_info.rect,
LocalClip::RoundedRect(
- prim_info.rect,
+ local_clip.unwrap(),
ComplexClipRegion::new(
prim_info.rect,
border_radius,
ClipMode::Clip
),
),
)
}
@@ -262,17 +271,20 @@ impl FrameBuilder {
// most of the pixels inside and just
// clipping out along the edges.
extra_clips.push(ClipSource::new_rounded_rect(
prim_info.rect,
border_radius,
ClipMode::ClipOut,
));
- let pic_info = LayerPrimitiveInfo::new(pic_rect);
+ let pic_info = LayerPrimitiveInfo::with_clip_rect(
+ pic_rect,
+ local_clip
+ );
self.add_primitive(
clip_and_scroll,
&pic_info,
extra_clips,
PrimitiveContainer::Picture(pic_prim),
);
}
BoxShadowClipMode::Inset => {
@@ -329,24 +341,29 @@ impl FrameBuilder {
cache_key,
pipeline_id,
);
pic_prim.add_primitive(
brush_prim_index,
clip_and_scroll
);
+ let local_clip = local_clip.intersection(&prim_info.rect);
+ if local_clip.is_none() {
+ return;
+ }
+
// Draw the picture one pixel outside the original
// rect to account for the inflate above. This
// extra edge will be clipped by the local clip
// rect set below.
let pic_rect = prim_info.rect.inflate(inflate_size + box_offset.x.abs(), inflate_size + box_offset.y.abs());
let pic_info = LayerPrimitiveInfo::with_clip_rect(
pic_rect,
- prim_info.rect
+ local_clip.unwrap()
);
// Add a normal clip to ensure nothing gets drawn
// outside the primitive rect.
if !border_radius.is_zero() {
extra_clips.push(ClipSource::new_rounded_rect(
prim_info.rect,
border_radius,
Markus, thanks. I have a branch for it: https://github.com/demo99/webrender/commit/b765e98f3cddb7a824fd801879efeba07ef94f43. There are two wrench tests for this clipping problem in the branch. IIRC, there is another wrench test that needs to be modified, but I lost my latest patch. I probably can't keep working on this issue. It would be good if you or anyone want to finish the patch. Thanks.
demo99
on 20 Jan 2018
❤3
😕3
This was fixed by #2405.
Darkspirit
on 20 Feb 2018
🎉1
Was this page helpful?
0 / 5 - 0 ratings
Related issues
mstange
路
3Comments
hgallagher1993
路
4Comments
Darkspirit
路
4Comments
mstange
路
3Comments
nical
路
4Comments
Most helpful comment
Markus, thanks. I have a branch for it: https://github.com/demo99/webrender/commit/b765e98f3cddb7a824fd801879efeba07ef94f43. There are two wrench tests for this clipping problem in the branch. IIRC, there is another wrench test that needs to be modified, but I lost my latest patch. I probably can't keep working on this issue. It would be good if you or anyone want to finish the patch. Thanks.