Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up sketch code in fj crate #1322

Merged
merged 4 commits into from
Nov 7, 2022
Merged

Clean up sketch code in fj crate #1322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7993309
Remove redundant comment
hannobraun Nov 7, 2022
c14e343
Make use of `ffi_safe::Vec` in `fj::PolyChain`
hannobraun Nov 7, 2022
20181e4
Implement Serde traits for `ffi_safe::Vec`
hannobraun Nov 7, 2022
57c627e
Move `impl` blocks closer to related type
hannobraun Nov 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions crates/fj/src/abi/ffi_safe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,35 @@ impl<T> Drop for Vec<T> {
unsafe impl<T: Send> Send for Vec<T> {}
unsafe impl<T: Sync> Sync for Vec<T> {}

#[cfg(feature = "serde")]
impl<T> serde::ser::Serialize for Vec<T>
where
T: serde::ser::Serialize,
{
fn serialize<S>(
&self,
serializer: S,
) -> std::result::Result<S::Ok, S::Error>
where
S: serde::ser::Serializer,
{
self.deref().serialize(serializer)
}
}

#[cfg(feature = "serde")]
impl<'de, T> serde::de::Deserialize<'de> for Vec<T>
where
T: serde::de::Deserialize<'de>,
{
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
where
D: serde::de::Deserializer<'de>,
{
Ok(std::vec::Vec::deserialize(deserializer)?.into())
}
}

/// A FFI-safe version of `Box<str>`.
#[repr(transparent)]
#[derive(Debug, PartialEq, Clone)]
Expand Down
240 changes: 20 additions & 220 deletions crates/fj/src/shape_2d.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use std::mem;
use std::sync::atomic;

use crate::Shape;
use crate::{abi::ffi_safe, Shape};

/// A 2-dimensional shape
#[derive(Clone, Debug, PartialEq)]
Expand Down Expand Up @@ -100,8 +97,6 @@ impl From<Difference2d> for Shape2d {
#[repr(C)]
pub struct Sketch {
chain: Chain,

// The color of the sketch in RGBA
color: [u8; 4],
}

Expand Down Expand Up @@ -139,6 +134,18 @@ impl Sketch {
}
}

impl From<Sketch> for Shape {
fn from(shape: Sketch) -> Self {
Self::Shape2d(shape.into())
}
}

impl From<Sketch> for Shape2d {
fn from(shape: Sketch) -> Self {
Shape2d::Sketch(shape)
}
}

/// A chain of elements that is part of a [`Sketch`]
#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
Expand Down Expand Up @@ -173,229 +180,22 @@ impl Circle {
}

/// A polygonal chain that is part of a [`Sketch`]
#[derive(Debug)]
#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[repr(C)]
pub struct PolyChain {
// The fields are the raw parts of a `Vec`. `Sketch` needs to be FFI-safe,
// meaning it can't store a `Vec` directly. It needs to take this detour.
ptr: *mut [f64; 2],
length: usize,
capacity: usize,

// The `Sketch` can be cloned, so we need to track the number of live
// instances, so as to free the buffer behind `ptr` only when the last
// one is dropped.
rc: *mut atomic::AtomicUsize,
points: ffi_safe::Vec<[f64; 2]>,
}

impl PolyChain {
/// Construct an instance from a list of points
pub fn from_points(mut points: Vec<[f64; 2]>) -> Self {
// This can be cleaned up, once `Vec::into_raw_parts` is stable.
let ptr = points.as_mut_ptr();
let length = points.len();
let capacity = points.capacity();

// We're taking ownership of the memory here, so we can't allow `points`
// to deallocate it.
mem::forget(points);

// Allocate the reference counter on the heap. It will be reclaimed
// alongside `points` when it reaches 0.
let rc = Box::new(atomic::AtomicUsize::new(1));
let rc = Box::leak(rc) as *mut _;

Self {
ptr,
length,
capacity,
rc,
}
}

/// Get a reference to the points in this [`PolyChain`].
fn points(&self) -> &[[f64; 2]] {
unsafe { std::slice::from_raw_parts(self.ptr, self.length) }
pub fn from_points(points: Vec<[f64; 2]>) -> Self {
let points = points.into();
Self { points }
}

/// Return the points that define the polygonal chain
pub fn to_points(&self) -> Vec<[f64; 2]> {
// This is sound. All invariants are automatically kept, as the raw
// parts come from an original `Vec` that is identical to the new one we
// create here, and aren't being modified anywhere.
let points = unsafe {
Vec::from_raw_parts(self.ptr, self.length, self.capacity)
};

// Ownership of the pointer in `self.raw_parts` transferred to `points`.
// We work around that, by returning a clone of `points` (hence not
// giving ownership to the caller).
let ret = points.clone();

// Now we just need to forget that `points` ever existed, and we keep
// ownership of the pointer.
mem::forget(points);

ret
}
}

impl Clone for PolyChain {
fn clone(&self) -> Self {
// Increment the reference counter
unsafe {
(*self.rc).fetch_add(1, atomic::Ordering::AcqRel);
}

Self {
ptr: self.ptr,
length: self.length,
capacity: self.capacity,
rc: self.rc,
}
}
}

impl PartialEq for PolyChain {
fn eq(&self, other: &Self) -> bool {
self.points() == other.points()
}
}

impl Drop for PolyChain {
fn drop(&mut self) {
// Decrement the reference counter
let rc_last =
unsafe { (*self.rc).fetch_sub(1, atomic::Ordering::AcqRel) };

// If the value of the refcount before decrementing was 1,
// then this must be the last Drop call. Reclaim all resources
// allocated on the heap.
if rc_last == 1 {
unsafe {
let points =
Vec::from_raw_parts(self.ptr, self.length, self.capacity);
let rc = Box::from_raw(self.rc);

drop(points);
drop(rc);
}
}
}
}

// `PolyChain` can be `Send`, because it encapsulates the raw pointer it
// contains, making sure memory ownership rules are observed.
unsafe impl Send for PolyChain {}

#[cfg(feature = "serde")]
impl serde::ser::Serialize for PolyChain {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::ser::Serializer,
{
let serde_sketch = PolyChainSerde {
points: self.to_points(),
};

serde_sketch.serialize(serializer)
}
}

#[cfg(feature = "serde")]
impl<'de> serde::de::Deserialize<'de> for PolyChain {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::de::Deserializer<'de>,
{
PolyChainSerde::deserialize(deserializer)
.map(|serde_sketch| PolyChain::from_points(serde_sketch.points))
}
}

/// An owned, non-repr-C [`PolyChain`]
///
/// De/serializing a non-trivial structure with raw pointers is a hassle.
/// This structure is a simple, owned intermediate form that can use the derive
/// macros provided by serde. The implementation of the `Serialize` and
/// `Deserialize` traits for [`PolyChain`] use this type as a stepping stone.
///
/// Note that constructing this requires cloning the points behind
/// [`PolyChain`]. If de/serialization turns out to be a bottleneck, a more
/// complete implementation will be required.
#[cfg(feature = "serde")]
#[derive(serde::Serialize, serde::Deserialize)]
#[serde(rename = "Polyline")]
struct PolyChainSerde {
points: Vec<[f64; 2]>,
}

impl From<Sketch> for Shape {
fn from(shape: Sketch) -> Self {
Self::Shape2d(shape.into())
}
}

impl From<Sketch> for Shape2d {
fn from(shape: Sketch) -> Self {
Shape2d::Sketch(shape)
}
}

#[cfg(test)]
mod tests {
use super::*;

fn test_points() -> Vec<[f64; 2]> {
vec![[1.0, 1.0], [2.0, 1.0], [2.0, 2.0], [1.0, 2.0]]
}

#[test]
fn test_poly_chain_preserve_points() {
let points = test_points();
let poly_chain = PolyChain::from_points(points.clone());

assert_eq!(poly_chain.to_points(), points);
}

#[test]
fn test_poly_chain_rc() {
let assert_rc = |poly_chain: &PolyChain, expected_rc: usize| {
let rc =
unsafe { (*poly_chain.rc).load(atomic::Ordering::Acquire) };
assert_eq!(
rc, expected_rc,
"Sketch has rc = {rc}, expected {expected_rc}"
);
};

let poly_chain = PolyChain::from_points(test_points());
assert_rc(&poly_chain, 1);

let (s2, s3) = (poly_chain.clone(), poly_chain.clone());
assert_rc(&poly_chain, 3);

drop(s2);
assert_rc(&poly_chain, 2);

drop(s3);
assert_rc(&poly_chain, 1);

// rc is deallocated after the last drop, so we can't assert that it's 0
}

#[cfg(feature = "serde")]
#[test]
fn test_poly_chain_serialize_loopback() {
use serde_json::{from_str, to_string};

let poly_chain = PolyChain::from_points(test_points());

let json = to_string(&poly_chain).expect("failed to serialize sketch");
let poly_chain_de: PolyChain =
from_str(&json).expect("failed to deserialize sketch");

// ensure same content
assert_eq!(poly_chain.to_points(), poly_chain_de.to_points());
self.points.clone().into()
}
}