From 2188a4bf48d802b516f4721a02a7b080edc8f46d Mon Sep 17 00:00:00 2001
From: Laurenz <laurmaedje@gmail.com>
Date: Fri, 17 May 2024 10:36:07 +0200
Subject: [PATCH] Fix equations in RTL text (#4150)

---
 crates/typst/src/layout/inline/mod.rs |  32 +++++++++++++++++++++-----
 crates/typst/src/math/equation.rs     |   6 +++++
 crates/typst/src/math/row.rs          |  10 ++++----
 tests/ref/issue-3696-equation-rtl.png | Bin 0 -> 660 bytes
 tests/ref/math-at-line-end.png        | Bin 0 -> 479 bytes
 tests/ref/math-at-line-start.png      | Bin 0 -> 476 bytes
 tests/ref/math-at-par-end.png         | Bin 0 -> 450 bytes
 tests/ref/math-at-par-start.png       | Bin 0 -> 418 bytes
 tests/ref/math-consecutive.png        | Bin 0 -> 176 bytes
 tests/ref/math-linebreaking-empty.png | Bin 615 -> 664 bytes
 tests/suite/math/equation.typ         |   5 ++++
 tests/suite/math/interactions.typ     |  20 ++++++++++++++++
 12 files changed, 61 insertions(+), 12 deletions(-)
 create mode 100644 tests/ref/issue-3696-equation-rtl.png
 create mode 100644 tests/ref/math-at-line-end.png
 create mode 100644 tests/ref/math-at-line-start.png
 create mode 100644 tests/ref/math-at-par-end.png
 create mode 100644 tests/ref/math-at-par-start.png
 create mode 100644 tests/ref/math-consecutive.png

diff --git a/crates/typst/src/layout/inline/mod.rs b/crates/typst/src/layout/inline/mod.rs
index f8b17f463..5a74d3d24 100644
--- a/crates/typst/src/layout/inline/mod.rs
+++ b/crates/typst/src/layout/inline/mod.rs
@@ -102,6 +102,13 @@ type Range = std::ops::Range<usize>;
 const SPACING_REPLACE: char = ' '; // Space
 const OBJ_REPLACE: char = '\u{FFFC}'; // Object Replacement Character
 
+// Unicode BiDi control characters.
+const LTR_EMBEDDING: char = '\u{202A}';
+const RTL_EMBEDDING: char = '\u{202B}';
+const POP_EMBEDDING: char = '\u{202C}';
+const LTR_ISOLATE: char = '\u{2066}';
+const POP_ISOLATE: char = '\u{2069}';
+
 /// A paragraph representation in which children are already layouted and text
 /// is already preshaped.
 ///
@@ -207,9 +214,12 @@ impl Segment<'_> {
             Self::Box(_, frac) => {
                 (if frac { SPACING_REPLACE } else { OBJ_REPLACE }).len_utf8()
             }
-            Self::Equation(ref par_items) => {
-                par_items.iter().map(MathParItem::text).map(char::len_utf8).sum()
-            }
+            Self::Equation(ref par_items) => par_items
+                .iter()
+                .map(MathParItem::text)
+                .chain([LTR_ISOLATE, POP_ISOLATE])
+                .map(char::len_utf8)
+                .sum(),
             Self::Meta => 0,
         }
     }
@@ -228,6 +238,9 @@ enum Item<'a> {
     Frame(Frame),
     /// Metadata.
     Meta(Frame),
+    /// An item that is invisible and needs to be skipped, e.g. a Unicode
+    /// isolate.
+    Skip(char),
 }
 
 impl<'a> Item<'a> {
@@ -254,6 +267,7 @@ impl<'a> Item<'a> {
             Self::Absolute(_) | Self::Fractional(_, _) => SPACING_REPLACE.len_utf8(),
             Self::Frame(_) => OBJ_REPLACE.len_utf8(),
             Self::Meta(_) => 0,
+            Self::Skip(c) => c.len_utf8(),
         }
     }
 
@@ -264,6 +278,7 @@ impl<'a> Item<'a> {
             Self::Absolute(v) => *v,
             Self::Frame(frame) => frame.width(),
             Self::Fractional(_, _) | Self::Meta(_) => Abs::zero(),
+            Self::Skip(_) => Abs::zero(),
         }
     }
 }
