From 7610448bfda33c77207780087e65e297ecff96c5 Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Tue, 28 Jan 2025 16:06:53 +0200 Subject: [PATCH 1/4] [arabic_high_hamza] Check the correct code point It should be checking U+0674 ARABIC LETTER HIGH HAMZA and not U+0675 ARABIC LETTER HIGH HAMZA ALEF. Fixes https://github.com/fonttools/fontbakery/issues/4290 --- CHANGELOG.md | 1 + Lib/fontbakery/checks/arabic_high_hamza.py | 12 ++++++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a81ca1dfc9..5d1202eeca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ A more detailed list of changes is available in the corresponding milestones for ### Changes to existing checks ### On the Universal profile. - **[no_debugging_tables]** and **[vttclean]** are now merged into unwanted_tables. (issues #4972 and #4975) + - **[arabic_high_hamza]** now checks U+0674 ARABIC LETTER HIGH HAMZA and not U+0675 ARABIC LETTER HIGH HAMZA ALEF. (issue #4290) ### On the OpenType Profile - **[opentype/slant_direction]:** SKIP instead of ERROR if a font does not contain 'H' (PR #4969) diff --git a/Lib/fontbakery/checks/arabic_high_hamza.py b/Lib/fontbakery/checks/arabic_high_hamza.py index 9ee952e274..1522f3dfe7 100644 --- a/Lib/fontbakery/checks/arabic_high_hamza.py +++ b/Lib/fontbakery/checks/arabic_high_hamza.py @@ -8,10 +8,10 @@ "https://github.com/googlefonts/fontbakery/issues/4290", ], rationale=""" - Many fonts incorrectly treat ARABIC LETTER HIGH HAMZA (U+0675) as a variant of + Many fonts incorrectly treat ARABIC LETTER HIGH HAMZA (U+0674) as a variant of ARABIC HAMZA ABOVE (U+0654) and make it a combining mark of the same size. - But U+0675 is a base letter and should be a variant of ARABIC LETTER HAMZA + But U+0674 is a base letter and should be a variant of ARABIC LETTER HAMZA (U+0621) but raised slightly above baseline. Not doing so effectively makes the font useless for Jawi and @@ -20,7 +20,7 @@ severity=4, ) def check_arabic_high_hamza(ttFont): - """Check that glyph for U+0675 ARABIC LETTER HIGH HAMZA is not a mark.""" + """Check that glyph for U+0674 ARABIC LETTER HIGH HAMZA is not a mark.""" from fontTools.pens.areaPen import AreaPen from copy import deepcopy @@ -30,13 +30,13 @@ def check_arabic_high_hamza(ttFont): ttFont_copy = deepcopy(ttFont) ARABIC_LETTER_HAMZA = 0x0621 - ARABIC_LETTER_HIGH_HAMZA = 0x0675 + ARABIC_LETTER_HIGH_HAMZA = 0x0674 cmap = ttFont_copy.getBestCmap() if ARABIC_LETTER_HAMZA not in cmap or ARABIC_LETTER_HIGH_HAMZA not in cmap: yield SKIP, Message( "glyphs-missing", - "This check will only run on fonts that have both glyphs U+0621 and U+0675", + "This check will only run on fonts that have both glyphs U+0621 and U+0674", ) return @@ -68,7 +68,7 @@ def check_arabic_high_hamza(ttFont): if abs((high_hamza_area - hamza_area) / hamza_area) > 0.1: yield FAIL, Message( "glyph-area", - "The arabic letter high hamza (U+0675) should have roughly" + "The arabic letter high hamza (U+0674) should have roughly" " the same size the arabic letter hamza (U+0621)," " but a different glyph outline area was detected.", ) From 633a7229cdb1b096ea3bd9d7b2ef6e8eaff9e888 Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Tue, 28 Jan 2025 16:16:10 +0200 Subject: [PATCH 2/4] =?UTF-8?q?[arabic=5Fhigh=5Fhamza]=20Check=20high=20ha?= =?UTF-8?q?mza=20isn=E2=80=99t=20mark=20even=20if=20hamza=20is=20missing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This part of the check doesn’t depend on the presence of regular hamza. --- Lib/fontbakery/checks/arabic_high_hamza.py | 31 +++++++++++++--------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/Lib/fontbakery/checks/arabic_high_hamza.py b/Lib/fontbakery/checks/arabic_high_hamza.py index 1522f3dfe7..0bbc25d7b0 100644 --- a/Lib/fontbakery/checks/arabic_high_hamza.py +++ b/Lib/fontbakery/checks/arabic_high_hamza.py @@ -24,26 +24,21 @@ def check_arabic_high_hamza(ttFont): from fontTools.pens.areaPen import AreaPen from copy import deepcopy - # This check modifies the font file with `.draw(pen)` - # so here we'll work with a copy of the object so that we - # do not affect other checks: - ttFont_copy = deepcopy(ttFont) - ARABIC_LETTER_HAMZA = 0x0621 ARABIC_LETTER_HIGH_HAMZA = 0x0674 - cmap = ttFont_copy.getBestCmap() - if ARABIC_LETTER_HAMZA not in cmap or ARABIC_LETTER_HIGH_HAMZA not in cmap: + cmap = ttFont.getBestCmap() + if ARABIC_LETTER_HIGH_HAMZA not in cmap: yield SKIP, Message( "glyphs-missing", - "This check will only run on fonts that have both glyphs U+0621 and U+0674", + "This check will only run on fonts that have U+0674 glyph", ) return - if "GDEF" in ttFont_copy and ttFont_copy["GDEF"].table.GlyphClassDef: - class_def = ttFont_copy["GDEF"].table.GlyphClassDef.classDefs - reverseCmap = ttFont_copy["cmap"].buildReversed() - glyphOrder = ttFont_copy.getGlyphOrder() + if "GDEF" in ttFont and ttFont["GDEF"].table.GlyphClassDef: + class_def = ttFont["GDEF"].table.GlyphClassDef.classDefs + reverseCmap = ttFont["cmap"].buildReversed() + glyphOrder = ttFont.getGlyphOrder() for name in glyphOrder: if ARABIC_LETTER_HIGH_HAMZA in reverseCmap.get(name, set()): if name in class_def and class_def[name] == 3: @@ -52,6 +47,18 @@ def check_arabic_high_hamza(ttFont): f'"{name}" is defined in GDEF as a mark (class 3).', ) + if ARABIC_LETTER_HAMZA not in cmap: + yield SKIP, Message( + "glyphs-missing", + "This check will only run on fonts that have both glyphs U+0621 and U+0674", + ) + return + + # This check modifies the font file with `.draw(pen)` + # so here we'll work with a copy of the object so that we + # do not affect other checks: + ttFont_copy = deepcopy(ttFont) + # Also validate the bounding box of the glyph and compare # it to U+0621 expecting them to have roughly the same size # (within a certain tolerance margin) From 633d83607655f8f5638ae644dfb5c8a6e0884c3f Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Tue, 28 Jan 2025 16:20:29 +0200 Subject: [PATCH 3/4] [arabic_high_hamza] Simplify getting high hamza glyph name --- Lib/fontbakery/checks/arabic_high_hamza.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/Lib/fontbakery/checks/arabic_high_hamza.py b/Lib/fontbakery/checks/arabic_high_hamza.py index 0bbc25d7b0..290292242b 100644 --- a/Lib/fontbakery/checks/arabic_high_hamza.py +++ b/Lib/fontbakery/checks/arabic_high_hamza.py @@ -37,15 +37,12 @@ def check_arabic_high_hamza(ttFont): if "GDEF" in ttFont and ttFont["GDEF"].table.GlyphClassDef: class_def = ttFont["GDEF"].table.GlyphClassDef.classDefs - reverseCmap = ttFont["cmap"].buildReversed() - glyphOrder = ttFont.getGlyphOrder() - for name in glyphOrder: - if ARABIC_LETTER_HIGH_HAMZA in reverseCmap.get(name, set()): - if name in class_def and class_def[name] == 3: - yield FAIL, Message( - "mark-in-gdef", - f'"{name}" is defined in GDEF as a mark (class 3).', - ) + name = get_glyph_name(ttFont, ARABIC_LETTER_HIGH_HAMZA) + if name in class_def and class_def[name] == 3: + yield FAIL, Message( + "mark-in-gdef", + f'"{name}" is defined in GDEF as a mark (class 3).', + ) if ARABIC_LETTER_HAMZA not in cmap: yield SKIP, Message( From 0a5aeba3caf6d6a89ace44742daaad63b444a474 Mon Sep 17 00:00:00 2001 From: Khaled Hosny Date: Tue, 28 Jan 2025 16:25:28 +0200 Subject: [PATCH 4/4] [arabic_high_hamza] Make the size of high hamza a warning It seems to be debatable since high hamza is used also in Kazakh where small high hamza is acceptable. Instead of complicated and possibly error prune language detection, lets make it a warning. --- CHANGELOG.md | 2 +- Lib/fontbakery/checks/arabic_high_hamza.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d1202eeca..1f76a6aa4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ A more detailed list of changes is available in the corresponding milestones for ### Changes to existing checks ### On the Universal profile. - **[no_debugging_tables]** and **[vttclean]** are now merged into unwanted_tables. (issues #4972 and #4975) - - **[arabic_high_hamza]** now checks U+0674 ARABIC LETTER HIGH HAMZA and not U+0675 ARABIC LETTER HIGH HAMZA ALEF. (issue #4290) + - **[arabic_high_hamza]** now checks U+0674 ARABIC LETTER HIGH HAMZA and not U+0675 ARABIC LETTER HIGH HAMZA ALEF, and the size check of _high hamza_ is now a warning instead of a fauilre. (issue #4290) ### On the OpenType Profile - **[opentype/slant_direction]:** SKIP instead of ERROR if a font does not contain 'H' (PR #4969) diff --git a/Lib/fontbakery/checks/arabic_high_hamza.py b/Lib/fontbakery/checks/arabic_high_hamza.py index 290292242b..df9c2cd812 100644 --- a/Lib/fontbakery/checks/arabic_high_hamza.py +++ b/Lib/fontbakery/checks/arabic_high_hamza.py @@ -1,4 +1,4 @@ -from fontbakery.prelude import FAIL, SKIP, Message, check +from fontbakery.prelude import FAIL, SKIP, WARN, Message, check from fontbakery.utils import get_glyph_name @@ -70,9 +70,9 @@ def check_arabic_high_hamza(ttFont): high_hamza_area = area_pen.value if abs((high_hamza_area - hamza_area) / hamza_area) > 0.1: - yield FAIL, Message( + yield WARN, Message( "glyph-area", "The arabic letter high hamza (U+0674) should have roughly" - " the same size the arabic letter hamza (U+0621)," - " but a different glyph outline area was detected.", + " the same size the arabic letter hamza (U+0621) while raised" + " above baseline, but a different glyph outline area was detected.", )