Closed Bug 1316223 Opened 3 years ago Closed 3 years ago

Move bindings to separate crate.

Categories

(Core :: Graphics: WebRender, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla54

People

(Reporter: mtseng, Assigned: mtseng)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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
Attachment #8811572 - Flags: review?(jmuizelaar)
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 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
Attachment #8811572 - Flags: feedback+
Assignee: nobody → mtseng
Addressed jeff and kats's comments.
Attachment #8812099 - Flags: review?(jmuizelaar)
Attachment #8812099 - Flags: review?(bugmail)
Attachment #8811572 - Attachment is obsolete: true
Attachment #8811572 - Flags: review?(jmuizelaar)
Comment on attachment 8812099 [details] [diff] [review]
Moving binding to separate folder.

Jeff gave his blessing to land this patch on IRC
Attachment #8812099 - Flags: review?(jmuizelaar) → review+
Pushed by kgupta@mozilla.com:
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?
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.