Skip to content

Commit

Permalink
[skrifa] hinting mode changes/remove feature gate (#710)
Browse files Browse the repository at this point in the history
- removes the hinting feature gate
- adds a `None` variant to `Hinting` enum to use as a replacement for `Option<Hinting>` so call sites taking this as a parameter have greater clarity
- updates the TrueType memory functions to take `Hinting` rather than `bool`

Fixes #708
  • Loading branch information
dfrg authored Nov 17, 2023
1 parent 889a5a6 commit 442cc4c
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 52 deletions.
5 changes: 0 additions & 5 deletions skrifa/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ repository = "https://github.com/googlefonts/fontations"
readme = "README.md"
categories = ["text-processing", "parsing", "graphics"]

[features]
default = ["scale"]
scale = []
hinting = []

[dependencies]
read-fonts = { version = "0.13.1", path = "../read-fonts" }

Expand Down
1 change: 0 additions & 1 deletion skrifa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ pub mod charmap;
pub mod font;
pub mod instance;
pub mod metrics;
#[cfg(feature = "scale")]
pub mod scale;
pub mod setting;
pub mod string;
Expand Down
14 changes: 7 additions & 7 deletions skrifa/src/scale/glyf/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use read_fonts::{
types::{F26Dot6, Fixed, Point},
};

use super::Outline;
use super::{Hinting, Outline};

/// Buffers used during glyph scaling.
pub struct OutlineMemory<'a> {
Expand All @@ -22,11 +22,11 @@ pub struct OutlineMemory<'a> {
}

