[HarfBuzz] mirroring support in hb_shape

[Behdad Esfahbod]

Thanks Jonathan.  Fixed it by doing substitute_default() before
ensure_native_direction().

behdad


On 04/02/2010 05:53 AM, Jonathan Kew wrote:
> Hi Behdad,
> 
> I think there's an issue with how (or rather *where*) mirroring is implemented -- currently hb_mirror_chars is called from hb_substitute_default, right before hb_map_glyphs. The problem with this is that by the time hb_substitute_default is called, the buffer has been forced to "native" direction, but AFAICS we need the mirroring to be based on the original direction instead.
> 
> This causes problems if we have text such as <RLO>Hello (world)<PDF>, which will form a RTL run; hb_ensure_native_direction will reverse it, in order that OT lookups can work properly, but this will prevent the parens being mirrored because hb_mirror_chars will see the run as being LTR.
> 
> To resolve this, I've moved the call to hb_mirror_chars out of hb_substitute_default, and put it before hb_ensure_native_direction, as shown below. Please see if you think this is a reasonable thing to do.
> 
> JK
> 
> diff --git a/src/hb-shape.c b/src/hb-shape.c
> --- a/src/hb-shape.c
> +++ b/src/hb-shape.c
> @@ -111,17 +111,16 @@ hb_map_glyphs (hb_font_t    *font,
>  
>  static void
>  hb_substitute_default (hb_font_t    *font,
>  		       hb_face_t    *face,
>  		       hb_buffer_t  *buffer,
>  		       hb_feature_t *features,
>  		       unsigned int  num_features)
>  {
> -  hb_mirror_chars (buffer);
>    hb_map_glyphs (font, face, buffer);
>  }
>  
>  static hb_bool_t
>  hb_substitute_complex (hb_font_t    *font,
>  		       hb_face_t    *face,
>  		       hb_buffer_t  *buffer,
>  		       hb_feature_t *features,
> @@ -224,18 +223,20 @@ hb_shape (hb_font_t    *font,
>  	  hb_buffer_t  *buffer,
>  	  hb_feature_t *features,
>  	  unsigned int  num_features)
>  {
>    hb_direction_t original_direction;
>    hb_bool_t substitute_fallback, position_fallback;
>  
>    hb_form_clusters (buffer);
> +
> +  hb_mirror_chars (buffer);
> +
>    original_direction = hb_ensure_native_direction (buffer);
> -
>    hb_substitute_default (font, face, buffer, features, num_features);
>  
>    substitute_fallback = !hb_substitute_complex (font, face, buffer, features, num_features);
>  
>    if (substitute_fallback)
>      hb_substitute_fallback (font, face, buffer, features, num_features);
>  
>    hb_position_default (font, face, buffer, features, num_features);
> 
>  
> _______________________________________________
> HarfBuzz mailing list
> HarfBuzz at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/harfbuzz
>