1372321 - Make the clip rect and mask rect relative to the content rect when pushing a clip item

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement, P3)

Description

[Kartikaya Gupta (email:[e-mail])]

When defining a clip item in the WR API, there are two things passed to it: a content rect and a "clip region". The clip region contains two rects - a clip rect and a mask rect. Normally these rects are relative to the stacking context, but in the special case where the clip region is attached to a clip item (as opposed to say a rect or gradient), the clip rect and mask rect need to be relative to the content rect.

See discussion on servo/webrender#1352

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1372321 - When pushing a clip item, ensure the clip rect and mask rect are relative to the content rect.

Review commit: https://reviewboard.mozilla.org/r/148196/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/148196/

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Amazingly this change appears to not break reftests: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5b5ccc0898c5ebd00af9f872084f3bd25daf7b3

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1372321 - Log the mask rect when passing a WrImageMask to WR.

Review commit: https://reviewboard.mozilla.org/r/148206/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/148206/

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8876868 [details]
Bug 1372321 - When pushing a clip item, ensure the clip rect and mask rect are relative to the content rect.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/148196/diff/1-2/

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8876879 [details]
Bug 1372321 - Log the mask rect when passing a WrImageMask to WR.

https://reviewboard.mozilla.org/r/148206/#review152562

Comment 6

[Martin Robinson (email:[e-mail])]

Comment on attachment 8876868 [details]
Bug 1372321 - When pushing a clip item, ensure the clip rect and mask rect are relative to the content rect.

https://reviewboard.mozilla.org/r/148196/#review152824

This looks good to me. I think this isn't causing reftest changes because internally in WebRender we are ignoring the origin of the ClipRegion rectangle. The mask origin is used though. In the future, this will likely change once https://github.com/servo/webrender/issues/1090 is fixed.

::: gfx/webrender_bindings/src/bindings.rs:1337
(Diff revision 2)
> -    let clip_rect = clip_rect.into();
> -    let mask = unsafe { mask.as_ref() }.map(|x| x.into());
> +    let content_rect: LayoutRect = rect.into();
> +
> +    // Both the clip rect and mask rect need to be relative to the
> +    // content rect when the clip region is being used as part of a clip item.
> +    let mut clip_rect: LayoutRect = rect.into();
> +    clip_rect.origin = clip_rect.origin - content_rect.origin;

Since these two rectangles appear to be the same, perhaps it would make more sense to write:

let clip_rect = LayerRect::new(LayoutPoint::zero(), content_rect.size);

I think that would be a bit clearer.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Thanks. I updated the patch as you suggested.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8876868 [details]
Bug 1372321 - When pushing a clip item, ensure the clip rect and mask rect are relative to the content rect.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/148196/diff/2-3/

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8876879 [details]
Bug 1372321 - Log the mask rect when passing a WrImageMask to WR.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/148206/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c585f465f4b1
When pushing a clip item, ensure the clip rect and mask rect are relative to the content rect. r=mrobinson+572841
https://hg.mozilla.org/integration/autoland/rev/9f7d877140e6
Log the mask rect when passing a WrImageMask to WR. r=jrmuizel

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/c585f465f4b1
https://hg.mozilla.org/mozilla-central/rev/9f7d877140e6