Simplify api and dependencies between piet family of crates
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 1.4k
- Forks
- 98
- PR merge metrics
- No merged PRs in 30d
Description
The current structure of piet— traits defined in the `piet` crate, with lots of associated types, and then implemented in the various backends, and then exposed as aliases in `piet-common`— is complicated and fragile-feeling.
For a while I've been trying to figure out a better way to do this, based on my experience refactoring `druid-shell`, and I think I might have a satisfying solution.
## structs instead of traits define common interfaces
The basic idea for this is that we use wrapper structs to define common interfaces. Let's take the `text` module as an example. We basically translate,
```rust
pub trait Text {
type FontBuilder: FontBuilder;
type Font: Font;
type TextLayoutBuilder: TextLayoutBuilder;
type TextLayout: TextLayout;
fn new_font_by_name(&mut self, name: &str, size: f64) -> Self::FontBuilder;
fn new_text_layout(&mut self, font: &Self::Font, text: &str) -> Self::TextLayoutBuilder;
}
pub trait FontBuilder {
type Out: Font;
fn build(self) -> Result;
}
pub trait Font {}
pub trait TextLayoutBuilder {
type Out: TextLayout;
fn build(self) -> Result;
}
```
Into,
```rust
use crate::platform;
struct Text(platform::Text);
struct Font(platform::Font);
struct FontBuilder(platform::FontBuilder);
struct TextLayout(platform::TextLayout);
struct TextLayoutBuilder(platform::TextLayoutBuilder);
impl Text {
fn new_font_by_name(&mut self, name: &str, size: f64) -> FontBuilder {
self.0.new_font_by_name(name, size).into()
}
fn new_text_layout(&mut self, font: &Font, text: &str) -> TextLayoutBuilder {
self.0.new_text_layout(&font.0, text).into()
}
}
impl FontBuilder {
fn build(self) -> Result {
self.0.build().map(Into::into)
}
}
//etc
```
In this world, `piet` and `piet-common` are unified. By default, in `piet`, the 'platform' is the `NullRenderer`; but we support a "platform" feature, which uses the best backend available for the platform. If the user wants to use a _specific_ backend, they can just use it directly; the API is identical, and this is enforced by the convention of having the methods on the types in `piet` directly call identical methods on the inner type.
## Downsides
The major downside to this strategy is that it doesn't let us have functions that are generic over piet backends. This isn't impossible to overcome, however; one simple (if a bit annoying) approach is to _also_ have traits that cover this API, and which can be used when necessary. A major question to ask is whether or not we _actually_ ever anticipate having more than one piet backend in use at a time, in a way that requires us to be generic. My hunch is that this is a very speculative concern, and one we really don't need to be too concerned about.
Am I missing anything? It's very possible that I am, but from my current understanding this really feels like a solid win: we can combine two or three crates into one (`piet`, `piet-common`, `piet-test`(?)), and we can get rid of the associated types and the way they complicate the API elsewhere (the really hair type aliases like `pub type Text<'a> = as piet_common::RenderContext>::Text`) which complicate API.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by comparing the `piet`, `piet-common`, and possibly `piet-test` crates, with the `text` module as the example described in the issue. Determine whether wrapper structs can replace the associated-type traits without losing needed generic backend support; done would be an agreed architecture for consolidating the crates and simplifying the API.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, computer-graphics
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100