Unexpected behaviour when swapping the order of stacking context and clip

[staktrace]

I don't yet have fully reduced test case for this but here's a quick summary. I have a stacking context (no transform, but nonzero top-left) and inside it I push a clip (with a mask). Inside that I push an image. This works fine. If I then invert the order of the stacking context and clip, adjusting the clip positioning accordingly, then I get different behaviour.

Here's a dump from the first (good) case:

WRDL: PushStackingContext b=(x=58.000000, y=63.000000, w=32.000000, h=32.000000) t=none
    WRDL: PushClip id=3 r=(x=-40.000000, y=-45.000000, w=280.000000, h=280.000000) m=0x7fff403bf5d0 b=(x=-40.000000, y=-45.000000, w=280.000000, h=280.000000)
        WRDL: PushClipRegion r=(x=0.000000, y=0.000000, w=32.000000, h=32.000000) m=(nil) b=none
        WRDL: PushImage b=(x=0.000000, y=0.000000, w=32.000000, h=32.000000) s=(w=32.000000, h=32.000000) t=(w=32.000000, h=32.000000)
    WRDL: PopClip id=3
WRDL: PopStackingContext

and here's the dump from the bad case:

WRDL: PushClip id=3 r=(x=18.000000, y=18.000000, w=280.000000, h=280.000000) m=0x7ffe9eae82f0 b=(x=18.000000, y=18.000000, w=280.000000, h=280.000000)
    WRDL: PushStackingContext b=(x=58.000000, y=63.000000, w=32.000000, h=32.000000) t=none
        WRDL: PushClipRegion r=(x=0.000000, y=0.000000, w=32.000000, h=32.000000) m=(nil) b=none
        WRDL: PushImage b=(x=0.000000, y=0.000000, w=32.000000, h=32.000000) s=(w=32.000000, h=32.000000) t=(w=32.000000, h=32.000000)
    WRDL: PopStackingContext
WRDL: PopClip id=3

Since the clip coordinates are adjusted for whether it's inside or outside the stacking context, I would expect everything else to remain the same and the two variations to behave the same. However they do not, it looks like the image is getting clipped by the mask in the second case while it doesn't in the first case.

[glennw]

I'm wondering if it's related to this comment - https://github.com/servo/webrender/blob/master/webrender/src/clip_scroll_node.rs#L130. Does that sound possible @mrobinson ?

[staktrace]

I made a sample app that reproduces the problem. You can get it by checking out the test branch in my webrender fork

https://github.com/staktrace/webrender/tree/53d84abd1cea754c74736e4a92dfd640a0dae576

Run cargo run --example test and observe the behavior. Then modify the "if true" block inside test.rs to be "if false" so that it goes down the other branch in the condition, and re-run to observe the behaviour difference.

[staktrace]

I strongly suspect this is related to using a mask in the clip. While debugging another reftest I'm seeing what appears to be another (possibly related) problem with a clip mask not showing up where I expect it to. I haven't yet reduced the test.

[mrobinson]

@staktrace I think the issue here is that mask coordinates and the clip rect itself should be defined relative to the origin of the rectangle passed as the first argument to the clip. With this diff, it seems that both code paths produce the same image:

diff --git a/webrender/examples/test.rs b/webrender/examples/test.rs
index f478c7d..bcbb4fd 100644
--- a/webrender/examples/test.rs
+++ b/webrender/examples/test.rs
@@ -71,12 +71,13 @@ fn body(api: &RenderApi,
                                       Vec::new());
 
         let clip_rect = (-5, 0).by(32, 32);
+        let mask_rect = (0, 0).by(32, 32);
         let mask = webrender_traits::ImageMask {
             image: mask_img,
-            rect: clip_rect.clone(),
+            rect: mask_rect.clone(),
             repeat: false,
         };
-        let clip_region = builder.push_clip_region(&clip_rect, vec![], Some(mask));
+        let clip_region = builder.push_clip_region(&mask_rect, vec![], Some(mask));
         builder.push_clip_node(clip_rect, clip_region, None);
 
         let rect_bounds = (-10, -10).by(32, 32);
@@ -91,12 +92,13 @@ fn body(api: &RenderApi,
         builder.pop_stacking_context();
     } else {
         let clip_rect = (5, 10).by(32, 32);
+        let mask_rect = (0, 0).by(32, 32);
         let mask = webrender_traits::ImageMask {
             image: mask_img,
-            rect: clip_rect.clone(),
+            rect: mask_rect.clone(),
             repeat: false,
         };
-        let clip_region = builder.push_clip_region(&clip_rect, vec![], Some(mask));
+        let clip_region = builder.push_clip_region(&mask_rect, vec![], Some(mask));
         builder.push_clip_node(clip_rect, clip_region, None);
 
         builder.push_stacking_context(webrender_traits::ScrollPolicy::Scrollable,

I'd like to get rid of this inconsistency at some point. See #1090.

[staktrace]

Making this change does appear to fix the original reftest failure I was investigating. Thanks!