1316223 - Move bindings to separate crate.

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Currently, bindings stuff stay in webrender folder. We can move it to separate crate. Thus, we can decouple the bindings code and rebase webrender easily.

Part of work in https://github.com/jrmuizel/gecko-dev/pull/21

Comment 1

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Moving binding to separate folder.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8811572 [details] [diff] [review]
Moving binding to separate folder.

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

I think it's probably worth keeping the webrender.h header name. "bindings.h" is too general.

Comment 3

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

Comment on attachment 8811572 [details] [diff] [review]
Moving binding to separate folder.

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

::: gfx/moz.build
@@ +23,5 @@
>      'vr',
>      'config',
>  ]
>  
> +EXPORTS += ['webrender_bindings/src/bindings.h']

I agree with jeff that I'd prefer this stayed webrender.h. However I would also prefer moving this export to EXPORTS.mozilla.gfx so that you have to #include "mozilla/gfx/webrender.h" rather than just #include "webrender.h". I don't feel too strongly about that, and it can be done later in another patch so feel free to leave it out.

::: gfx/webrender_bindings/Cargo.toml
@@ +1,4 @@
> +[package]
> +name = "webrender_bindings"
> +version = "0.0.1"
> +authors = ["The Servo Project Developers"]

This should probably be "The Mozilla Project Developers" since it's not really a part of servo. And I think the convention for version numbers is to start 0.1.0.

::: gfx/webrender_bindings/src/bindings.rs
@@ +237,5 @@
>  
>  #[no_mangle]
>  pub extern fn wr_init_window(root_pipeline_id: u64) -> *mut WrWindowState {
>      // hack to find the directory for the shaders
> +    let res_path = concat!(env!("CARGO_MANIFEST_DIR"),"/../webrender/res");

You'll need to rebase on the stuff I landed yesterday

Comment 4

[Morris Tseng [:mtseng] [:Morris] (Inactive)]

Attachment: Moving binding to separate folder.

Addressed jeff and kats's comments.

Comment 5

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

Rebased to tip of graphics, pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0dd2d196d61e9cb0fac9d32bcefb0c33a92ca252

Comment 6

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

Sorry that was a bad rebase, here's a better one: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8dbecec18d1dfcf0b236991822974afdac50b472

Comment 7

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

Comment on attachment 8812099 [details] [diff] [review]
Moving binding to separate folder.

Jeff gave his blessing to land this patch on IRC

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/5f62856653d3
Moving binding to separate folder. r=jrmuizel,kats
https://hg.mozilla.org/projects/graphics/rev/c78b0565b19f
Update README.webrender. r=morris?