@@ -466,8 +481,8 @@ fn collect<'a>(
             if dir != outer_dir {
                 // Insert "Explicit Directional Embedding".
                 match dir {
-                    Dir::LTR => full.push('\u{202A}'),
-                    Dir::RTL => full.push('\u{202B}'),
+                    Dir::LTR => full.push(LTR_EMBEDDING),
+                    Dir::RTL => full.push(RTL_EMBEDDING),
                     _ => {}
                 }
             }
@@ -480,7 +495,7 @@ fn collect<'a>(
 
             if dir != outer_dir {
                 // Insert "Pop Directional Formatting".
-                full.push('\u{202C}');
+                full.push(POP_EMBEDDING);
             }
             Segment::Text(full.len() - prev)
         } else if let Some(elem) = child.to_packed::<HElem>() {
@@ -535,7 +550,9 @@ fn collect<'a>(
                 let MathParItem::Frame(frame) = item else { continue };
                 frame.meta(styles, false);
             }
+            full.push(LTR_ISOLATE);
             full.extend(items.iter().map(MathParItem::text));
+            full.push(POP_ISOLATE);
             Segment::Equation(items)
         } else if let Some(elem) = child.to_packed::<BoxElem>() {
             let frac = elem.width(styles).is_fractional();
@@ -608,6 +625,7 @@ fn prepare<'a>(
                 }
             },
             Segment::Equation(par_items) => {
+                items.push(Item::Skip(LTR_ISOLATE));
                 for item in par_items {
                     match item {
                         MathParItem::Space(s) => items.push(Item::Absolute(s)),
@@ -617,6 +635,7 @@ fn prepare<'a>(
                         }
                     }
                 }
