Initial cut of primitives from piet-scene #1
Conversation
Pulls in the primitives from the piet-scene crate, replacing the geometry and path types with those from kurbo. There are a few alterations to naming a docs and the addition of some convenience "builder" methods to making constructing various types easier. There are more types to add to this crate-- specifically things related to fonts and text. I though I'd keep the initial PR smallish, covering just enough to serve current piet-scene needs. I have a working branch with updates to the full piet-gpu repo to support these changes that is ready to go after this lands.
| name = "peniko" | ||
| version = "0.1.0" | ||
| license = "MIT/Apache-2.0" | ||
| edition = "2021" |
There was a problem hiding this comment.
It would be nice to have this a little more filled out - author, description, readme link, keywords.
Cargo.toml
Outdated
| edition = "2021" | ||
|
|
||
| [dependencies] | ||
| kurbo = { git = "https://github.com/linebender/kurbo" } |
There was a problem hiding this comment.
Point to 0.9 when that's released?
There was a problem hiding this comment.
Yes, that's the intention, but I didn't want to block on it.
src/blend.rs
Outdated
| Saturation = 13, | ||
| Color = 14, | ||
| Luminosity = 15, | ||
| // Clip is the same as normal, but doesn't always push a blend group. |
There was a problem hiding this comment.
This is terse and confusing. May I suggest: "Clip is the same as normal, but the latter always creates an isolated blend group and the former can optimize that out."
There was a problem hiding this comment.
Is it bad form to claim that I simply copied your comment from the original code? :) I'll update it. I also intend to document all the others in this file when I have a moment. I'm trying to make that a priority.
There was a problem hiding this comment.
The original code had it in context of a full rendering pipeline, where the intent was crystal clear. Now it's in a separate crate, so without that context it's extremely confusing.
More seriously, yes, we should work toward documenting all this better. It was hard to motivate while everything was so fluid, but it feels like it's starting to converge a bit now.
src/brush/color.rs
Outdated
| Self { r, g, b, a } | ||
| } | ||
|
|
||
| /// Creates a color from hue, saturation, and lightness values. |
There was a problem hiding this comment.
I'm not a massive fan of hsl color spaces, as they're not colorimetrically sound. But color is one of those incredibly deep things. We should just do the RGB basics for now, though I am interested in doing "proper" color at some point.
I see there is hlc, which I like seeing - it should be compatible with Freiefarbe.
There was a problem hiding this comment.
I'm not either, but one of the things I'd like to be able to offer in this library is reasonable support for the general set of relevant web style properties (in the same way that kurbo can parse SVG path data). This means being able to represent all of the color spaces (and then some?). If you'd prefer to limit the scope, I can do that.
|
|
||
| pub use blend::*; | ||
| pub use brush::*; | ||
| pub use parse::ParseError; |
There was a problem hiding this comment.
If the only thing in there is an error, maybe the module should be named "error"? I'd expect parsing code. Minor nit though.
There was a problem hiding this comment.
This is a bit of a stub for now. As above for color, my future goal is to support parsing additional things like font-family, font-weight, font-style, etc. I have this mostly written in another project, but didn't want to overload this PR with new stuff.
I suppose there is a general question of whether parsing is out of scope for this crate?
* add additional metadata to Cargo.toml * better comment on Mix::Clip and document all of the others
also removes the Color::hsl[a] methods
|
Cargo metadata and kurbo version have been updated. I removed the hsl constructors for color (but kept hlc). Happy to spend some time in the future discussing high quality color support as I think this crate is now a suitable place for it. I kept the (currently) unfortunately named parse module for now because I intend to fill it with some actual parsing machinery in the next PR. Merging this so we can move forward! |
Pulls in the primitives from the piet-scene crate, replacing the geometry and path types with those from kurbo. There are a few alterations to naming and docs and the addition of some convenience "builder" methods to making constructing various types easier.
There are more types to add to this crate-- specifically things related to fonts and text. I though I'd keep the initial PR smallish, covering just enough to serve current piet-scene needs.
I have a working branch with updates to the full piet-gpu repo to support these changes that is ready to go after this lands.