From 05d977f60b2d4bb59e9b1f2bce3b50b6787fdeec Mon Sep 17 00:00:00 2001 From: Chad Brokaw Date: Mon, 9 Oct 2023 23:42:01 -0400 Subject: [PATCH] review feedback * collapse test metrics asserts to single expression * reduce CBDT test case size * remove useless comments * add links to issue for improving codegen to relevant places --- font-codegen/src/fields.rs | 4 +- font-codegen/src/record.rs | 2 + .../test_data/ttf/embedded_bitmaps.ttf | Bin 1352 -> 476 bytes .../test_data/ttx/embedded_bitmaps.ttx | 93 +++--------------- read-fonts/src/tables/cbdt.rs | 93 +++--------------- read-fonts/src/tables/ebdt.rs | 56 +++++------ 6 files changed, 60 insertions(+), 188 deletions(-) diff --git a/font-codegen/src/fields.rs b/font-codegen/src/fields.rs index 5bda0d3f0..ccce0039d 100644 --- a/font-codegen/src/fields.rs +++ b/font-codegen/src/fields.rs @@ -401,7 +401,8 @@ fn traversal_arm_for_field( let data = fld.offset_getter_data_src(); quote!(Field::new(#name_str, traversal::FieldType::var_array(#typ_str, self.#name()#maybe_unwrap, #data))) } - // HACK: who wouldn't want to hard-code ValueRecord handling + // See if there are better ways to handle these hardcoded types + // FieldType::Struct { typ } if typ == "ValueRecord" || typ == "SbitLineMetrics" => { let offset_data = pass_data .cloned() @@ -479,6 +480,7 @@ impl Field { // catch `ValueRecord` so use this attribute to ignore it. // Fields that require args for reading can't be read "zerocopy" // anyway. + // self.attrs.read_with_args.is_none() && matches!( self.typ, diff --git a/font-codegen/src/record.rs b/font-codegen/src/record.rs index 2c4a8e1d0..ddf5763ea 100644 --- a/font-codegen/src/record.rs +++ b/font-codegen/src/record.rs @@ -55,6 +55,8 @@ pub(crate) fn generate(item: &Record, all_items: &Items) -> syn::Result if item.name == "SbitLineMetrics" { use quote::TokenStreamExt; let mut copy_trait = quote!(Copy,); diff --git a/font-test-data/test_data/ttf/embedded_bitmaps.ttf b/font-test-data/test_data/ttf/embedded_bitmaps.ttf index eeba70af3d68d4a91f2f7f5619b31faa9b604f97..9a06b939f871462e85b134ae270392a54d2a4398 100644 GIT binary patch delta 146 zcmX@Xb%!~GfsuiMfsMg|fr&xE*~uj&NipgyP-G1di#j{`I9s|HbOPB&fH=SvD9;^d zuED^-C<5ebx&q}ho!MJ}d(HY?9&dVjmHCdM>kcmNXas$f=0G7QVZvX%Q delta 1030 zcmcb^e1a>4fsuiMfsMg|fr&xE*~ukD<*|J*P-G1Q19PLZlaI5di}6I(1)adj{7 ziHV6REiKK>%`GY_Dk&++%FN8k$;r&j%+Jry&d$!v$S5u<3JVP_C@27`0g5ChBxGb{ zq@|^$Bqsw=NN{j;WF$~9C@>JnPKb}MkBf_ojg5_tjt&e6h>DB^l0doeurMG4!Un4I z_wxfHAQ!>_l3Pzac?oouUP+K&FoQya!|VTn0tJf?_y0He|Nno(|NpQ4C+z>fem;=% zzu>>X`Tq{@`~NrW{~z%Ge%kFd4GauS*`6+rAr*{o>uj4beJFhBQgyrTO-@<0 z_x~%lC$f9bx~=qR!BqRLFYcSg`LtHD9g|4zF+3(Ae6_Aq@#uC5NB@xeotYgs7KG(H z`Mz%5;`*kbXW_Ej%BF&^Ws+CC{Nlm9Gp*mzzu)!5(K}WFm*ouvv!_($uE^x+klA;3 zBb({ghi4aPe^0CTFexwfkW%7f7nvPkEOYn7!^eUvoqdasxa1VRcJP(?H9NL1d4;6E zNAx@1<7`*9sJrmlCkkFES*GH)&OY^majNeVsXIU;ywZyDEy@-yGgUA8!MF7#Q`W9x zrSj5ahHbN+&N(5jT+C+r_i9b)X18@)lenj>Fx>b0Tv&=-e9@iyHASC!x9(^u-ourj z|NV7n-NEoryBgi4_lp|+JH%e8zoezPM&wf~|D+nRPd}P>JWXd@>D;-fFS*6q-UtDkFc=n{QWzDI;;&59}moH}=&)OOM>ANkL6_fF!<+h~P zkqdG{C7t^zOB)Bdvo-JXqMT+hqm1d>eYC8 z?tkZrI&?T*`(~)vt=Y_1bpGvF&3Q$}>2SLLr@YIRZR(W`{tMN9Jzlp?`(H{*@KfeL zyB_(wT|57gL4M!jZ#&;=*EQVFv^%i5_(8r~br<)G!jmtTA8~v!zb^6sB*`TEeBH-< jLsMSy+@_bHJk8+g>gTe~DWM6-oOr~KiMe*N2;&I=ITOCu diff --git a/font-test-data/test_data/ttx/embedded_bitmaps.ttx b/font-test-data/test_data/ttx/embedded_bitmaps.ttx index 9cd11bea7..06a4b2696 100644 --- a/font-test-data/test_data/ttx/embedded_bitmaps.ttx +++ b/font-test-data/test_data/ttx/embedded_bitmaps.ttx @@ -24,10 +24,10 @@ - - - - + + + + @@ -38,7 +38,7 @@
- + @@ -50,7 +50,7 @@ eeaeea - + @@ -61,22 +61,10 @@ f0f0f0f0 - - - - - - - - - - - abababab - - + aabbccdd 00112233 ffee1234 424242aa 88990011 @@ -128,8 +116,8 @@ - - + + @@ -184,7 +172,7 @@ - + @@ -192,7 +180,7 @@
- + @@ -201,62 +189,7 @@ - 89504e47 0d0a1a0a 0000000d 49484452 - 00000088 00000080 08030000 00e737d1 - 0d000000 8a504c54 4547704c 5c5c5c75 - 75756d6d 6d727272 7474746a 69696c6c - 6c696969 6f6f6f6b 6b6b6968 68737272 - 56555570 70706d6d 6d6b6b6b 61606068 - 68686666 66646363 64636354 53535b59 - 59616060 52515154 5353605f 5f5e5e5e - 5d5d5d5b 5b5b5150 505a5959 51505052 - 51515756 56515050 51505052 51515453 - 534f4e4e 4f4e4e51 50504f4e 4e4f4e4e - 4f4e4eb5 c8e4e900 00002e74 524e5300 - 208040eb ff511070 a3c38fff 30ffffff - 80ffffea ff60bfff af9fffff ffff70ff - 10cfff40 ef8fff80 bfff50ff df66dbac - 80000002 6b494441 547801ed d8878eea - 400c85e1 13c2d27b 872dd909 1d26efff - 78b76107 4b9adb22 e2a0953f b5e8df36 - 5e4c8579 06c61863 8c31c618 13d57e89 - 21c5b718 414f547f b96988d8 a0566f42 - 4deb85b4 45ec708c a1a6db23 3511eb1c - 63a8e9f4 4803b966 8f414f8f 45c8c5dc - 3a50d31f 30116b94 7a6da869 0c881cbe - cdb10635 b5e1cda0 2bf76648 34777548 - 1a220e07 149b5033 1cddc8e1 e311a943 - 4d73c444 6c71eb40 4d1cfa9b 5d8e63a8 - 194f485b ee0dc706 d4b42744 0e3f6111 - d474a624 46ae3f65 d033654d e41adc34 - 77754a66 726f3876 a1a63527 72f80eb5 - e9026aba 73227775 c631869a e59cc817 - 23730635 fdd57c75 b346aeb5 620b94a8 - 31beebce 56643e5f 72dcac72 f30db5b8 - 8473bc0a 6f6ff7eb 557ec157 f2ba8147 - 1bbf1532 fec20779 2fa48483 7c14f285 - 0f927c16 f2f883b8 e56701a9 4389a274 - cb960e64 b7cf63e2 a064b765 1172074a - 9f29a07e 903de441 c8016a92 23598ab8 - e4b8809a cd919c44 dc73dc41 cd39307c - 7464d073 617de476 dc52a8e9 5f988827 - 6e1ba8b9 7a22875f 733c41cd c913397c - ca517357 3d4944f4 cc41cd3e 30fc8edb - 1e6a5c68 f884db19 4ac4f099 88872a77 - 550e7fd6 3f88cb3c eb83f5bd ce4db338 - df653eb7 3f337f97 715b9470 0e5fc8a2 - 84ad28e4 6407b183 a81f24f1 85247834 - b7f1059c 1d4a9478 9645206e edd9c641 - 4b1af8c7 f79feec5 48f6ac2f 46ecc5c8 - 156a36a1 e13d8ba0 260d0cdf f70c5ac2 - c35f2bd9 5516da9b 03d42cfe b8ab09d4 - 1c42c367 4ff26ed3 79862779 804fa126 - fae3aeae 2bfe6464 53e503fc bae24f46 - d6cff100 1fbea3f6 b9ed2b7e 80df693e - c0b373e0 6f467b8a 0be871c9 e9a7c441 - e89f7e59 c018638c 31c61863 4cf5be03 - d8291f21 ceb2e953 00000000 49454e44 - ae426082 + 89504e47 0d0a1a0a @@ -305,7 +238,7 @@ - + diff --git a/read-fonts/src/tables/cbdt.rs b/read-fonts/src/tables/cbdt.rs index 37fb6b792..81b7d13e9 100644 --- a/read-fonts/src/tables/cbdt.rs +++ b/read-fonts/src/tables/cbdt.rs @@ -22,77 +22,22 @@ mod tests { let cbdt = font.cbdt().unwrap(); let size = &cblc.bitmap_sizes()[0]; // Metrics for size at index 0 - assert_eq!(size.hori.ascender(), 101); - assert_eq!(size.hori.descender(), -27); - assert_eq!(size.hori.width_max(), 136); - assert_eq!(size.vert.ascender(), 101); - assert_eq!(size.vert.descender(), -27); - assert_eq!(size.vert.width_max(), 136); - assert_eq!(size.start_glyph_index(), GlyphId::new(4)); - assert_eq!(size.end_glyph_index(), GlyphId::new(4)); - assert_eq!(size.ppem_x(), 109); - assert_eq!(size.ppem_y(), 109); - assert_eq!(size.bit_depth(), 32); + assert!( + size.hori.ascender() == 101 + && size.hori.descender() == -27 + && size.hori.width_max() == 136 + && size.vert.ascender() == 101 + && size.vert.descender() == -27 + && size.vert.width_max() == 136 + && size.start_glyph_index() == GlyphId::new(4) + && size.end_glyph_index() == GlyphId::new(4) + && size.ppem_x() == 109 + && size.ppem_y() == 109 + && size.bit_depth() == 32 + ); let expected: &[(GlyphId, &[u8], SmallGlyphMetrics)] = &[( GlyphId::new(4), - &raw_image_data( - r#"89504e47 0d0a1a0a 0000000d 49484452 - 00000088 00000080 08030000 00e737d1 - 0d000000 8a504c54 4547704c 5c5c5c75 - 75756d6d 6d727272 7474746a 69696c6c - 6c696969 6f6f6f6b 6b6b6968 68737272 - 56555570 70706d6d 6d6b6b6b 61606068 - 68686666 66646363 64636354 53535b59 - 59616060 52515154 5353605f 5f5e5e5e - 5d5d5d5b 5b5b5150 505a5959 51505052 - 51515756 56515050 51505052 51515453 - 534f4e4e 4f4e4e51 50504f4e 4e4f4e4e - 4f4e4eb5 c8e4e900 00002e74 524e5300 - 208040eb ff511070 a3c38fff 30ffffff - 80ffffea ff60bfff af9fffff ffff70ff - 10cfff40 ef8fff80 bfff50ff df66dbac - 80000002 6b494441 547801ed d8878eea - 400c85e1 13c2d27b 872dd909 1d26efff - 78b76107 4b9adb22 e2a0953f b5e8df36 - 5e4c8579 06c61863 8c31c618 13d57e89 - 21c5b718 414f547f b96988d8 a0566f42 - 4deb85b4 45ec708c a1a6db23 3511eb1c - 63a8e9f4 4803b966 8f414f8f 45c8c5dc - 3a50d31f 30116b94 7a6da869 0c881cbe - cdb10635 b5e1cda0 2bf76648 34777548 - 1a220e07 149b5033 1cddc8e1 e311a943 - 4d73c444 6c71eb40 4d1cfa9b 5d8e63a8 - 194f485b ee0dc706 d4b42744 0e3f6111 - d474a624 46ae3f65 d033654d e41adc34 - 77754a66 726f3876 a1a63527 72f80eb5 - e9026aba 73227775 c631869a e59cc817 - 23730635 fdd57c75 b346aeb5 620b94a8 - 31beebce 56643e5f 72dcac72 f30db5b8 - 8473bc0a 6f6ff7eb 557ec157 f2ba8147 - 1bbf1532 fec20779 2fa48483 7c14f285 - 0f927c16 f2f883b8 e56701a9 4389a274 - cb960e64 b7cf63e2 a064b765 1172074a - 9f29a07e 903de441 c8016a92 23598ab8 - e4b8809a cd919c44 dc73dc41 cd39307c - 7464d073 617de476 dc52a8e9 5f988827 - 6e1ba8b9 7a22875f 733c41cd c913397c - ca517357 3d4944f4 cc41cd3e 30fc8edb - 1e6a5c68 f884db19 4ac4f099 88872a77 - 550e7fd6 3f88cb3c eb83f5bd ce4db338 - df653eb7 3f337f97 715b9470 0e5fc8a2 - 84ad28e4 6407b183 a81f24f1 85247834 - b7f1059c 1d4a9478 9645206e edd9c641 - 4b1af8c7 f79feec5 48f6ac2f 46ecc5c8 - 156a36a1 e13d8ba0 260d0cdf f70c5ac2 - c35f2bd9 5516da9b 03d42cfe b8ab09d4 - 1c42c367 4ff26ed3 79862779 804fa126 - fae3aeae 2bfe6464 53e503fc bae24f46 - d6cff100 1fbea3f6 b9ed2b7e 80df693e - c0b373e0 6f467b8a 0be871c9 e9a7c441 - e89f7e59 c018638c 31c61863 4cf5be03 - d8291f21 ceb2e953 00000000 49454e44 - ae426082"#, - ), + &[0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a], SmallGlyphMetrics { height: 128, width: 136, @@ -103,22 +48,12 @@ mod tests { )]; for (gid, data, metrics) in expected { let location = size.location(cblc.offset_data(), *gid).unwrap(); - // all glyphs have data format == 17 assert_eq!(location.format, 17); let bitmap_data = cbdt.data(&location).unwrap(); let (img_fmt, img_data) = bitmap_data.content.extract_data(); - // all glyphs are PNG assert_eq!(img_fmt, BitmapDataFormat::Png); assert_eq!(img_data, *data); assert_eq!(bitmap_data.extract_small_metrics(), metrics); } } - - fn raw_image_data(raw: &str) -> Vec { - raw.replace([' ', '\n', '\r'], "") - .as_bytes() - .chunks(2) - .map(|str_hex| u8::from_str_radix(std::str::from_utf8(str_hex).unwrap(), 16).unwrap()) - .collect() - } } diff --git a/read-fonts/src/tables/ebdt.rs b/read-fonts/src/tables/ebdt.rs index a6b133167..b0f643b9c 100644 --- a/read-fonts/src/tables/ebdt.rs +++ b/read-fonts/src/tables/ebdt.rs @@ -43,18 +43,20 @@ mod tests { let ebdt = font.ebdt().unwrap(); let size = &eblc.bitmap_sizes()[0]; // Metrics for size at index 0 - assert_eq!(size.hori.ascender(), 6); - assert_eq!(size.hori.descender(), 2); - assert_eq!(size.hori.width_max(), 4); - assert_eq!(size.hori.max_before_bl(), 6); - assert_eq!(size.hori.min_after_bl(), -2); - assert_eq!(size.vert.ascender(), 6); - assert_eq!(size.vert.descender(), 2); - assert_eq!(size.start_glyph_index(), GlyphId::new(1)); - assert_eq!(size.end_glyph_index(), GlyphId::new(2)); - assert_eq!(size.ppem_x(), 7); - assert_eq!(size.ppem_y(), 7); - assert_eq!(size.bit_depth(), 1); + assert!( + size.hori.ascender() == 6 + && size.hori.descender() == 2 + && size.hori.width_max() == 4 + && size.hori.max_before_bl() == 6 + && size.hori.min_after_bl() == -2 + && size.vert.ascender() == 6 + && size.vert.descender() == 2 + && size.start_glyph_index() == GlyphId::new(1) + && size.end_glyph_index() == GlyphId::new(2) + && size.ppem_x() == 7 + && size.ppem_y() == 7 + && size.bit_depth() == 1 + ); // Bit aligned formats in this strike: let expected: &[(GlyphId, &[u8], SmallGlyphMetrics)] = &[ ( @@ -82,11 +84,9 @@ mod tests { ]; for (gid, data, metrics) in expected { let location = size.location(eblc.offset_data(), *gid).unwrap(); - // all glyphs have data format == 2 assert_eq!(location.format, 2); let bitmap_data = ebdt.data(&location).unwrap(); let (img_fmt, img_data) = bitmap_data.content.extract_data(); - // all glyphs are bit aligned assert_eq!(img_fmt, BitmapDataFormat::BitAligned); assert_eq!(img_data, *data); assert_eq!(bitmap_data.extract_small_metrics(), metrics); @@ -100,18 +100,20 @@ mod tests { let ebdt = font.ebdt().unwrap(); let size = &eblc.bitmap_sizes()[1]; // Metrics for size at index 1 - assert_eq!(size.hori.ascender(), 12); - assert_eq!(size.hori.descender(), 5); - assert_eq!(size.hori.width_max(), 9); - assert_eq!(size.hori.max_before_bl(), 12); - assert_eq!(size.hori.min_after_bl(), -5); - assert_eq!(size.vert.ascender(), 12); - assert_eq!(size.vert.descender(), 5); - assert_eq!(size.start_glyph_index(), GlyphId::new(3)); - assert_eq!(size.end_glyph_index(), GlyphId::new(3)); - assert_eq!(size.ppem_x(), 15); - assert_eq!(size.ppem_y(), 15); - assert_eq!(size.bit_depth(), 1); + assert!( + size.hori.ascender() == 12 + && size.hori.descender() == 5 + && size.hori.width_max() == 9 + && size.hori.max_before_bl() == 12 + && size.hori.min_after_bl() == -5 + && size.vert.ascender() == 12 + && size.vert.descender() == 5 + && size.start_glyph_index() == GlyphId::new(3) + && size.end_glyph_index() == GlyphId::new(3) + && size.ppem_x() == 15 + && size.ppem_y() == 15 + && size.bit_depth() == 1 + ); let expected: &[(GlyphId, &[u8])] = &[( GlyphId::new(3), &[ @@ -135,11 +137,9 @@ mod tests { vert_advance: 0, }) ); - // all glyphs have data format == 5 assert_eq!(location.format, 5); let bitmap_data = ebdt.data(&location).unwrap(); let (img_fmt, img_data) = bitmap_data.content.extract_data(); - // all glyphs are bit aligned assert_eq!(img_fmt, BitmapDataFormat::BitAligned); assert_eq!(img_data, *data); }