googlefonts / googlefonts/fontations
[skrifa] Location/LocationRef type ergonomics
- Dominant language
- Rust
- Stars
- 826
- Forks
- 75
- Avg merge
- 22h 33m
- Merged PRs (30d)
- 75
Description
The `Location` and `LocationRef` types were intended to serve a similar purpose to other owned/borrowed type pairs in Rust (`String`/`&str`, `Vec`/`&[T]`, etc) but actual usage doesn't actually follow the same patterns leading to confusion and frustration.
The main issue is that `Location` cannot impl `Deref` due to the requirements of the `Deref` trait. Here's one potential solution along with some proposed renaming for consistency and to make the purpose of these types more clear at a glance:
```rust
/// Coordinate that represents the location of a single axis in variation
/// space, normalized to the range `-1.0..=1.0`.
pub type VarCoord = F2Dot14;
/// Borrowed type for a collection of normalized coordinates, one for each
/// axis in the same order as the `fvar` table, which represents a location
/// in variation space.
///
/// For the owned version of this type, see [`VarLocation`].
pub type VarLocationRef = [VarCoord];
/// Owned type for a collection of normalized coordinates, one for each
/// axis in the same order as the `fvar` table, which represents a location
/// in variation space.
///
/// (Other docs for how to construct this, examples, etc)
///
/// For the borrowed version of this type, see [`VarLocationRef`].
#[derive(Default)]
pub struct VarLocation(SmallArray);
impl Deref for VarLocation {
type Target = VarLocationRef;
fn deref(&self) -> &Self::Target {
&self.0
}
}
```
This renames `NormalizedCoord` -> `VarCoord` to make the relation clear and adds a `Var` prefix to the location types.
Notably `VarLocationRef` now just becomes a type alias for `[VarCoord]` which is an unsized type similar to `str` leading to the same usage patterns (you can never use it without a `&`).
All types that accept a location would now take a `&VarLocationRef` parameter which can be passed as follows:
```rust
pub fn uses_variations(var_loc: &VarLocationRef) {}
// with an owned location
let owned_loc = font.axes().location(...);
uses_variations(&owned_loc);
// vello style with coords accumulated in a scene
struct Scene {
// ...
all_coords: Vec,
}
uses_variations(&scene.all_coords[start..end]);
// passing default (or empty) locations
uses_variations(Default::default());
uses_variations(&VarLocation::default());
```
This change can be done incrementally without breaking Skia.
We _could_ make `VarLocationRef` itself a real unsized type to also enable this pattern but that would require unsafe code and IMO a borrowed location is really just a slice of coordinates.
@rsheeter wdyt? @cmyr thoughts on general rustiness of this approach?
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by locating the skrifa definitions and usages of Location, LocationRef, and NormalizedCoord. Compare their current API patterns with the proposed VarCoord, VarLocation, and VarLocationRef aliases, then trace the types that accept locations. Done means the ownership and borrowing ergonomics are consistent across callers without breaking the stated incremental compatibility with Skia.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100