1381949 - Expose functions for dealing Arc<Vec<u8>>

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, enhancement)

Description

[Jeff Muizelaar [:jrmuizel]]

We'll need this for interacting with the font data from C++

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: Expose functions for dealing with Arc<Vec<u8>>

Comment 2

[Ryan Hunt [:rhunt] (on leave until early May)]

Comment on attachment 8887616 [details] [diff] [review]
Expose functions for dealing with Arc<Vec<u8>>

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

::: gfx/webrender_bindings/src/bindings.rs
@@ +1728,5 @@
> +pub type VecU8 = Vec<u8>;
> +pub type ArcVecU8 = Arc<VecU8>;
> +
> +#[no_mangle]
> +pub unsafe extern "C" fn wr_add_ref_arc(arc: &ArcVecU8) -> *const VecU8 {

nit: The bindings file is all over the place on this, but I think we should be using *const instead of & in FFI function interfaces.

They should codegen to be equivalent, but it emphasizes that the pointer might not follow rust's reference rules.

@@ +1733,5 @@
> +    Arc::into_raw(arc.clone())
> +}
> +
> +#[no_mangle]
> +pub extern "C" fn wr_dec_ref_arc(arc: *const VecU8) {

nit: you can annotate this function unsafe like the other one.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

(In reply to Ryan Hunt [:rhunt] from comment #2)
> Comment on attachment 8887616 [details] [diff] [review]
> Expose functions for dealing with Arc<Vec<u8>>
> 
> Review of attachment 8887616 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/webrender_bindings/src/bindings.rs
> @@ +1728,5 @@
> > +pub type VecU8 = Vec<u8>;
> > +pub type ArcVecU8 = Arc<VecU8>;
> > +
> > +#[no_mangle]
> > +pub unsafe extern "C" fn wr_add_ref_arc(arc: &ArcVecU8) -> *const VecU8 {
> 
> nit: The bindings file is all over the place on this, but I think we should
> be using *const instead of & in FFI function interfaces.
> 
> They should codegen to be equivalent, but it emphasizes that the pointer
> might not follow rust's reference rules.

In this case I actually want the rust reference rules to be followed.

Comment 4

[Ryan Hunt [:rhunt] (on leave until early May)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #3)
> (In reply to Ryan Hunt [:rhunt] from comment #2)
> > Comment on attachment 8887616 [details] [diff] [review]
> > Expose functions for dealing with Arc<Vec<u8>>
> > 
> > Review of attachment 8887616 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: gfx/webrender_bindings/src/bindings.rs
> > @@ +1728,5 @@
> > > +pub type VecU8 = Vec<u8>;
> > > +pub type ArcVecU8 = Arc<VecU8>;
> > > +
> > > +#[no_mangle]
> > > +pub unsafe extern "C" fn wr_add_ref_arc(arc: &ArcVecU8) -> *const VecU8 {
> > 
> > nit: The bindings file is all over the place on this, but I think we should
> > be using *const instead of & in FFI function interfaces.
> > 
> > They should codegen to be equivalent, but it emphasizes that the pointer
> > might not follow rust's reference rules.
> 
> In this case I actually want the rust reference rules to be followed.

That's okay then. It won't be enforced on C/C++ code, but at the least it's written somewhere.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8000ce443309
Expose functions for dealing Arc<Vec<u8>> r=rhunt

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8000ce443309