From 187df7d7a9a1d9cd67cb2f72d4d6ed8cae1eed61 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Wed, 10 Oct 2018 17:12:52 +0330 Subject: [PATCH 01/26] [circleci] Add an iOS bot (#1233) --- .circleci/config.yml | 12 ++++ CMakeLists.txt | 136 +++++++++++++++++++++++++++---------------- 2 files changed, 98 insertions(+), 50 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 69520b014..1cf4bc885 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -27,6 +27,17 @@ jobs: # Ignoring assembler complains, https://stackoverflow.com/a/39867021 - run: make 2>&1 | grep -v -e '^/var/folders/*' -e '^[[:space:]]*\.section' -e '^[[:space:]]*\^[[:space:]]*~*' + macos-notest-ios: + macos: + xcode: "10.0.0" + steps: + - checkout + - run: brew update-reset + - run: brew install cmake + # not needed to be a framework but we like to test that also + - run: cmake -DBUILD_FRAMEWORK=ON -H. -Bbuild -GXcode -DHB_IOS=ON + - run: cd build && xcodebuild -sdk iphoneos12.0 -configuration Release build -arch arm64 + distcheck: docker: - image: ubuntu:17.10 @@ -274,6 +285,7 @@ workflows: # macOS - macos-llvm-gcc-4.2 - macos-notest-apple-gcc-i686-4.2 + - macos-notest-ios # both autotools and cmake - distcheck diff --git a/CMakeLists.txt b/CMakeLists.txt index d97f7c269..83ebed7c6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -82,6 +82,16 @@ if (HB_CHECK) endif () endif () +set (HB_DISABLE_SUBSET OFF) +set (HB_DISABLE_TESTS OFF) +option(HB_IOS "Apply iOS specific build flags" OFF) +if (HB_IOS) + # We should fix their issue and enable them + set (HB_DISABLE_SUBSET ON) + set (HB_DISABLE_TESTS ON) + set (HB_HAVE_CORETEXT OFF) +endif () + include_directories(AFTER ${PROJECT_SOURCE_DIR}/src ${PROJECT_BINARY_DIR}/src @@ -359,12 +369,32 @@ if (APPLE AND HB_HAVE_CORETEXT) list(APPEND project_sources ${PROJECT_SOURCE_DIR}/src/hb-coretext.cc) list(APPEND project_headers ${PROJECT_SOURCE_DIR}/src/hb-coretext.h) - find_library(APPLICATION_SERVICES_FRAMEWORK ApplicationServices) - if (APPLICATION_SERVICES_FRAMEWORK) - list(APPEND THIRD_PARTY_LIBS ${APPLICATION_SERVICES_FRAMEWORK}) - endif (APPLICATION_SERVICES_FRAMEWORK) + if (HB_IOS) + find_library(COREFOUNDATION CoreFoundation) + if (COREFOUNDATION) + list(APPEND THIRD_PARTY_LIBS ${COREFOUNDATION}) + endif () + mark_as_advanced(COREFOUNDATION) - mark_as_advanced(APPLICATION_SERVICES_FRAMEWORK) + find_library(CORETEXT CoreText) + if (CORETEXT) + list(APPEND THIRD_PARTY_LIBS ${CORETEXT}) + endif () + mark_as_advanced(CORETEXT) + + find_library(COREGRAPHICS CoreGraphics) + if (COREGRAPHICS) + list(APPEND THIRD_PARTY_LIBS ${COREGRAPHICS}) + endif () + mark_as_advanced(COREGRAPHICS) + else () + find_library(APPLICATION_SERVICES_FRAMEWORK ApplicationServices) + if (APPLICATION_SERVICES_FRAMEWORK) + list(APPEND THIRD_PARTY_LIBS ${APPLICATION_SERVICES_FRAMEWORK}) + endif () + + mark_as_advanced(APPLICATION_SERVICES_FRAMEWORK) + endif () endif () if (WIN32 AND HB_HAVE_UNISCRIBE) @@ -527,12 +557,14 @@ add_library(harfbuzz ${project_sources} ${project_extra_sources} ${project_heade target_link_libraries(harfbuzz ${THIRD_PARTY_LIBS}) ## Define harfbuzz-subset library -add_library(harfbuzz-subset ${subset_project_sources} ${subset_project_headers}) -add_dependencies(harfbuzz-subset harfbuzz) -target_link_libraries(harfbuzz-subset harfbuzz ${THIRD_PARTY_LIBS}) +if (NOT HB_DISABLE_SUBSET) + add_library(harfbuzz-subset ${subset_project_sources} ${subset_project_headers}) + add_dependencies(harfbuzz-subset harfbuzz) + target_link_libraries(harfbuzz-subset harfbuzz ${THIRD_PARTY_LIBS}) -if (BUILD_SHARED_LIBS) - set_target_properties(harfbuzz harfbuzz-subset PROPERTIES VISIBILITY_INLINES_HIDDEN TRUE) + if (BUILD_SHARED_LIBS) + set_target_properties(harfbuzz harfbuzz-subset PROPERTIES VISIBILITY_INLINES_HIDDEN TRUE) + endif () endif () if (UNIX OR MINGW) @@ -549,7 +581,9 @@ if (UNIX OR MINGW) set (CMAKE_CXX_IMPLICIT_LINK_LIBRARIES "m") # libm set (CMAKE_CXX_IMPLICIT_LINK_DIRECTORIES "") set_target_properties(harfbuzz PROPERTIES LINKER_LANGUAGE C) - set_target_properties(harfbuzz-subset PROPERTIES LINKER_LANGUAGE C) + if (NOT HB_DISABLE_SUBSET) + set_target_properties(harfbuzz-subset PROPERTIES LINKER_LANGUAGE C) + endif () # No threadsafe statics as we do it ourselves set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-threadsafe-statics") @@ -828,51 +862,53 @@ if (UNIX AND CMAKE_GENERATOR STREQUAL "Ninja") endif () -## src/ executables -foreach (prog main test test-would-substitute test-size-params test-buffer-serialize hb-ot-tag test-unicode-ranges) - set (prog_name ${prog}) - if (${prog_name} STREQUAL "test") - # test can not be used as a valid executable name on cmake, lets special case it - set (prog_name test-test) - endif () - add_executable(${prog_name} ${PROJECT_SOURCE_DIR}/src/${prog}.cc) - target_link_libraries(${prog_name} harfbuzz ${THIRD_PARTY_LIBS}) -endforeach () -set_target_properties(hb-ot-tag PROPERTIES COMPILE_FLAGS "-DMAIN") +if (NOT HB_DISABLE_TESTS) + ## src/ executables + foreach (prog main test test-would-substitute test-size-params test-buffer-serialize hb-ot-tag test-unicode-ranges) + set (prog_name ${prog}) + if (${prog_name} STREQUAL "test") + # test can not be used as a valid executable name on cmake, lets special case it + set (prog_name test-test) + endif () + add_executable(${prog_name} ${PROJECT_SOURCE_DIR}/src/${prog}.cc) + target_link_libraries(${prog_name} harfbuzz ${THIRD_PARTY_LIBS}) + endforeach () + set_target_properties(hb-ot-tag PROPERTIES COMPILE_FLAGS "-DMAIN") -## Tests -if (UNIX OR MINGW) - if (BUILD_SHARED_LIBS) - # generate harfbuzz.def after build completion - add_custom_command(TARGET harfbuzz POST_BUILD - COMMAND "${PYTHON_EXECUTABLE}" ${PROJECT_SOURCE_DIR}/src/gen-def.py ${PROJECT_BINARY_DIR}/harfbuzz.def ${project_headers} - WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/src) + ## Tests + if (UNIX OR MINGW) + if (BUILD_SHARED_LIBS) + # generate harfbuzz.def after build completion + add_custom_command(TARGET harfbuzz POST_BUILD + COMMAND "${PYTHON_EXECUTABLE}" ${PROJECT_SOURCE_DIR}/src/gen-def.py ${PROJECT_BINARY_DIR}/harfbuzz.def ${project_headers} + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/src) - add_test(NAME check-static-inits.sh - COMMAND ${PROJECT_SOURCE_DIR}/src/check-static-inits.sh - WORKING_DIRECTORY ${PROJECT_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/harfbuzz.dir/src # ugly hack - ) - add_test(NAME check-libstdc++.sh COMMAND ${PROJECT_SOURCE_DIR}/src/check-libstdc++.sh) - add_test(NAME check-symbols.sh COMMAND ${PROJECT_SOURCE_DIR}/src/check-symbols.sh) + add_test(NAME check-static-inits.sh + COMMAND ${PROJECT_SOURCE_DIR}/src/check-static-inits.sh + WORKING_DIRECTORY ${PROJECT_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/harfbuzz.dir/src # ugly hack + ) + add_test(NAME check-libstdc++.sh COMMAND ${PROJECT_SOURCE_DIR}/src/check-libstdc++.sh) + add_test(NAME check-symbols.sh COMMAND ${PROJECT_SOURCE_DIR}/src/check-symbols.sh) + set_tests_properties( + check-static-inits.sh check-libstdc++.sh check-symbols.sh + PROPERTIES + ENVIRONMENT "libs=.;srcdir=${PROJECT_SOURCE_DIR}/src" + SKIP_RETURN_CODE 77) + endif () + + add_test(NAME check-c-linkage-decls.sh COMMAND ./check-c-linkage-decls.sh) + add_test(NAME check-header-guards.sh COMMAND ./check-header-guards.sh) + add_test(NAME check-externs.sh COMMAND ./check-externs.sh) + add_test(NAME check-includes.sh COMMAND ./check-includes.sh) set_tests_properties( - check-static-inits.sh check-libstdc++.sh check-symbols.sh + check-c-linkage-decls.sh check-header-guards.sh check-externs.sh check-includes.sh PROPERTIES - ENVIRONMENT "libs=.;srcdir=${PROJECT_SOURCE_DIR}/src" + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/src SKIP_RETURN_CODE 77) endif () - add_test(NAME check-c-linkage-decls.sh COMMAND ./check-c-linkage-decls.sh) - add_test(NAME check-header-guards.sh COMMAND ./check-header-guards.sh) - add_test(NAME check-externs.sh COMMAND ./check-externs.sh) - add_test(NAME check-includes.sh COMMAND ./check-includes.sh) - set_tests_properties( - check-c-linkage-decls.sh check-header-guards.sh check-externs.sh check-includes.sh - PROPERTIES - WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}/src - SKIP_RETURN_CODE 77) + # Needs to come last so that variables defined above are passed to + # subdirectories. + add_subdirectory(test) endif () - -# Needs to come last so that variables defined above are passed to -# subdirectories. -add_subdirectory(test) From 754cf440bf80ced36461a98a5d4607a700f44fd3 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 10:04:05 -0400 Subject: [PATCH 02/26] Minor --- src/hb-ot-shape-fallback.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/hb-ot-shape-fallback.cc b/src/hb-ot-shape-fallback.cc index 556c34089..62104b3a8 100644 --- a/src/hb-ot-shape-fallback.cc +++ b/src/hb-ot-shape-fallback.cc @@ -460,6 +460,8 @@ _hb_ot_shape_fallback_kern (const hb_ot_shape_plan_t *plan, hb_font_t *font, hb_buffer_t *buffer) { + if (!font->has_glyph_h_kerning_func () && !font->has_glyph_v_kerning_func ()) + return; hb_ot_shape_fallback_kern_driver_t driver (font, buffer); hb_kern_machine_t machine (driver); machine.kern (font, buffer, plan->kern_mask); From e655fd38cf20eefb1c071a52282a4caccb6f08ea Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 10:16:09 -0400 Subject: [PATCH 03/26] Apply TT or fallback kerning when GPOS does not have kern feature Previously we only did if there was no GPOS whatsoever. This applies to Arial, Times New Roman, etc in Win7. Was not kerning before. It is now. --- src/hb-ot-shape.cc | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 6753ceb29..5555327b2 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -52,27 +52,34 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, map.compile (plan.map, coords, num_coords); plan.rtlm_mask = plan.map.get_1_mask (HB_TAG ('r','t','l','m')); + plan.frac_mask = plan.map.get_1_mask (HB_TAG ('f','r','a','c')); plan.numr_mask = plan.map.get_1_mask (HB_TAG ('n','u','m','r')); plan.dnom_mask = plan.map.get_1_mask (HB_TAG ('d','n','o','m')); - - plan.kern_mask = plan.map.get_mask (HB_DIRECTION_IS_HORIZONTAL (plan.props.direction) ? - HB_TAG ('k','e','r','n') : HB_TAG ('v','k','r','n')); - plan.has_frac = plan.frac_mask || (plan.numr_mask && plan.dnom_mask); - plan.kerning_requested = !!plan.kern_mask; - plan.has_gpos_mark = !!plan.map.get_1_mask (HB_TAG ('m','a','r','k')); + /* Decide who provides glyph classes. GDEF or Unicode. */ + plan.fallback_glyph_classes = !hb_ot_layout_has_glyph_classes (face); + + /* Decide who does substitutions. GSUB, morx, or fallback. */ plan.apply_morx = !hb_ot_layout_has_substitution (face) && hb_aat_layout_has_substitution (face); + /* Decide who does positioning. GPOS, kerx, kern, or fallback. */ bool disable_gpos = plan.shaper->gpos_tag && plan.shaper->gpos_tag != plan.map.chosen_script[1]; plan.apply_gpos = !disable_gpos && hb_ot_layout_has_positioning (face); - plan.apply_kern = !plan.apply_gpos && hb_ot_layout_has_kerning (face); - plan.fallback_kerning = !plan.apply_gpos && !plan.apply_kern; + + hb_tag_t kern_tag = HB_DIRECTION_IS_HORIZONTAL (plan.props.direction) ? + HB_TAG ('k','e','r','n') : HB_TAG ('v','k','r','n'); + plan.kern_mask = plan.map.get_mask (kern_tag); + plan.kerning_requested = !!plan.kern_mask; + bool has_gpos_kern = plan.map.get_feature_index (0, kern_tag) != HB_OT_LAYOUT_NO_FEATURE_INDEX; + plan.apply_kern = !has_gpos_kern && hb_ot_layout_has_kerning (face); + plan.fallback_kerning = !has_gpos_kern && !plan.apply_kern; + + plan.has_gpos_mark = !!plan.map.get_1_mask (HB_TAG ('m','a','r','k')); plan.fallback_mark_positioning = !plan.apply_gpos; - plan.fallback_glyph_classes = !hb_ot_layout_has_glyph_classes (face); } From 8d00c39bfc558895c63e22148d88db51cde39164 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 10:18:39 -0400 Subject: [PATCH 04/26] [kern] Minor --- src/hb-ot-shape-fallback.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/hb-ot-shape-fallback.cc b/src/hb-ot-shape-fallback.cc index 62104b3a8..655f45ca4 100644 --- a/src/hb-ot-shape-fallback.cc +++ b/src/hb-ot-shape-fallback.cc @@ -460,7 +460,9 @@ _hb_ot_shape_fallback_kern (const hb_ot_shape_plan_t *plan, hb_font_t *font, hb_buffer_t *buffer) { - if (!font->has_glyph_h_kerning_func () && !font->has_glyph_v_kerning_func ()) + if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction) ? + !font->has_glyph_h_kerning_func () : + !font->has_glyph_v_kerning_func ()) return; hb_ot_shape_fallback_kern_driver_t driver (font, buffer); hb_kern_machine_t machine (driver); From 2091b509e3e3b7fb7315539679fae81da2879280 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 10:41:08 -0400 Subject: [PATCH 05/26] [kerx] Hook up to shaper --- src/hb-aat-layout.cc | 31 ++++++++++++++++++++++++------- src/hb-aat-layout.hh | 3 +++ src/hb-ot-shape.cc | 6 ++++-- src/hb-ot-shape.hh | 1 + 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/hb-aat-layout.cc b/src/hb-aat-layout.cc index 71932e798..3fd4f9f89 100644 --- a/src/hb-aat-layout.cc +++ b/src/hb-aat-layout.cc @@ -54,6 +54,21 @@ _get_morx (hb_face_t *face, hb_blob_t **blob = nullptr) *blob = hb_ot_face_data (face)->morx.get_blob (); return morx; } +static inline const AAT::kerx& +_get_kerx (hb_face_t *face, hb_blob_t **blob = nullptr) +{ + if (unlikely (!hb_ot_shaper_face_data_ensure (face))) + { + if (blob) + *blob = hb_blob_get_empty (); + return Null(AAT::kerx); + } + const AAT::kerx& kerx = *(hb_ot_face_data (face)->kerx.get ()); + if (blob) + *blob = hb_ot_face_data (face)->kerx.get_blob (); + return kerx; +} + hb_bool_t hb_aat_layout_has_substitution (hb_face_t *face) @@ -73,19 +88,21 @@ hb_aat_layout_substitute (hb_ot_shape_plan_t *plan, morx.apply (&c); } + +hb_bool_t +hb_aat_layout_has_positioning (hb_face_t *face) +{ + return _get_kerx (face).has_data (); +} + void hb_aat_layout_position (hb_ot_shape_plan_t *plan, hb_font_t *font, hb_buffer_t *buffer) { -#if 0 hb_blob_t *blob; - const AAT::ankr& ankr = _get_ankr (font->face, &blob); const AAT::kerx& kerx = _get_kerx (font->face, &blob); - const AAT::trak& trak = _get_trak (font->face, &blob); - AAT::hb_aat_apply_context_t c (font, buffer, blob); - kerx.apply (&c, &ankr); - trak.apply (&c); -#endif + AAT::hb_aat_apply_context_t c (plan, font, buffer, blob); + kerx.apply (&c); } diff --git a/src/hb-aat-layout.hh b/src/hb-aat-layout.hh index 8b12833d9..aafc3278f 100644 --- a/src/hb-aat-layout.hh +++ b/src/hb-aat-layout.hh @@ -39,6 +39,9 @@ hb_aat_layout_substitute (hb_ot_shape_plan_t *plan, hb_font_t *font, hb_buffer_t *buffer); +HB_INTERNAL hb_bool_t +hb_aat_layout_has_positioning (hb_face_t *face); + HB_INTERNAL void hb_aat_layout_position (hb_ot_shape_plan_t *plan, hb_font_t *font, diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 5555327b2..8f665d37e 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -69,14 +69,16 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, bool disable_gpos = plan.shaper->gpos_tag && plan.shaper->gpos_tag != plan.map.chosen_script[1]; plan.apply_gpos = !disable_gpos && hb_ot_layout_has_positioning (face); + plan.apply_kerx = !plan.apply_gpos && + hb_aat_layout_has_positioning (face); hb_tag_t kern_tag = HB_DIRECTION_IS_HORIZONTAL (plan.props.direction) ? HB_TAG ('k','e','r','n') : HB_TAG ('v','k','r','n'); plan.kern_mask = plan.map.get_mask (kern_tag); plan.kerning_requested = !!plan.kern_mask; bool has_gpos_kern = plan.map.get_feature_index (0, kern_tag) != HB_OT_LAYOUT_NO_FEATURE_INDEX; - plan.apply_kern = !has_gpos_kern && hb_ot_layout_has_kerning (face); - plan.fallback_kerning = !has_gpos_kern && !plan.apply_kern; + plan.apply_kern = !has_gpos_kern && !plan.apply_kerx && hb_ot_layout_has_kerning (face); + plan.fallback_kerning = !has_gpos_kern && !plan.apply_kerx && !plan.apply_kern; plan.has_gpos_mark = !!plan.map.get_1_mask (HB_TAG ('m','a','r','k')); plan.fallback_mark_positioning = !plan.apply_gpos; diff --git a/src/hb-ot-shape.hh b/src/hb-ot-shape.hh index 43852d8ff..7b76a5d59 100644 --- a/src/hb-ot-shape.hh +++ b/src/hb-ot-shape.hh @@ -51,6 +51,7 @@ struct hb_ot_shape_plan_t bool fallback_mark_positioning : 1; bool apply_morx : 1; + bool apply_kerx : 1; bool apply_kern : 1; bool apply_gpos : 1; From 961ab46b24ca9f3ef42a56398646191f106bf5bd Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 10:42:10 -0400 Subject: [PATCH 06/26] More reshuffle plan compile --- src/hb-ot-shape.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 8f665d37e..0d2a3b96d 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -52,11 +52,18 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, map.compile (plan.map, coords, num_coords); plan.rtlm_mask = plan.map.get_1_mask (HB_TAG ('r','t','l','m')); - plan.frac_mask = plan.map.get_1_mask (HB_TAG ('f','r','a','c')); plan.numr_mask = plan.map.get_1_mask (HB_TAG ('n','u','m','r')); plan.dnom_mask = plan.map.get_1_mask (HB_TAG ('d','n','o','m')); plan.has_frac = plan.frac_mask || (plan.numr_mask && plan.dnom_mask); + hb_tag_t kern_tag = HB_DIRECTION_IS_HORIZONTAL (plan.props.direction) ? + HB_TAG ('k','e','r','n') : HB_TAG ('v','k','r','n'); + plan.kern_mask = plan.map.get_mask (kern_tag); + plan.kerning_requested = !!plan.kern_mask; + + bool has_gpos_kern = plan.map.get_feature_index (0, kern_tag) != HB_OT_LAYOUT_NO_FEATURE_INDEX; + bool disable_gpos = plan.shaper->gpos_tag && + plan.shaper->gpos_tag != plan.map.chosen_script[1]; /* Decide who provides glyph classes. GDEF or Unicode. */ plan.fallback_glyph_classes = !hb_ot_layout_has_glyph_classes (face); @@ -66,17 +73,10 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, hb_aat_layout_has_substitution (face); /* Decide who does positioning. GPOS, kerx, kern, or fallback. */ - bool disable_gpos = plan.shaper->gpos_tag && - plan.shaper->gpos_tag != plan.map.chosen_script[1]; plan.apply_gpos = !disable_gpos && hb_ot_layout_has_positioning (face); plan.apply_kerx = !plan.apply_gpos && hb_aat_layout_has_positioning (face); - hb_tag_t kern_tag = HB_DIRECTION_IS_HORIZONTAL (plan.props.direction) ? - HB_TAG ('k','e','r','n') : HB_TAG ('v','k','r','n'); - plan.kern_mask = plan.map.get_mask (kern_tag); - plan.kerning_requested = !!plan.kern_mask; - bool has_gpos_kern = plan.map.get_feature_index (0, kern_tag) != HB_OT_LAYOUT_NO_FEATURE_INDEX; plan.apply_kern = !has_gpos_kern && !plan.apply_kerx && hb_ot_layout_has_kerning (face); plan.fallback_kerning = !has_gpos_kern && !plan.apply_kerx && !plan.apply_kern; From d1be805e784dfaadf2ce9caa830a3f851fdd67da Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 10:49:45 -0400 Subject: [PATCH 07/26] More rewriting plan compile Hopefully more clear. --- src/hb-ot-shape.cc | 55 ++++++++++++++++++++++++++++++++-------------- src/hb-ot-shape.hh | 1 - 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 0d2a3b96d..936b01fd3 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -59,29 +59,54 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, hb_tag_t kern_tag = HB_DIRECTION_IS_HORIZONTAL (plan.props.direction) ? HB_TAG ('k','e','r','n') : HB_TAG ('v','k','r','n'); plan.kern_mask = plan.map.get_mask (kern_tag); - plan.kerning_requested = !!plan.kern_mask; + bool kerning_requested = !!plan.kern_mask; bool has_gpos_kern = plan.map.get_feature_index (0, kern_tag) != HB_OT_LAYOUT_NO_FEATURE_INDEX; bool disable_gpos = plan.shaper->gpos_tag && plan.shaper->gpos_tag != plan.map.chosen_script[1]; - /* Decide who provides glyph classes. GDEF or Unicode. */ - plan.fallback_glyph_classes = !hb_ot_layout_has_glyph_classes (face); + /* + * Decide who provides glyph classes. GDEF or Unicode. + */ - /* Decide who does substitutions. GSUB, morx, or fallback. */ - plan.apply_morx = !hb_ot_layout_has_substitution (face) && - hb_aat_layout_has_substitution (face); + if (!hb_ot_layout_has_glyph_classes (face)) + plan.fallback_glyph_classes = true; - /* Decide who does positioning. GPOS, kerx, kern, or fallback. */ - plan.apply_gpos = !disable_gpos && hb_ot_layout_has_positioning (face); - plan.apply_kerx = !plan.apply_gpos && - hb_aat_layout_has_positioning (face); + /* + * Decide who does substitutions. GSUB, morx, or fallback. + */ - plan.apply_kern = !has_gpos_kern && !plan.apply_kerx && hb_ot_layout_has_kerning (face); - plan.fallback_kerning = !has_gpos_kern && !plan.apply_kerx && !plan.apply_kern; + if (!hb_ot_layout_has_substitution (face)) + { /* No GSUB. */ + if (hb_aat_layout_has_substitution (face)) + plan.apply_morx = true; + } + + /* + * Decide who does positioning. GPOS, kerx, kern, or fallback. + */ + + if (!disable_gpos && hb_ot_layout_has_positioning (face)) + plan.apply_gpos = true; + else if (hb_aat_layout_has_positioning (face)) + plan.apply_kerx = true; + + if (kerning_requested) + { + if (plan.apply_kerx) + ;/* kerx supercedes kern. */ + else if (!has_gpos_kern) + { + if (hb_ot_layout_has_kerning (face)) + plan.apply_kern = true; + else + plan.fallback_kerning = true; + } + } plan.has_gpos_mark = !!plan.map.get_1_mask (HB_TAG ('m','a','r','k')); - plan.fallback_mark_positioning = !plan.apply_gpos; + if (!plan.apply_gpos) + plan.fallback_mark_positioning = true; } @@ -844,9 +869,7 @@ hb_ot_position (const hb_ot_shape_context_t *c) /* Visual fallback goes here. */ - if (!c->plan->kerning_requested) - ; - else if (c->plan->apply_kern) + if (c->plan->apply_kern) hb_ot_layout_kern (c->font, c->buffer, c->plan->kern_mask); else if (c->plan->fallback_kerning) _hb_ot_shape_fallback_kern (c->plan, c->font, c->buffer); diff --git a/src/hb-ot-shape.hh b/src/hb-ot-shape.hh index 7b76a5d59..fc444b252 100644 --- a/src/hb-ot-shape.hh +++ b/src/hb-ot-shape.hh @@ -44,7 +44,6 @@ struct hb_ot_shape_plan_t hb_mask_t kern_mask; bool has_frac : 1; - bool kerning_requested : 1; bool has_gpos_mark : 1; bool fallback_glyph_classes : 1; bool fallback_kerning : 1; From a03850a3567d532c3a4d7655aa71bfe73dfb0e33 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 10:57:28 -0400 Subject: [PATCH 08/26] Fix GPOS/kern interaction Oops. Was checking for kern feature in GSUB, not GPOS. --- src/hb-ot-shape.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 936b01fd3..21e069387 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -61,7 +61,7 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, plan.kern_mask = plan.map.get_mask (kern_tag); bool kerning_requested = !!plan.kern_mask; - bool has_gpos_kern = plan.map.get_feature_index (0, kern_tag) != HB_OT_LAYOUT_NO_FEATURE_INDEX; + bool has_gpos_kern = plan.map.get_feature_index (1, kern_tag) != HB_OT_LAYOUT_NO_FEATURE_INDEX; bool disable_gpos = plan.shaper->gpos_tag && plan.shaper->gpos_tag != plan.map.chosen_script[1]; From c8f2d9334c0f91ec30f1c7821eb44bb5149bd31c Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 11:36:28 -0400 Subject: [PATCH 09/26] Move code In preparation to move add per-subtable set digests... --- src/hb-ot-layout-gsubgpos.hh | 50 +++++++++++++++++++++++++++++++ src/hb-ot-layout.cc | 57 +++--------------------------------- 2 files changed, 54 insertions(+), 53 deletions(-) diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index bdaf35a95..07744aefb 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -259,6 +259,56 @@ struct hb_add_coverage_context_t : }; +struct hb_get_subtables_context_t : + hb_dispatch_context_t +{ + template + static inline bool apply_to (const void *obj, OT::hb_ot_apply_context_t *c) + { + const Type *typed_obj = (const Type *) obj; + return typed_obj->apply (c); + } + + typedef bool (*hb_apply_func_t) (const void *obj, OT::hb_ot_apply_context_t *c); + + struct hb_applicable_t + { + inline void init (const void *obj_, hb_apply_func_t apply_func_) + { + obj = obj_; + apply_func = apply_func_; + } + + inline bool apply (OT::hb_ot_apply_context_t *c) const { return apply_func (obj, c); } + + private: + const void *obj; + hb_apply_func_t apply_func; + }; + + typedef hb_auto_t > array_t; + + /* Dispatch interface. */ + inline const char *get_name (void) { return "GET_SUBTABLES"; } + template + inline return_t dispatch (const T &obj) + { + hb_applicable_t *entry = array.push(); + entry->init (&obj, apply_to); + return HB_VOID; + } + static return_t default_return_value (void) { return HB_VOID; } + bool stop_sublookup_iteration (return_t r HB_UNUSED) const { return false; } + + hb_get_subtables_context_t (array_t &array_) : + array (array_), + debug_depth (0) {} + + array_t &array; + unsigned int debug_depth; +}; + + struct hb_ot_apply_context_t : hb_dispatch_context_t { diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index 975b7f8d4..fc83873ce 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -1112,59 +1112,10 @@ struct GPOSProxy }; -struct hb_get_subtables_context_t : - hb_dispatch_context_t -{ - template - static inline bool apply_to (const void *obj, OT::hb_ot_apply_context_t *c) - { - const Type *typed_obj = (const Type *) obj; - return typed_obj->apply (c); - } - - typedef bool (*hb_apply_func_t) (const void *obj, OT::hb_ot_apply_context_t *c); - - struct hb_applicable_t - { - inline void init (const void *obj_, hb_apply_func_t apply_func_) - { - obj = obj_; - apply_func = apply_func_; - } - - inline bool apply (OT::hb_ot_apply_context_t *c) const { return apply_func (obj, c); } - - private: - const void *obj; - hb_apply_func_t apply_func; - }; - - typedef hb_auto_t > array_t; - - /* Dispatch interface. */ - inline const char *get_name (void) { return "GET_SUBTABLES"; } - template - inline return_t dispatch (const T &obj) - { - hb_applicable_t *entry = array.push(); - entry->init (&obj, apply_to); - return HB_VOID; - } - static return_t default_return_value (void) { return HB_VOID; } - bool stop_sublookup_iteration (return_t r HB_UNUSED) const { return false; } - - hb_get_subtables_context_t (array_t &array_) : - array (array_), - debug_depth (0) {} - - array_t &array; - unsigned int debug_depth; -}; - static inline bool apply_forward (OT::hb_ot_apply_context_t *c, const hb_ot_layout_lookup_accelerator_t &accel, - const hb_get_subtables_context_t::array_t &subtables) + const OT::hb_get_subtables_context_t::array_t &subtables) { bool ret = false; hb_buffer_t *buffer = c->buffer; @@ -1194,7 +1145,7 @@ apply_forward (OT::hb_ot_apply_context_t *c, static inline bool apply_backward (OT::hb_ot_apply_context_t *c, const hb_ot_layout_lookup_accelerator_t &accel, - const hb_get_subtables_context_t::array_t &subtables) + const OT::hb_get_subtables_context_t::array_t &subtables) { bool ret = false; hb_buffer_t *buffer = c->buffer; @@ -1232,8 +1183,8 @@ apply_string (OT::hb_ot_apply_context_t *c, c->set_lookup_props (lookup.get_props ()); - hb_get_subtables_context_t::array_t subtables; - hb_get_subtables_context_t c_get_subtables (subtables); + OT::hb_get_subtables_context_t::array_t subtables; + OT::hb_get_subtables_context_t c_get_subtables (subtables); lookup.dispatch (&c_get_subtables); if (likely (!lookup.is_reverse ())) From 97e5913d5ac2cd313fb3923e9602358d7f75f11d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 11:41:05 -0400 Subject: [PATCH 10/26] Move more code --- src/hb-ot-layout-gsubgpos.hh | 17 +++++++++++++++++ src/hb-ot-layout.cc | 12 ++++++------ src/hb-ot-layout.hh | 20 ++------------------ src/hb-ot-shape-complex-arabic-fallback.hh | 2 +- 4 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index 07744aefb..a11d5dcc5 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -2612,6 +2612,23 @@ struct Extension * GSUB/GPOS Common */ +struct hb_ot_layout_lookup_accelerator_t +{ + template + inline void init (const TLookup &lookup) + { + digest.init (); + lookup.add_coverage (&digest); + } + inline void fini (void) {} + + inline bool may_have (hb_codepoint_t g) const + { return digest.may_have (g); } + + private: + hb_set_digest_t digest; +}; + struct GSUBGPOS { inline bool has_data (void) const { return version.to_int () != 0; } diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index fc83873ce..572e4cb92 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -1094,7 +1094,7 @@ struct GSUBProxy accels (hb_ot_face_data (face)->GSUB->accels) {} const OT::GSUB &table; - const hb_ot_layout_lookup_accelerator_t *accels; + const OT::hb_ot_layout_lookup_accelerator_t *accels; }; struct GPOSProxy @@ -1108,13 +1108,13 @@ struct GPOSProxy accels (hb_ot_face_data (face)->GPOS->accels) {} const OT::GPOS &table; - const hb_ot_layout_lookup_accelerator_t *accels; + const OT::hb_ot_layout_lookup_accelerator_t *accels; }; static inline bool apply_forward (OT::hb_ot_apply_context_t *c, - const hb_ot_layout_lookup_accelerator_t &accel, + const OT::hb_ot_layout_lookup_accelerator_t &accel, const OT::hb_get_subtables_context_t::array_t &subtables) { bool ret = false; @@ -1144,7 +1144,7 @@ apply_forward (OT::hb_ot_apply_context_t *c, static inline bool apply_backward (OT::hb_ot_apply_context_t *c, - const hb_ot_layout_lookup_accelerator_t &accel, + const OT::hb_ot_layout_lookup_accelerator_t &accel, const OT::hb_get_subtables_context_t::array_t &subtables) { bool ret = false; @@ -1174,7 +1174,7 @@ template static inline void apply_string (OT::hb_ot_apply_context_t *c, const typename Proxy::Lookup &lookup, - const hb_ot_layout_lookup_accelerator_t &accel) + const OT::hb_ot_layout_lookup_accelerator_t &accel) { hb_buffer_t *buffer = c->buffer; @@ -1270,7 +1270,7 @@ void hb_ot_map_t::position (const hb_ot_shape_plan_t *plan, hb_font_t *font, hb_ void hb_ot_layout_substitute_lookup (OT::hb_ot_apply_context_t *c, const OT::SubstLookup &lookup, - const hb_ot_layout_lookup_accelerator_t &accel) + const OT::hb_ot_layout_lookup_accelerator_t &accel) { apply_string (c, lookup, accel); } diff --git a/src/hb-ot-layout.hh b/src/hb-ot-layout.hh index f5ffe3112..64b3d7480 100644 --- a/src/hb-ot-layout.hh +++ b/src/hb-ot-layout.hh @@ -112,32 +112,16 @@ hb_ot_layout_substitute_start (hb_font_t *font, hb_buffer_t *buffer); -struct hb_ot_layout_lookup_accelerator_t -{ - template - inline void init (const TLookup &lookup) - { - digest.init (); - lookup.add_coverage (&digest); - } - inline void fini (void) {} - - inline bool may_have (hb_codepoint_t g) const - { return digest.may_have (g); } - - private: - hb_set_digest_t digest; -}; - namespace OT { struct hb_ot_apply_context_t; struct SubstLookup; + struct hb_ot_layout_lookup_accelerator_t; } HB_INTERNAL void hb_ot_layout_substitute_lookup (OT::hb_ot_apply_context_t *c, const OT::SubstLookup &lookup, - const hb_ot_layout_lookup_accelerator_t &accel); + const OT::hb_ot_layout_lookup_accelerator_t &accel); /* Should be called before all the position_lookup's are done. */ diff --git a/src/hb-ot-shape-complex-arabic-fallback.hh b/src/hb-ot-shape-complex-arabic-fallback.hh index 0ef60f643..2aa036728 100644 --- a/src/hb-ot-shape-complex-arabic-fallback.hh +++ b/src/hb-ot-shape-complex-arabic-fallback.hh @@ -201,7 +201,7 @@ struct arabic_fallback_plan_t hb_mask_t mask_array[ARABIC_FALLBACK_MAX_LOOKUPS]; OT::SubstLookup *lookup_array[ARABIC_FALLBACK_MAX_LOOKUPS]; - hb_ot_layout_lookup_accelerator_t accel_array[ARABIC_FALLBACK_MAX_LOOKUPS]; + OT::hb_ot_layout_lookup_accelerator_t accel_array[ARABIC_FALLBACK_MAX_LOOKUPS]; }; #if (defined(_WIN32) || defined(__CYGWIN__)) && !defined(HB_NO_WIN1256) From 78c09bf21335a0f2b538b37de6647af08e3b1161 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 11:50:46 -0400 Subject: [PATCH 11/26] Move subtable array into lookup accel --- src/hb-null.hh | 2 +- src/hb-ot-layout-gsubgpos.hh | 14 +++++++++++--- src/hb-ot-layout.cc | 16 ++++++---------- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/hb-null.hh b/src/hb-null.hh index 88c1c9cb6..2509296fb 100644 --- a/src/hb-null.hh +++ b/src/hb-null.hh @@ -36,7 +36,7 @@ /* Global nul-content Null pool. Enlarge as necessary. */ -#define HB_NULL_POOL_SIZE 264 +#define HB_NULL_POOL_SIZE 512 extern HB_INTERNAL hb_vector_size_impl_t const _hb_NullPool[(HB_NULL_POOL_SIZE + sizeof (hb_vector_size_impl_t) - 1) / sizeof (hb_vector_size_impl_t)]; diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index a11d5dcc5..21a45d50e 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -286,7 +286,7 @@ struct hb_get_subtables_context_t : hb_apply_func_t apply_func; }; - typedef hb_auto_t > array_t; + typedef hb_vector_t array_t; /* Dispatch interface. */ inline const char *get_name (void) { return "GET_SUBTABLES"; } @@ -2619,14 +2619,22 @@ struct hb_ot_layout_lookup_accelerator_t { digest.init (); lookup.add_coverage (&digest); + + subtables.init (); + OT::hb_get_subtables_context_t c_get_subtables (subtables); + lookup.dispatch (&c_get_subtables); + } + inline void fini (void) + { + subtables.fini (); } - inline void fini (void) {} inline bool may_have (hb_codepoint_t g) const { return digest.may_have (g); } - private: + public: hb_set_digest_t digest; + hb_get_subtables_context_t::array_t subtables; }; struct GSUBGPOS diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index 572e4cb92..adb1da28a 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -1114,11 +1114,11 @@ struct GPOSProxy static inline bool apply_forward (OT::hb_ot_apply_context_t *c, - const OT::hb_ot_layout_lookup_accelerator_t &accel, - const OT::hb_get_subtables_context_t::array_t &subtables) + const OT::hb_ot_layout_lookup_accelerator_t &accel) { bool ret = false; hb_buffer_t *buffer = c->buffer; + const OT::hb_get_subtables_context_t::array_t &subtables = accel.subtables; while (buffer->idx < buffer->len && buffer->successful) { bool applied = false; @@ -1144,11 +1144,11 @@ apply_forward (OT::hb_ot_apply_context_t *c, static inline bool apply_backward (OT::hb_ot_apply_context_t *c, - const OT::hb_ot_layout_lookup_accelerator_t &accel, - const OT::hb_get_subtables_context_t::array_t &subtables) + const OT::hb_ot_layout_lookup_accelerator_t &accel) { bool ret = false; hb_buffer_t *buffer = c->buffer; + const OT::hb_get_subtables_context_t::array_t &subtables = accel.subtables; do { if (accel.may_have (buffer->cur().codepoint) && @@ -1183,10 +1183,6 @@ apply_string (OT::hb_ot_apply_context_t *c, c->set_lookup_props (lookup.get_props ()); - OT::hb_get_subtables_context_t::array_t subtables; - OT::hb_get_subtables_context_t c_get_subtables (subtables); - lookup.dispatch (&c_get_subtables); - if (likely (!lookup.is_reverse ())) { /* in/out forward substitution/positioning */ @@ -1195,7 +1191,7 @@ apply_string (OT::hb_ot_apply_context_t *c, buffer->idx = 0; bool ret; - ret = apply_forward (c, accel, subtables); + ret = apply_forward (c, accel); if (ret) { if (!Proxy::inplace) @@ -1211,7 +1207,7 @@ apply_string (OT::hb_ot_apply_context_t *c, buffer->remove_output (); buffer->idx = buffer->len - 1; - apply_backward (c, accel, subtables); + apply_backward (c, accel); } } From e78549edfb4df617128a5f5ddd12692f1d0af4bf Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 11:54:48 -0400 Subject: [PATCH 12/26] Move apply down into subtables accel --- src/hb-ot-layout-gsubgpos.hh | 10 +++++++++- src/hb-ot-layout.cc | 17 +++-------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index 21a45d50e..0ddadcf6b 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -2632,7 +2632,15 @@ struct hb_ot_layout_lookup_accelerator_t inline bool may_have (hb_codepoint_t g) const { return digest.may_have (g); } - public: + inline bool apply (hb_ot_apply_context_t *c) const + { + for (unsigned int i = 0; i < subtables.len; i++) + if (subtables[i].apply (c)) + return true; + return false; + } + + private: hb_set_digest_t digest; hb_get_subtables_context_t::array_t subtables; }; diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc index adb1da28a..4908076e8 100644 --- a/src/hb-ot-layout.cc +++ b/src/hb-ot-layout.cc @@ -1118,7 +1118,6 @@ apply_forward (OT::hb_ot_apply_context_t *c, { bool ret = false; hb_buffer_t *buffer = c->buffer; - const OT::hb_get_subtables_context_t::array_t &subtables = accel.subtables; while (buffer->idx < buffer->len && buffer->successful) { bool applied = false; @@ -1126,12 +1125,7 @@ apply_forward (OT::hb_ot_apply_context_t *c, (buffer->cur().mask & c->lookup_mask) && c->check_glyph_property (&buffer->cur(), c->lookup_props)) { - for (unsigned int i = 0; i < subtables.len; i++) - if (subtables[i].apply (c)) - { - applied = true; - break; - } + applied = accel.apply (c); } if (applied) @@ -1148,19 +1142,14 @@ apply_backward (OT::hb_ot_apply_context_t *c, { bool ret = false; hb_buffer_t *buffer = c->buffer; - const OT::hb_get_subtables_context_t::array_t &subtables = accel.subtables; do { if (accel.may_have (buffer->cur().codepoint) && (buffer->cur().mask & c->lookup_mask) && c->check_glyph_property (&buffer->cur(), c->lookup_props)) { - for (unsigned int i = 0; i < subtables.len; i++) - if (subtables[i].apply (c)) - { - ret = true; - break; - } + if (accel.apply (c)) + ret = true; } /* The reverse lookup doesn't "advance" cursor (for good reason). */ buffer->idx--; From b3390990f508def9c375716614b92fc7b0038228 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 12:07:49 -0400 Subject: [PATCH 13/26] Add per-subtable set-digests This speeds up Roboto shaping by ~10%. I was hoping for more. Still, good defense against lookups with many subtables. --- src/hb-null.hh | 2 +- src/hb-ot-layout-gsubgpos.hh | 108 +++++++++++++++++++---------------- 2 files changed, 59 insertions(+), 51 deletions(-) diff --git a/src/hb-null.hh b/src/hb-null.hh index 2509296fb..bf01b3af7 100644 --- a/src/hb-null.hh +++ b/src/hb-null.hh @@ -36,7 +36,7 @@ /* Global nul-content Null pool. Enlarge as necessary. */ -#define HB_NULL_POOL_SIZE 512 +#define HB_NULL_POOL_SIZE 1024 extern HB_INTERNAL hb_vector_size_impl_t const _hb_NullPool[(HB_NULL_POOL_SIZE + sizeof (hb_vector_size_impl_t) - 1) / sizeof (hb_vector_size_impl_t)]; diff --git a/src/hb-ot-layout-gsubgpos.hh b/src/hb-ot-layout-gsubgpos.hh index 0ddadcf6b..3a09803ee 100644 --- a/src/hb-ot-layout-gsubgpos.hh +++ b/src/hb-ot-layout-gsubgpos.hh @@ -259,56 +259,6 @@ struct hb_add_coverage_context_t : }; -struct hb_get_subtables_context_t : - hb_dispatch_context_t -{ - template - static inline bool apply_to (const void *obj, OT::hb_ot_apply_context_t *c) - { - const Type *typed_obj = (const Type *) obj; - return typed_obj->apply (c); - } - - typedef bool (*hb_apply_func_t) (const void *obj, OT::hb_ot_apply_context_t *c); - - struct hb_applicable_t - { - inline void init (const void *obj_, hb_apply_func_t apply_func_) - { - obj = obj_; - apply_func = apply_func_; - } - - inline bool apply (OT::hb_ot_apply_context_t *c) const { return apply_func (obj, c); } - - private: - const void *obj; - hb_apply_func_t apply_func; - }; - - typedef hb_vector_t array_t; - - /* Dispatch interface. */ - inline const char *get_name (void) { return "GET_SUBTABLES"; } - template - inline return_t dispatch (const T &obj) - { - hb_applicable_t *entry = array.push(); - entry->init (&obj, apply_to); - return HB_VOID; - } - static return_t default_return_value (void) { return HB_VOID; } - bool stop_sublookup_iteration (return_t r HB_UNUSED) const { return false; } - - hb_get_subtables_context_t (array_t &array_) : - array (array_), - debug_depth (0) {} - - array_t &array; - unsigned int debug_depth; -}; - - struct hb_ot_apply_context_t : hb_dispatch_context_t { @@ -671,6 +621,64 @@ struct hb_ot_apply_context_t : }; +struct hb_get_subtables_context_t : + hb_dispatch_context_t +{ + template + static inline bool apply_to (const void *obj, OT::hb_ot_apply_context_t *c) + { + const Type *typed_obj = (const Type *) obj; + return typed_obj->apply (c); + } + + typedef bool (*hb_apply_func_t) (const void *obj, OT::hb_ot_apply_context_t *c); + + struct hb_applicable_t + { + template + inline void init (const T &obj_, hb_apply_func_t apply_func_) + { + obj = &obj_; + apply_func = apply_func_; + digest.init (); + obj_.get_coverage ().add_coverage (&digest); + } + + inline bool apply (OT::hb_ot_apply_context_t *c) const + { + return digest.may_have (c->buffer->cur().codepoint) && apply_func (obj, c); + } + + private: + const void *obj; + hb_apply_func_t apply_func; + hb_set_digest_t digest; + }; + + typedef hb_vector_t array_t; + + /* Dispatch interface. */ + inline const char *get_name (void) { return "GET_SUBTABLES"; } + template + inline return_t dispatch (const T &obj) + { + hb_applicable_t *entry = array.push(); + entry->init (obj, apply_to); + return HB_VOID; + } + static return_t default_return_value (void) { return HB_VOID; } + bool stop_sublookup_iteration (return_t r HB_UNUSED) const { return false; } + + hb_get_subtables_context_t (array_t &array_) : + array (array_), + debug_depth (0) {} + + array_t &array; + unsigned int debug_depth; +}; + + + typedef bool (*intersects_func_t) (const hb_set_t *glyphs, const HBUINT16 &value, const void *data); typedef void (*collect_glyphs_func_t) (hb_set_t *glyphs, const HBUINT16 &value, const void *data); From 7727e737566ddc826647e19fc645b296ad5a0cac Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 13:24:51 -0400 Subject: [PATCH 14/26] [kerx] Actually hook up, and fix crash --- src/hb-aat-layout-common.hh | 6 ++++++ src/hb-aat-layout-kerx-table.hh | 4 ++-- src/hb-ot-shape.cc | 4 ++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/hb-aat-layout-common.hh b/src/hb-aat-layout-common.hh index 5845ab516..37ab53539 100644 --- a/src/hb-aat-layout-common.hh +++ b/src/hb-aat-layout-common.hh @@ -260,6 +260,12 @@ struct Lookup } } + inline const T& get_value_or_null (hb_codepoint_t glyph_id, unsigned int num_glyphs) const + { + const T *v = get_value (glyph_id, num_glyphs); + return v ? *v : Null(T); + } + inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index e5934c39f..a4a4dab71 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -109,8 +109,8 @@ struct KerxSubTableFormat2 inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, const char *end, unsigned int num_glyphs) const { - unsigned int l = *(this+leftClassTable).get_value (left, num_glyphs); - unsigned int r = *(this+rightClassTable).get_value (right, num_glyphs); + unsigned int l = (this+leftClassTable).get_value_or_null (left, num_glyphs); + unsigned int r = (this+rightClassTable).get_value_or_null (right, num_glyphs); unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+array), offset); if (unlikely ((const char *) v < (const char *) &array || diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 21e069387..cf808c2a5 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -826,6 +826,8 @@ hb_ot_position_complex (const hb_ot_shape_context_t *c) if (c->plan->apply_gpos) c->plan->position (c->font, c->buffer); + else if (c->plan->apply_kerx) + hb_aat_layout_position (c->plan, c->font, c->buffer); switch (c->plan->shaper->zero_width_marks) { @@ -875,8 +877,6 @@ hb_ot_position (const hb_ot_shape_context_t *c) _hb_ot_shape_fallback_kern (c->plan, c->font, c->buffer); _hb_buffer_deallocate_gsubgpos_vars (c->buffer); - - //hb_aat_layout_position (c->font, c->buffer); } static inline void From 1e8fdd285f90b7b715b6d9ca9222a3c91cbea6b8 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 16:32:35 -0400 Subject: [PATCH 15/26] Remove HAVE_OT We never tested compiling without it. Just kill it. We always build our own shaper. --- CMakeLists.txt | 1 - configure.ac | 6 ------ src/Makefile.am | 2 -- src/hb-shaper-list.hh | 2 -- src/hb.hh | 2 -- test/api/Makefile.am | 4 +--- test/api/test-c.c | 5 +---- util/Makefile.am | 4 ---- util/options.cc | 4 ---- util/options.hh | 2 -- 10 files changed, 2 insertions(+), 30 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 83ebed7c6..760883fda 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -97,7 +97,6 @@ include_directories(AFTER ${PROJECT_BINARY_DIR}/src ) -add_definitions(-DHAVE_OT) add_definitions(-DHAVE_FALLBACK) # We need PYTHON_EXECUTABLE to be set for running the tests... diff --git a/configure.ac b/configure.ac index 3aa41ff21..1b9ddfe7c 100644 --- a/configure.ac +++ b/configure.ac @@ -148,12 +148,6 @@ AM_CONDITIONAL(HAVE_PTHREAD, $have_pthread) dnl ========================================================================== -have_ot=true -if $have_ot; then - AC_DEFINE(HAVE_OT, 1, [Have native OpenType Layout backend]) -fi -AM_CONDITIONAL(HAVE_OT, $have_ot) - have_fallback=true if $have_fallback; then AC_DEFINE(HAVE_FALLBACK, 1, [Have simple TrueType Layout backend]) diff --git a/src/Makefile.am b/src/Makefile.am index 2eca356b4..c4ae2bcb2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -29,11 +29,9 @@ HBSOURCES = $(HB_BASE_sources) HBSOURCES += $(HB_BASE_RAGEL_GENERATED_sources) HBHEADERS = $(HB_BASE_headers) -if HAVE_OT HBSOURCES += $(HB_OT_sources) HBSOURCES += $(HB_OT_RAGEL_GENERATED_sources) HBHEADERS += $(HB_OT_headers) -endif if HAVE_FALLBACK HBSOURCES += $(HB_FALLBACK_sources) diff --git a/src/hb-shaper-list.hh b/src/hb-shaper-list.hh index b0835d31a..1fdb64810 100644 --- a/src/hb-shaper-list.hh +++ b/src/hb-shaper-list.hh @@ -39,9 +39,7 @@ HB_SHAPER_IMPLEMENT (graphite2) HB_SHAPER_IMPLEMENT (coretext_aat) #endif -#ifdef HAVE_OT HB_SHAPER_IMPLEMENT (ot) /* <--- This is our main OpenType shaper. */ -#endif #ifdef HAVE_UNISCRIBE HB_SHAPER_IMPLEMENT (uniscribe) diff --git a/src/hb.hh b/src/hb.hh index 152285221..f37be7ad0 100644 --- a/src/hb.hh +++ b/src/hb.hh @@ -45,10 +45,8 @@ #include "hb.h" #define HB_H_IN -#ifdef HAVE_OT #include "hb-ot.h" #define HB_OT_H_IN -#endif #include #include diff --git a/test/api/Makefile.am b/test/api/Makefile.am index 02e878034..3ff7f5a8a 100644 --- a/test/api/Makefile.am +++ b/test/api/Makefile.am @@ -69,13 +69,12 @@ test_unicode_LDADD += $(top_builddir)/src/libharfbuzz-icu.la $(ICU_LIBS) endif -if HAVE_OT - TEST_PROGS += \ test-ot-color \ test-ot-tag \ $(NULL) + if HAVE_PTHREAD if HAVE_FREETYPE TEST_PROGS += test-multithread @@ -95,7 +94,6 @@ test_ot_math_LDADD = $(LDADD) $(FREETYPE_LIBS) test_ot_math_CPPFLAGS = $(AM_CPPFLAGS) $(FREETYPE_CFLAGS) endif # HAVE_FREETYPE -endif # HAVE_OT # Tests for header compilation TEST_PROGS += \ diff --git a/test/api/test-c.c b/test/api/test-c.c index 78d6e974d..061f35cdc 100644 --- a/test/api/test-c.c +++ b/test/api/test-c.c @@ -32,6 +32,7 @@ #endif #include +#include #ifdef HAVE_GLIB #include @@ -45,10 +46,6 @@ #include #endif -#ifdef HAVE_OT -#include -#endif - #ifdef HAVE_UNISCRIBE #include #endif diff --git a/util/Makefile.am b/util/Makefile.am index b8bf88418..85f9edaa0 100644 --- a/util/Makefile.am +++ b/util/Makefile.am @@ -52,14 +52,11 @@ hb_subset_LDADD = \ $(top_builddir)/src/libharfbuzz-subset.la bin_PROGRAMS += hb-subset -if HAVE_OT hb_ot_shape_closure_SOURCES = $(HB_OT_SHAPE_CLOSURE_sources) bin_PROGRAMS += hb-ot-shape-closure -endif # HAVE_OT endif # HAVE_GLIB -#if HAVE_OT #if HAVE_FONTCONFIG #hb_fc_list_SOURCES = \ # hb-fc.cc \ @@ -72,6 +69,5 @@ endif # HAVE_GLIB # $(NULL) #bin_PROGRAMS += hb-fc-list #endif # HAVE_FONTCONFIG -#endif # HAVE_OT -include $(top_srcdir)/git.mk diff --git a/util/options.cc b/util/options.cc index 090a9c254..26b0bd0a8 100644 --- a/util/options.cc +++ b/util/options.cc @@ -29,9 +29,7 @@ #ifdef HAVE_FREETYPE #include #endif -#ifdef HAVE_OT #include -#endif static struct supported_font_funcs_t { char name[4]; @@ -41,9 +39,7 @@ static struct supported_font_funcs_t { #ifdef HAVE_FREETYPE {"ft", hb_ft_font_set_funcs}, #endif -#ifdef HAVE_OT {"ot", hb_ot_font_set_funcs}, -#endif }; diff --git a/util/options.hh b/util/options.hh index 5088adaba..3749b99be 100644 --- a/util/options.hh +++ b/util/options.hh @@ -46,9 +46,7 @@ #endif #include -#ifdef HAVE_OT #include -#endif #include #include From 44f09afd5bd4f4f1ea47ca54ac9d605219b06910 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 17:32:32 -0400 Subject: [PATCH 16/26] [kerx] Skip variation subtables --- src/hb-aat-layout-kerx-table.hh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index a4a4dab71..005208c60 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -312,21 +312,21 @@ struct kerx { bool reverse; + if (table->coverage & (KerxTable::CrossStream | KerxTable::Variation)) + goto skip; /* We do NOT handle cross-stream or variation kerning. */ + if (HB_DIRECTION_IS_VERTICAL (c->buffer->props.direction) != bool (table->coverage & KerxTable::Vertical)) - goto skip; - - if (table->coverage & KerxTable::CrossStream) - goto skip; /* We do NOT handle cross-stream kerning. None of Apple fonts use it. */ + goto skip; reverse = bool (table->coverage & KerxTable::Backwards) != HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction); if (!c->buffer->message (c->font, "start kerx subtable %d", c->lookup_index)) - goto skip; + goto skip; if (reverse) - c->buffer->reverse (); + c->buffer->reverse (); c->sanitizer.set_object (*table); @@ -337,7 +337,7 @@ struct kerx table->dispatch (c); if (reverse) - c->buffer->reverse (); + c->buffer->reverse (); (void) c->buffer->message (c->font, "end kerx subtable %d", c->lookup_index); From 38a7a8a89ed035a1d1fc34a675a1860ad660b6ff Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 17:44:46 -0400 Subject: [PATCH 17/26] Allow HB_OPTIONS=aat to prefer AAT tables over OT Fixes https://github.com/harfbuzz/harfbuzz/issues/322 --- src/hb-common.cc | 23 +++++++++++++++++++++-- src/hb-debug.hh | 7 ++++--- src/hb-ot-shape.cc | 24 ++++++++++++++++-------- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/src/hb-common.cc b/src/hb-common.cc index 41b1601de..f698a4406 100644 --- a/src/hb-common.cc +++ b/src/hb-common.cc @@ -47,8 +47,27 @@ _hb_options_init (void) u.i = 0; u.opts.initialized = 1; - char *c = getenv ("HB_OPTIONS"); - u.opts.uniscribe_bug_compatible = c && strstr (c, "uniscribe-bug-compatible"); + const char *c = getenv ("HB_OPTIONS"); + if (c) + { + while (*c) + { + const char *p = strchr (c, ':'); + if (!p) + p = c + strlen (c); + +#define OPTION(name, symbol) \ + if (0 == strncmp (c, name, p - c)) u.opts.symbol = true; + + OPTION ("uniscribe-bug-compatible", uniscribe_bug_compatible); + OPTION ("aat", aat); + +#undef OPTION + + c = *p ? p + 1 : p; + } + + } /* This is idempotent and threadsafe. */ _hb_options.set_relaxed (u.i); diff --git a/src/hb-debug.hh b/src/hb-debug.hh index 12b6c49ae..58c190d27 100644 --- a/src/hb-debug.hh +++ b/src/hb-debug.hh @@ -43,9 +43,10 @@ struct hb_options_t { - unsigned int unused : 1; /* In-case sign bit is here. */ - unsigned int initialized : 1; - unsigned int uniscribe_bug_compatible : 1; + bool unused : 1; /* In-case sign bit is here. */ + bool initialized : 1; + bool uniscribe_bug_compatible : 1; + bool aat : 1; }; union hb_options_union_t { diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index cf808c2a5..7a15d5234 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -42,6 +42,17 @@ #include "hb-aat-layout.hh" +static bool +_hb_apply_morx (hb_face_t *face) +{ + if (hb_options ().aat && + hb_aat_layout_has_substitution (face)) + return true; + + return !hb_ot_layout_has_substitution (face) && + hb_aat_layout_has_substitution (face); +} + void hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, const int *coords, @@ -76,17 +87,15 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, * Decide who does substitutions. GSUB, morx, or fallback. */ - if (!hb_ot_layout_has_substitution (face)) - { /* No GSUB. */ - if (hb_aat_layout_has_substitution (face)) - plan.apply_morx = true; - } + plan.apply_morx = _hb_apply_morx (face); /* * Decide who does positioning. GPOS, kerx, kern, or fallback. */ - if (!disable_gpos && hb_ot_layout_has_positioning (face)) + if (hb_options ().aat && hb_aat_layout_has_positioning (face)) + plan.apply_kerx = true; + else if (!disable_gpos && hb_ot_layout_has_positioning (face)) plan.apply_gpos = true; else if (hb_aat_layout_has_positioning (face)) plan.apply_kerx = true; @@ -263,8 +272,7 @@ _hb_ot_shaper_shape_plan_data_create (hb_shape_plan_t *shape_plan, /* Ugly that we have to do this here... * If we are going to apply morx, choose default shaper. */ - if (!hb_ot_layout_has_substitution (planner.face) && - hb_aat_layout_has_substitution (planner.face)) + if (_hb_apply_morx (planner.face)) planner.shaper = &_hb_ot_complex_shaper_default; else planner.shaper = hb_ot_shape_complex_categorize (&planner); From 60f86d32d7c735ccf783b382e18ecdc096eaa682 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 18:10:05 -0400 Subject: [PATCH 18/26] [kerx] Don't loop over kerning subtables if kerning disabled --- src/hb-aat-layout-kerx-table.hh | 12 ++++++++++++ src/hb-ot-shape.cc | 4 ++-- src/hb-ot-shape.hh | 1 + 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 005208c60..f5540b6e9 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -59,6 +59,9 @@ struct KerxSubTableFormat0 { TRACE_APPLY (this); + if (!c->plan->requested_kerning) + return false; + hb_kern_machine_t machine (*this); machine.kern (c->font, c->buffer, c->plan->kern_mask); @@ -85,6 +88,9 @@ struct KerxSubTableFormat1 { TRACE_APPLY (this); + if (!c->plan->requested_kerning) + return false; + /* TODO */ return_trace (true); @@ -123,6 +129,9 @@ struct KerxSubTableFormat2 { TRACE_APPLY (this); + if (!c->plan->requested_kerning) + return false; + accelerator_t accel (*this, c->sanitizer.end, c->face->get_num_glyphs ()); @@ -203,6 +212,9 @@ struct KerxSubTableFormat6 { TRACE_APPLY (this); + if (!c->plan->requested_kerning) + return false; + /* TODO */ return_trace (true); diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc index 7a15d5234..3b1c93db4 100644 --- a/src/hb-ot-shape.cc +++ b/src/hb-ot-shape.cc @@ -71,7 +71,7 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, HB_TAG ('k','e','r','n') : HB_TAG ('v','k','r','n'); plan.kern_mask = plan.map.get_mask (kern_tag); - bool kerning_requested = !!plan.kern_mask; + plan.requested_kerning = !!plan.kern_mask; bool has_gpos_kern = plan.map.get_feature_index (1, kern_tag) != HB_OT_LAYOUT_NO_FEATURE_INDEX; bool disable_gpos = plan.shaper->gpos_tag && plan.shaper->gpos_tag != plan.map.chosen_script[1]; @@ -100,7 +100,7 @@ hb_ot_shape_planner_t::compile (hb_ot_shape_plan_t &plan, else if (hb_aat_layout_has_positioning (face)) plan.apply_kerx = true; - if (kerning_requested) + if (plan.requested_kerning) { if (plan.apply_kerx) ;/* kerx supercedes kern. */ diff --git a/src/hb-ot-shape.hh b/src/hb-ot-shape.hh index fc444b252..4943c5155 100644 --- a/src/hb-ot-shape.hh +++ b/src/hb-ot-shape.hh @@ -43,6 +43,7 @@ struct hb_ot_shape_plan_t hb_mask_t rtlm_mask, frac_mask, numr_mask, dnom_mask; hb_mask_t kern_mask; + bool requested_kerning : 1; bool has_frac : 1; bool has_gpos_mark : 1; bool fallback_glyph_classes : 1; From 5d34164d98f04816aafaa0abfc44cd899c7d70b3 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 18:14:41 -0400 Subject: [PATCH 19/26] [kern/kerx] Fix offset base Disable kern Format2. Fix kerx Format2. Manually tested this with Tamil MN font and it works: $ HB_OPTIONS=aat ./hb-shape Tamil\ MN.ttc -u 0B94,0B95 [tgv_au=0+3435|tgc_ka=1@-75,0+1517] HB_OPTIONS=aat ./hb-shape Tamil\ MN.ttc -u 0B94,0B95 --features=-kern [tgv_au=0+3510|tgc_ka=1+1592] --- src/hb-aat-layout-kerx-table.hh | 73 ++++++++++++++++++++------------- src/hb-ot-kern-table.hh | 7 ++++ 2 files changed, 52 insertions(+), 28 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index f5540b6e9..2214265b5 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -44,6 +44,22 @@ namespace AAT { using namespace OT; +struct KerxSubTableHeader +{ + inline bool sanitize (hb_sanitize_context_t *c) const + { + TRACE_SANITIZE (this); + return_trace (likely (c->check_struct (this))); + } + + public: + HBUINT32 length; + HBUINT32 coverage; + HBUINT32 tupleCount; + public: + DEFINE_SIZE_STATIC (12); +}; + struct KerxSubTableFormat0 { inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right) const @@ -76,10 +92,11 @@ struct KerxSubTableFormat0 } protected: + KerxSubTableHeader header; BinSearchArrayOf - pairs; /* Sorted kern records. */ + pairs; /* Sorted kern records. */ public: - DEFINE_SIZE_ARRAY (16, pairs); + DEFINE_SIZE_ARRAY (28, pairs); }; struct KerxSubTableFormat1 @@ -104,10 +121,11 @@ struct KerxSubTableFormat1 } protected: + KerxSubTableHeader header; StateTable stateHeader; LOffsetTo > valueTable; public: - DEFINE_SIZE_STATIC (20); + DEFINE_SIZE_STATIC (32); }; struct KerxSubTableFormat2 @@ -168,18 +186,18 @@ struct KerxSubTableFormat2 }; protected: - HBUINT32 rowWidth; /* The width, in bytes, of a row in the table. */ + KerxSubTableHeader header; + HBUINT32 rowWidth; /* The width, in bytes, of a row in the table. */ LOffsetTo > - leftClassTable; /* Offset from beginning of this subtable to - * left-hand class table. */ + leftClassTable; /* Offset from beginning of this subtable to + * left-hand class table. */ LOffsetTo > - rightClassTable;/* Offset from beginning of this subtable to - * right-hand class table. */ - LOffsetTo - array; /* Offset from beginning of this subtable to - * the start of the kerning array. */ + rightClassTable;/* Offset from beginning of this subtable to + * right-hand class table. */ + LOffsetTo array; /* Offset from beginning of this subtable to + * the start of the kerning array. */ public: - DEFINE_SIZE_STATIC (16); + DEFINE_SIZE_STATIC (28); }; struct KerxSubTableFormat4 @@ -202,8 +220,9 @@ struct KerxSubTableFormat4 } protected: + KerxSubTableHeader header; public: - DEFINE_SIZE_STATIC (1); + DEFINE_SIZE_STATIC (12); }; struct KerxSubTableFormat6 @@ -231,23 +250,24 @@ struct KerxSubTableFormat6 } protected: - HBUINT32 flags; - HBUINT16 rowCount; - HBUINT16 columnCount; + KerxSubTableHeader header; + HBUINT32 flags; + HBUINT16 rowCount; + HBUINT16 columnCount; LOffsetTo > rowIndexTable; LOffsetTo > columnIndexTable; LOffsetTo > kerningArray; LOffsetTo > kerningVector; public: - DEFINE_SIZE_STATIC (24); + DEFINE_SIZE_STATIC (36); }; struct KerxTable { friend struct kerx; - inline unsigned int get_size (void) const { return length; } - inline unsigned int get_type (void) const { return coverage & SubtableType; } + inline unsigned int get_size (void) const { return u.header.length; } + inline unsigned int get_type (void) const { return u.header.coverage & SubtableType; } enum Coverage { @@ -281,19 +301,16 @@ struct KerxTable inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); - if (!length.sanitize (c) || - length < min_size || - !c->check_range (this, length)) + if (!u.header.sanitize (c) || + !c->check_range (this, u.header.length)) return_trace (false); return_trace (dispatch (c)); } protected: - HBUINT32 length; - HBUINT32 coverage; - HBUINT32 tupleCount; union { + KerxSubTableHeader header; KerxSubTableFormat0 format0; KerxSubTableFormat1 format1; KerxSubTableFormat2 format2; @@ -324,14 +341,14 @@ struct kerx { bool reverse; - if (table->coverage & (KerxTable::CrossStream | KerxTable::Variation)) + if (table->u.header.coverage & (KerxTable::CrossStream | KerxTable::Variation)) goto skip; /* We do NOT handle cross-stream or variation kerning. */ if (HB_DIRECTION_IS_VERTICAL (c->buffer->props.direction) != - bool (table->coverage & KerxTable::Vertical)) + bool (table->u.header.coverage & KerxTable::Vertical)) goto skip; - reverse = bool (table->coverage & KerxTable::Backwards) != + reverse = bool (table->u.header.coverage & KerxTable::Backwards) != HB_DIRECTION_IS_BACKWARD (c->buffer->props.direction); if (!c->buffer->message (c->font, "start kerx subtable %d", c->lookup_index)) diff --git a/src/hb-ot-kern-table.hh b/src/hb-ot-kern-table.hh index ccb666e8c..dab7a805f 100644 --- a/src/hb-ot-kern-table.hh +++ b/src/hb-ot-kern-table.hh @@ -191,6 +191,12 @@ struct KernSubTableFormat2 { inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, const char *end) const { + /* This subtable is disabled. It's not cleaer to me *exactly* where the offests are + * based from. I *think* they should be based from beginning of kern subtable wrapper, + * *NOT* "this". Since we know of no fonts that use this subtable, we are disabling + * it. Someday fix it and re-enable. Better yet, find fonts that use it... Meh, + * Windows doesn't implement it. Maybe just remove... */ + return 0; unsigned int l = (this+leftClassTable).get_class (left); unsigned int r = (this+rightClassTable).get_class (right); unsigned int offset = l + r; @@ -204,6 +210,7 @@ struct KernSubTableFormat2 inline bool sanitize (hb_sanitize_context_t *c) const { TRACE_SANITIZE (this); + return_trace (true); /* Disabled. See above. */ return_trace (rowWidth.sanitize (c) && leftClassTable.sanitize (c, this) && rightClassTable.sanitize (c, this) && From 7e6e5bf6147596d6d096e2ba37f3a6eefd7429cd Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 18:59:07 -0400 Subject: [PATCH 20/26] Fix option string matching --- src/hb-common.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hb-common.cc b/src/hb-common.cc index f698a4406..eda41dd87 100644 --- a/src/hb-common.cc +++ b/src/hb-common.cc @@ -57,7 +57,7 @@ _hb_options_init (void) p = c + strlen (c); #define OPTION(name, symbol) \ - if (0 == strncmp (c, name, p - c)) u.opts.symbol = true; + if (0 == strncmp (c, name, p - c) && strlen (name) == p - c) u.opts.symbol = true; OPTION ("uniscribe-bug-compatible", uniscribe_bug_compatible); OPTION ("aat", aat); From 7fa69e92ca3dd9d8fa92aba0e01098165d2b7975 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 19:02:32 -0400 Subject: [PATCH 21/26] Comment --- src/hb-machinery.hh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/hb-machinery.hh b/src/hb-machinery.hh index 8feb3773c..56914beae 100644 --- a/src/hb-machinery.hh +++ b/src/hb-machinery.hh @@ -257,6 +257,13 @@ struct hb_sanitize_context_t : inline void set_max_ops (int max_ops_) { max_ops = max_ops_; } + /* TODO + * This set_object() thing is to use sanitize at runtime lookup + * application time. This is very distinct from the regular + * sanitizer operation, so, eventually, separate into another + * type and make hb_aat_apply_context_t use that one instead + * of abusing this one. + */ template inline void set_object (const T& obj) { From 7ed5366d3cfca9c533250cb419e8cc878f32505d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 19:11:30 -0400 Subject: [PATCH 22/26] [kerx] No-op Tested that Format0 works with Kannada MN font: $ make -j5 lib -s && HB_OPTIONS=aat ./hb-shape Kannada\ MN.ttc -u 0C95,0CC2 [kn_ka=0+1000|kn_matra_uu=0@-30,0+1345] $ make -j5 lib -s && HB_OPTIONS=aat ./hb-shape Kannada\ MN.ttc -u 0C95,0CC2 --features=-kern [kn_ka=0+1030|kn_matra_uu=0+1375] Note that GPOS does the same with 'dist' feature, and applies the whole difference to the same glyph: $ make -j5 lib -s && ./hb-shape Kannada\ MN.ttc -u 0C95,0CC2 [kn_ka=0+970|kn_matra_uu=0+1375] $ make -j5 lib -s && ./hb-shape Kannada\ MN.ttc -u 0C95,0CC2 --features=-dist [kn_ka=0+1030|kn_matra_uu=0+1375] --- src/hb-aat-layout-kerx-table.hh | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 2214265b5..c683242f6 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -66,9 +66,7 @@ struct KerxSubTableFormat0 { hb_glyph_pair_t pair = {left, right}; int i = pairs.bsearch (pair); - if (i == -1) - return 0; - return pairs[i].get_kerning (); + return i == -1 ? 0 : pairs[i].get_kerning (); } inline bool apply (hb_aat_apply_context_t *c) const From f6aaad9b4ffb42e6cd8398f6439fe420e393c8f6 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 19:20:06 -0400 Subject: [PATCH 23/26] [kerx] When rejecting variable kerning, also check for tupleCount --- src/hb-aat-layout-kerx-table.hh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index c683242f6..cd8dc458c 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -339,7 +339,8 @@ struct kerx { bool reverse; - if (table->u.header.coverage & (KerxTable::CrossStream | KerxTable::Variation)) + if (table->u.header.coverage & (KerxTable::CrossStream | KerxTable::Variation) || + table->u.header.tupleCount) goto skip; /* We do NOT handle cross-stream or variation kerning. */ if (HB_DIRECTION_IS_VERTICAL (c->buffer->props.direction) != From 22955b23cdeb48e46cdffd0eb906a855a420c4d1 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 19:58:20 -0400 Subject: [PATCH 24/26] [kerx] Start fleshing out Format6 --- src/hb-aat-layout-kerx-table.hh | 42 ++++++++++++++++++++++++++------- src/hb-open-type.hh | 3 +++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index cd8dc458c..05b28f3cf 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -225,6 +225,13 @@ struct KerxSubTableFormat4 struct KerxSubTableFormat6 { + enum Flags + { + ValuesAreLong = 0x00000001, + }; + + inline bool is_long (void) const { return flags & ValuesAreLong; } + inline bool apply (hb_aat_apply_context_t *c) const { TRACE_APPLY (this); @@ -241,10 +248,16 @@ struct KerxSubTableFormat6 { TRACE_SANITIZE (this); return_trace (likely (c->check_struct (this) && - rowIndexTable.sanitize (c, this) && - columnIndexTable.sanitize (c, this) && - kerningArray.sanitize (c, this) && - kerningVector.sanitize (c, this))); + is_long () ? + ( + u.l.rowIndexTable.sanitize (c, this) && + u.l.columnIndexTable.sanitize (c, this) && + u.l.kerningArray.sanitize (c, this) + ) : ( + u.s.rowIndexTable.sanitize (c, this) && + u.s.columnIndexTable.sanitize (c, this) && + u.s.kerningArray.sanitize (c, this) + ))); } protected: @@ -252,12 +265,23 @@ struct KerxSubTableFormat6 HBUINT32 flags; HBUINT16 rowCount; HBUINT16 columnCount; - LOffsetTo > rowIndexTable; - LOffsetTo > columnIndexTable; - LOffsetTo > kerningArray; - LOffsetTo > kerningVector; + union + { + struct + { + LOffsetTo > rowIndexTable; + LOffsetTo > columnIndexTable; + LOffsetTo kerningArray; + } s; + struct + { + LOffsetTo > rowIndexTable; + LOffsetTo > columnIndexTable; + LOffsetTo kerningArray; + } l; + } u; public: - DEFINE_SIZE_STATIC (36); + DEFINE_SIZE_STATIC (32); }; struct KerxTable diff --git a/src/hb-open-type.hh b/src/hb-open-type.hh index 9a0873369..2eae09d5c 100644 --- a/src/hb-open-type.hh +++ b/src/hb-open-type.hh @@ -92,6 +92,9 @@ typedef IntType HBUINT24; /* 24-bit unsigned integer. */ /* 16-bit signed integer (HBINT16) that describes a quantity in FUnits. */ typedef HBINT16 FWORD; +/* 32-bit signed integer (HBINT32) that describes a quantity in FUnits. */ +typedef HBINT32 FWORD32; + /* 16-bit unsigned integer (HBUINT16) that describes a quantity in FUnits. */ typedef HBUINT16 UFWORD; From c9a2ce9e05f91730a2150b9214dc6a49f31555c1 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 20:00:44 -0400 Subject: [PATCH 25/26] [kerx] Move bounds-checking to subtable length itself --- src/hb-aat-layout-kerx-table.hh | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 05b28f3cf..31b650ada 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -129,14 +129,14 @@ struct KerxSubTableFormat1 struct KerxSubTableFormat2 { inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, - const char *end, unsigned int num_glyphs) const + unsigned int num_glyphs) const { unsigned int l = (this+leftClassTable).get_value_or_null (left, num_glyphs); unsigned int r = (this+rightClassTable).get_value_or_null (right, num_glyphs); unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+array), offset); if (unlikely ((const char *) v < (const char *) &array || - (const char *) v > (const char *) end - 2)) + (const char *) v + 2 - (const char *) this <= header.length)) return 0; return *v; } @@ -149,7 +149,6 @@ struct KerxSubTableFormat2 return false; accelerator_t accel (*this, - c->sanitizer.end, c->face->get_num_glyphs ()); hb_kern_machine_t machine (accel); machine.kern (c->font, c->buffer, c->plan->kern_mask); @@ -170,16 +169,15 @@ struct KerxSubTableFormat2 struct accelerator_t { const KerxSubTableFormat2 &table; - const char *end; unsigned int num_glyphs; inline accelerator_t (const KerxSubTableFormat2 &table_, - const char *end_, unsigned int num_glyphs_) - : table (table_), end (end_), num_glyphs (num_glyphs_) {} + unsigned int num_glyphs_) + : table (table_), num_glyphs (num_glyphs_) {} inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right) const { - return table.get_kerning (left, right, end, num_glyphs); + return table.get_kerning (left, right, num_glyphs); } }; From ab1f30bd059f1d2270793e9726b60666b328d2b8 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 10 Oct 2018 20:10:20 -0400 Subject: [PATCH 26/26] [kerx] Implement Format6 Untested. The only Apple font shipping with this format is San Francisco fonts that use this for their kerx variation tables, which we don't support. --- src/hb-aat-layout-kerx-table.hh | 73 +++++++++++++++++++++++++++------ 1 file changed, 60 insertions(+), 13 deletions(-) diff --git a/src/hb-aat-layout-kerx-table.hh b/src/hb-aat-layout-kerx-table.hh index 31b650ada..3cff334a9 100644 --- a/src/hb-aat-layout-kerx-table.hh +++ b/src/hb-aat-layout-kerx-table.hh @@ -136,7 +136,7 @@ struct KerxSubTableFormat2 unsigned int offset = l + r; const FWORD *v = &StructAtOffset (&(this+array), offset); if (unlikely ((const char *) v < (const char *) &array || - (const char *) v + 2 - (const char *) this <= header.length)) + (const char *) v + v->static_size - (const char *) this <= header.length)) return 0; return *v; } @@ -230,6 +230,35 @@ struct KerxSubTableFormat6 inline bool is_long (void) const { return flags & ValuesAreLong; } + inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right, + unsigned int num_glyphs) const + { + if (is_long ()) + { + const U::Long &t = u.l; + unsigned int l = (this+t.rowIndexTable).get_value_or_null (left, num_glyphs); + unsigned int r = (this+t.columnIndexTable).get_value_or_null (right, num_glyphs); + unsigned int offset = l + r; + const FWORD32 *v = &StructAtOffset (&(this+t.array), offset * sizeof (FWORD32)); + if (unlikely ((const char *) v < (const char *) &t.array || + (const char *) v + v->static_size - (const char *) this <= header.length)) + return 0; + return *v; + } + else + { + const U::Short &t = u.s; + unsigned int l = (this+t.rowIndexTable).get_value_or_null (left, num_glyphs); + unsigned int r = (this+t.columnIndexTable).get_value_or_null (right, num_glyphs); + unsigned int offset = l + r; + const FWORD *v = &StructAtOffset (&(this+t.array), offset * sizeof (FWORD)); + if (unlikely ((const char *) v < (const char *) &t.array || + (const char *) v + v->static_size - (const char *) this <= header.length)) + return 0; + return *v; + } + } + inline bool apply (hb_aat_apply_context_t *c) const { TRACE_APPLY (this); @@ -237,7 +266,10 @@ struct KerxSubTableFormat6 if (!c->plan->requested_kerning) return false; - /* TODO */ + accelerator_t accel (*this, + c->face->get_num_glyphs ()); + hb_kern_machine_t machine (accel); + machine.kern (c->font, c->buffer, c->plan->kern_mask); return_trace (true); } @@ -250,33 +282,48 @@ struct KerxSubTableFormat6 ( u.l.rowIndexTable.sanitize (c, this) && u.l.columnIndexTable.sanitize (c, this) && - u.l.kerningArray.sanitize (c, this) + u.l.array.sanitize (c, this) ) : ( u.s.rowIndexTable.sanitize (c, this) && u.s.columnIndexTable.sanitize (c, this) && - u.s.kerningArray.sanitize (c, this) + u.s.array.sanitize (c, this) ))); } + struct accelerator_t + { + const KerxSubTableFormat6 &table; + unsigned int num_glyphs; + + inline accelerator_t (const KerxSubTableFormat6 &table_, + unsigned int num_glyphs_) + : table (table_), num_glyphs (num_glyphs_) {} + + inline int get_kerning (hb_codepoint_t left, hb_codepoint_t right) const + { + return table.get_kerning (left, right, num_glyphs); + } + }; + protected: KerxSubTableHeader header; HBUINT32 flags; HBUINT16 rowCount; HBUINT16 columnCount; - union + union U { - struct - { - LOffsetTo > rowIndexTable; - LOffsetTo > columnIndexTable; - LOffsetTo kerningArray; - } s; - struct + struct Long { LOffsetTo > rowIndexTable; LOffsetTo > columnIndexTable; - LOffsetTo kerningArray; + LOffsetTo array; } l; + struct Short + { + LOffsetTo > rowIndexTable; + LOffsetTo > columnIndexTable; + LOffsetTo array; + } s; } u; public: DEFINE_SIZE_STATIC (32);