1438017 - Update wr bindings for filters and add filters to command builder

Status: RESOLVED FIXED

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

Description

[Eitan Isaacson [:eeejay]]

This is a followup to https://github.com/servo/webrender/pull/2323.

Also, I added API for defining filters in WebRenderCommandBuilder::BuildWebRenderCommands to make it useful in bug 1431466.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Update wr bindings with ColorMatrix filter and add filters to command builder. r?kats

Comment 2

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

Comment on attachment 8950746 [details] [diff] [review]
Update wr bindings with ColorMatrix filter and add filters to command builder. r?kats

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

Minus for now but only because regenerating the FFI header (see below) will probably remove the operator== on that struct and I'm not sure if that'll cause build failures or not. If the comments are addressed without problems I'm happy to r+ this.

::: gfx/layers/wr/WebRenderCommandBuilder.h
@@ +51,5 @@
>                                nsDisplayList* aDisplayList,
>                                nsDisplayListBuilder* aDisplayListBuilder,
>                                WebRenderScrollData& aScrollData,
> +                              wr::LayoutSize& aContentSize,
> +                              const nsTArray<wr::WrFilterOp>& aFilters = nsTArray<wr::WrFilterOp>());

Drop the default value here, you've already provided a value at the only call site of this function

::: gfx/webrender_bindings/webrender_ffi_generated.h
@@ +676,5 @@
>    WrFilterOpType filter_type;
>    float argument;
>    LayoutVector2D offset;
>    ColorF color;
> +  float matrix[20];

Looks like you hand-modified this? This file is meant to be autogenerated, although right now the step to do that is slightly broken. You have to:
(1) apply https://hg.mozilla.org/try/rev/9d8d9b8971961e87116ca5a1684aa9f5889d9e82 locally
(2) follow the instructions at the top/bottom of this file to regenerate it
(3) unapply the thing from step (1)

Comment 3

[Eitan Isaacson [:eeejay]]

Attachment: Update wr bindings with ColorMatrix filter and add filters to command builder. r?kats

This still builds without the operator==, but will it introduce regressions? Running a try on this.

Comment 4

[Eitan Isaacson [:eeejay]]

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

Comment 5

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

Comment on attachment 8951004 [details] [diff] [review]
Update wr bindings with ColorMatrix filter and add filters to command builder. r?kats

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

LGTM. If it compiles it's probably fine - the operator is autogenerated but probably not being used anywhere.

Comment 6

[Eitan Isaacson [:eeejay]]

Nope, need to rebase this manually first.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ae79bc0bbd31
Update wr bindings with ColorMatrix filter and add filters to command builder. r=kats

Comment 8

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/ae79bc0bbd31