Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
26 changes: 20 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions piet-scene/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ edition = "2021"
[dependencies]
bytemuck = { version = "1.7.2", features = ["derive"] }
smallvec = "1.8.0"
pinot = "0.1.5"
moscato = "0.1.2"
moscato = { git = "https://github.com/dfrg/pinot" }
kurbo = { version = "0.8.3", optional = true }
2 changes: 1 addition & 1 deletion piet-scene/src/glyph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
//
// Also licensed under MIT license, at your choice.

pub use pinot;
pub use moscato::pinot;

use crate::brush::{Brush, Color};
use crate::geometry::Affine;
Expand Down
93 changes: 93 additions & 0 deletions piet-scene/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,96 @@ pub mod glyph;
pub mod path;
pub mod resource;
pub mod scene;

/// Implement conversions to and from Kurbo types when the `kurbo` feature is
/// enabled.
#[cfg(feature = "kurbo")]
mod kurbo_conv {
use super::geometry::{Affine, Point, Rect};
use super::path::Element;

impl From<kurbo::Point> for Point {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ergonomic but doesn't exactly follow Rust conventions - usually From is for lossless conversions, so there's f32->f64 but not the other way around. There was a proposal for a standard lossy conversion trait but I'm not sure it went anywhere. I think this is good enough for now but want to check with a Rust expert to see if there is a better pattern. In any case, we can change it later.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. If you'd feel better with explicit to_kurbo/from_kurbo methods, I actually don't mind going that direction. I found that impl Into in the builder interface isn't all that useful anyway due to these types being mostly buried in options and iterators.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ->kurbo direction is fine. If "into" is not actually a big ergonomic win in practice then I think having an explicit method is a better choice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the From impls and replaced them with from_kurbo methods for the lossy conversions.

fn from(p: kurbo::Point) -> Self {
Self::new(p.x as f32, p.y as f32)
}
}

impl From<Point> for kurbo::Point {
fn from(p: Point) -> kurbo::Point {
Self::new(p.x as f64, p.y as f64)
}
}

impl From<kurbo::Affine> for Affine {
fn from(a: kurbo::Affine) -> Self {
let c = a.as_coeffs();
Self {
xx: c[0] as f32,
yx: c[1] as f32,
xy: c[2] as f32,
yy: c[3] as f32,
dx: c[4] as f32,
dy: c[5] as f32,
}
}
}

impl From<Affine> for kurbo::Affine {
fn from(a: Affine) -> Self {
Self::new([
a.xx as f64,
a.yx as f64,
a.yx as f64,
a.yy as f64,
a.dx as f64,
a.dy as f64,
])
}
}

impl From<kurbo::Rect> for Rect {
fn from(r: kurbo::Rect) -> Self {
Self {
min: Point::new(r.x0 as f32, r.y0 as f32),
max: Point::new(r.x1 as f32, r.y1 as f32),
}
}
}

impl From<Rect> for kurbo::Rect {
fn from(r: Rect) -> Self {
Self {
x0: r.min.x as f64,
y0: r.min.y as f64,
x1: r.max.x as f64,
y1: r.max.y as f64,
}
}
}

impl From<kurbo::PathEl> for Element {
fn from(e: kurbo::PathEl) -> Self {
use kurbo::PathEl::*;
match e {
MoveTo(p0) => Self::MoveTo(p0.into()),
LineTo(p0) => Self::LineTo(p0.into()),
QuadTo(p0, p1) => Self::QuadTo(p0.into(), p1.into()),
CurveTo(p0, p1, p2) => Self::CurveTo(p0.into(), p1.into(), p2.into()),
ClosePath => Self::Close,
}
}
}

impl From<Element> for kurbo::PathEl {
fn from(e: Element) -> Self {
use Element::*;
match e {
MoveTo(p0) => Self::MoveTo(p0.into()),
LineTo(p0) => Self::LineTo(p0.into()),
QuadTo(p0, p1) => Self::QuadTo(p0.into(), p1.into()),
CurveTo(p0, p1, p2) => Self::CurveTo(p0.into(), p1.into(), p2.into()),
Close => Self::ClosePath,
}
}
}
}
3 changes: 1 addition & 2 deletions piet-scene/src/scene/blend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ pub enum Compose {
DestAtop = 10,
Xor = 11,
Plus = 12,
PlusDarker = 13,
PlusLighter = 14,
PlusLighter = 13,
}

/// Blend mode consisting of mixing and composition functions.
Expand Down
7 changes: 4 additions & 3 deletions piet-scene/src/scene/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use core::borrow::Borrow;
const MAX_BLEND_STACK: usize = 256;

/// Creates a new builder for constructing a scene.
pub fn build_scene<'a>(scene: &'a mut Scene, resources: &'a mut ResourceContext) -> Builder<'a> {
Builder::new(&mut scene.data, ResourceData::Scene(resources))
pub fn build_scene<'a>(scene: &'a mut Scene, rcx: &'a mut ResourceContext) -> Builder<'a> {
Builder::new(&mut scene.data, ResourceData::Scene(rcx))
}

/// Creates a new builder for construction a scene fragment.
Expand Down Expand Up @@ -62,7 +62,8 @@ impl<'a> Builder<'a> {
}

/// Pushes a transform matrix onto the stack.
pub fn push_transform(&mut self, transform: Affine) {
pub fn push_transform(&mut self, transform: impl Into<Affine>) {
let transform = transform.into();
self.transform(transform);
self.transforms.push(transform);
}
Expand Down
19 changes: 18 additions & 1 deletion piet-scene/src/scene/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ pub use builder::{build_fragment, build_scene, Builder};
pub use style::*;

use super::brush::*;
use super::geometry::{Affine, Point, Rect};
use super::geometry::{Affine, Point};
use super::path::Element;
use super::resource::ResourceContext;

use core::ops::Range;

/// Raw data streams describing an encoded scene.
#[derive(Default)]
pub struct SceneData {
pub transform_stream: Vec<Affine>,
Expand Down Expand Up @@ -83,6 +85,13 @@ pub struct Scene {
}

impl Scene {
/// Creates a new builder for filling the scene. Any current content in
/// the scene is cleared.
pub fn build<'a>(&'a mut self, rcx: &'a mut ResourceContext) -> Builder<'a> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this name is confusing, usually "build" is at the end of a fluent-style chain, converting a builder type into a (usually non-mutable) instance of what's being build. Perhaps "start_build"? It's also possible I don't really understand the logic here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this name is poorly chosen. start_build could work, or maybe new_builder. I think I'll just remove these and keep the original free functions for now.

Copy link
Collaborator Author

@dfrg dfrg May 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were redundant anyway so they've been removed. Easiest way to solve a naming issue :)

build_scene(self, rcx)
}

/// Returns the raw encoded scene data streams.
pub fn data(&self) -> &SceneData {
&self.data
}
Expand All @@ -96,9 +105,17 @@ pub struct Fragment {
}

impl Fragment {
/// Returns the underlying stream of points that defined all encoded path
/// segments.
pub fn points(&self) -> &[Point] {
bytemuck::cast_slice(&self.data.pathseg_stream)
}

/// Creates a new builder for filling the fragment. Any current content in
/// the fragment is cleared.
pub fn build<'a>(&'a mut self) -> Builder<'a> {
build_fragment(self)
}
}

#[derive(Default)]
Expand Down