Closed Bug 1356371 Opened 3 years ago Closed 3 years ago

Use cbindgen for WebRender binding generation

Categories

(Core :: Graphics: WebRender, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: rhunt, Assigned: rhunt)

Details

(Whiteboard: [gfx-noted])

Attachments

(3 files)

I've forked wr-binding and called it cbindgen to continue development to make it general purpose. cbindgen does slightly more than wr-binding (like generate opaque structs), so it there are a few things that need to be fixed for it to generate webrender_ffi_generated.h. 

With these changes it produces nice output and I'm planning on maintaing/enhancing it, so I think we should switch over.
Whiteboard: [gfx-noted]
I think that the webrender_ffi_generated.h file is different because dependencies are found after everything has been parsed. And also because enums are sorted to the top for nicer output.

There shouldn't be as much churn in the future, but it's an autogenerated file so I don't think it matters much.
Forgot to add, the command to generate is different,

`cbindgen -c wr gfx/webrender_bindings gfx/webrender_bindings/webrender_ffi_generated.h`

-c controls the output style, wr is hardcoded for our needs, in the future it will be customizable. If the output file isn't specified it goes to stdout.
Comment on attachment 8858109 [details]
Bug 1356371 - Create a WrRenderedEpochs struct

https://reviewboard.mozilla.org/r/130076/#review132690
Attachment #8858109 - Flags: review?(bugmail) → review+
Comment on attachment 8858110 [details]
Bug 1356371 - Create a WrExternalImageId because ExternalImageId is not repr(C)

https://reviewboard.mozilla.org/r/130078/#review132692
Attachment #8858110 - Flags: review?(bugmail) → review+
Comment on attachment 8858111 [details]
Bug 1356371 - Use cbindgen for WebRender bindings generation

https://reviewboard.mozilla.org/r/130080/#review132688

I'm happy to rubberstamp this once there's some docs in place on how to regenerate the generated file.

::: gfx/webrender_bindings/webrender_ffi_generated.h:5
(Diff revision 1)
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> -/* THIS FILE IS GENERATED! DO NOT MODIFY MANUALLY! See https://github.com/jrmuizel/wr-binding! */
> +/* DO NOT MODIFY THIS MANUALLY! This file was generated using cbindgen. */

This comment needs a pointer to instructions on how to run the generator tool and re-generate the file.
Attachment #8858111 - Flags: review?(bugmail)
I've updated the tool to generate a better message, and updated the patch with it.

I'll also send a follow up message to the 'Generated bindings has landed' dev tech gfx message when this lands.
Comment on attachment 8858111 [details]
Bug 1356371 - Use cbindgen for WebRender bindings generation

https://reviewboard.mozilla.org/r/130080/#review132798

::: commit-message-2599d:5
(Diff revision 2)
> +Bug 1356371 - Use cbindgen for WebRender bindings generation r?kats
> +
> +This commit adds some directives to help cbindgen generate correct
> +structs and functions, removes some old FFI code that isn't needed
> +anymore, and autogenerates webrender_ffi_generated.h

s/autogenerates/regenerates/
Attachment #8858111 - Flags: review?(bugmail) → review+
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/projects/graphics/rev/acece5d3ba51
Create a WrRenderedEpochs struct r=kats
https://hg.mozilla.org/projects/graphics/rev/41b26890a99d
Create a WrExternalImageId because ExternalImageId is not repr(C) r=kats
https://hg.mozilla.org/projects/graphics/rev/cc6078dce31c
Use cbindgen for WebRender bindings generation 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.