Closed Bug 1345975 Opened 8 years ago Closed 8 years ago

Clean up WebRender bindings and layer code

Categories

(Core :: Graphics: WebRender, enhancement)

Product:
Core
Component:
Graphics: WebRender
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla55

People

(Reporter: rhunt, Assigned: rhunt)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(5 files, 4 obsolete files)

Bug 1345975 - Clean up WebRenderLayer code
(deleted), text/x-review-board-request
kats
: review+
Details
Bug 1345975 - Rename WrLayoutSize to WrSize
(deleted), text/x-review-board-request
kats
: review+
Details
Bug 1345975 - Sort Webrender binding definitions
(deleted), text/x-review-board-request
kats
: review+
Details
Bug 1345975 - Use some enums with repr(u32) directly
(deleted), text/x-review-board-request
kats
: review+
Details
Bug 1345975 - Run bindings.rs through rustfmt
(deleted), text/x-review-board-request
kats
: review+
Details
No description provided.
Attached patch cleanup-1-webrender-layer.patch (obsolete) (deleted) — — 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)
Attached patch cleanup-2-rename-wrlayoutsize.patch (obsolete) (deleted) — — Splinter Review
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) (deleted) — — 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)
Attached patch cleanup-4-organize-bindings.patch (obsolete) (deleted) — — Splinter Review
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 8845668 [details] Bug 1345975 - Clean up WebRenderLayer code https://reviewboard.mozilla.org/r/118814/#review120740
Attachment #8845668 - Flags: review?(bugmail) → review+
Comment on attachment 8845672 [details] Bug 1345975 - Run bindings.rs through rustfmt https://reviewboard.mozilla.org/r/118822/#review120742
Attachment #8845672 - Flags: review?(bugmail) → review+
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: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: