Remove intermediate types in bindings.rs

RESOLVED FIXED in Firefox 56

Status

()

Core
Graphics: WebRender
P3
normal
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: rhunt, Assigned: rhunt)

Tracking

unspecified
mozilla56
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

(Whiteboard: [gfx-noted])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(8 attachments)

(Assignee)

Description

3 months ago
This is now possible. I'll upload patches soon.
(Assignee)

Comment 1

3 months ago
I apologize in advance for the churn. I tried to split out these patches as much as I could while writing them. There should be no functional changes here.

The biggest changes from these patches:
1. Everything in webrender_ffi_generated.h is put into the mozilla::wr namespace
2. Most intermediate types (including for euclid) are dropped.
3. The Wr prefix is gone for types that were using intermediate types.

I haven't gotten to converting every type, but this is a first step. In the future I'd like to drop the Wr prefix from all types, but that was too much work for today.

One hiccup is from cbindgen and euclid types. The use of LayoutRect is causing the generation of,
1. struct TypedPoint2D_f32__LayerPixel
2. struct TypedSize2D_f32__LayerPixel

These types are technically equivalent to LayoutPoint and LayoutSize, but it's difficult for cbindgen to figure that out. They're harmless and used only once so I think it's okay to have them for now.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=94f2be3362ad3aa470bded3c9ce126214bc89109
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

3 months ago
mozreview-review
Comment on attachment 8887829 [details]
Bug 1382128 part 1 - Use a namespace in webrender_bindings.

https://reviewboard.mozilla.org/r/158740/#review164136
Attachment #8887829 - Flags: review?(bugmail) → review+

Comment 11

3 months ago
mozreview-review
Comment on attachment 8887830 [details]
Bug 1382128 part 2 - Remove WrPoint, WrSize, WrRect, WrMatrix, and WrColor.

https://reviewboard.mozilla.org/r/158742/#review164142

::: gfx/webrender_bindings/WebRenderTypes.h:244
(Diff revision 1)
> +static inline wr::LayoutVector2D ToLayoutVector2D(const gfx::IntPointTyped<T>& point)
> +{
> +  return ToLayoutVector2D(IntPointToPoint(point));
> +}
> +
> +static inline wr::LayoutVector2D ToLayoutVector2D(const gfx::Point& point)

You shouldn't need this, I think. gfx::Point is a typedef of gfx::PointTyped<UnknownUnits> which should be handled by the template function you added a few lines up.
Attachment #8887830 - Flags: review?(bugmail) → review+

Comment 12

3 months ago
mozreview-review
Comment on attachment 8887831 [details]
Bug 1382128 part 3 - Rename WrByteSlice to ByteSlice.

https://reviewboard.mozilla.org/r/158744/#review164144
Attachment #8887831 - Flags: review?(bugmail) → review+

Comment 13

3 months ago
mozreview-review
Comment on attachment 8887832 [details]
Bug 1382128 part 4 - Remove WrGlyphInstance.

https://reviewboard.mozilla.org/r/158746/#review164146
Attachment #8887832 - Flags: review?(bugmail) → review+

Comment 14

3 months ago
mozreview-review
Comment on attachment 8887833 [details]
Bug 1382128 part 5 - Remove WrGradientStop and WrGradientExtendMode.

https://reviewboard.mozilla.org/r/158748/#review164150
Attachment #8887833 - Flags: review?(bugmail) → review+

Comment 15

3 months ago
mozreview-review
Comment on attachment 8887834 [details]
Bug 1382128 part 6 - Remove WrBorderSides, WrBorderRadius, WrBorderWidths, WrNinePatchDescriptors.

https://reviewboard.mozilla.org/r/158750/#review164156
Attachment #8887834 - Flags: review?(bugmail) → review+

Comment 16

3 months ago
mozreview-review
Comment on attachment 8887835 [details]
Bug 1382128 part 7 - Organize types in webrender_bindings.

https://reviewboard.mozilla.org/r/158752/#review164158
Attachment #8887835 - Flags: review?(bugmail) → review+

Comment 17

3 months ago
mozreview-review
Comment on attachment 8887836 [details]
Bug 1382128 part 8 - Remove Wr prefix from some type aliases.

https://reviewboard.mozilla.org/r/158754/#review164164
Attachment #8887836 - Flags: review?(bugmail) → review+
Thanks for doing all this cleanup! It's great to get rid of so much boilerplate/intermediate type goop. :)

Comment 19

3 months ago
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ec2b3b09f00b
part 1 - Use a namespace in webrender_bindings. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8b85719bc59
part 2 - Remove WrPoint, WrSize, WrRect, WrMatrix, and WrColor. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd7ad2246d6f
part 3 - Rename WrByteSlice to ByteSlice. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/569d535ea9f3
part 4 - Remove WrGlyphInstance. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/ef0d072d52f9
part 5 - Remove WrGradientStop and WrGradientExtendMode. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1bcea28512d
part 6 - Remove WrBorderSides, WrBorderRadius, WrBorderWidths, WrNinePatchDescriptors. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/792d552a2bcc
part 7 - Organize types in webrender_bindings. r=kats
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e1508068c1b
part 8 - Remove Wr prefix from some type aliases. r=kats

Comment 20

3 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ec2b3b09f00b
https://hg.mozilla.org/mozilla-central/rev/b8b85719bc59
https://hg.mozilla.org/mozilla-central/rev/cd7ad2246d6f
https://hg.mozilla.org/mozilla-central/rev/569d535ea9f3
https://hg.mozilla.org/mozilla-central/rev/ef0d072d52f9
https://hg.mozilla.org/mozilla-central/rev/e1bcea28512d
https://hg.mozilla.org/mozilla-central/rev/792d552a2bcc
https://hg.mozilla.org/mozilla-central/rev/3e1508068c1b
Status: NEW → RESOLVED
Last Resolved: 3 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.