Closed Bug 1345975 Opened 3 years ago Closed 3 years ago

Clean up WebRender bindings and layer code

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55

People

(Reporter: rhunt, Assigned: rhunt)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(5 files, 4 obsolete files)

No description provided.
Attached patch cleanup-1-webrender-layer.patch (obsolete) — Splinter Review
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.
Attachment #8845565 - Flags: review?(bugmail)
I think technically we should change WrPoint and WrRect to be WrLayoutPoint and WrLayoutRect, but this is easier for now.
Attachment #8845587 - Flags: review?(bugmail)
Attached patch cleanup-3-enums-u32.patch (obsolete) — Splinter Review
We expose ImageRendering in bindings.rs because it's repr(u32). I think we can do that with two other enums.
Attachment #8845588 - Flags: review?(bugmail)
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.
Attachment #8845593 - Flags: review?(bugmail)
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.
(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.
Attachment #8845593 - Attachment is obsolete: true
Attachment #8845593 - Flags: review?(bugmail)
Attachment #8845588 - Attachment is obsolete: true
Attachment #8845588 - Flags: review?(bugmail)
Attachment #8845587 - Attachment is obsolete: true
Attachment #8845587 - Flags: review?(bugmail)
Attachment #8845565 - Attachment is obsolete: true
Attachment #8845565 - Flags: review?(bugmail)
Comment on attachment 8845671 [details]
Bug 1345975 - Use some enums with repr(u32) directly

https://reviewboard.mozilla.org/r/118820/#review120748
Attachment #8845671 - Flags: review?(bugmail) → review+
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.
Attachment #8845669 - Flags: review?(bugmail) → review+
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.
Attachment #8845670 - Flags: review?(bugmail) → review+
(In reply to Kartikaya Gupta (email:kats@mozilla.com) 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:kats@mozilla.com) 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.
Try looks green to me.
Pushed by rhunt@eqrion.net:
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
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.