Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression: After a7bd6d7a4c53ff61 hb_ot_layout_lookup_collect_glyphs fails to collect glyphs when params before, input, after are NULL #657

Closed
drott opened this issue Dec 19, 2017 · 5 comments
Labels
Chrome Chrome/Chromium project related issues and requests

Comments

@drott
Copy link
Collaborator

drott commented Dec 19, 2017

See also https://bugs.chromium.org/p/chromium/issues/detail?id=796124

Patching afshaper as follows:

diff --git a/src/autofit/afshaper.c b/src/autofit/afshaper.c
index 8ee9ba1a..4e03c7bc 100644
--- a/src/autofit/afshaper.c
+++ b/src/autofit/afshaper.c
@@ -108,6 +108,9 @@
     hb_set_t*  gsub_glyphs  = NULL; /* glyphs covered by GSUB lookups  */
     hb_set_t*  gpos_lookups = NULL; /* GPOS lookups for a given script */
     hb_set_t*  gpos_glyphs  = NULL; /* glyphs covered by GPOS lookups  */
+    hb_set_t* dummy_glyphs_1 = NULL;
+    hb_set_t* dummy_glyphs_2 = NULL;
+    hb_set_t* dummy_glyphs_3 = NULL;
 
     hb_script_t      script;
     const hb_tag_t*  coverage_tags;
@@ -183,6 +186,9 @@
 #endif
 
     gsub_glyphs = hb_set_create();
+    dummy_glyphs_1 = hb_set_create();
+    dummy_glyphs_2 = hb_set_create();
+    dummy_glyphs_3 = hb_set_create();
     for ( idx = HB_SET_VALUE_INVALID; hb_set_next( gsub_lookups, &idx ); )
     {
 #ifdef FT_DEBUG_LEVEL_TRACE
@@ -191,12 +197,19 @@
 #endif
 
       /* get output coverage of GSUB feature */
+      /* hb_ot_layout_lookup_collect_glyphs( face, */
+      /*                                     HB_OT_TAG_GSUB, */
+      /*                                     idx, */
+      /*                                     NULL, */
+      /*                                     NULL, */
+      /*                                     NULL, */
+      /*                                     gsub_glyphs ); */
       hb_ot_layout_lookup_collect_glyphs( face,
                                           HB_OT_TAG_GSUB,
                                           idx,
-                                          NULL,
-                                          NULL,
-                                          NULL,
+                                          dummy_glyphs_1,
+                                          dummy_glyphs_2,
+                                          dummy_glyphs_3,
                                           gsub_glyphs );
     }
 

produces a glyph with autohinting, while without this patch, it doesn't. This is after HarfBuzz commit a7bd6d7.

freetype_dummy_sets

This breaks a Noto Devanagari layout test on Chrome as the new result does not have autohinting.

@drott drott added the Chrome Chrome/Chromium project related issues and requests label Dec 19, 2017
@drott
Copy link
Collaborator Author

drott commented Dec 19, 2017

https://gist.github.com/drott/c78215e20bf23648f11a05ff3850a13c was used for testing, building against libfreetype_harfbuzz.so as part of the Chromium checkout.

@behdad
Copy link
Member

behdad commented Dec 19, 2017

Thanks. Let me check.

@behdad behdad closed this as completed in 2fe5f88 Dec 19, 2017
@drott
Copy link
Collaborator Author

drott commented Dec 20, 2017

Thanks very much, locally verified fixed. Roll CL up in https://chromium-review.googlesource.com/c/chromium/src/+/831946

@drott
Copy link
Collaborator Author

drott commented Dec 20, 2017

@behdad
Copy link
Member

behdad commented Dec 20, 2017

Great. Thanks. Released 1.7.4 as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Chrome Chrome/Chromium project related issues and requests
Projects
None yet
Development

No branches or pull requests

2 participants