1345975 - Clean up WebRender bindings and layer code

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Comment 1

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: cleanup-1-webrender-layer.patch

This patch makes the common parts of the WebRenderLayers more consistent, and some other small changes. It also removes WrScrollFrameStackingContextGenerator as it is no longer used.

It's based on the changes in bug 1345907, so that would have to land first.

Comment 2

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: cleanup-2-rename-wrlayoutsize.patch

I think technically we should change WrPoint and WrRect to be WrLayoutPoint and WrLayoutRect, but this is easier for now.

Comment 3

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: cleanup-3-enums-u32.patch

We expose ImageRendering in bindings.rs because it's repr(u32). I think we can do that with two other enums.

Comment 4

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: cleanup-4-organize-bindings.patch

bindings.rs is a bit of a mess.

This patch sorts bindings.rs to be in sync with webrender_ffi.h, and in a few cases moves functions in webrender_ffi.h.

Comment 5

[Ryan Hunt [:rhunt] (on leave until early May)]

I think mozreview handles moving lines around in a file better than just reading a patch. I can reuppload there if that is easier. The last patch is a bit ugly like that.

Comment 6

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

(In reply to Ryan Hunt [:rhunt] from comment #5)
> I think mozreview handles moving lines around in a file better than just
> reading a patch. I can reuppload there if that is easier. The last patch is
> a bit ugly like that.

Yeah, that would probably be better, if you don't mind.

Comment 7

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Bug 1345975 - Clean up WebRenderLayer code

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

Comment 8

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Bug 1345975 - Rename WrLayoutSize to WrSize

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

Comment 9

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Bug 1345975 - Sort Webrender binding definitions

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

Comment 10

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Bug 1345975 - Use some enums with repr(u32) directly

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

Comment 11

[Ryan Hunt [:rhunt] (on leave until early May)]

Attachment: Bug 1345975 - Run bindings.rs through rustfmt

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

Comment 12

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8845668 [details]
Bug 1345975 - Clean up WebRenderLayer code

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

Comment 13

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8845669 [details]
Bug 1345975 - Rename WrLayoutSize to WrSize

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

Comment 14

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8845670 [details]
Bug 1345975 - Sort Webrender binding definitions

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

Comment 15

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8845671 [details]
Bug 1345975 - Use some enums with repr(u32) directly

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

Comment 16

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8845672 [details]
Bug 1345975 - Run bindings.rs through rustfmt

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

Comment 17

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

Comment on attachment 8845668 [details]
Bug 1345975 - Clean up WebRenderLayer code

https://reviewboard.mozilla.org/r/118814/#review120740

Comment 18

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

Comment on attachment 8845672 [details]
Bug 1345975 - Run bindings.rs through rustfmt

https://reviewboard.mozilla.org/r/118822/#review120742

Comment 19

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

Comment on attachment 8845671 [details]
Bug 1345975 - Use some enums with repr(u32) directly

https://reviewboard.mozilla.org/r/118820/#review120748

Comment 20

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

Comment on attachment 8845669 [details]
Bug 1345975 - Rename WrLayoutSize to WrSize

https://reviewboard.mozilla.org/r/118816/#review120750

I'll r+ this for now but I think eventually we will want to make sure we properly represent sizes of different units. Using strongly-typed dimensions has saved us from numerous problems in the past.

Comment 21

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

Comment on attachment 8845670 [details]
Bug 1345975 - Sort Webrender binding definitions

https://reviewboard.mozilla.org/r/118818/#review120752

This also removes the wr_api_generate_font_key and wr_api_generate_image_key functions from bindings.rs, which I presume are unused.

Comment 22

[Ryan Hunt [:rhunt] (on leave until early May)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #20)
> Comment on attachment 8845669 [details]
> Bug 1345975 - Rename WrLayoutSize to WrSize
> 
> https://reviewboard.mozilla.org/r/118816/#review120750
> 
> I'll r+ this for now but I think eventually we will want to make sure we
> properly represent sizes of different units. Using strongly-typed dimensions
> has saved us from numerous problems in the past.

Yes I agree. I think we should definitely do that sometime soon.

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #21)
> Comment on attachment 8845670 [details]
> Bug 1345975 - Sort Webrender binding definitions
> 
> https://reviewboard.mozilla.org/r/118818/#review120752
> 
> This also removes the wr_api_generate_font_key and wr_api_generate_image_key
> functions from bindings.rs, which I presume are unused.

Yes they are, that should have been done in a different patch probably.

Thank you for dealing with the churn, it was a lot.

Comment 23

[Ryan Hunt [:rhunt] (on leave until early May)]

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

Comment 24

[Ryan Hunt [:rhunt] (on leave until early May)]

Try looks green to me.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/2479f38345d6
Clean up WebRenderLayer code r=kats
https://hg.mozilla.org/projects/graphics/rev/5de79be2222a
Rename WrLayoutSize to WrSize r=kats
https://hg.mozilla.org/projects/graphics/rev/70a8e7b62e18
Sort WebRender binding definitions r=kats
https://hg.mozilla.org/projects/graphics/rev/fd6318907241
Use some enums with repr(u32) directly r=kats
https://hg.mozilla.org/projects/graphics/rev/855b9296185f
Run WebRender bindings through rustfmt r=kats

Comment 26

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

https://hg.mozilla.org/mozilla-central/rev/2479f38345d6
https://hg.mozilla.org/mozilla-central/rev/5de79be2222a
https://hg.mozilla.org/mozilla-central/rev/70a8e7b62e18
https://hg.mozilla.org/mozilla-central/rev/fd6318907241
https://hg.mozilla.org/mozilla-central/rev/855b9296185f