1415034 - Convert drop-shadow CSS filter to WebRender

Status: RESOLVED FIXED

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

Description

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

+++ This bug was initially created as a clone of Bug #1370564 +++

Comment 1

[Peter Chang[:pchang]]

This bug is related fb performance. I think we probably want to address this for wr-nightly based on bug 1411813 comment 13.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Sure, we can move it to nightly.  Still want to wait a bit before we work on it, until we clear out some of the non-fallback types of performance issues.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Here's the corresponding webrender issue: https://github.com/servo/webrender/issues/2044

Comment 5

[Peter Chang[:pchang]]

(In reply to Milan Sreckovic [:milan] from comment #2)
> Sure, we can move it to nightly.  Still want to wait a bit before we work on
> it, until we clear out some of the non-fallback types of performance issues.

I think this bug is needed to reduce the fallback items on fb and MotionMark. It's not improving the fallback path performance. Change to P2?

Comment 6

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

I'm starting work on this.

Comment 7

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Bug 1415034 - Add drop-shadow support.

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

Comment 8

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Upload a gecko wip. Still depend on webrender pull request. pr is here https://github.com/servo/webrender/pull/2091

Comment 9

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

 https://treeherder.mozilla.org/#/jobs?repo=try&revision=65b78085e1add61ec44a86095dd07e978e470292

Comment 10

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Add drop-shadow support.

The long-chain.html failure is tracking by
https://github.com/servo/webrender/issues/2197

MozReview-Commit-ID: FECidSvTQrY

Comment 11

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=11ea28ab783e2dfc76a0d9322bb93a872b738fa6&selectedJob=150092732

Comment 12

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

Comment on attachment 8935625 [details] [diff] [review]
Add drop-shadow support.

Review of attachment 8935625 [details] [diff] [review]:
-----------------------------------------------------------------

r+. One suggestion below. I don't feel too strongly about it but it would make it a little cleaner I think.

::: gfx/webrender_bindings/src/bindings.rs
@@ +365,5 @@
>  #[repr(C)]
>  #[derive(Copy, Clone)]
>  pub struct WrFilterOp {
>      filter_type: WrFilterOpType,
> +    argument: [c_float; 7],

Instead of lumping all 7 floats into one array it might be a bit cleaner to separate this into three fields:

argument: c_float, // holds radius for DropShadow; value for other filters
offset: LayoutVector2D, // only used for DropShadow
color: ColorF, // only used for DropShadow

and then all the existing filters can just use the same c_filter.argument unchanged, and you can use the extra fields in a more strongly-typed fashion for the DropShadow filter. cbindgen should be able to deal with this correctly.

Comment 13

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8931585 [details]
Bug 1415034 - Add drop-shadow support.

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

Comment 14

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Thanks for suggestion. The code becomes much cleaner now.

Comment 15

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6fb6fb7ceadc45e17d035b2816de4efe13463913

Comment 16

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Comment on attachment 8931585 [details]
Bug 1415034 - Add drop-shadow support.

oops, accidentally sending a review.

Comment 17

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Add drop-shadow support. (carry r+: kats)

The long-chain.html failure is tracking by
https://github.com/servo/webrender/issues/2197

MozReview-Commit-ID: FECidSvTQrY

Comment 18

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Add drop-shadow support. (carry r+: kats)

Fix try failures.

MozReview-Commit-ID: FECidSvTQrY

Comment 19

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=451c6bffc7ee39b1c9be4254c922b06fd9c1d8fe

Comment 20

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

latest try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=af007644c312909a80a4844f5809d91294f0ddb2&selectedJob=151327728

Comment 21

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Add drop-shadow support. (carry r+: kats)

The long-chain.html failure is tracking by
https://github.com/servo/webrender/issues/2197

MozReview-Commit-ID: FECidSvTQrY

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd2a2e17aef3
Add drop-shadow support. r=kats

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for build bustage at layout/painting/nsDisplayList.cpp:9992:

https://hg.mozilla.org/integration/mozilla-inbound/rev/bcae439efd9ce769f1412b14d2c4ce3205117eb9

Push with bustage: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=fd2a2e17aef386ad47cfbd2259b7dd791999d194&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Build log: https://treeherder.mozilla.org/logviewer.html#?job_id=151474795&repo=mozilla-inbound

/builds/worker/workspace/build/src/layout/painting/nsDisplayList.cpp:9992:11: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
/builds/worker/workspace/build/src/layout/painting/nsDisplayList.cpp:9994:11: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8859d342fd59
Add drop-shadow support. r=kats

Comment 25

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Sorry for that, submit another patch to fix it.

Comment 26

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/8859d342fd59