From 7c704d898210cd88f6fbf1096b9a75123d33cccd Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 28 Jan 2022 12:38:32 -0700 Subject: [PATCH 01/10] [buffer] Make hb_buffer_append() take a const argument --- src/hb-buffer.cc | 2 +- src/hb-buffer.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hb-buffer.cc b/src/hb-buffer.cc index 3e09260cf..d6bafc064 100644 --- a/src/hb-buffer.cc +++ b/src/hb-buffer.cc @@ -1790,7 +1790,7 @@ hb_buffer_add_codepoints (hb_buffer_t *buffer, **/ HB_EXTERN void hb_buffer_append (hb_buffer_t *buffer, - hb_buffer_t *source, + const hb_buffer_t *source, unsigned int start, unsigned int end) { diff --git a/src/hb-buffer.h b/src/hb-buffer.h index 001e6dca8..5df17170d 100644 --- a/src/hb-buffer.h +++ b/src/hb-buffer.h @@ -522,7 +522,7 @@ hb_buffer_add_codepoints (hb_buffer_t *buffer, HB_EXTERN void hb_buffer_append (hb_buffer_t *buffer, - hb_buffer_t *source, + const hb_buffer_t *source, unsigned int start, unsigned int end); From 61823838f9c3cdc93b2452451126dddcacfbe61d Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 28 Jan 2022 13:45:25 -0700 Subject: [PATCH 02/10] [buffer] Add HB_BUFFER_FLAG_VERIFY Move buffer verification code inside the library, from util/. Part of https://github.com/harfbuzz/harfbuzz/issues/3010 --- src/Makefile.sources | 1 + src/harfbuzz.cc | 1 + src/hb-buffer-verify.cc | 389 ++++++++++++++++++++++++++++++++++++++++ src/hb-buffer.h | 3 +- src/hb-buffer.hh | 14 ++ src/hb-shape.cc | 18 ++ src/meson.build | 1 + util/shape-options.hh | 344 +---------------------------------- 8 files changed, 427 insertions(+), 344 deletions(-) create mode 100644 src/hb-buffer-verify.cc diff --git a/src/Makefile.sources b/src/Makefile.sources index eeff2ca1b..ce6501443 100644 --- a/src/Makefile.sources +++ b/src/Makefile.sources @@ -25,6 +25,7 @@ HB_BASE_sources = \ hb-blob.cc \ hb-blob.hh \ hb-buffer-serialize.cc \ + hb-buffer-verify.cc \ hb-buffer.cc \ hb-buffer.hh \ hb-cache.hh \ diff --git a/src/harfbuzz.cc b/src/harfbuzz.cc index fe0010097..b6a5957c4 100644 --- a/src/harfbuzz.cc +++ b/src/harfbuzz.cc @@ -2,6 +2,7 @@ #include "hb-aat-map.cc" #include "hb-blob.cc" #include "hb-buffer-serialize.cc" +#include "hb-buffer-verify.cc" #include "hb-buffer.cc" #include "hb-common.cc" #include "hb-draw.cc" diff --git a/src/hb-buffer-verify.cc b/src/hb-buffer-verify.cc new file mode 100644 index 000000000..905e54877 --- /dev/null +++ b/src/hb-buffer-verify.cc @@ -0,0 +1,389 @@ +/* + * Copyright © 2022 Behdad Esfahbod + * + * This is part of HarfBuzz, a text shaping library. + * + * Permission is hereby granted, without written agreement and without + * license or royalty fees, to use, copy, modify, and distribute this + * software and its documentation for any purpose, provided that the + * above copyright notice and the following two paragraphs appear in + * all copies of this software. + * + * IN NO EVENT SHALL THE COPYRIGHT HOLDER BE LIABLE TO ANY PARTY FOR + * DIRECT, INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES + * ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS DOCUMENTATION, EVEN + * IF THE COPYRIGHT HOLDER HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH + * DAMAGE. + * + * THE COPYRIGHT HOLDER SPECIFICALLY DISCLAIMS ANY WARRANTIES, INCLUDING, + * BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND + * FITNESS FOR A PARTICULAR PURPOSE. THE SOFTWARE PROVIDED HEREUNDER IS + * ON AN "AS IS" BASIS, AND THE COPYRIGHT HOLDER HAS NO OBLIGATION TO + * PROVIDE MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS. + * + * Google Author(s): Behdad Esfahbod + */ + +#include "hb.hh" + +#ifndef HB_NO_BUFFER_VERIFY + +#include "hb-buffer.hh" + + +static inline void +buffer_verify_error (hb_buffer_t *buffer, + hb_font_t *font, + const char *message) +{ + if (buffer->messaging ()) + buffer->message (font, "%s", message); + else + fprintf (stderr, "%s\n", message); +} + +static bool +buffer_verify_monotone (hb_buffer_t *buffer, + hb_font_t *font) +{ + /* Check that clusters are monotone. */ + if (buffer->cluster_level == HB_BUFFER_CLUSTER_LEVEL_MONOTONE_GRAPHEMES || + buffer->cluster_level == HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS) + { + bool is_forward = HB_DIRECTION_IS_FORWARD (hb_buffer_get_direction (buffer)); + + unsigned int num_glyphs; + hb_glyph_info_t *info = hb_buffer_get_glyph_infos (buffer, &num_glyphs); + + for (unsigned int i = 1; i < num_glyphs; i++) + if (info[i-1].cluster != info[i].cluster && + (info[i-1].cluster < info[i].cluster) != is_forward) + { + buffer_verify_error (buffer, font, "clusters are not monotone."); + return false; + } + } + + return true; +} + +static bool +buffer_verify_unsafe_to_break (hb_buffer_t *buffer, + hb_buffer_t *text_buffer, + hb_font_t *font, + const hb_feature_t *features, + unsigned int num_features, + const char * const *shapers) +{ + if (buffer->cluster_level != HB_BUFFER_CLUSTER_LEVEL_MONOTONE_GRAPHEMES && + buffer->cluster_level != HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS) + { + /* Cannot perform this check without monotone clusters. */ + return true; + } + + /* Check that breaking up shaping at safe-to-break is indeed safe. */ + + hb_buffer_t *fragment = hb_buffer_create_similar (buffer); + hb_buffer_set_flags (fragment, hb_buffer_get_flags (fragment) & ~HB_BUFFER_FLAG_VERIFY); + hb_buffer_t *reconstruction = hb_buffer_create_similar (buffer); + hb_buffer_set_flags (reconstruction, hb_buffer_get_flags (reconstruction) & ~HB_BUFFER_FLAG_VERIFY); + + unsigned int num_glyphs; + hb_glyph_info_t *info = hb_buffer_get_glyph_infos (buffer, &num_glyphs); + + unsigned int num_chars; + hb_glyph_info_t *text = hb_buffer_get_glyph_infos (text_buffer, &num_chars); + + /* Chop text and shape fragments. */ + bool forward = HB_DIRECTION_IS_FORWARD (hb_buffer_get_direction (buffer)); + unsigned int start = 0; + unsigned int text_start = forward ? 0 : num_chars; + unsigned int text_end = text_start; + for (unsigned int end = 1; end < num_glyphs + 1; end++) + { + if (end < num_glyphs && + (info[end].cluster == info[end-1].cluster || + info[end-(forward?0:1)].mask & HB_GLYPH_FLAG_UNSAFE_TO_BREAK)) + continue; + + /* Shape segment corresponding to glyphs start..end. */ + if (end == num_glyphs) + { + if (forward) + text_end = num_chars; + else + text_start = 0; + } + else + { + if (forward) + { + unsigned int cluster = info[end].cluster; + while (text_end < num_chars && text[text_end].cluster < cluster) + text_end++; + } + else + { + unsigned int cluster = info[end - 1].cluster; + while (text_start && text[text_start - 1].cluster >= cluster) + text_start--; + } + } + assert (text_start < text_end); + + if (0) + printf("start %d end %d text start %d end %d\n", start, end, text_start, text_end); + + hb_buffer_clear_contents (fragment); + + hb_buffer_flags_t flags = hb_buffer_get_flags (fragment); + if (0 < text_start) + flags = (hb_buffer_flags_t) (flags & ~HB_BUFFER_FLAG_BOT); + if (text_end < num_chars) + flags = (hb_buffer_flags_t) (flags & ~HB_BUFFER_FLAG_EOT); + hb_buffer_set_flags (fragment, flags); + + hb_buffer_append (fragment, text_buffer, text_start, text_end); + if (!hb_shape_full (font, fragment, features, num_features, shapers)) + { + buffer_verify_error (buffer, font, "All shapers failed while shaping fragment."); + hb_buffer_destroy (reconstruction); + hb_buffer_destroy (fragment); + return false; + } + hb_buffer_append (reconstruction, fragment, 0, -1); + + start = end; + if (forward) + text_start = text_end; + else + text_end = text_start; + } + + bool ret = true; + hb_buffer_diff_flags_t diff = hb_buffer_diff (reconstruction, buffer, (hb_codepoint_t) -1, 0); + if (diff) + { + buffer_verify_error (buffer, font, "unsafe-to-break test failed."); + ret = false; + + /* Return the reconstructed result instead so it can be inspected. */ + hb_buffer_set_length (buffer, 0); + hb_buffer_append (buffer, reconstruction, 0, -1); + } + + hb_buffer_destroy (reconstruction); + hb_buffer_destroy (fragment); + + return ret; +} + +static bool +buffer_verify_unsafe_to_concat (hb_buffer_t *buffer, + hb_buffer_t *text_buffer, + hb_font_t *font, + const hb_feature_t *features, + unsigned int num_features, + const char * const *shapers) +{ + if (buffer->cluster_level != HB_BUFFER_CLUSTER_LEVEL_MONOTONE_GRAPHEMES && + buffer->cluster_level != HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS) + { + /* Cannot perform this check without monotone clusters. */ + return true; + } + + /* Check that shuffling up text before shaping at safe-to-concat points + * is indeed safe. */ + + /* This is what we do: + * + * 1. We shape text once. Then segment the text at all the safe-to-concat + * points; + * + * 2. Then we create two buffers, one containing all the even segments and + * one all the odd segments. + * + * 3. Because all these segments were safe-to-concat at both ends, we + * expect that concatenating them and shaping should NOT change the + * shaping results of each segment. As such, we expect that after + * shaping the two buffers, we still get cluster boundaries at the + * segment boundaries, and that those all are safe-to-concat points. + * Moreover, that there are NOT any safe-to-concat points within the + * segments. + * + * 4. Finally, we reconstruct the shaping results of the original text by + * simply interleaving the shaping results of the segments from the two + * buffers, and assert that the total shaping results is the same as + * the one from original buffer in step 1. + */ + + hb_buffer_t *fragments[2] {hb_buffer_create_similar (buffer), + hb_buffer_create_similar (buffer)}; + hb_buffer_set_flags (fragments[0], hb_buffer_get_flags (fragments[0]) & ~HB_BUFFER_FLAG_VERIFY); + hb_buffer_set_flags (fragments[1], hb_buffer_get_flags (fragments[1]) & ~HB_BUFFER_FLAG_VERIFY); + hb_buffer_t *reconstruction = hb_buffer_create_similar (buffer); + hb_buffer_set_flags (reconstruction, hb_buffer_get_flags (reconstruction) & ~HB_BUFFER_FLAG_VERIFY); + hb_segment_properties_t props; + hb_buffer_get_segment_properties (buffer, &props); + hb_buffer_set_segment_properties (fragments[0], &props); + hb_buffer_set_segment_properties (fragments[1], &props); + hb_buffer_set_segment_properties (reconstruction, &props); + + unsigned num_glyphs; + hb_glyph_info_t *info = hb_buffer_get_glyph_infos (buffer, &num_glyphs); + + unsigned num_chars; + hb_glyph_info_t *text = hb_buffer_get_glyph_infos (text_buffer, &num_chars); + + bool forward = HB_DIRECTION_IS_FORWARD (hb_buffer_get_direction (buffer)); + + if (!forward) + hb_buffer_reverse (buffer); + + /* + * Split text into segments and collect into to fragment streams. + */ + { + unsigned fragment_idx = 0; + unsigned start = 0; + unsigned text_start = 0; + unsigned text_end = 0; + for (unsigned end = 1; end < num_glyphs + 1; end++) + { + if (end < num_glyphs && + (info[end].cluster == info[end-1].cluster || + info[end].mask & HB_GLYPH_FLAG_UNSAFE_TO_CONCAT)) + continue; + + /* Accumulate segment corresponding to glyphs start..end. */ + if (end == num_glyphs) + text_end = num_chars; + else + { + unsigned cluster = info[end].cluster; + while (text_end < num_chars && text[text_end].cluster < cluster) + text_end++; + } + assert (text_start < text_end); + + if (0) + printf("start %d end %d text start %d end %d\n", start, end, text_start, text_end); + +#if 0 + hb_buffer_flags_t flags = hb_buffer_get_flags (fragment); + if (0 < text_start) + flags = (hb_buffer_flags_t) (flags & ~HB_BUFFER_FLAG_BOT); + if (text_end < num_chars) + flags = (hb_buffer_flags_t) (flags & ~HB_BUFFER_FLAG_EOT); + hb_buffer_set_flags (fragment, flags); +#endif + + hb_buffer_append (fragments[fragment_idx], text_buffer, text_start, text_end); + + start = end; + text_start = text_end; + fragment_idx = 1 - fragment_idx; + } + } + + bool ret = true; + hb_buffer_diff_flags_t diff; + + /* + * Shape the two fragment streams. + */ + if (!hb_shape_full (font, fragments[0], features, num_features, shapers)) + { + buffer_verify_error (buffer, font, "All shapers failed while shaping fragment."); + ret = false; + goto out; + } + if (!hb_shape_full (font, fragments[1], features, num_features, shapers)) + { + buffer_verify_error (buffer, font, "All shapers failed while shaping fragment."); + ret = false; + goto out; + } + + if (!forward) + { + hb_buffer_reverse (fragments[0]); + hb_buffer_reverse (fragments[1]); + } + + /* + * Reconstruct results. + */ + { + unsigned fragment_idx = 0; + unsigned fragment_start[2] {0, 0}; + unsigned fragment_num_glyphs[2]; + hb_glyph_info_t *fragment_info[2]; + for (unsigned i = 0; i < 2; i++) + fragment_info[i] = hb_buffer_get_glyph_infos (fragments[i], &fragment_num_glyphs[i]); + while (fragment_start[0] < fragment_num_glyphs[0] || + fragment_start[1] < fragment_num_glyphs[1]) + { + unsigned fragment_end = fragment_start[fragment_idx] + 1; + while (fragment_end < fragment_num_glyphs[fragment_idx] && + (fragment_info[fragment_idx][fragment_end].cluster == fragment_info[fragment_idx][fragment_end - 1].cluster || + fragment_info[fragment_idx][fragment_end].mask & HB_GLYPH_FLAG_UNSAFE_TO_CONCAT)) + fragment_end++; + + hb_buffer_append (reconstruction, fragments[fragment_idx], fragment_start[fragment_idx], fragment_end); + + fragment_start[fragment_idx] = fragment_end; + fragment_idx = 1 - fragment_idx; + } + } + + if (!forward) + { + hb_buffer_reverse (buffer); + hb_buffer_reverse (reconstruction); + } + + /* + * Diff results. + */ + diff = hb_buffer_diff (reconstruction, buffer, (hb_codepoint_t) -1, 0); + if (diff) + { + buffer_verify_error (buffer, font, "unsafe-to-concat test failed."); + ret = false; + + /* Return the reconstructed result instead so it can be inspected. */ + hb_buffer_set_length (buffer, 0); + hb_buffer_append (buffer, reconstruction, 0, -1); + } + + +out: + hb_buffer_destroy (reconstruction); + hb_buffer_destroy (fragments[0]); + hb_buffer_destroy (fragments[1]); + + return ret; +} + +bool +hb_buffer_t::verify (hb_buffer_t *text_buffer, + hb_font_t *font, + const hb_feature_t *features, + unsigned int num_features, + const char * const *shapers) +{ + bool ret = true; + if (!buffer_verify_monotone (this, font)) + ret = false; + if (!buffer_verify_unsafe_to_break (this, text_buffer, font, features, num_features, shapers)) + ret = false; + if (!buffer_verify_unsafe_to_concat (this, text_buffer, font, features, num_features, shapers)) + ret = false; + return ret; +} + + +#endif diff --git a/src/hb-buffer.h b/src/hb-buffer.h index 5df17170d..fb76c9007 100644 --- a/src/hb-buffer.h +++ b/src/hb-buffer.h @@ -368,7 +368,8 @@ typedef enum { /*< flags >*/ HB_BUFFER_FLAG_EOT = 0x00000002u, /* End-of-text */ HB_BUFFER_FLAG_PRESERVE_DEFAULT_IGNORABLES = 0x00000004u, HB_BUFFER_FLAG_REMOVE_DEFAULT_IGNORABLES = 0x00000008u, - HB_BUFFER_FLAG_DO_NOT_INSERT_DOTTED_CIRCLE = 0x00000010u + HB_BUFFER_FLAG_DO_NOT_INSERT_DOTTED_CIRCLE = 0x00000010u, + HB_BUFFER_FLAG_VERIFY = 0x00000020u } hb_buffer_flags_t; HB_EXTERN void diff --git a/src/hb-buffer.hh b/src/hb-buffer.hh index adf4aa2b6..3402da719 100644 --- a/src/hb-buffer.hh +++ b/src/hb-buffer.hh @@ -212,6 +212,20 @@ struct hb_buffer_t HB_INTERNAL void enter (); HB_INTERNAL void leave (); +#ifndef HB_NO_BUFFER_VERIFY + HB_INTERNAL +#endif + bool verify (hb_buffer_t *text_buffer, + hb_font_t *font, + const hb_feature_t *features, + unsigned int num_features, + const char * const *shapers) +#ifndef HB_NO_BUFFER_VERIFY + ; +#else + { return true; } +#endif + unsigned int backtrack_len () const { return have_output ? out_len : idx; } unsigned int lookahead_len () const { return len - idx; } uint8_t next_serial () { return ++serial ? serial : ++serial; } diff --git a/src/hb-shape.cc b/src/hb-shape.cc index c1f619c81..3407e1af4 100644 --- a/src/hb-shape.cc +++ b/src/hb-shape.cc @@ -126,6 +126,13 @@ hb_shape_full (hb_font_t *font, unsigned int num_features, const char * const *shaper_list) { + hb_buffer_t *text_buffer = nullptr; + if (buffer->flags & HB_BUFFER_FLAG_VERIFY) + { + text_buffer = hb_buffer_create (); + hb_buffer_append (text_buffer, buffer, 0, -1); + } + hb_shape_plan_t *shape_plan = hb_shape_plan_create_cached2 (font->face, &buffer->props, features, num_features, font->coords, font->num_coords, @@ -133,6 +140,17 @@ hb_shape_full (hb_font_t *font, hb_bool_t res = hb_shape_plan_execute (shape_plan, font, buffer, features, num_features); hb_shape_plan_destroy (shape_plan); + if (text_buffer) + { + if (res && !buffer->verify (text_buffer, + font, + features, + num_features, + shaper_list)) + res = false; + hb_buffer_destroy (text_buffer); + } + return res; } diff --git a/src/meson.build b/src/meson.build index d70dc8799..802b03db0 100644 --- a/src/meson.build +++ b/src/meson.build @@ -29,6 +29,7 @@ hb_base_sources = files( 'hb-blob.cc', 'hb-blob.hh', 'hb-buffer-serialize.cc', + 'hb-buffer-verify.cc', 'hb-buffer.cc', 'hb-buffer.hh', 'hb-cache.hh', diff --git a/util/shape-options.hh b/util/shape-options.hh index 7847fdb6b..ec32425dc 100644 --- a/util/shape-options.hh +++ b/util/shape-options.hh @@ -51,6 +51,7 @@ struct shape_options_t (HB_BUFFER_FLAG_DEFAULT | (bot ? HB_BUFFER_FLAG_BOT : 0) | (eot ? HB_BUFFER_FLAG_EOT : 0) | + (verify ? HB_BUFFER_FLAG_VERIFY : 0) | (preserve_default_ignorables ? HB_BUFFER_FLAG_PRESERVE_DEFAULT_IGNORABLES : 0) | (remove_default_ignorables ? HB_BUFFER_FLAG_REMOVE_DEFAULT_IGNORABLES : 0) | 0)); @@ -90,13 +91,6 @@ struct shape_options_t hb_bool_t shape (hb_font_t *font, hb_buffer_t *buffer, const char **error=nullptr) { - hb_buffer_t *text_buffer = nullptr; - if (verify) - { - text_buffer = hb_buffer_create (); - hb_buffer_append (text_buffer, buffer, 0, -1); - } - if (!hb_shape_full (font, buffer, features, num_features, shapers)) { if (error) @@ -107,348 +101,12 @@ struct shape_options_t if (normalize_glyphs) hb_buffer_normalize_glyphs (buffer); - if (verify && !verify_buffer (buffer, text_buffer, font, error)) - goto fail; - - if (text_buffer) - hb_buffer_destroy (text_buffer); - return true; fail: - if (text_buffer) - hb_buffer_destroy (text_buffer); - return false; } - bool verify_buffer (hb_buffer_t *buffer, - hb_buffer_t *text_buffer, - hb_font_t *font, - const char **error=nullptr) - { - if (!verify_buffer_monotone (buffer, error)) - return false; - if (!verify_buffer_unsafe_to_break (buffer, text_buffer, font, error)) - return false; - if (!verify_buffer_unsafe_to_concat (buffer, text_buffer, font, error)) - return false; - return true; - } - - bool verify_buffer_monotone (hb_buffer_t *buffer, const char **error=nullptr) - { - /* Check that clusters are monotone. */ - if (cluster_level == HB_BUFFER_CLUSTER_LEVEL_MONOTONE_GRAPHEMES || - cluster_level == HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS) - { - bool is_forward = HB_DIRECTION_IS_FORWARD (hb_buffer_get_direction (buffer)); - - unsigned int num_glyphs; - hb_glyph_info_t *info = hb_buffer_get_glyph_infos (buffer, &num_glyphs); - - for (unsigned int i = 1; i < num_glyphs; i++) - if (info[i-1].cluster != info[i].cluster && - (info[i-1].cluster < info[i].cluster) != is_forward) - { - if (error) - *error = "clusters are not monotone."; - return false; - } - } - - return true; - } - - bool verify_buffer_unsafe_to_break (hb_buffer_t *buffer, - hb_buffer_t *text_buffer, - hb_font_t *font, - const char **error=nullptr) - { - if (cluster_level != HB_BUFFER_CLUSTER_LEVEL_MONOTONE_GRAPHEMES && - cluster_level != HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS) - { - /* Cannot perform this check without monotone clusters. */ - return true; - } - - /* Check that breaking up shaping at safe-to-break is indeed safe. */ - - hb_buffer_t *fragment = hb_buffer_create_similar (buffer); - hb_buffer_t *reconstruction = hb_buffer_create_similar (buffer); - - unsigned int num_glyphs; - hb_glyph_info_t *info = hb_buffer_get_glyph_infos (buffer, &num_glyphs); - - unsigned int num_chars; - hb_glyph_info_t *text = hb_buffer_get_glyph_infos (text_buffer, &num_chars); - - /* Chop text and shape fragments. */ - bool forward = HB_DIRECTION_IS_FORWARD (hb_buffer_get_direction (buffer)); - unsigned int start = 0; - unsigned int text_start = forward ? 0 : num_chars; - unsigned int text_end = text_start; - for (unsigned int end = 1; end < num_glyphs + 1; end++) - { - if (end < num_glyphs && - (info[end].cluster == info[end-1].cluster || - info[end-(forward?0:1)].mask & HB_GLYPH_FLAG_UNSAFE_TO_BREAK)) - continue; - - /* Shape segment corresponding to glyphs start..end. */ - if (end == num_glyphs) - { - if (forward) - text_end = num_chars; - else - text_start = 0; - } - else - { - if (forward) - { - unsigned int cluster = info[end].cluster; - while (text_end < num_chars && text[text_end].cluster < cluster) - text_end++; - } - else - { - unsigned int cluster = info[end - 1].cluster; - while (text_start && text[text_start - 1].cluster >= cluster) - text_start--; - } - } - assert (text_start < text_end); - - if (0) - printf("start %d end %d text start %d end %d\n", start, end, text_start, text_end); - - hb_buffer_clear_contents (fragment); - - hb_buffer_flags_t flags = hb_buffer_get_flags (fragment); - if (0 < text_start) - flags = (hb_buffer_flags_t) (flags & ~HB_BUFFER_FLAG_BOT); - if (text_end < num_chars) - flags = (hb_buffer_flags_t) (flags & ~HB_BUFFER_FLAG_EOT); - hb_buffer_set_flags (fragment, flags); - - hb_buffer_append (fragment, text_buffer, text_start, text_end); - if (!hb_shape_full (font, fragment, features, num_features, shapers)) - { - if (error) - *error = "All shapers failed while shaping fragment."; - hb_buffer_destroy (reconstruction); - hb_buffer_destroy (fragment); - return false; - } - hb_buffer_append (reconstruction, fragment, 0, -1); - - start = end; - if (forward) - text_start = text_end; - else - text_end = text_start; - } - - bool ret = true; - hb_buffer_diff_flags_t diff = hb_buffer_diff (reconstruction, buffer, (hb_codepoint_t) -1, 0); - if (diff) - { - if (error) - *error = "unsafe-to-break test failed."; - ret = false; - - /* Return the reconstructed result instead so it can be inspected. */ - hb_buffer_set_length (buffer, 0); - hb_buffer_append (buffer, reconstruction, 0, -1); - } - - hb_buffer_destroy (reconstruction); - hb_buffer_destroy (fragment); - - return ret; - } - - bool verify_buffer_unsafe_to_concat (hb_buffer_t *buffer, - hb_buffer_t *text_buffer, - hb_font_t *font, - const char **error=nullptr) - { - if (cluster_level != HB_BUFFER_CLUSTER_LEVEL_MONOTONE_GRAPHEMES && - cluster_level != HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS) - { - /* Cannot perform this check without monotone clusters. */ - return true; - } - - /* Check that shuffling up text before shaping at safe-to-concat points - * is indeed safe. */ - - /* This is what we do: - * - * 1. We shape text once. Then segment the text at all the safe-to-concat - * points; - * - * 2. Then we create two buffers, one containing all the even segments and - * one all the odd segments. - * - * 3. Because all these segments were safe-to-concat at both ends, we - * expect that concatenating them and shaping should NOT change the - * shaping results of each segment. As such, we expect that after - * shaping the two buffers, we still get cluster boundaries at the - * segment boundaries, and that those all are safe-to-concat points. - * Moreover, that there are NOT any safe-to-concat points within the - * segments. - * - * 4. Finally, we reconstruct the shaping results of the original text by - * simply interleaving the shaping results of the segments from the two - * buffers, and assert that the total shaping results is the same as - * the one from original buffer in step 1. - */ - - hb_buffer_t *fragments[2] {hb_buffer_create_similar (buffer), - hb_buffer_create_similar (buffer)}; - hb_buffer_t *reconstruction = hb_buffer_create_similar (buffer); - hb_segment_properties_t props; - hb_buffer_get_segment_properties (buffer, &props); - hb_buffer_set_segment_properties (fragments[0], &props); - hb_buffer_set_segment_properties (fragments[1], &props); - hb_buffer_set_segment_properties (reconstruction, &props); - - unsigned num_glyphs; - hb_glyph_info_t *info = hb_buffer_get_glyph_infos (buffer, &num_glyphs); - - unsigned num_chars; - hb_glyph_info_t *text = hb_buffer_get_glyph_infos (text_buffer, &num_chars); - - bool forward = HB_DIRECTION_IS_FORWARD (hb_buffer_get_direction (buffer)); - - if (!forward) - hb_buffer_reverse (buffer); - - /* - * Split text into segments and collect into to fragment streams. - */ - { - unsigned fragment_idx = 0; - unsigned start = 0; - unsigned text_start = 0; - unsigned text_end = 0; - for (unsigned end = 1; end < num_glyphs + 1; end++) - { - if (end < num_glyphs && - (info[end].cluster == info[end-1].cluster || - info[end].mask & HB_GLYPH_FLAG_UNSAFE_TO_CONCAT)) - continue; - - /* Accumulate segment corresponding to glyphs start..end. */ - if (end == num_glyphs) - text_end = num_chars; - else - { - unsigned cluster = info[end].cluster; - while (text_end < num_chars && text[text_end].cluster < cluster) - text_end++; - } - assert (text_start < text_end); - - if (0) - printf("start %d end %d text start %d end %d\n", start, end, text_start, text_end); - -#if 0 - hb_buffer_flags_t flags = hb_buffer_get_flags (fragment); - if (0 < text_start) - flags = (hb_buffer_flags_t) (flags & ~HB_BUFFER_FLAG_BOT); - if (text_end < num_chars) - flags = (hb_buffer_flags_t) (flags & ~HB_BUFFER_FLAG_EOT); - hb_buffer_set_flags (fragment, flags); -#endif - - hb_buffer_append (fragments[fragment_idx], text_buffer, text_start, text_end); - - start = end; - text_start = text_end; - fragment_idx = 1 - fragment_idx; - } - } - - bool ret = true; - hb_buffer_diff_flags_t diff; - - /* - * Shape the two fragment streams. - */ - if (!hb_shape_full (font, fragments[0], features, num_features, shapers) || - !hb_shape_full (font, fragments[1], features, num_features, shapers)) - { - if (error) - *error = "All shapers failed while shaping fragments."; - ret = false; - goto out; - } - - if (!forward) - { - hb_buffer_reverse (fragments[0]); - hb_buffer_reverse (fragments[1]); - } - - /* - * Reconstruct results. - */ - { - unsigned fragment_idx = 0; - unsigned fragment_start[2] {0, 0}; - unsigned fragment_num_glyphs[2]; - hb_glyph_info_t *fragment_info[2]; - for (unsigned i = 0; i < 2; i++) - fragment_info[i] = hb_buffer_get_glyph_infos (fragments[i], &fragment_num_glyphs[i]); - while (fragment_start[0] < fragment_num_glyphs[0] || - fragment_start[1] < fragment_num_glyphs[1]) - { - unsigned fragment_end = fragment_start[fragment_idx] + 1; - while (fragment_end < fragment_num_glyphs[fragment_idx] && - (fragment_info[fragment_idx][fragment_end].cluster == fragment_info[fragment_idx][fragment_end - 1].cluster || - fragment_info[fragment_idx][fragment_end].mask & HB_GLYPH_FLAG_UNSAFE_TO_CONCAT)) - fragment_end++; - - hb_buffer_append (reconstruction, fragments[fragment_idx], fragment_start[fragment_idx], fragment_end); - - fragment_start[fragment_idx] = fragment_end; - fragment_idx = 1 - fragment_idx; - } - } - - if (!forward) - { - hb_buffer_reverse (buffer); - hb_buffer_reverse (reconstruction); - } - - /* - * Diff results. - */ - diff = hb_buffer_diff (reconstruction, buffer, (hb_codepoint_t) -1, 0); - if (diff) - { - if (error) - *error = "unsafe-to-concat test failed."; - ret = false; - - /* Return the reconstructed result instead so it can be inspected. */ - hb_buffer_set_length (buffer, 0); - hb_buffer_append (buffer, reconstruction, 0, -1); - } - - - out: - hb_buffer_destroy (reconstruction); - hb_buffer_destroy (fragments[0]); - hb_buffer_destroy (fragments[1]); - - return ret; - } - void shape_closure (const char *text, int text_len, hb_font_t *font, hb_buffer_t *buffer, hb_set_t *glyphs) From 5b1d813b698488fb86b4f20a596bb1c046e61eed Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 28 Jan 2022 13:49:21 -0700 Subject: [PATCH 03/10] [config] Enable HB_NO_BUFFER_VERIFY in HB_LEAN --- src/hb-config.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hb-config.hh b/src/hb-config.hh index 7d00d9088..5141ad834 100644 --- a/src/hb-config.hh +++ b/src/hb-config.hh @@ -55,6 +55,7 @@ #define HB_NO_ATEXIT #define HB_NO_BUFFER_MESSAGE #define HB_NO_BUFFER_SERIALIZE +#define HB_NO_BUFFER_VERIFY #define HB_NO_BITMAP #define HB_NO_CFF #define HB_NO_COLOR From 6596e42d160a0ae2cd2cd3b42a9f8823197cd716 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 28 Jan 2022 13:55:24 -0700 Subject: [PATCH 04/10] [fuzz] Verify shape results --- test/fuzzing/hb-shape-fuzzer.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/fuzzing/hb-shape-fuzzer.cc b/test/fuzzing/hb-shape-fuzzer.cc index 661ea9ffa..2952a93ec 100644 --- a/test/fuzzing/hb-shape-fuzzer.cc +++ b/test/fuzzing/hb-shape-fuzzer.cc @@ -33,6 +33,7 @@ extern "C" int LLVMFuzzerTestOneInput (const uint8_t *data, size_t size) { const char text[] = "ABCDEXYZ123@_%&)*$!"; hb_buffer_t *buffer = hb_buffer_create (); + hb_buffer_set_flags (buffer, HB_BUFFER_FLAG_VERIFY); hb_buffer_add_utf8 (buffer, text, -1, 0, -1); hb_buffer_guess_segment_properties (buffer); hb_shape (font, buffer, nullptr, 0); @@ -50,6 +51,7 @@ extern "C" int LLVMFuzzerTestOneInput (const uint8_t *data, size_t size) text32[10] = test_font (font, text32[15]) % 256; hb_buffer_t *buffer = hb_buffer_create (); + hb_buffer_set_flags (buffer, HB_BUFFER_FLAG_VERIFY); hb_buffer_add_utf32 (buffer, text32, sizeof (text32) / sizeof (text32[0]), 0, -1); hb_buffer_guess_segment_properties (buffer); hb_shape (font, buffer, nullptr, 0); From 61856359cb90f4d53eced1159b0810defa342ec6 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 28 Jan 2022 14:07:29 -0700 Subject: [PATCH 05/10] [fuzz] Disable verification for now. --- test/fuzzing/hb-shape-fuzzer.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/fuzzing/hb-shape-fuzzer.cc b/test/fuzzing/hb-shape-fuzzer.cc index 2952a93ec..0022a89ec 100644 --- a/test/fuzzing/hb-shape-fuzzer.cc +++ b/test/fuzzing/hb-shape-fuzzer.cc @@ -33,7 +33,7 @@ extern "C" int LLVMFuzzerTestOneInput (const uint8_t *data, size_t size) { const char text[] = "ABCDEXYZ123@_%&)*$!"; hb_buffer_t *buffer = hb_buffer_create (); - hb_buffer_set_flags (buffer, HB_BUFFER_FLAG_VERIFY); + // hb_buffer_set_flags (buffer, HB_BUFFER_FLAG_VERIFY); hb_buffer_add_utf8 (buffer, text, -1, 0, -1); hb_buffer_guess_segment_properties (buffer); hb_shape (font, buffer, nullptr, 0); @@ -51,7 +51,7 @@ extern "C" int LLVMFuzzerTestOneInput (const uint8_t *data, size_t size) text32[10] = test_font (font, text32[15]) % 256; hb_buffer_t *buffer = hb_buffer_create (); - hb_buffer_set_flags (buffer, HB_BUFFER_FLAG_VERIFY); +// hb_buffer_set_flags (buffer, HB_BUFFER_FLAG_VERIFY); hb_buffer_add_utf32 (buffer, text32, sizeof (text32) / sizeof (text32[0]), 0, -1); hb_buffer_guess_segment_properties (buffer); hb_shape (font, buffer, nullptr, 0); From 476a6377a574291025ce8acc0cecfc53408c8d3c Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Fri, 28 Jan 2022 15:05:10 -0700 Subject: [PATCH 06/10] [buffer] Document HB_BUFFER_FLAG_VERIFY --- src/hb-buffer.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/hb-buffer.h b/src/hb-buffer.h index fb76c9007..8cd43f2dd 100644 --- a/src/hb-buffer.h +++ b/src/hb-buffer.h @@ -357,6 +357,14 @@ hb_buffer_guess_segment_properties (hb_buffer_t *buffer); * flag indicating that a dotted circle should * not be inserted in the rendering of incorrect * character sequences (such at <0905 093E>). Since: 2.4 + * @HB_BUFFER_FLAG_VERIFY: + * flag indicating that the hb_shape() call and its variants + * should perform various verification processes on the results + * of the shaping operation on the buffer. If the verification + * fails, then either a buffer message is sent, if a message + * handler is installed on the buffer, or a message is written + * to standard error. In either case, the shaping result might + * be modified to show the failed output. * * Flags for #hb_buffer_t. * From 3972e0a8f1504783a509096e069ca718d25af8d9 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sat, 29 Jan 2022 08:22:19 -0700 Subject: [PATCH 07/10] [buffer] Whitespace --- src/hb-buffer.h | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/hb-buffer.h b/src/hb-buffer.h index 8cd43f2dd..ec9235f19 100644 --- a/src/hb-buffer.h +++ b/src/hb-buffer.h @@ -628,24 +628,24 @@ hb_buffer_serialize_glyphs (hb_buffer_t *buffer, HB_EXTERN unsigned int hb_buffer_serialize_unicode (hb_buffer_t *buffer, - unsigned int start, - unsigned int end, - char *buf, - unsigned int buf_size, - unsigned int *buf_consumed, - hb_buffer_serialize_format_t format, - hb_buffer_serialize_flags_t flags); + unsigned int start, + unsigned int end, + char *buf, + unsigned int buf_size, + unsigned int *buf_consumed, + hb_buffer_serialize_format_t format, + hb_buffer_serialize_flags_t flags); HB_EXTERN unsigned int hb_buffer_serialize (hb_buffer_t *buffer, - unsigned int start, - unsigned int end, - char *buf, - unsigned int buf_size, - unsigned int *buf_consumed, - hb_font_t *font, - hb_buffer_serialize_format_t format, - hb_buffer_serialize_flags_t flags); + unsigned int start, + unsigned int end, + char *buf, + unsigned int buf_size, + unsigned int *buf_consumed, + hb_font_t *font, + hb_buffer_serialize_format_t format, + hb_buffer_serialize_flags_t flags); HB_EXTERN hb_bool_t hb_buffer_deserialize_glyphs (hb_buffer_t *buffer, @@ -657,10 +657,10 @@ hb_buffer_deserialize_glyphs (hb_buffer_t *buffer, HB_EXTERN hb_bool_t hb_buffer_deserialize_unicode (hb_buffer_t *buffer, - const char *buf, - int buf_len, - const char **end_ptr, - hb_buffer_serialize_format_t format); + const char *buf, + int buf_len, + const char **end_ptr, + hb_buffer_serialize_format_t format); From d35f380126830872611c85d664c3710deb46cd6b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sat, 29 Jan 2022 09:08:20 -0700 Subject: [PATCH 08/10] [util] Change "All shapers failed." message to "Shaping failed." Since we now emit this when verification fails as well. --- util/shape-options.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/shape-options.hh b/util/shape-options.hh index ec32425dc..806b34b62 100644 --- a/util/shape-options.hh +++ b/util/shape-options.hh @@ -94,7 +94,7 @@ struct shape_options_t if (!hb_shape_full (font, buffer, features, num_features, shapers)) { if (error) - *error = "All shapers failed."; + *error = "Shaping failed."; goto fail; } From e986c12075a69300a5e114fe139ae5acd762ef1b Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sat, 29 Jan 2022 09:08:48 -0700 Subject: [PATCH 09/10] [verify] Show buffer input text when verification fails --- src/hb-buffer-verify.cc | 50 +++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/src/hb-buffer-verify.cc b/src/hb-buffer-verify.cc index 905e54877..c97729403 100644 --- a/src/hb-buffer-verify.cc +++ b/src/hb-buffer-verify.cc @@ -31,15 +31,32 @@ #include "hb-buffer.hh" +#define BUFFER_VERIFY_ERROR "buffer verify error: " static inline void buffer_verify_error (hb_buffer_t *buffer, hb_font_t *font, - const char *message) + const char *fmt, + ...) HB_PRINTF_FUNC(3, 4); + +static inline void +buffer_verify_error (hb_buffer_t *buffer, + hb_font_t *font, + const char *fmt, + ...) { + va_list ap; + va_start (ap, fmt); if (buffer->messaging ()) - buffer->message (font, "%s", message); + { + buffer->message_impl (font, fmt, ap); + } else - fprintf (stderr, "%s\n", message); + { + fprintf (stderr, "harfbuzz "); + vfprintf (stderr, fmt, ap); + fprintf (stderr, "\n"); + } + va_end (ap); } static bool @@ -59,7 +76,7 @@ buffer_verify_monotone (hb_buffer_t *buffer, if (info[i-1].cluster != info[i].cluster && (info[i-1].cluster < info[i].cluster) != is_forward) { - buffer_verify_error (buffer, font, "clusters are not monotone."); + buffer_verify_error (buffer, font, BUFFER_VERIFY_ERROR "clusters are not monotone."); return false; } } @@ -147,7 +164,7 @@ buffer_verify_unsafe_to_break (hb_buffer_t *buffer, hb_buffer_append (fragment, text_buffer, text_start, text_end); if (!hb_shape_full (font, fragment, features, num_features, shapers)) { - buffer_verify_error (buffer, font, "All shapers failed while shaping fragment."); + buffer_verify_error (buffer, font, BUFFER_VERIFY_ERROR "shaping failed while shaping fragment."); hb_buffer_destroy (reconstruction); hb_buffer_destroy (fragment); return false; @@ -165,7 +182,7 @@ buffer_verify_unsafe_to_break (hb_buffer_t *buffer, hb_buffer_diff_flags_t diff = hb_buffer_diff (reconstruction, buffer, (hb_codepoint_t) -1, 0); if (diff) { - buffer_verify_error (buffer, font, "unsafe-to-break test failed."); + buffer_verify_error (buffer, font, BUFFER_VERIFY_ERROR "unsafe-to-break test failed."); ret = false; /* Return the reconstructed result instead so it can be inspected. */ @@ -296,13 +313,13 @@ buffer_verify_unsafe_to_concat (hb_buffer_t *buffer, */ if (!hb_shape_full (font, fragments[0], features, num_features, shapers)) { - buffer_verify_error (buffer, font, "All shapers failed while shaping fragment."); + buffer_verify_error (buffer, font, BUFFER_VERIFY_ERROR "shaping failed while shaping fragment."); ret = false; goto out; } if (!hb_shape_full (font, fragments[1], features, num_features, shapers)) { - buffer_verify_error (buffer, font, "All shapers failed while shaping fragment."); + buffer_verify_error (buffer, font, BUFFER_VERIFY_ERROR "shaping failed while shaping fragment."); ret = false; goto out; } @@ -351,7 +368,7 @@ buffer_verify_unsafe_to_concat (hb_buffer_t *buffer, diff = hb_buffer_diff (reconstruction, buffer, (hb_codepoint_t) -1, 0); if (diff) { - buffer_verify_error (buffer, font, "unsafe-to-concat test failed."); + buffer_verify_error (buffer, font, BUFFER_VERIFY_ERROR "unsafe-to-concat test failed."); ret = false; /* Return the reconstructed result instead so it can be inspected. */ @@ -382,6 +399,21 @@ hb_buffer_t::verify (hb_buffer_t *text_buffer, ret = false; if (!buffer_verify_unsafe_to_concat (this, text_buffer, font, features, num_features, shapers)) ret = false; + if (!ret) + { + unsigned len = text_buffer->len; + hb_vector_t bytes; + if (likely (bytes.resize (len * 10 + 16))) + { + hb_buffer_serialize_unicode (text_buffer, + 0, len, + bytes.arrayZ, bytes.length, + &len, + HB_BUFFER_SERIALIZE_FORMAT_TEXT, + HB_BUFFER_SERIALIZE_FLAG_NO_CLUSTERS); + buffer_verify_error (this, font, BUFFER_VERIFY_ERROR "text was: %s.", bytes.arrayZ); + } + } return ret; } From b47b3b99725888fd27273d8d9b9ee3d5d6cf0400 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Sat, 29 Jan 2022 10:24:38 -0700 Subject: [PATCH 10/10] [fallback-kern] Move buffer message to correct position --- src/hb-ot-shape-fallback.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hb-ot-shape-fallback.cc b/src/hb-ot-shape-fallback.cc index 671f30327..b2eedb027 100644 --- a/src/hb-ot-shape-fallback.cc +++ b/src/hb-ot-shape-fallback.cc @@ -497,14 +497,14 @@ _hb_ot_shape_fallback_kern (const hb_ot_shape_plan_t *plan, #endif #ifndef HB_DISABLE_DEPRECATED - if (!buffer->message (font, "start fallback kern")) - return; - if (HB_DIRECTION_IS_HORIZONTAL (buffer->props.direction) ? !font->has_glyph_h_kerning_func () : !font->has_glyph_v_kerning_func ()) return; + if (!buffer->message (font, "start fallback kern")) + return; + bool reverse = HB_DIRECTION_IS_BACKWARD (buffer->props.direction); if (reverse)