diff --git a/api/sixtyfps-cpp/include/sixtyfps.h b/api/sixtyfps-cpp/include/sixtyfps.h index 6b571f72a..7db05c303 100644 --- a/api/sixtyfps-cpp/include/sixtyfps.h +++ b/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 { diff --git a/api/sixtyfps-cpp/include/sixtyfps_string.h b/api/sixtyfps-cpp/include/sixtyfps_string.h new file mode 100644 index 000000000..25dfc06b4 --- /dev/null +++ b/api/sixtyfps-cpp/include/sixtyfps_string.h @@ -0,0 +1,36 @@ +#include +#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 +}; +} diff --git a/api/sixtyfps-rs/lib.rs b/api/sixtyfps-rs/lib.rs index 1f4f1ca2b..0eb32ebc7 100644 --- a/api/sixtyfps-rs/lib.rs +++ b/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, *}; diff --git a/api/sixtyfps-rs/sixtyfps-rs-macro/Cargo.toml b/api/sixtyfps-rs/sixtyfps-rs-macro/Cargo.toml index 3e92fc611..495983d40 100644 --- a/api/sixtyfps-rs/sixtyfps-rs-macro/Cargo.toml +++ b/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"] } diff --git a/api/sixtyfps-rs/sixtyfps-rs-macro/lib.rs b/api/sixtyfps-rs/sixtyfps-rs-macro/lib.rs index a9bcdc236..fe8916f37 100644 --- a/api/sixtyfps-rs/sixtyfps-rs-macro/lib.rs +++ b/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(); - let tok = TokenTree::Literal(Literal::byte_string(c_str.as_bytes_with_nul())); - quote!(#tok as *const u8).into() + quote!(sixtyfps::re_exports::SharedString::from(#s)) } Expression::NumberLiteral(n) => quote!(#n), }; diff --git a/examples/cpptest/hello.60 b/examples/cpptest/hello.60 index f9dc75a94..83f7907da 100644 --- a/examples/cpptest/hello.60 +++ b/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"; } } diff --git a/sixtyfps_runtime/corelib/Cargo.toml b/sixtyfps_runtime/corelib/Cargo.toml index c440427b9..3463e5322 100644 --- a/sixtyfps_runtime/corelib/Cargo.toml +++ b/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" diff --git a/sixtyfps_runtime/corelib/abi/datastructures.rs b/sixtyfps_runtime/corelib/abi/datastructures.rs index 7661909ed..299990381 100644 --- a/sixtyfps_runtime/corelib/abi/datastructures.rs +++ b/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), Image(OpaqueImageHandle, AspectRatio), Text(String)*/ diff --git a/sixtyfps_runtime/corelib/abi/primitives.rs b/sixtyfps_runtime/corelib/abi/primitives.rs index 8b7e1d03f..36c51a0e5 100644 --- a/sixtyfps_runtime/corelib/abi/primitives.rs +++ b/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, - std::ffi::CStr::from_ptr(self.source).to_str().unwrap(), - ) - } + RenderingInfo::Image(self.x, self.y, self.source.clone()) } fn layouting_info(&self) -> LayoutInfo { diff --git a/sixtyfps_runtime/corelib/abi/string.rs b/sixtyfps_runtime/corelib/abi/string.rs new file mode 100644 index 000000000..020111312 --- /dev/null +++ b/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] + /// buffer[0..=len] is initialized and valid utf8, and buffer[len] is '\0' + inner: ThinArc>, +} + +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>> = + 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::()), + [MaybeUninit::new(0); core::mem::align_of::()].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; + fn next(&mut self) -> Option> { + let pos = self.pos; + self.pos += 1; + let align = core::mem::align_of::(); + 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) { + 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::(); + 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)); +} diff --git a/sixtyfps_runtime/corelib/build.rs b/sixtyfps_runtime/corelib/build.rs index ec0e36f22..ef56e9a14 100644 --- a/sixtyfps_runtime/corelib/build.rs +++ b/sixtyfps_runtime/corelib/build.rs @@ -8,6 +8,8 @@ fn main() { .map(|x| x.to_string()) .collect::>(); + let exclude = ["SharedString"].iter().map(|x| x.to_string()).collect::>(); + 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_header("#include ") + .with_src(format!("{}/abi/datastructures.rs", crate_dir)) + .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"); diff --git a/sixtyfps_runtime/corelib/item_rendering.rs b/sixtyfps_runtime/corelib/item_rendering.rs index f20a0169b..86d6fe3f2 100644 --- a/sixtyfps_runtime/corelib/item_rendering.rs +++ b/sixtyfps_runtime/corelib/item_rendering.rs @@ -35,7 +35,7 @@ pub(crate) fn update_item_rendering_data( { 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(); diff --git a/sixtyfps_runtime/corelib/lib.rs b/sixtyfps_runtime/corelib/lib.rs index dd6ac6634..620016f19 100644 --- a/sixtyfps_runtime/corelib/lib.rs +++ b/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 diff --git a/tools/viewer/main.rs b/tools/viewer/main.rs index 48189e440..1044985f8 100644 --- a/tools/viewer/main.rs +++ b/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) => { - // FIXME that's a leak - std::ffi::CString::new(v.as_str()).unwrap().into_raw() as _ - } + Expression::StringLiteral(v) => (**v).into(), _ => 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:: as _)), ("width", (offsets.width.get_byte_offset(), set_property:: as _)), ("height", (offsets.height.get_byte_offset(), set_property:: as _)), - ("source", (offsets.source.get_byte_offset(), set_property::<*const i8> as _)), + ("source", (offsets.source.get_byte_offset(), set_property:: as _)), ] .iter() .cloned()