From b9599236e1b18ddb79e74f6976525ae2075597e5 Mon Sep 17 00:00:00 2001
From: Georg Mischler <gmischler@users.noreply.github.com>
Date: Thu, 27 Jun 2024 19:05:32 +0200
Subject: [PATCH] Make paragraph top margin trigger column break if necessary
 (#1216)

* fixing #1214

* Update CHANGELOG.md
---
 CHANGELOG.md                                |   1 +
 fpdf/text_region.py                         |   7 ++--
 test/text_region/tcols_break_top_margin.pdf | Bin 0 -> 2992 bytes
 test/text_region/test_text_columns.py       |  41 ++++++++------------
 4 files changed, 21 insertions(+), 28 deletions(-)
 create mode 100644 test/text_region/tcols_break_top_margin.pdf

diff --git a/CHANGELOG.md b/CHANGELOG.md
index dfc446a43..6b53a865f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -36,6 +36,7 @@ This can also be enabled programmatically with `warnings.simplefilter('default',
 * line size calculation for fragments when [text shaping](https://py-pdf.github.io/fpdf2/TextShaping.html) is used
 * [`FPDF.write_html()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.write_html): fixed incoherent indentation of long `<ul>` list entries - _cf._ [issue #1073](https://github.com/py-pdf/fpdf2/issues/1073) - thanks to @lcgeneralprojects
 * default values for `top_margin` and `bottom_margin` in `HTML2FPDF._new_paragraph()` calls are now correctly converted into chosen document units.
+* In [text_columns()](https://py-pdf.github.io/fpdf2/extColumns.html), paragraph top/bottom margins didn't correctly trigger column breaks; [issue #1214](https://github.com/py-pdf/fpdf2/issues/1214)
 ### Removed
 * an obscure and undocumented [feature](https://github.com/py-pdf/fpdf2/issues/1198) of [`FPDF.write_html()`](https://py-pdf.github.io/fpdf2/fpdf/fpdf.html#fpdf.fpdf.FPDF.write_html), which used to magically pass local variables as arguments.
 ### Changed
diff --git a/fpdf/text_region.py b/fpdf/text_region.py
index 2b1a02e0b..085779bba 100644
--- a/fpdf/text_region.py
+++ b/fpdf/text_region.py
@@ -524,10 +524,9 @@ def _render_column_lines(self, text_lines, top, bottom):
                     and self.pdf.y > self.pdf.t_margin
                 ):
                     self.pdf.y += cur_paragraph.top_margin
-                else:
-                    if self.pdf.y + text_line.height > bottom:
-                        last_line_height = prev_line_height
-                        break
+                if self.pdf.y + text_line.height > bottom:
+                    last_line_height = prev_line_height
+                    break
                 prev_line_height = last_line_height
                 last_line_height = text_line.height
                 col_left, col_right = self.current_x_extents(self.pdf.y, 0)
diff --git a/test/text_region/tcols_break_top_margin.pdf b/test/text_region/tcols_break_top_margin.pdf
new file mode 100644
index 0000000000000000000000000000000000000000..cd6e35c4af902be3057d031af7d28f57cd6ef507
GIT binary patch
literal 2992
zcmcImdpwkB8?Ho}wiM&gf!<XTYv=IJ!Hfnm2g#W*LhaziFo(<mBjl@`rEEw>A4-IY
z)uJSatfFXZNgI+yCuUnhnlG|DQ`z;c-*3OazUPnk`90VB+}C|S&-=Tt_cCy%(7r|^
zFfcTL0=Rx5FcJxdBy)uv0RX~~cFX`i;Ej>La)sd(PhS|)0SaI;thteZH%gA+h&TjD
z00>w#f<TnN-6uK(0Z3;?5X6UBSim4ofPxfUMDBD5;#m0$m|V_hW&lZJ3ibdTA&Vs^
zP=wkuL|mZ&Kss?CGM6P}b3Va*KvAUYLJ~4Vz+iEM7Nt=X`qvpa0*E8v1K3Y7&J3P>
zxxnBG@wq~tKg0(JizyY!7x5`V7x-vQmH^_(Yh^J65C!t*20$>RJ;VtT1Or4<P!Yu!
z@E`^o7MXWB`%d~3{Q89PDY&p&t=Pb2jSxq&SXrktDq2PxCVWwSBD-p<B~{C4rl?ot
z3yomaF`Bgd;8(w|n2&6y_cXstbkdf|3=(eyc&VHTio8LBAAJMMHSrg%)zp@R43XVe
zd+_L*b8jbB$l&+A5CvLCYj8eM?VGc+G<_O>sJDFSexl&aYn>rxBJ=6Y+gDbVZDmh?
zcz!T?b#x4wxz}dMWTY-$65`ck88X&W7IEsdo|ChsnoQ;KzO5xaXMfHtEPgbcuB&%M
z5YpRI6}4=>R=0Z|!+>J8NyH@%!uq}q)#YJlJ)IMq`06950|a`<m0Z{7<ezquKRAb$
zMAS(|D~PYokdYH*Dh+L6bx!6U#hUQvsP-zc6<a*%l0Fru>O2|S0_M+?g3bd_<<udQ
zoir=clOs>=12*wGF)Z2FcT4x@%TmG|Q>Gt|9(cFo-j}}*abja1oy!jM89AN*eV4#+
z_+j9-)w;_zS!xKPbiV6v=^t*rPDo}uP&y|QN=>(j`Kv9Gxn`&zzlvE6^H3cYww*tm
zoQ%0*5}i?=UF<#ab<Rys9FurkG-t9>gU#>e$fhsd+x|cZ?sYJmhPP=p>fzFPnk_L`
zhAZwMj#Nm8t2@sqc_!LD-MGnudb7{5@!Y`9Di)a-Vp`()0c>x2sY&hit1mBYYDiVu
z9JUv}P8J<aP7ZwJTRAk<cqFQ(wCBd%KrmfGww)~gFc=(Jl#k|DF>hJ4G>`bZdh`;n
z#oEtZb0WOo=aCZ8B~EaO75{c=L>SzhFo;d_71wS*Fq!~kdfxgMH7xK{{YAeobFkiO
z7!&pD{`4ZR!w)+nOzqCbFE5qOgzq-#&uX{2hje-)8exm@g6Cj_<9z=ICV!?|*hvi!
z(O_E@>d_xU=Y*BfO1W`WqjRp;290j4lcw-<Pu59`#)?9}+r7^G==GDkZh92L*^b?z
znnwKw#iy=)?;}g}O|9-o>)W*lQL#Nms?m<m9k<?lET63%l}FDk+4F1Yk%wmP`6u-3
zYELt^Cyx2s41C4^cYHf_g9@d~#i69OA#Wp&_1+2YRsB|k)fA3;oETK9R(qXC9}^lX
z`^2q2c$Uv$N6$za^?cq*v|T};mZtF>)zh8e2E>UVeGlVqO0}ROIntJ;QK&B4v1V^@
zUecL|#0_AfQ2!h2aZ0n`MRMewCmKWF_8F$ykL4%jJleehbNQHl;HEWHNv2*((x^TB
zO;_E{4!gh$N!br<HlPYK^uO6}NbM~0NI6y70C(xmHR`Z`QJgeW9zJH7HITGiAz+`|
z`m>ZNWN<+f$lT8wga2oZA^wBLC_)$b{z+p9DC~b}Oib2&P9AFg?eSSSpM`{@boiHZ
z^~w$yv-O&lxnnTY^1NqncFi}9Ms1@eQHR=02ct0+Lv5uded`W59lSsB{IIgSTc$xC
za^jGMxst_pcNcU)nx=XDl&ZUDro)<-p0a5-wx3GfZ&3+tN?J8r#d=npD2@D~Iqi)T
zQr(N8q<FYNsY!r_bjN%W1Q(Oyki7H)7YCX0<6Ai;KY_u<E^geg_i>3=tt5#LyqEp*
zy)UxOre#yqHkNY-B7wX`)vWbK+1lglA`bZJ(L&Uuso$EFTN|lENv2h6Cx1!Yw`RiD
zBDAh0HRoAIoBPcu53D)g-BBZdm9pO;^%BoRLx1<gY(mz}`7&er>&%8rwv{_+%>^&K
zPQYX)>$km4Z@Q9Kt9JXU@|TVoJ}Sh!KRwQybBY+_WgL4Z^3m?BOu2nrEzIv~R?*71
z)+S{YqC-yYbPXxvQO0988_Jo8>ZgY!FSi^`9#_wKi7QkVx?tKiPaE5$-`?n@)i~+E
zJFPVbGn46S-FZnm*-C>;#yP_Y-1}YjN9z%^(ke1S<fHrED)sKFAC04`<qGoDGXFa1
z@)xh0@BMNw_2Q6s1zr5CzxSTH*rMjxdt$@gj@BdPYpK0~v*n{r4IN6mv<yJC8?OvA
zg98VG)()!uXX7mde#%H1pl9o5S61;m7rO%N%&DV$qiyJsvG%#OX~#C--kvsl^ul=G
z*qHA#1mlHMhpzB=@v`t|J5b^5zU-A3cNq)njnyR5SHK&)Th<S-{c`!(qGEpeiNsv{
zMZNf<QFokk-{86=XjV&fNi><0Z`x&wesDPg;W$70foVQ_NLRyy#+Fq2mTnSX=j=__
zU-<x@R=CD)DArZgL7KAObZdytHJxi}bp4XF0!~cHV|>l|$|uwNsV&8=SoMf(<iY-$
zSK((}4zIF(oJ9$bS<zb%4>#M~!kX=xYQ3JUkN$b+(%Z7ejpo@b!yooI4zy7QLiz5+
zcfHQ39IwC7cdl<nT^GlCXXzC%&>e~GseV(w<JaX^KWWFe>WG{4SN3_{-udR`m8*Vk
zI{bcBw^?)g(%v;))N9rZK17rEFM!+wvBDt%)1LuDQaSSfI+GIwAU&BJD-NHz0KkxL
zLO;PrZ=-NH=mig=U|3`r6lUVjW>%1sNJttQP~>%$8w8{?kL&LS2>@@T+;#!TeNd!4
zzb#wtI;=m><j=FM?;=`-e*7P3(Fy~%z)O~U7lkRK$n6cVlVXM@;)!SwgGLik^06OH
zG(@2cjTRLZ$%6u6C;*LyDK>l>03L_K;DA8jFHBy$VgQ^!F*HhU2mgkFD7miw9V0Bk
z&=_zD9~z5Vf`Q=DSP(;4k{86{m&W37a!p>SMZjY)SrG4|Ep}r@L5lMtUAbJjJyv{F
z7}Ay#$OS&$q+&q<yh#KQPqC(8t?(2Io{FPb<1siao<JevOmQHHwkBHq?;RgMK3~A#
S2|nI3h({A(1_o3o8tf0K9Ii?L

literal 0
HcmV?d00001

diff --git a/test/text_region/test_text_columns.py b/test/text_region/test_text_columns.py
index 306892a37..ac540669c 100644
--- a/test/text_region/test_text_columns.py
+++ b/test/text_region/test_text_columns.py
@@ -307,30 +307,23 @@ def test_tcols_bad_uses():
     assert str(error.value) == expected_msg
 
 
-@pytest.mark.skip(reason="unfinished")
-def test_tcols_text_shaping(tmp_path):
+def test_tcols_break_top_margin(tmp_path):  # regression test for #1214
+    """Ensure that the top/bottom margins of a paragraph trigger
+    a column break.
+    """
     pdf = FPDF()
     pdf.add_page()
-    pdf.t_margin = 50
-    pdf.set_text_shaping(True)
-    pdf.set_font("Helvetica", "", 6)
-    tsfontpath = HERE.parent / "text_shaping"
-    pdf.add_font(
-        family="KFGQPC", fname=tsfontpath / "KFGQPC Uthmanic Script HAFS Regular.otf"
-    )
-    pdf.add_font(family="Mangal", fname=tsfontpath / "Mangal 400.ttf")
-    cols = pdf.text_columns(text_align="L", ncols=3, gutter=20)
-    with cols:
-        #        pdf.set_font("Times", "", 12)
-        #        cols.write(text=LOREM_IPSUM[:101])
-        pdf.set_font("KFGQPC", size=12)
-        cols.write(text=" مثال على اللغة العربية. محاذاة لليمين.")
-        pdf.set_font("Mangal", size=12)
-        cols.write(text="इण्टरनेट पर हिन्दी के साधन")
-    #        pdf.set_font("Helvetica", "", 12)
-    #        pdf.set_font("Times", "", 14)
-    #        cols.write(text=LOREM_IPSUM)
-    #        pdf.set_font("Courier", "", 16)
-    #        cols.write(text=LOREM_IPSUM)
+    pdf.set_font("Helvetica", "", 14)
+    pdf.set_top_margin(50)
+    pdf.set_auto_page_break(True, 50)
+    pdf.rect(pdf.l_margin, pdf.t_margin, pdf.epw, pdf.eph)
 
-    assert_pdf_equal(pdf, HERE / "tcols_text_shaping.pdf", tmp_path)
+    with pdf.text_columns(text_align="J", ncols=2) as cols:
+        for _ in range(3):
+            with cols.paragraph(
+                text_align="J",
+                bottom_margin=pdf.font_size * 5,
+                top_margin=pdf.font_size * 5,
+            ) as par:
+                par.write(text=LOREM_IPSUM)
+    assert_pdf_equal(pdf, HERE / "tcols_break_top_margin.pdf", tmp_path)