impl<'a> OutlineMemory<'a> {
pub(super) fn new(outline: &Outline, buf: &'a mut [u8], with_hinting: bool) -> Option<Self> {
pub(super) fn new(outline: &Outline, buf: &'a mut [u8], hinting: Hinting) -> Option<Self> {
let (scaled, buf) = alloc_slice(buf, outline.points)?;
let (unscaled, buf) = alloc_slice(buf, outline.max_other_points)?;
// We only need original scaled points when hinting
let (original_scaled, buf) = if outline.has_hinting && with_hinting {
let (original_scaled, buf) = if outline.has_hinting && hinting != Hinting::None {
alloc_slice(buf, outline.max_other_points)?
} else {
(Default::default(), buf)
Expand Down Expand Up @@ -157,9 +157,9 @@ mod tests {
has_variations: true,
has_overlaps: false,
};
let required_size = outline_info.required_buffer_size(false);
let required_size = outline_info.required_buffer_size(Hinting::None);
let mut buf = vec![0u8; required_size];
let memory = OutlineMemory::new(&outline_info, &mut buf, false).unwrap();
let memory = OutlineMemory::new(&outline_info, &mut buf, Hinting::None).unwrap();
assert_eq!(memory.scaled.len(), outline_info.points);
assert_eq!(memory.unscaled.len(), outline_info.max_other_points);
// We don't allocate this buffer when hinting is disabled
Expand Down Expand Up @@ -190,8 +190,8 @@ mod tests {
};
// Required size adds 4 bytes slop to account for internal alignment
// requirements. So subtract 5 to force a failure.
let not_enough = outline_info.required_buffer_size(false) - 5;
let not_enough = outline_info.required_buffer_size(Hinting::None) - 5;
let mut buf = vec![0u8; not_enough];
assert!(OutlineMemory::new(&outline_info, &mut buf, false).is_none());
assert!(OutlineMemory::new(&outline_info, &mut buf, Hinting::None).is_none());
}
}
2 changes: 1 addition & 1 deletion skrifa/src/scale/glyf/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub use hint::HinterOutline;
pub use mem::OutlineMemory;
pub use outline::{Outline, ScaledOutline};

use super::Error;
use super::{Error, Hinting};

use read_fonts::{
tables::{
Expand Down
10 changes: 5 additions & 5 deletions skrifa/src/scale/glyf/outline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use read_fonts::{
types::{F26Dot6, Fixed, GlyphId, Pen, Point},
};

use super::OutlineMemory;
use super::{Hinting, OutlineMemory};

/// Represents the information necessary to scale a glyph outline.
///
Expand Down Expand Up @@ -48,9 +48,9 @@ pub struct Outline<'a> {
impl<'a> Outline<'a> {
/// Returns the minimum size in bytes required to scale an outline based
/// on the computed sizes.
pub fn required_buffer_size(&self, with_hinting: bool) -> usize {
pub fn required_buffer_size(&self, hinting: Hinting) -> usize {
let mut size = 0;
let hinting = with_hinting && self.has_hinting;
let hinting = self.has_hinting && hinting != Hinting::None;
// Scaled, unscaled and (for hinting) original scaled points
size += self.points * size_of::<Point<F26Dot6>>();
// Unscaled and (if hinted) original scaled points
Expand Down Expand Up @@ -82,9 +82,9 @@ impl<'a> Outline<'a> {
pub fn memory_from_buffer(
&self,
buf: &'a mut [u8],
with_hinting: bool,
hinting: Hinting,
) -> Option<OutlineMemory<'a>> {
OutlineMemory::new(self, buf, with_hinting)
OutlineMemory::new(self, buf, hinting)
}
}

Expand Down
5 changes: 3 additions & 2 deletions skrifa/src/scale/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,9 +167,11 @@ use super::{
/// Modes for hinting.
///
/// Only the `glyf` source supports all hinting modes.
#[cfg(feature = "hinting")]
#[derive(Copy, Clone, PartialEq, Eq, Default, Debug)]
pub enum Hinting {
/// Hinting is disabled.
#[default]
None,
/// "Full" hinting mode. May generate rough outlines and poor horizontal
/// spacing.
Full,
Expand All @@ -181,7 +183,6 @@ pub enum Hinting {
LightSubpixel,
/// Same as light subpixel, but always prevents adjustment in the
/// horizontal direction. This is the default mode.
#[default]
VerticalSubpixel,
}

Expand Down
44 changes: 13 additions & 31 deletions skrifa/src/scale/scaler.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
use super::{
cff, glyf, Context, Error, NormalizedCoord, Pen, Result, Size, UniqueId, VariationSetting,
cff, glyf, Context, Error, Hinting, NormalizedCoord, Pen, Result, Size, UniqueId,
VariationSetting,
};

#[cfg(feature = "hinting")]
use super::Hinting;

use core::borrow::Borrow;
use read_fonts::{
types::{Fixed, GlyphId},
Expand Down Expand Up @@ -33,8 +31,7 @@ pub struct ScalerBuilder<'a> {
context: &'a mut Context,
cache_key: Option<UniqueId>,
size: Size,
#[cfg(feature = "hinting")]
hint: Option<Hinting>,
hinting: Hinting,
}

impl<'a> ScalerBuilder<'a> {
Expand All @@ -46,8 +43,7 @@ impl<'a> ScalerBuilder<'a> {
context,
cache_key: None,
size: Size::unscaled(),
#[cfg(feature = "hinting")]
hint: None,
hinting: Hinting::None,
}
}

Expand All @@ -70,9 +66,8 @@ impl<'a> ScalerBuilder<'a> {
/// Sets the hinting mode.
///
/// Passing `None` will disable hinting.
#[cfg(feature = "hinting")]
pub fn hint(mut self, hint: Option<Hinting>) -> Self {
self.hint = hint;
pub fn hint(mut self, hint: Hinting) -> Self {
self.hinting = hint;
self
}

Expand Down Expand Up @@ -153,8 +148,7 @@ impl<'a> ScalerBuilder<'a> {
Scaler {
size,
coords,
#[cfg(feature = "hinting")]
hint: self.hint,
hinting: self.hinting,
outlines,
}
}
Expand Down Expand Up @@ -206,8 +200,7 @@ impl<'a> ScalerBuilder<'a> {
pub struct Scaler<'a> {
size: f32,
coords: &'a [NormalizedCoord],
#[cfg(feature = "hinting")]
hint: Option<Hinting>,
hinting: Hinting,
outlines: Option<Outlines<'a>>,
}

Expand All @@ -226,14 +219,7 @@ impl<'a> Scaler<'a> {
/// in the given pen for the sequence of path commands that define the outline.
pub fn outline(&mut self, glyph_id: GlyphId, pen: &mut impl Pen) -> Result<ScalerMetrics> {
if let Some(outlines) = &mut self.outlines {
outlines.outline(
glyph_id,
self.size,
self.coords,
#[cfg(feature = "hinting")]
self.hint,
pen,
)
outlines.outline(glyph_id, self.size, self.coords, self.hinting, pen)
} else {
Err(Error::NoSources)
}
Expand All @@ -254,18 +240,18 @@ impl<'a> Outlines<'a> {
glyph_id: GlyphId,
size: f32,
coords: &'a [NormalizedCoord],
#[cfg(feature = "hinting")] hint: Option<Hinting>,
hinting: Hinting,
pen: &mut impl Pen,
) -> Result<ScalerMetrics> {
match self {
Self::TrueType(scaler, buf) => {
let glyph = scaler.outline(glyph_id)?;
let buf_size = glyph.required_buffer_size(false);
let buf_size = glyph.required_buffer_size(hinting);
if buf.len() < buf_size {
buf.resize(buf_size, 0);
}
let memory = glyph
.memory_from_buffer(&mut buf[..], false)
.memory_from_buffer(&mut buf[..], hinting)
.ok_or(Error::InsufficientMemory)?;
let outline = scaler.scale(memory, &glyph, size, coords)?;
outline.to_path(pen)?;
Expand All @@ -281,11 +267,7 @@ impl<'a> Outlines<'a> {
}
// CFF only has a single hinting mode and FT enables it
// if any of the hinting load flags are set.
#[cfg(feature = "hinting")]
let hint = hint.is_some();
#[cfg(not(feature = "hinting"))]
let hint = false;
scaler.outline(subfont, glyph_id, coords, hint, pen)?;
scaler.outline(subfont, glyph_id, coords, hinting != Hinting::None, pen)?;
// CFF does not have overlap flags and hinting never adjusts
// horizontal metrics
Ok(ScalerMetrics::default())
Expand Down

0 comments on commit 442cc4c

Please sign in to comment.