Add a shared string that can be used in properties

api/sixtyfps-cpp/include/sixtyfps.h

@@ -1,16 +1,9 @@
-
 namespace sixtyfps::internal {
-// FIXME: this is just required because of something wrong
-// with &str in cbindgen, but one should not have &str anyway
-using str = char;
-
 // Workaround https://github.com/eqrion/cbindgen/issues/43
 struct ComponentVTable;
 }
-
-
-#include "sixtyfps_gl_internal.h"
 #include "sixtyfps_internal.h"
+#include "sixtyfps_gl_internal.h"
 
 namespace sixtyfps {
 

api/sixtyfps-cpp/include/sixtyfps_string.h

@@ -0,0 +1,36 @@
+#include <string_view>
+#include "sixtyfps_string_internal.h"
+
+namespace sixtyfps {
+
+struct SharedString
+{
+    SharedString() { internal::sixtyfps_shared_string_from_bytes(this, "", 0); }
+    SharedString(std::string_view s)
+    {
+        internal::sixtyfps_shared_string_from_bytes(this, s.data(), s.size());
+    }
+    SharedString(const SharedString &other)
+    {
+        internal::sixtyfps_shared_string_clone(this, &other);
+    }
+    ~SharedString() { internal::sixtyfps_shared_string_drop(this); }
+    SharedString &operator=(const SharedString &other)
+    {
+        internal::sixtyfps_shared_string_drop(this);
+        internal::sixtyfps_shared_string_clone(this, &other);
+    }
+    SharedString &operator=(std::string_view s)
+    {
+        internal::sixtyfps_shared_string_drop(this);
+        internal::sixtyfps_shared_string_from_bytes(this, s.data(), s.size());
+    }
+    SharedString &operator=(SharedString &&other) { std::swap(inner, other.inner); }
+
+    operator std::string_view() const { return internal::sixtyfps_shared_string_bytes(this); }
+    auto data() const -> const char * { return internal::sixtyfps_shared_string_bytes(this); }
+
+private:
+    void *inner; // opaque
+};
+}

api/sixtyfps-rs/lib.rs

@@ -6,6 +6,7 @@ pub mod re_exports {
     pub use corelib::abi::datastructures::{Component, ComponentTO, ComponentVTable, ItemTreeNode};
     pub use corelib::abi::primitives::{Image, ImageVTable, Rectangle, RectangleVTable};
     pub use corelib::ComponentVTable_static;
+    pub use corelib::SharedString;
     pub use gl::sixtyfps_runtime_run_component_with_gl_renderer;
     pub use once_cell::sync::Lazy;
     pub use vtable::{self, *};

api/sixtyfps-rs/sixtyfps-rs-macro/Cargo.toml

@@ -10,6 +10,5 @@ path = "lib.rs"
 
 [dependencies]
 quote = "1.0"
-proc-macro2 = "1.0"
 sixtyfps_compiler = { path = "../../../sixtyfps_compiler", features = ["proc_macro_span"] }
 

api/sixtyfps-rs/sixtyfps-rs-macro/lib.rs

@@ -1,7 +1,6 @@
 extern crate proc_macro;
 use object_tree::Expression;
 use proc_macro::TokenStream;
-use proc_macro2::{Literal, TokenTree};
 use quote::quote;
 use sixtyfps_compiler::*;
 
@@ -126,9 +125,7 @@ pub fn sixtyfps(stream: TokenStream) -> TokenStream {
                 // That's an error
                 Expression::Identifier(_) => quote!(),
                 Expression::StringLiteral(s) => {
-                    let c_str: std::ffi::CString = std::ffi::CString::new(s.as_bytes()).unwrap();
+                    quote!(sixtyfps::re_exports::SharedString::from(#s))
-                    let tok = TokenTree::Literal(Literal::byte_string(c_str.as_bytes_with_nul()));
-                    quote!(#tok as *const u8).into()
                 }
                 Expression::NumberLiteral(n) => quote!(#n),
             };

examples/cpptest/hello.60

@@ -26,7 +26,7 @@ SuperSimple = Rectangle {
     Image {
         x: 200;
         y: 200;
-        source: "../../../examples/graphicstest/logo.png";
+        source: "../../examples/graphicstest/logo.png";
     }
 }
 

sixtyfps_runtime/corelib/Cargo.toml

@@ -16,6 +16,8 @@ const-field-offset = { path = "../../helper_crates/const-field-offset" }
 vtable = { path = "../../helper_crates/vtable" }
 winit = "0.22.1"
 lyon = { version = "0.15.8" }
+servo_arc = "0.1.1" #we need the Arc::from_header_and_iter
+once_cell = "1.4"
 
 [build-dependencies]
 cbindgen = "0.14.2"

sixtyfps_runtime/corelib/abi/datastructures.rs

@@ -103,7 +103,7 @@ pub struct LayoutInfo {
 pub enum RenderingInfo {
     NoContents,
     Rectangle(f32, f32, f32, f32, u32), // Should be a beret structure
-    Image(f32, f32, &'static str),
+    Image(f32, f32, crate::SharedString),
     /*Path(Vec<PathElement>),
     Image(OpaqueImageHandle, AspectRatio),
     Text(String)*/

sixtyfps_runtime/corelib/abi/primitives.rs

@@ -38,15 +38,11 @@ impl ItemConsts for Rectangle {
     > = Rectangle::field_offsets().cached_rendering_data;
 }
 
-// FIXME: remove  (or use the libc one)
-#[allow(non_camel_case_types)]
-type c_char = i8;
-
 #[repr(C)]
-#[derive(const_field_offset::FieldOffsets)]
+#[derive(const_field_offset::FieldOffsets, Default)]
 pub struct Image {
     /// FIXME: make it a image source
-    pub source: *const c_char,
+    pub source: crate::SharedString,
     pub x: f32,
     pub y: f32,
     pub width: f32,
@@ -54,29 +50,10 @@ pub struct Image {
     pub cached_rendering_data: super::datastructures::CachedRenderingData,
 }
 
-impl Default for Image {
-    fn default() -> Self {
-        Image {
-            source: (b"\0").as_ptr() as *const _,
-            x: 0.,
-            y: 0.,
-            width: 0.,
-            height: 0.,
-            cached_rendering_data: Default::default(),
-        }
-    }
-}
-
 impl Item for Image {
     fn geometry(&self) {}
     fn rendering_info(&self) -> RenderingInfo {
-        unsafe {
+        RenderingInfo::Image(self.x, self.y, self.source.clone())
-            RenderingInfo::Image(
-                self.x,
-                self.y,
-                std::ffi::CStr::from_ptr(self.source).to_str().unwrap(),
-            )
-        }
     }
 
     fn layouting_info(&self) -> LayoutInfo {

sixtyfps_runtime/corelib/abi/string.rs

@@ -0,0 +1,136 @@
+use core::mem::MaybeUninit;
+use servo_arc::ThinArc;
+use std::{fmt::Debug, ops::Deref};
+
+/// The string type suitable for properties. It is shared meaning passing copies
+/// around will not allocate, and that different properties with the same string
+/// can share the same buffer.
+/// It is also ffi-friendly as the buffer always ends with `'\0'`
+/// Internally, this is an implicitly shared type to a null terminated string
+#[derive(Clone)]
+pub struct SharedString {
+    /// Invariant: The usize header is the `len` of the vector, the contained buffer is [MaybeUninit<u8>]
+    /// buffer[0..=len] is initialized and valid utf8, and buffer[len] is '\0'
+    inner: ThinArc<usize, MaybeUninit<u8>>,
+}
+
+impl SharedString {
+    fn as_ptr(&self) -> *const u8 {
+        self.inner.slice.as_ptr() as *const u8
+    }
+
+    pub fn len(&self) -> usize {
+        self.inner.header.header
+    }
+
+    pub fn as_str(&self) -> &str {
+        unsafe {
+            core::str::from_utf8_unchecked(core::slice::from_raw_parts(self.as_ptr(), self.len()))
+        }
+    }
+}
+
+impl Deref for SharedString {
+    type Target = str;
+    fn deref(&self) -> &Self::Target {
+        self.as_str()
+    }
+}
+
+impl Default for SharedString {
+    fn default() -> Self {
+        // Unfortunately, the Arc constructor is not const, so we must use a Lazy static for that
+        static NULL: once_cell::sync::Lazy<ThinArc<usize, MaybeUninit<u8>>> =
+            once_cell::sync::Lazy::new(|| {
+                servo_arc::Arc::into_thin(servo_arc::Arc::from_header_and_iter(
+                    servo_arc::HeaderWithLength::new(0, core::mem::align_of::<usize>()),
+                    [MaybeUninit::new(0); core::mem::align_of::<usize>()].iter().cloned(),
+                ))
+            });
+
+        SharedString { inner: NULL.clone() }
+    }
+}
+
+impl From<&str> for SharedString {
+    fn from(value: &str) -> Self {
+        struct AddNullIter<'a> {
+            pos: usize,
+            str: &'a [u8],
+        }
+
+        impl<'a> Iterator for AddNullIter<'a> {
+            type Item = MaybeUninit<u8>;
+            fn next(&mut self) -> Option<MaybeUninit<u8>> {
+                let pos = self.pos;
+                self.pos += 1;
+                let align = core::mem::align_of::<usize>();
+                if pos < self.str.len() {
+                    Some(MaybeUninit::new(self.str[pos]))
+                } else if pos < (self.str.len() + align) & !(align - 1) {
+                    Some(MaybeUninit::new(0))
+                } else {
+                    None
+                }
+            }
+            fn size_hint(&self) -> (usize, Option<usize>) {
+                let l = self.str.len() + 1;
+                // add some padding at the end since the sice of the inner will anyway have to be padded
+                let align = core::mem::align_of::<usize>();
+                let l = (l + align - 1) & !(align - 1);
+                let l = l - self.pos;
+                (l, Some(l))
+            }
+        }
+        impl<'a> core::iter::ExactSizeIterator for AddNullIter<'a> {}
+
+        let iter = AddNullIter { str: value.as_bytes(), pos: 0 };
+
+        SharedString {
+            inner: servo_arc::Arc::into_thin(servo_arc::Arc::from_header_and_iter(
+                servo_arc::HeaderWithLength::new(value.len(), iter.size_hint().0),
+                iter,
+            )),
+        }
+    }
+}
+
+impl Debug for SharedString {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        Debug::fmt(self.as_str(), f)
+    }
+}
+
+/// for cbingen.
+#[allow(non_camel_case_types)]
+type c_char = u8;
+
+#[no_mangle]
+pub extern "C" fn sixtyfps_shared_string_bytes(ss: &SharedString) -> *const c_char {
+    ss.as_ptr()
+}
+
+#[no_mangle]
+/// Destroy the shared string
+pub unsafe extern "C" fn sixtyfps_shared_string_drop(ss: *const SharedString) {
+    core::ptr::read(ss);
+}
+
+#[no_mangle]
+/// Increment the reference count of the string.
+/// the resulting structure must be passed to sixtyfps_shared_string_drop
+pub unsafe extern "C" fn sixtyfps_shared_string_clone(out: *mut SharedString, ss: &SharedString) {
+    core::ptr::write(out, ss.clone())
+}
+
+#[no_mangle]
+/// Safety: bytes must be a valid utf-8 string of size len wihout null inside.
+/// the resulting structure must be passed to sixtyfps_shared_string_drop
+pub unsafe extern "C" fn sixtyfps_shared_string_from_bytes(
+    out: *mut SharedString,
+    bytes: *const c_char,
+    len: usize,
+) {
+    let str = core::str::from_utf8_unchecked(core::slice::from_raw_parts(bytes, len));
+    core::ptr::write(out, SharedString::from(str));
+}

sixtyfps_runtime/corelib/build.rs

@@ -8,6 +8,8 @@ fn main() {
         .map(|x| x.to_string())
         .collect::<Vec<String>>();
 
+    let exclude = ["SharedString"].iter().map(|x| x.to_string()).collect::<Vec<String>>();
+
     let config = cbindgen::Config {
         pragma_once: true,
         include_version: true,
@@ -18,15 +20,26 @@ fn main() {
         language: cbindgen::Language::Cxx,
         cpp_compat: true,
         documentation: true,
-        export: cbindgen::ExportConfig { include, ..Default::default() },
+        export: cbindgen::ExportConfig { include, exclude, ..Default::default() },
         ..Default::default()
     };
 
     let crate_dir = env::var("CARGO_MANIFEST_DIR").unwrap();
+    cbindgen::Builder::new()
+        .with_config(config.clone())
+        .with_src(format!("{}/abi/string.rs", crate_dir))
+        .with_after_include("namespace sixtyfps { struct SharedString; }")
+        .generate()
+        .expect("Unable to generate bindings")
+        .write_to_file(env::var("OUT_DIR").unwrap() + "/sixtyfps_string_internal.h");
+
     cbindgen::Builder::new()
         .with_config(config)
-        .with_crate(crate_dir)
+        .with_src(format!("{}/abi/datastructures.rs", crate_dir))
-        .with_header("#include <vtable.h>")
+        .with_src(format!("{}/abi/primitives.rs", crate_dir))
+        .with_src(format!("{}/abi/model.rs", crate_dir))
+        .with_include("vtable.h")
+        .with_include("sixtyfps_string.h")
         .generate()
         .expect("Unable to generate bindings")
         .write_to_file(env::var("OUT_DIR").unwrap() + "/sixtyfps_internal.h");

sixtyfps_runtime/corelib/item_rendering.rs

@@ -35,7 +35,7 @@ pub(crate) fn update_item_rendering_data<Backend: GraphicsBackend>(
             {
                 let mut image_path = std::env::current_exe().unwrap();
                 image_path.pop(); // pop of executable name
-                image_path.push(_source);
+                image_path.push(&*_source);
                 let image = image::open(image_path.as_path()).unwrap().into_rgba();
                 let source_size = image.dimensions();
 

sixtyfps_runtime/corelib/lib.rs

@@ -5,8 +5,12 @@ pub mod abi {
     pub mod datastructures;
     pub mod model;
     pub mod primitives;
+    pub mod string;
 }
 
+#[doc(inline)]
+pub use abi::string::SharedString;
+
 mod item_rendering;
 
 pub struct MainWindow<GraphicsBackend>

tools/viewer/main.rs

@@ -1,5 +1,6 @@
 use core::ptr::NonNull;
 use corelib::abi::datastructures::{ComponentBox, ComponentRef, ComponentRefMut, ComponentVTable};
+use corelib::SharedString;
 use sixtyfps_compiler::object_tree::Expression;
 use std::collections::HashMap;
 use structopt::StructOpt;
@@ -34,16 +35,13 @@ impl PropertyWriter for u32 {
     }
 }
 
-impl PropertyWriter for *const i8 {
+impl PropertyWriter for SharedString {
     unsafe fn write(ptr: *mut u8, value: &Expression) {
         let val: Self = match value {
-            Expression::StringLiteral(v) => {
+            Expression::StringLiteral(v) => (**v).into(),
-                // FIXME that's a leak
-                std::ffi::CString::new(v.as_str()).unwrap().into_raw() as _
-            }
             _ => todo!(),
         };
-        std::ptr::write(ptr as *mut Self, val);
+        *(ptr as *mut Self) = val.clone();
     }
 }
 
@@ -128,7 +126,7 @@ fn main() -> std::io::Result<()> {
                 ("y", (offsets.y.get_byte_offset(), set_property::<f32> as _)),
                 ("width", (offsets.width.get_byte_offset(), set_property::<f32> as _)),
                 ("height", (offsets.height.get_byte_offset(), set_property::<f32> as _)),
-                ("source", (offsets.source.get_byte_offset(), set_property::<*const i8> as _)),
+                ("source", (offsets.source.get_byte_offset(), set_property::<SharedString> as _)),
             ]
             .iter()
             .cloned()