Closed Bug 1354951 Opened 5 years ago Closed 5 years ago

Add type aliases in bindings.rs to have the type names match the C++ side exactly

Categories

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

Other Branch
enhancement

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

The binding generator (see bug 1354944) currently doesn't hard-code any mapping of type names. However, there are type name mismatches across our existing bindings. By this I mean that we have a function signature using e.g. PipelineId on the Rust side but WrPipelineId on the C++ side. I added some support in the binding generator to handle this with type aliases. So if we add `type WrPipelineId = PipelineId;` on the Rust side, and use WrPipelineId in the function signatures, then the binding generator will know to generate a C struct called WrPipelineId but using the definition from the PipelineId repr(C) struct in Rust.

So the upshot of this is we need to update bindings.rs to add these type aliases and use the right types in the function signatures for the generated bindings to come out right. I have a patch locally that does this although it will probably rot quite fast.
Comment on attachment 8856295 [details] [diff] [review]
Patch

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

This is mostly a straightforward search-and-replace type of patch. The only interesting replacement is WrRenderedEpochs which maps to a Vec of tuples. That one is opaque on the C++ side (https://hg.mozilla.org/projects/graphics/file/4bbcc581a6d6/gfx/webrender_bindings/webrender_ffi.h#l475) and the binding generator won't generate anything for it.
Attachment #8856295 - Flags: review?(rhunt)
Attachment #8856295 - Flags: review?(rhunt) → review+
Apparently type aliases don't work exactly how I expected. We can do stuff like use `WrMixBlendMode::Normal` in bindings.rs. Instead we have to use `MixBlendMode::Normal`. I'm not sure why it compiled for me locally but the former failed on try, so I changed it to the latter.

Green try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=e0b9d1573a96eb4e5a60a05cf95c46085d5f0cde, will land with that change. It doesn't affect the binding side of things.
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/projects/graphics/rev/22a9e4fe2c6f
Add some typedefs in bindings.rs to make the rust function signatures better match the signatures in webrender_ffi.h. r=rhunt
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.