From d47d9545d81dffc424496e35aa6c111320def762 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Thu, 21 Mar 2024 13:25:48 -0400 Subject: [PATCH 1/2] [clippy] Push the rock back up the hill --- font-codegen/src/parsing.rs | 3 +++ skrifa/src/color/traversal_tests/mod.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/font-codegen/src/parsing.rs b/font-codegen/src/parsing.rs index ea543f191..5c21b6795 100644 --- a/font-codegen/src/parsing.rs +++ b/font-codegen/src/parsing.rs @@ -257,6 +257,9 @@ pub(crate) struct SinceVersion { /// ``` #[derive(Clone, Debug)] pub(crate) enum Count { + // the field isn't used, but it's nice to hold onto if we want to print errors + // in the future + #[allow(dead_code)] All(syn::token::DotDot), SingleArg(CountArg), Complicated { diff --git a/skrifa/src/color/traversal_tests/mod.rs b/skrifa/src/color/traversal_tests/mod.rs index b68702e5c..87b2e84be 100644 --- a/skrifa/src/color/traversal_tests/mod.rs +++ b/skrifa/src/color/traversal_tests/mod.rs @@ -296,6 +296,7 @@ fn colrv1_traversal_test( let mut file = OpenOptions::new() .write(true) .create(true) + .truncate(true) .open(dumpfile_path) .unwrap(); From 657d08e58dc07b8ad3cd9b672309a5071d5e7588 Mon Sep 17 00:00:00 2001 From: Chad Brokaw Date: Thu, 21 Mar 2024 21:33:45 -0400 Subject: [PATCH 2/2] [skrifa] tthint tests batch 1 (#847) Working toward #803 Covers GETINFO, GETVARIATION, GETDATA, GC, SCFS and MD --- skrifa/src/outline/glyf/hint/engine/data.rs | 122 +++++++++++- skrifa/src/outline/glyf/hint/engine/misc.rs | 208 ++++++++++++++++++-- skrifa/src/outline/glyf/hint/engine/mod.rs | 18 +- 3 files changed, 327 insertions(+), 21 deletions(-) diff --git a/skrifa/src/outline/glyf/hint/engine/data.rs b/skrifa/src/outline/glyf/hint/engine/data.rs index 1a3aaf9f5..ba646aa8b 100644 --- a/skrifa/src/outline/glyf/hint/engine/data.rs +++ b/skrifa/src/outline/glyf/hint/engine/data.rs @@ -144,7 +144,8 @@ impl<'a> Engine<'a> { #[cfg(test)] mod tests { - use super::super::MockEngine; + use super::super::{super::zone::ZonePointer, math, Engine, MockEngine}; + use raw::types::F26Dot6; #[test] fn measure_ppem_and_point_size() { @@ -157,4 +158,123 @@ mod tests { engine.op_mps().unwrap(); assert_eq!(engine.value_stack.pop().unwrap(), ppem * 64); } + + #[test] + fn gc() { + let mut mock = MockEngine::new(); + let mut engine = mock.engine(); + set_test_vectors(&mut engine); + // current point projected coord + let point = engine.graphics.zones[1].point_mut(1).unwrap(); + point.x = F26Dot6::from_bits(132); + point.y = F26Dot6::from_bits(-256); + engine.value_stack.push(1).unwrap(); + engine.op_gc(0).unwrap(); + assert_eq!(engine.value_stack.pop().unwrap(), 4); + // original point projected coord + let point = engine.graphics.zones[1].original_mut(1).unwrap(); + point.x = F26Dot6::from_bits(-64); + point.y = F26Dot6::from_bits(521); + engine.value_stack.push(1).unwrap(); + engine.op_gc(1).unwrap(); + assert_eq!(engine.value_stack.pop().unwrap(), 176); + } + + #[test] + fn scfs() { + let mut mock = MockEngine::new(); + let mut engine = mock.engine(); + set_test_vectors(&mut engine); + // This instruction is a nop in backward compatibility mode + // and before IUP. + engine.graphics.backward_compatibility = false; + engine.graphics.did_iup_x = true; + engine.graphics.did_iup_y = true; + // use the twilight zone to test the optional code path + engine.graphics.zp2 = ZonePointer::Twilight; + let point = engine.graphics.zones[0].point_mut(1).unwrap(); + point.x = F26Dot6::from_bits(132); + point.y = F26Dot6::from_bits(-256); + // assert we're not currently the same + assert_ne!( + engine.graphics.zones[0].point(1).unwrap(), + engine.graphics.zones[0].original(1).unwrap() + ); + // push point number + engine.value_stack.push(1).unwrap(); + // push value to match + engine.value_stack.push(42).unwrap(); + // set coordinate from stack! + engine.op_scfs().unwrap(); + let point = engine.graphics.zones[0].point(1).unwrap(); + assert_eq!(point.x.to_bits(), 166); + assert_eq!(point.y.to_bits(), -239); + // ensure that we set original = point + assert_eq!(point, engine.graphics.zones[0].original(1).unwrap()); + } + + #[test] + fn md_scaled() { + let mut mock = MockEngine::new(); + let mut engine = mock.engine(); + set_test_vectors(&mut engine); + // first path, measure in grid fitted outline + let zone = engine.graphics.zone_mut(ZonePointer::Glyph); + let point1 = zone.point_mut(1).unwrap(); + point1.x = F26Dot6::from_bits(132); + point1.y = F26Dot6::from_bits(-256); + let point2 = zone.point_mut(3).unwrap(); + point2.x = F26Dot6::from_bits(-64); + point2.y = F26Dot6::from_bits(100); + // now measure + engine.value_stack.push(1).unwrap(); + engine.value_stack.push(3).unwrap(); + engine.op_md(1).unwrap(); + assert_eq!(engine.value_stack.pop().unwrap(), 16); + } + + #[test] + fn md_unscaled() { + let mut mock = MockEngine::new(); + let mut engine = mock.engine(); + set_test_vectors(&mut engine); + // second path, measure in original unscaled outline. + // unscaled points are set in mock engine but we need a scale + engine.graphics.scale = 375912; + engine.value_stack.push(1).unwrap(); + engine.value_stack.push(3).unwrap(); + engine.op_md(0).unwrap(); + assert_eq!(engine.value_stack.pop().unwrap(), 11); + } + + #[test] + fn md_twilight() { + let mut mock = MockEngine::new(); + let mut engine = mock.engine(); + set_test_vectors(&mut engine); + // final path, measure in original outline, in twilight zone + engine.graphics.zp0 = ZonePointer::Twilight; + engine.graphics.zp1 = ZonePointer::Twilight; + // set some points + let zone = engine.graphics.zone_mut(ZonePointer::Twilight); + let point1 = zone.original_mut(1).unwrap(); + point1.x = F26Dot6::from_bits(132); + point1.y = F26Dot6::from_bits(-256); + let point2 = zone.original_mut(3).unwrap(); + point2.x = F26Dot6::from_bits(-64); + point2.y = F26Dot6::from_bits(100); + // now measure + engine.value_stack.push(1).unwrap(); + engine.value_stack.push(3).unwrap(); + engine.op_md(0).unwrap(); + assert_eq!(engine.value_stack.pop().unwrap(), 16); + } + + fn set_test_vectors(engine: &mut Engine) { + let v = math::normalize14(100, 50); + engine.graphics.proj_vector = v; + engine.graphics.dual_proj_vector = v; + engine.graphics.freedom_vector = v; + engine.graphics.update_projection_state(); + } } diff --git a/skrifa/src/outline/glyf/hint/engine/misc.rs b/skrifa/src/outline/glyf/hint/engine/misc.rs index bad060640..7acfe3047 100644 --- a/skrifa/src/outline/glyf/hint/engine/misc.rs +++ b/skrifa/src/outline/glyf/hint/engine/misc.rs @@ -22,69 +22,53 @@ impl<'a> Engine<'a> { /// See /// and pub(super) fn op_getinfo(&mut self) -> OpResult { + use getinfo::*; let selector = self.value_stack.pop()?; let mut result = 0; // Interpreter version (selector bit: 0, result bits: 0-7) - const VERSION_SELECTOR_BIT: i32 = 1 << 0; if (selector & VERSION_SELECTOR_BIT) != 0 { - result = 42; + result = 40; } // Glyph rotated (selector bit: 1, result bit: 8) - const GLYPH_ROTATED_SELECTOR_BIT: i32 = 1 << 1; if (selector & GLYPH_ROTATED_SELECTOR_BIT) != 0 && self.graphics.is_rotated { - const GLYPH_ROTATED_RESULT_BIT: i32 = 1 << 8; result |= GLYPH_ROTATED_RESULT_BIT; } // Glyph stretched (selector bit: 2, result bit: 9) - const GLYPH_STRETCHED_SELECTOR_BIT: i32 = 1 << 2; if (selector & GLYPH_STRETCHED_SELECTOR_BIT) != 0 && self.graphics.is_stretched { - const GLYPH_STRETCHED_RESULT_BIT: i32 = 1 << 9; result |= GLYPH_STRETCHED_RESULT_BIT; } // Font variations (selector bit: 3, result bit: 10) - const FONT_VARIATIONS_SELECTOR_BIT: i32 = 1 << 3; if (selector & FONT_VARIATIONS_SELECTOR_BIT) != 0 && self.axis_count != 0 { - const FONT_VARIATIONS_RESULT_BIT: i32 = 1 << 10; result |= FONT_VARIATIONS_RESULT_BIT; } // The following only apply for smooth hinting. if self.graphics.mode.is_smooth() { // Subpixel hinting [cleartype enabled] (selector bit: 6, result bit: 13) // (always enabled) - const SUBPIXEL_HINTING_SELECTOR_BIT: i32 = 1 << 6; if (selector & SUBPIXEL_HINTING_SELECTOR_BIT) != 0 { - const SUBPIXEL_HINTING_RESULT_BIT: i32 = 1 << 13; result |= SUBPIXEL_HINTING_RESULT_BIT; } // Vertical LCD subpixels? (selector bit: 8, result bit: 15) - const VERTICAL_LCD_SELECTOR_BIT: i32 = 1 << 8; if (selector & VERTICAL_LCD_SELECTOR_BIT) != 0 && self.graphics.mode.is_vertical_lcd() { - const VERTICAL_LCD_RESULT_BIT: i32 = 1 << 15; result |= VERTICAL_LCD_RESULT_BIT; } // Subpixel positioned? (selector bit: 10, result bit: 17) // (always enabled) - const SUBPIXEL_POSITIONED_SELECTOR_BIT: i32 = 1 << 10; if (selector & SUBPIXEL_POSITIONED_SELECTOR_BIT) != 0 { - const SUBPIXEL_POSITIONED_RESULT_BIT: i32 = 1 << 17; result |= SUBPIXEL_POSITIONED_RESULT_BIT; } // Symmetrical smoothing (selector bit: 11, result bit: 18) // Note: FreeType always enables this but we deviate when our own // preserve linear metrics flag is enabled. - const SYMMETRICAL_SMOOTHING_SELECTOR_BIT: i32 = 1 << 11; if (selector & SYMMETRICAL_SMOOTHING_SELECTOR_BIT) != 0 && !self.graphics.mode.preserve_linear_metrics() { - const SYMMETRICAL_SMOOTHING_RESULT_BIT: i32 = 1 << 18; result |= SYMMETRICAL_SMOOTHING_RESULT_BIT; } // ClearType hinting and grayscale rendering (selector bit: 12, result bit: 19) - const GRAYSCALE_CLEARTYPE_SELECTOR_BIT: i32 = 1 << 12; if (selector & GRAYSCALE_CLEARTYPE_SELECTOR_BIT) != 0 && self.graphics.mode.is_grayscale_cleartype() { - const GRAYSCALE_CLEARTYPE_RESULT_BIT: i32 = 1 << 19; result |= GRAYSCALE_CLEARTYPE_RESULT_BIT; } } @@ -145,3 +129,191 @@ impl<'a> Engine<'a> { } } } + +/// Constants for the GETINFO instruction. Extracted here +/// to enable access from tests. +mod getinfo { + // Interpreter version (selector bit: 0, result bits: 0-7) + pub const VERSION_SELECTOR_BIT: i32 = 1 << 0; + + // Glyph rotated (selector bit: 1, result bit: 8) + pub const GLYPH_ROTATED_SELECTOR_BIT: i32 = 1 << 1; + pub const GLYPH_ROTATED_RESULT_BIT: i32 = 1 << 8; + + // Glyph stretched (selector bit: 2, result bit: 9) + pub const GLYPH_STRETCHED_SELECTOR_BIT: i32 = 1 << 2; + pub const GLYPH_STRETCHED_RESULT_BIT: i32 = 1 << 9; + + // Font variations (selector bit: 3, result bit: 10) + pub const FONT_VARIATIONS_SELECTOR_BIT: i32 = 1 << 3; + pub const FONT_VARIATIONS_RESULT_BIT: i32 = 1 << 10; + + // Subpixel hinting [cleartype enabled] (selector bit: 6, result bit: 13) + // (always enabled) + pub const SUBPIXEL_HINTING_SELECTOR_BIT: i32 = 1 << 6; + pub const SUBPIXEL_HINTING_RESULT_BIT: i32 = 1 << 13; + + // Vertical LCD subpixels? (selector bit: 8, result bit: 15) + pub const VERTICAL_LCD_SELECTOR_BIT: i32 = 1 << 8; + pub const VERTICAL_LCD_RESULT_BIT: i32 = 1 << 15; + + // Subpixel positioned? (selector bit: 10, result bit: 17) + // (always enabled) + pub const SUBPIXEL_POSITIONED_SELECTOR_BIT: i32 = 1 << 10; + pub const SUBPIXEL_POSITIONED_RESULT_BIT: i32 = 1 << 17; + + // Symmetrical smoothing (selector bit: 11, result bit: 18) + // Note: FreeType always enables this but we deviate when our own + // preserve linear metrics flag is enabled. + pub const SYMMETRICAL_SMOOTHING_SELECTOR_BIT: i32 = 1 << 11; + pub const SYMMETRICAL_SMOOTHING_RESULT_BIT: i32 = 1 << 18; + + // ClearType hinting and grayscale rendering (selector bit: 12, result bit: 19) + pub const GRAYSCALE_CLEARTYPE_SELECTOR_BIT: i32 = 1 << 12; + pub const GRAYSCALE_CLEARTYPE_RESULT_BIT: i32 = 1 << 19; +} + +#[cfg(test)] +mod tests { + use super::super::{ + super::{super::super::LcdLayout, HintingMode}, + Engine, HintErrorKind, MockEngine, + }; + use raw::types::F2Dot14; + use read_fonts::tables::glyf::bytecode::Opcode; + + #[test] + fn getinfo() { + use super::getinfo::*; + let mut mock = MockEngine::new(); + let mut engine = mock.engine(); + // version + engine.getinfo_test(VERSION_SELECTOR_BIT, 40); + // not rotated + engine.getinfo_test(GLYPH_ROTATED_SELECTOR_BIT, 0); + // rotated + engine.graphics.is_rotated = true; + engine.getinfo_test(GLYPH_ROTATED_SELECTOR_BIT, GLYPH_ROTATED_RESULT_BIT); + // not stretched + engine.getinfo_test(GLYPH_STRETCHED_SELECTOR_BIT, 0); + // stretched + engine.graphics.is_stretched = true; + engine.getinfo_test(GLYPH_STRETCHED_SELECTOR_BIT, GLYPH_STRETCHED_RESULT_BIT); + // stretched and rotated + engine.getinfo_test( + GLYPH_ROTATED_SELECTOR_BIT | GLYPH_STRETCHED_SELECTOR_BIT, + GLYPH_ROTATED_RESULT_BIT | GLYPH_STRETCHED_RESULT_BIT, + ); + // not variable + engine.getinfo_test(FONT_VARIATIONS_SELECTOR_BIT, 0); + // variable + engine.axis_count = 1; + engine.getinfo_test(FONT_VARIATIONS_SELECTOR_BIT, FONT_VARIATIONS_RESULT_BIT); + // in strong hinting mode, the following selectors are always disabled + engine.graphics.mode = HintingMode::Strong; + for selector in [ + SUBPIXEL_HINTING_SELECTOR_BIT, + VERTICAL_LCD_SELECTOR_BIT, + SUBPIXEL_POSITIONED_SELECTOR_BIT, + SYMMETRICAL_SMOOTHING_SELECTOR_BIT, + GRAYSCALE_CLEARTYPE_SELECTOR_BIT, + ] { + engine.getinfo_test(selector, 0); + } + // set back to smooth mode + engine.graphics.mode = HintingMode::default(); + for (selector, result) in [ + // default smooth mode is grayscale cleartype + ( + GRAYSCALE_CLEARTYPE_SELECTOR_BIT, + GRAYSCALE_CLEARTYPE_RESULT_BIT, + ), + // always enabled in smooth mode + (SUBPIXEL_HINTING_SELECTOR_BIT, SUBPIXEL_HINTING_RESULT_BIT), + ( + SUBPIXEL_POSITIONED_SELECTOR_BIT, + SUBPIXEL_POSITIONED_RESULT_BIT, + ), + ] { + engine.getinfo_test(selector, result); + } + // vertical lcd + engine.graphics.mode = HintingMode::Smooth { + lcd_subpixel: Some(LcdLayout::Vertical), + preserve_linear_metrics: true, + }; + engine.getinfo_test(VERTICAL_LCD_SELECTOR_BIT, VERTICAL_LCD_RESULT_BIT); + // symmetical smoothing is disabled when preserving linear metrics + engine.getinfo_test(SYMMETRICAL_SMOOTHING_SELECTOR_BIT, 0); + // grayscale cleartype is disabled when lcd_subpixel is not None + engine.getinfo_test(GRAYSCALE_CLEARTYPE_SELECTOR_BIT, 0); + // reset to default to disable preserve linear metrics + engine.graphics.mode = HintingMode::default(); + // now symmetrical smoothing is enabled + engine.getinfo_test( + SYMMETRICAL_SMOOTHING_SELECTOR_BIT, + SYMMETRICAL_SMOOTHING_RESULT_BIT, + ); + } + + #[test] + fn getvariation() { + let mut mock = MockEngine::new(); + let mut engine = mock.engine(); + // no variations should trigger unknown opcode + assert!(matches!( + engine.op_getvariation(), + Err(HintErrorKind::UnhandledOpcode(Opcode::GETVARIATION)) + )); + // set the axis count to a non-zero value to enable variations + engine.axis_count = 2; + // and creates some coords + let coords = [ + F2Dot14::from_f32(-1.0), + F2Dot14::from_f32(0.5), + F2Dot14::from_f32(1.0), + ]; + let coords_bits = coords.map(|x| x.to_bits() as i32); + // too few, pad with zeros + engine.coords = &coords[0..1]; + engine.op_getvariation().unwrap(); + assert_eq!(engine.value_stack.len(), 2); + assert_eq!(engine.value_stack.values(), &[coords_bits[0], 0]); + engine.value_stack.clear(); + // too many, truncate + engine.coords = &coords[0..3]; + engine.op_getvariation().unwrap(); + assert_eq!(engine.value_stack.len(), 2); + assert_eq!(engine.value_stack.values(), &coords_bits[0..2]); + engine.value_stack.clear(); + // just right + engine.coords = &coords[0..2]; + engine.op_getvariation().unwrap(); + assert_eq!(engine.value_stack.len(), 2); + assert_eq!(engine.value_stack.values(), &coords_bits[0..2]); + } + + #[test] + fn getdata() { + let mut mock = MockEngine::new(); + let mut engine = mock.engine(); + // no variations should trigger unknown opcode + assert!(matches!( + engine.op_getdata(), + Err(HintErrorKind::UnhandledOpcode(Opcode::GETDATA)) + )); + // set the axis count to a non-zero value to enable variations + engine.axis_count = 1; + engine.op_getdata().unwrap(); + // :shrug: + assert_eq!(engine.value_stack.pop().unwrap(), 17); + } + + impl<'a> Engine<'a> { + fn getinfo_test(&mut self, selector: i32, expected: i32) { + self.value_stack.push(selector).unwrap(); + self.op_getinfo().unwrap(); + assert_eq!(self.value_stack.pop().unwrap(), expected); + } + } +} diff --git a/skrifa/src/outline/glyf/hint/engine/mod.rs b/skrifa/src/outline/glyf/hint/engine/mod.rs index a713ab2f0..8089bcf28 100644 --- a/skrifa/src/outline/glyf/hint/engine/mod.rs +++ b/skrifa/src/outline/glyf/hint/engine/mod.rs @@ -177,6 +177,8 @@ mod mock { points: Vec>, point_flags: Vec, contours: Vec, + twilight: Vec>, + twilight_flags: Vec, } impl MockEngine { @@ -189,6 +191,8 @@ mod mock { points: vec![Default::default(); 64], point_flags: vec![Default::default(); 32], contours: vec![32], + twilight: vec![Default::default(); 32], + twilight_flags: vec![Default::default(); 32], } } @@ -202,6 +206,11 @@ mod mock { DefinitionMap::Mut(function_defs), DefinitionMap::Mut(instruction_defs), ); + for (i, point) in self.unscaled.iter_mut().enumerate() { + let i = i as i32; + point.x = 57 + i * 2; + point.y = -point.x * 3; + } let (points, original) = self.points.split_at_mut(32); let glyph_zone = Zone::new( &self.unscaled, @@ -210,8 +219,13 @@ mod mock { &mut self.point_flags, &self.contours, ); - let mut graphics_state = GraphicsState::default(); - graphics_state.zones[1] = glyph_zone; + let (points, original) = self.twilight.split_at_mut(16); + let twilight_zone = Zone::new(&[], original, points, &mut self.twilight_flags, &[]); + let mut graphics_state = GraphicsState { + zones: [twilight_zone, glyph_zone], + ..Default::default() + }; + graphics_state.update_projection_state(); Engine { graphics: graphics_state, cvt: CowSlice::new_mut(cvt).into(),