Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Automatic fractions are broken for RTL scripts #395

Closed
khaledhosny opened this issue Jan 14, 2017 · 3 comments
Closed

Automatic fractions are broken for RTL scripts #395

khaledhosny opened this issue Jan 14, 2017 · 3 comments

Comments

@khaledhosny
Copy link
Collaborator

Automatic fractions (with fraction slash) seem to be incorrectly handled for RTL scripts; it seems the numr/dnom features are reversed:

$ hb-shape amiri-regular.ttf "123⁄456" --direction=ltr --script=latn
[one.numr|two.numr|three.numr|fraction|four.small|five.small|six.small]

latn

$ hb-shape amiri-regular.ttf "123⁄456" --direction=ltr --script=arab
[one.small|two.small|three.small|fraction|four.numr|five.numr|six.numr]

arab

@khaledhosny
Copy link
Collaborator Author

Some thing like this does the trick, though I’m not sure if it is the right approach:

diff --git a/src/hb-ot-shape.cc b/src/hb-ot-shape.cc
index ddd6662e..6b38739c 100644
--- a/src/hb-ot-shape.cc
+++ b/src/hb-ot-shape.cc
@@ -362,6 +362,18 @@ hb_ot_shape_setup_masks_fraction (hb_ot_shape_context_t *c)
 
   hb_buffer_t *buffer = c->buffer;
 
+  hb_mask_t pre_mask, post_mask;
+  if (HB_DIRECTION_IS_FORWARD (buffer->props.direction))
+  {
+    pre_mask = c->plan->numr_mask | c->plan->frac_mask;
+    post_mask = c->plan->frac_mask | c->plan->dnom_mask;
+  }
+  else
+  {
+    pre_mask = c->plan->frac_mask | c->plan->dnom_mask;
+    post_mask = c->plan->numr_mask | c->plan->frac_mask;
+  }
+
   /* TODO look in pre/post context text also. */
   unsigned int count = buffer->len;
   hb_glyph_info_t *info = buffer->info;
@@ -380,10 +392,10 @@ hb_ot_shape_setup_masks_fraction (hb_ot_shape_context_t *c)
         end++;
 
       for (unsigned int j = start; j < i; j++)
-        info[j].mask |= c->plan->numr_mask | c->plan->frac_mask;
+        info[j].mask |= pre_mask;
       info[i].mask |= c->plan->frac_mask;
       for (unsigned int j = i + 1; j < end; j++)
-        info[j].mask |= c->plan->frac_mask | c->plan->dnom_mask;
+        info[j].mask |= post_mask;
 
       i = end - 1;
     }

@KrasnayaPloshchad
Copy link

Is it possible to avoid it by RLM/LRM characters?

@behdad
Copy link
Member

behdad commented Jan 17, 2017

Some thing like this does the trick, though I’m not sure if it is the right approach:

Sounds about right. This is one of the artefacts of the fact that we process Arabic numbers in RTL direction. The reason MS doesn't suffer from such problems is that their itemizer attaches a distinct script code to Arabic numerals, so shaper knows that they should be processed LTR, not RTL.

behdad pushed a commit that referenced this issue Jan 18, 2017
The numbers for right-to-left scripts are processed also from right to
left, so the order of applying “numr” and “dnom” features should be
reversed in such case.

Fixes #395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants