1356371 - Use cbindgen for WebRender binding generation

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Ryan Hunt [:rhunt]]

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.

Comment 1

[Ryan Hunt [:rhunt]]

Attachment: Bug 1356371 - Create a WrRenderedEpochs struct

This helps cbindgen know to generate a opaque struct for WrRenderedEpochs.

Review commit: https://reviewboard.mozilla.org/r/130076/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/130076/

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 1356371 - Create a WrExternalImageId because ExternalImageId is not repr(C)

This is only temporary as ExternalImageId has been made repr(C) upstream.

Review commit: https://reviewboard.mozilla.org/r/130078/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/130078/

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 1356371 - Use cbindgen for WebRender bindings generation

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

Review commit: https://reviewboard.mozilla.org/r/130080/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/130080/

Comment 4

[Ryan Hunt [:rhunt]]

Started a try run here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=47287b0680af25ecb28904fc08c12f4b29ca75bb

Comment 5

[Ryan Hunt [:rhunt]]

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.

Comment 6

[Ryan Hunt [:rhunt]]

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 7

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8858109 [details]
Bug 1356371 - Create a WrRenderedEpochs struct

https://reviewboard.mozilla.org/r/130076/#review132690

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Comment on attachment 8858110 [details]
Bug 1356371 - Create a WrExternalImageId because ExternalImageId is not repr(C)

https://reviewboard.mozilla.org/r/130078/#review132692

Comment 9

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 10

[Ryan Hunt [:rhunt]]

Comment on attachment 8858111 [details]
Bug 1356371 - Use cbindgen for WebRender bindings generation

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/130080/diff/1-2/

Comment 11

[Ryan Hunt [:rhunt]]

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 12

[Kartikaya Gupta (email:[e-mail])]

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/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/mozilla-central/rev/acece5d3ba51789d4011f20c556991d403e80281
https://hg.mozilla.org/mozilla-central/rev/41b26890a99d928f4016cd69aa4797a28bad4b93
https://hg.mozilla.org/mozilla-central/rev/cc6078dce31cb8ca747f4b67994fafe7e2017be8