+                items.push(Item::Skip(POP_ISOLATE));
             }
             Segment::Box(elem, _) => {
                 if let Sizing::Fr(v) = elem.width(styles) {
@@ -1407,6 +1426,7 @@ fn commit(
             Item::Frame(frame) | Item::Meta(frame) => {
                 push(&mut offset, frame.clone());
             }
+            Item::Skip(_) => {}
         }
     }
 
diff --git a/crates/typst/src/math/equation.rs b/crates/typst/src/math/equation.rs
index eb3fcc4c6..947f01f25 100644
--- a/crates/typst/src/math/equation.rs
+++ b/crates/typst/src/math/equation.rs
@@ -223,6 +223,12 @@ impl Packed<EquationElem> {
             vec![MathParItem::Frame(run.into_fragment(&ctx, styles).into_frame())]
         };
 
+        // An empty equation should have a height, so we still create a frame
+        // (which is then resized in the loop).
+        if items.is_empty() {
+            items.push(MathParItem::Frame(Frame::soft(Size::zero())));
+        }
+
         for item in &mut items {
             let MathParItem::Frame(frame) = item else { continue };
 
diff --git a/crates/typst/src/math/row.rs b/crates/typst/src/math/row.rs
index 59661f722..6454f491e 100644
--- a/crates/typst/src/math/row.rs
+++ b/crates/typst/src/math/row.rs
@@ -3,7 +3,7 @@ use std::iter::once;
 use unicode_math_class::MathClass;
 
 use crate::foundations::{Resolve, StyleChain};
-use crate::layout::{Abs, AlignElem, Em, Frame, FrameKind, Point, Size};
+use crate::layout::{Abs, AlignElem, Em, Frame, Point, Size};
 use crate::math::{
     alignments, scaled_font_size, spacing, EquationElem, FrameFragment, MathContext,
     MathFragment, MathParItem, MathSize,
@@ -257,7 +257,7 @@ impl MathRun {
         let mut x = Abs::zero();
         let mut ascent = Abs::zero();
         let mut descent = Abs::zero();
-        let mut frame = Frame::new(Size::zero(), FrameKind::Soft);
+        let mut frame = Frame::soft(Size::zero());
         let mut empty = true;
 
         let finalize_frame = |frame: &mut Frame, x, ascent, descent| {
@@ -301,10 +301,8 @@ impl MathRun {
                 || (class == MathClass::Relation
                     && !iter.peek().map(is_relation).unwrap_or_default())
             {
-                let mut frame_prev = std::mem::replace(
-                    &mut frame,
-                    Frame::new(Size::zero(), FrameKind::Soft),
-                );
+                let mut frame_prev =
+                    std::mem::replace(&mut frame, Frame::soft(Size::zero()));
 
                 finalize_frame(&mut frame_prev, x, ascent, descent);
                 items.push(MathParItem::Frame(frame_prev));
diff --git a/tests/ref/issue-3696-equation-rtl.png b/tests/ref/issue-3696-equation-rtl.png
new file mode 100644
index 0000000000000000000000000000000000000000..1ebf2dc217216952319062bfbc58a17dedc8da9d
GIT binary patch
literal 660
zcmV;F0&D$=P)<h;3K|Lk000e1NJLTq005Q%000{Z00000yF@8C00078Nkl<Zc-rmO
z+e=dc9Kdn^j-WrF-pZZ~A{G_+u$KZs2*JoGlofQb@RAqGWoAlwS)yakL_KDWG-;V6
zh|B59wY+6WYFeiw4ZLj5v-8b|oxx=43Ja0*efm9qUVcA*aqP(AG?vEFSQ_ikSVHCs
zo3&V1O{o4<BAdE2@UK|;0XA>A6t#1#tKas2Su?c{cOvUo-}%31<qAUgXtu?+JY=+3
zrCdvk7x?L<N^E%|v`V%I^<7FVtd0g3;aUK2sVsl(ooN-3*%g%)m_cb4KB1PX(;c2k
z((g|jKBZ3(F6tAH1Cs~5Hxk>A>2&qI3Hm*cV_;P@ti+?H6hyKlQ4|#bme!`FXe>_w
z=`MFN5UvT}GEj*Ls=OelA2lxm&Pm@h7P4Ys?UOwQ))i)1OeRwRfPuBi6^#Y6M`$*o
z))*ucpcu%Y66_+hyRRJz&c2~$!s!6x)+}El_gkH)jf7bXO8_gYz;^(=fFuJ`_(Vis
zh~;&-4V-;9PN-x^0Vo8LFt}$k0Q(72@~j=y_Pl6<!mU|UKfhh{t+&YD44@<djAoy1
zqbUa%w&&ZB5BdS4!vJ?{KsrNrG521FSd(T-TCY?D+RE&m0N0@ipd5nbc;ffg0bdeU
z<-Eorgo%?@D35g$Kc^`dSl0mHk@{}IAi$#QDk$$a+{BKOA3587AYQfgmwAZj>l}cb
zJr53mVCPNv;NOr{I&QW~+r{bwfN7t9kSNK<k1qjW`4RfaBe!ghV2v?9#x}7=p02W4
ugIQU<q48~d4-rYioc7pgERD50Sib@2V?a`3f}{li0000<MNUMnLSTYiX({ah

literal 0
HcmV?d00001

diff --git a/tests/ref/math-at-line-end.png b/tests/ref/math-at-line-end.png
new file mode 100644
index 0000000000000000000000000000000000000000..6e4173a7f96284b32860c5a6ef9b0b85f87b8e46
GIT binary patch
literal 479
zcmV<50U-W~P)<h;3K|Lk000e1NJLTq004LZ001Zm000005P@O&0004|Nkl<Zc-rmM
z&nv@m9LI4NSO0{QgF^Bnj6xA2Kaz5A5r&!)5wdKWUz;t>?-`4!S&X8TWI6cNC=^>5
zYkr0K@n!qk_I;7ta`o8`KF{O(@~HRW6@&m4L}3b3n8Lq$@&i2H2G|;8J~w?le0nzs
z{vKw7yMdk?-y07;IbWg)yjl>x205df=lb`|l8esMMd>>mqtkm~(XL_G*!KVcBw*1=
z=#`u~EXKf~Ct_tk>QbU-!#+CN@AzDfbu7ck5QDK0BVo=<pc)3x?Z|*;faT#Rd%AF1
z^ROTW2*KWVpe2O4&W2Kf1okl<YQ;CK6E9~1+?<FQtjs?HSL#BgxUpb6eo32bttflR
zQun$zzCh*R7uf*34keDb=yFl`|H89x0fN_A05})<3qGE`w5Uv5>iba#RwwL=R0G!4
zR`T#(_h+oTdS=q%Ot2Yc=8c_d90=_z<l(yJDLLm!GXVMa+oUe{ZVjGOge-ZuSpwj;
zCyzs(&Eyu0?N(vm(<&to+tY+I=G>?9+Ji`$Qd)1ikJD;8n>KZQbU-LfVG2|Dr!T!j
VNQ7b3mBRo4002ovPDHLkV1j?0;5h&Q

literal 0
HcmV?d00001

diff --git a/tests/ref/math-at-line-start.png b/tests/ref/math-at-line-start.png
new file mode 100644
index 0000000000000000000000000000000000000000..05221db179ba389d20140071e83d5ccd264f6e5e
GIT binary patch
literal 476
zcmV<20VDp2P)<h;3K|Lk000e1NJLTq004LZ001Zm000005P@O&0004_Nkl<Zc-rmM
zNlOB87zXg5uh2*65EMnxLP$n+C@2bpHo6tnAqs@3Ca9o{F73OG)U+=as6=#VgCGjQ
z#9#`sP*hR`wH$?UdjfABI?i<P{~X>I-pdc(!;^-8N*aL)Oke{49m}-{@LG8g_U5Ui
zn^7VHjEpv5-j{UZrDDJn4RcuJREoG<%huoZ9j<pf_$-F<KzB5Q!R2<FP_W`B=YEkh
zw_<YxrL4U;rrhOAaMH~--APUA77l{JU^C_1XB)PZkVbqp*jwSHc$Mw+Iqgx+G=zdf
z6mXu-c`HDTqA2L906oh~u(q$y=L6qIV}ik|;SXRl0@ajy;<^7yBezbD2gVo;a5AJ0
zPf~W$Tm()nIHn&)Qob(4f%*3WB*p)fSd>ffR~IH`M61Io6WV^cy@c~v=g3>KxlhkT
zfO~v%dxk2{EVP|y1dUoZT6tP{*q?Qve1xVd4pSHAt4*l$fUeXF4+k^C0CqZVVC;l}
z%}1RZ21{N{I5<k@u6hA1$kyMM6z1%QQHfQ?3}B?!(ZckjEJ_N5zyv06io?IAy+~`*
SV_MDt0000<MNUMnLSTZpn${2i

literal 0
HcmV?d00001

diff --git a/tests/ref/math-at-par-end.png b/tests/ref/math-at-par-end.png
new file mode 100644
index 0000000000000000000000000000000000000000..dd3393faa6df6de3f3c57c10fec20b377e95286a
GIT binary patch
literal 450
zcmV;z0X_bSP)<h;3K|Lk000e1NJLTq004LZ000{Z00000l5Z6H0004rNkl<Zc-rmM
z%PVXF0LO9v$E~}%+gmO-M9IoRrids*%A+Qfm}Vr8GN$o3Y~(cv>CDJk%t%>{=ZKPH
zjEGnaA(KbOqmkF~Wn*EbW|ZQ5_rE^7Uj$!NgbJ&$3jaX(^7G--?&0PWKFD0iyLOxa
zz?H)oA{<P)<tXangTb1PcU|qi&2RpP2zLvBscA-BY(D04Y!vXD4+R&!?m*lAvejep
zk653**@JT+i=JS+-1-EzaW?E@0E7yc*#N1)|7T;g8tuJF0yI5=zeo*~%-{a@*foLt
zEwAQ2S6a0w{}g@cG~m>eiWC-XY5@l79ury9`6BJ&HjuwdER!Y-ZsJ&~q<Y4yLo(Ku
z+r&qU6ToetBXK1x*cTauOy6Asa`D-(2NWQ8n`owtl(~pzp`0q$E8O5{)1%Pn0GO@3
zllS7nf{`mm`qr_>?+7W!U0Ult>?L<?#!Lirgrd@^D6cvFAfskIPweD6yWV!1rMm$P
sX$RUN0XXkW>Q~g~QehQV;qMH;0M3LteebHM7XSbN07*qoM6N<$g38g@761SM

literal 0
HcmV?d00001

diff --git a/tests/ref/math-at-par-start.png b/tests/ref/math-at-par-start.png
new file mode 100644
index 0000000000000000000000000000000000000000..d69b214a8e6d3988fea5c210f8e12b30c8b7a49e
GIT binary patch
literal 418
zcmV;T0bTxyP)<h;3K|Lk000e1NJLTq004LZ000{Z00000l5Z6H0004LNkl<Zc-rmM
zIWL0&7{GC#!fuop#O4SwIAan*9D_j`q814j(ul<!S3^XC9<<8Rfq{h(v~~0#D3Mmh
z5z!-h>D#|IK0uc?G4y?=fAZwXZ+kNMr^%3D36|irg#|wl_ofz}iX=EN?E@fF0DZF5
z!X^^Tn+pKrFkq6W2Hw+H)Io3#g?|k5FMv&F2hK(=?#Db1{S~{L>hr~Y>^dvPt_3q@
zIRTv}Y1nR64XQj4%MIz*9M@tfs0Ju?hG*h~V&7#QJSfKnF)_+QWyd1i5}a5`7LL<n
z0iY!OT&#!!7EMkV*dE&QajDMpepZ5hQp{YxahQo&v-~w%tF|Qz-$n|BV}PRXL!sLM
zk8PBOfLOs2?q;gK-YY;m>T0FjC&KM#Cmn&z5JFT)4jx(T-#7x!^Vg=8y)O8uU#~U%
zZEo<^X&W@kqSJ`1>8VazzrNF{T4kWdz1+CqoZFH<mjp|&1g8c33vDz_YP$p&t^fc4
M07*qoM6N<$f&i|z`2YX_

literal 0
HcmV?d00001

diff --git a/tests/ref/math-consecutive.png b/tests/ref/math-consecutive.png
new file mode 100644
index 0000000000000000000000000000000000000000..63209657fa27a342d1ebd7ed72e1e48bc8265ea9
GIT binary patch
literal 176
zcmeAS@N?(olHy`uVBq!ia0vp^6+kS_0VEhE<%|3RQstg5jv*Ddl7HAcG$dYm6xi*q
zE4Q`cN>Y)|z0F_VOv=Ch{NL}&rhQ`ZJGbn}`*?Y2=J~SnOV=OO|L;Ek@BjbXf9rYY
z*4BJp{`G$H|9tVk<-eWdyRFm1)>!=d?|;_mJfr8;%lE#^NC|XLUvtwcs-Y*!ChOyh
bqb`gL7x(n2?ls){5abk3S3j3^P6<r_p%+oZ

literal 0
HcmV?d00001

diff --git a/tests/ref/math-linebreaking-empty.png b/tests/ref/math-linebreaking-empty.png
index 8fd4dbb9eb64ef00a61f86a5d47cc58180585294..2b0917a67eeedf1e9fbd43ed423d45355079f3c8 100644
GIT binary patch
delta 639
zcmV-_0)YMJ1egVoB!6E?L_t(|+U?X^NRt5=hw*N^?kb29B1DRiY|iK)$<V-vGN=ei
zN-q>VB?Sc$h(^+ZT4YjqXfr|!OdZmoz-(k#9?-F`9h~W=ZDYE%`N`(yx2GC{H(~=j
z+<ec~d-vnL`ETGqwLykuScYX-hGkfWWmtw~_;137LHO3AXMefAGXU;6R?8awgRpVq
zN2H4e0K(0n0MLJ6wdy^r+^kybe4;QpJKU&6!X^px;Yl(ia}2j9W{&^obG<VH%h^Sz
zhw&hhkw9SgR|yE(iJ7rP#YsXFqJ{8*QSXF1X7?rl*S2OZsl}P%&nF!3_ZdiOsV<4;
zHJOtd)yL@6)_<s7_jC}?ojRsD8!LJeEf~ITOX-cm;N6Z&OHxx!?$c9!J<07Qb)LOC
z`+ludC?o<?Ua1zVU(j~VdldE%RPji|!4L{6tfQ9+ODuO@u$<#@b1Xv}MG25s-%?KI
zg8vAu4JZgFlO5!3@uHRl;2NTb6ynwAyt(oVR<e*4hkv6eX}_<TL1G2A=Kq8Pb5&KO
z17!6BnUU`~JbOq6D9H{$dP5)qSbyA)dy6=pJ62&2?Q9$0>@jSgzEJAiHelN{Rr{*j
z$gM7*%m)<MB557@Dnp&2j8x4cw5eX}T}gwzDgLS=X{*qv<@6mF4@xoK{4H!1d@u{a
zPeHH+oOpuQA-DvSOPF>EZgr2YupfYS2XNBIyxHfC7>B^&0dN{)vx0L<2$)PuX~RzN
z+oAgli__~H5LF1R6M!KI_FrO+=66B7f!pg}u95oAl_?47MAoS5Mj!EB&dOUY!!rC2
Z`T?xWCRNFEft&yU002ovPDHLkV1m!9Gn)Va

delta 590
zcmV-U0<ry=1?L2iB!4bRL_t(|+U?a%NRt5=z;O><J9dgL1%2S556I0Z3J(fp(4inj
zO-YUNkk+y^D;8k}vL#JN+1AW$)mk}<6*>AOW3yS{+FX#bQ6Y1UMK1T*`wtu9W!PX)
zLGS<kz>nwhJbC=3@<dpKMOcJIScFAbghlv&gv0hf1*>-__<va40RT~4YXHnPGIjPd
z%oQbL2_ajU#RowV0^rL|kOD+vWGcjbQZiN=${Idhbd9bEI-Lf~+I|WjHa<x<KX6w&
zWOtb|DLV(>&Fwi^`$5y4YI%%FV{|UpMj@$JzwzvI9I;((nz}DH)q8#UD*J(f#EEBF
zz~_Q1N4g?rC4Xm(!BSZ9$zO_6hr}5zK<2R@<DL`JmAhS)T++mge+(f3kl%P&h}7v{
z7gHNz%cfmBDE@|AgNkfmUavkGX|nR@@GLChdl5Z-Y)K(OYc}3uz8cT_DlThLZM3K1
zZPvGAY)yPEL9apzt7hTgCeN!Z;CLHo7DaTYFz7<X0)OiXZ${aRWQKgskNChg3V46P
zW0TvFE8O1kU2FiTQ4>Ns6D&fjpn&imN`eA4`-x8c7QS+59NH5?XK#-{UGLK}m!)#}
z=y6Ad^~GL?p|-AS+BpH#yMZ!49qrwX-nQ-`T00XEY`8EyT4}vg?Q50Am-b%_(AONm
zavirKw?_j~0k%N^3`w9owZN<(fpq#fkQ>~f7`QD!x<HGS1u`so4gj*7<P0N-PeO!6
c_+N#80!I!d{?1j@T>t<807*qoM6N<$f>9F;{Qv*}

diff --git a/tests/suite/math/equation.typ b/tests/suite/math/equation.typ
index d5771f956..d4fba36b2 100644
--- a/tests/suite/math/equation.typ
+++ b/tests/suite/math/equation.typ
@@ -224,3 +224,8 @@ $ <quadratic>
 // Error: 14-24 cannot reference equation without numbering
 // Hint: 14-24 you can enable equation numbering with `#set math.equation(numbering: "1.")`
 Looks at the @quadratic formula.
+
+--- issue-3696-equation-rtl ---
+#set page(width: 150pt)
+#set text(lang: "he")
+תהא סדרה $a_n$: $[a_n: 1, 1/2, 1/3, dots]$
diff --git a/tests/suite/math/interactions.typ b/tests/suite/math/interactions.typ
index 03d5a6e7f..9a9b13ec4 100644
--- a/tests/suite/math/interactions.typ
+++ b/tests/suite/math/interactions.typ
@@ -35,6 +35,26 @@ $#here[f] := #here[Hi there]$.
 // Test boxes with a baseline are respected
 #box(stroke: 0.2pt, $a #box(baseline:0.5em, stroke: 0.2pt, $a$)$)
 
+--- math-at-par-start ---
+// Test that equation at start of paragraph works fine.
+$x$ is a variable.
+
+--- math-at-par-end ---
+// Test that equation at end of paragraph works fine.
+One number is $1$
+
+--- math-at-line-start ---
+// Test math at the natural end of a line.
+#h(60pt) Number $1$ exists.
+
+--- math-at-line-end ---
+// Test math at the natural end of a line.
+#h(50pt) Number $1$ exists.
+
+--- math-consecutive ---
+// Test immediately consecutive equations.
+$x$$y$
+
 --- issue-2821-missing-fields ---
 // Issue #2821: Setting a figure's supplement to none removes the field
 #show figure.caption: it => {