From fb6b7da5bd748a3e49b22da796ee1ca35cb365c6 Mon Sep 17 00:00:00 2001 From: arnaud Date: Thu, 3 Jul 2025 10:06:08 -0500 Subject: [PATCH 1/2] Fix crash caused by glyph splitting --- crates/resvg/tests/integration/render.rs | 1 + .../tests/tests/text/text/glyph-splitting.png | Bin 0 -> 13486 bytes .../tests/tests/text/text/glyph-splitting.svg | 17 +++++++++++++++++ crates/usvg/src/text/layout.rs | 11 +++++++++-- 4 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 crates/resvg/tests/tests/text/text/glyph-splitting.png create mode 100644 crates/resvg/tests/tests/text/text/glyph-splitting.svg diff --git a/crates/resvg/tests/integration/render.rs b/crates/resvg/tests/integration/render.rs index 3118508a3..ee0bae263 100644 --- a/crates/resvg/tests/integration/render.rs +++ b/crates/resvg/tests/integration/render.rs @@ -1502,6 +1502,7 @@ use crate::render; #[test] fn text_text_filter_bbox() { assert_eq!(render("tests/text/text/filter-bbox"), 0); } #[test] fn text_text_ligatures_handling_in_mixed_fonts_1() { assert_eq!(render("tests/text/text/ligatures-handling-in-mixed-fonts-1"), 0); } #[test] fn text_text_ligatures_handling_in_mixed_fonts_2() { assert_eq!(render("tests/text/text/ligatures-handling-in-mixed-fonts-2"), 0); } +#[test] fn text_text_glyph_splitting() { assert_eq!(render("tests/text/text/glyph-splitting"), 0); } #[test] fn text_text_mm_coordinates() { assert_eq!(render("tests/text/text/mm-coordinates"), 0); } #[test] fn text_text_nested() { assert_eq!(render("tests/text/text/nested"), 0); } #[test] fn text_text_no_coordinates() { assert_eq!(render("tests/text/text/no-coordinates"), 0); } diff --git a/crates/resvg/tests/tests/text/text/glyph-splitting.png b/crates/resvg/tests/tests/text/text/glyph-splitting.png new file mode 100644 index 0000000000000000000000000000000000000000..11196b26efb13235bd7ca73e17148f51a98720d2 GIT binary patch literal 13486 zcmeHOe^gWF8BRzEoi^IFLprH)*15SeTGOr=K|^x7>T%8uwsn@%fx#WyMF$uyMG^xE znP+tsg+F>)TCikCkKC6$N2JUT@vK@qb)I%|*a4j_j}&wect!|?km44*eY4Fa*047kZk+e=HCbe!bjony<+eeU2Q{^ zK#)_lZSzyV)e3K+qQ9fe_U>Rar^Gh3P;!6&)#FLum9ZA`jaOgnF${3 z{^Oy)9obYiQivWPPM@DV)7}{}-_qP;levpasz!DcMX}k9DXPrCW5n({#0frrZ_@aB zlelk*c9qZ~iL7vML^YM*|91M6gJmKl)z(lqDmErkzsjDv0j7(Qmt%Pel<~(N16mZH4xrQ}=3_PlbjSXvZ1) zImWOm##(erfD8dUsz&HXS;JVftLpGC&1Mbb&HkVEHh!~G;i?=P>{5iS%J2mMG9$9^ zVCe$0H_z-Xf#xoU=8wx){O zS+#Mt8o-@4EQA>K7O+d<>MfrFc3BYs(U4!i0L?z~U%;+B@`YkxmpS?ghz0~-7&yi@ zU>By7Izqs%X%ZlqiY^3ubW`mxFimg&2-u}IZU9k8`WMC(?=t4(8Ghhh7DLZG%I4Dy zADAq^FzJ%#vKY0()Bn8c;xlH9=;cL5jCkio#{av>h+wn32y0!FSkkMMld+O3A~Y+Q-C!y`3S;?4lw&h~GMG!o>{-`a964&rc#Z zlCTfK#EN>gK8pZV4RUqyFVHU+o6-*SYebfdJLuQADa6S1tp&G&zz#t%am#Tl@HJc zcBf5fnV0kzN3yI|Yvn3*Y-}t)g5SO&cXXf~@-{@MXl|!Yacj;*9yKZ}N9c6MG10Bu ziZPM@XafkVY{m_tMlKW+Jt`Gdw9pS(BsSonO{m5z)j#f5$sHu9i0YlarZf*YJ@>mu z@~2M=@_la?M$1$=SrF}tBN;=7O@8R|Kn3Lm{=HeLE$B`G^)!8DA>Lr1W7{I(TABMf zxVmMCewg)D9;ybL0vVz0!i0*Jj3I#8e(Sf0xKdtsrDg*(YfxYuLZ(4bPS_P`fpW_G zhHFidXG*yO3{c@FMmvb0erxSAx81hKkg0vF^_PNp%+xzinJjagyucVg3k@|7$k!f7 z5&+5pwaCnF5*dUZX}wihhdH-+9cY=SJS7m6wx>eUaT@zH$}Mk5=xG2V!{p1`)1_)59F7h!q_o)ec4oO!6| zGWU%L=3&AXZbF|0AyKamKTKG4S0Xa^@8m6l_?Ab;5aV*@i!t6%HtGr{&d^KX)4S)o ziLfolG;Zo$YXCf%hpMg{bs8HO5`>!sn_%?F9WH?xz_c4Mw*4M=T=T@XXWr#8-k^1t zD<1${#C8kkdx>+g)P_58@_qEMj^VSr^k|U5SEI16Io6|L+Y0G*jrYW*9dtT0iHPY* zvb`cY2;=qc@M-N`@BVek-cJ>pvhYx^W6YQWSc8VEM7 zt&2~KNB!cydb$Ss48!#zydUK7vCP0;)k}9l7`X_feFgwa*%ZvIHwCFrd4vm3 z8ZIzPA~{8X2(JQ60~sp*&OpVkjKGgs)(bufQq0tw$)|+fwIGV*7N}h^W*7kh zvkFAlLl}N%ZjrY!MC3?aoicq}cY5>Lrn0;9ka$p(Q7gs~N1-yqg%b~VNLLG`XhtT^iO z!J|GItRT*5WkS&Gj%g}mQ@L&jy4_QbOKi*OL-ToZ4jD|kz6)f;FuZPD0g3+_9Ni>l zc)3w3iOBXJQ-}8vwSiGKpTX?zB705bguO3KtD&mCmhR)ECtX>?h!c432%wTi@#_0( z*1NCSzDTGLmd;*9HKGA+7bzY)lJNeg59%)EK87+F;xUPYV> znm_#te^g~OVFsOnx+kx8gIf`s+)~4Ph<0sPv8hRTP#y(JFURgjCfI7`nZj{(0`?IBQB+qd%NDgAYM2e59 zVt4I89YEAFSE|`{7km;cIbESd5lbJ^NNP6hKbE}?&N|m)Q(tLm z)k|~X1dB`^b%(gw!m7GJp<9pm^^KITl=c-mh3sF0)uAymI@WENe8wF5NPca2c=$oZ z%%_T(`e4=s{C_<3c46&gvG*6t_17;NLixBi-`GrVF7>3Bdj6@`>l>YKvMUM8k@qUL zTp~RhcahOo)E)U4i+;RTWa}@s^=BZL2W^)tY(&CoDh;U^RD5y9DzY3GPo5P&+Hp7eQ~7z zGj_yBZ-i_ZD`~-;?i-B(BkV~QAmF^*>(P!9pYeT;rEt>8nF=aLp-^8s>p;VT?iWYs zKP+wzT|!D~GU&4)xgBYzx?wtwAGdSoAQ9K0y+ntuKqIkz2b%(^Z2Sy;9H#47jSI%B zoKYm54~SI;%DFa3onR+!V*t)7<42(pKd`3Fw3}7@JLndm12{u}Zu^-E_vV1)1gN;S z4h%`@$C5DYNg$cQS30k!s>HDCv4LAAB(Mzge1sM<6mefsRKx8qz_D@sR61z>yxr88 zvQ9`sJ(J`&a@`}SS_IE=#p374PnY?DJaZ3*8xs5-2L9Ryd%=gpN&HQjn4*TiDZ^hb zVs005MF@W%i@(CgUQ*?+eK55FFEZe5=GaTa{Iw5eV9tw-#cE()Wbh*6KQA))Hyrr) zN#-;2f8{lpz#ec%YVPfw($eY>U{ytI?+zWU|cL8J7&XV>gCZIr93!Fx!8 OZCeU9cm8zOpZ*Pn#&`Mv literal 0 HcmV?d00001 diff --git a/crates/resvg/tests/tests/text/text/glyph-splitting.svg b/crates/resvg/tests/tests/text/text/glyph-splitting.svg new file mode 100644 index 000000000..e2568f7e3 --- /dev/null +++ b/crates/resvg/tests/tests/text/text/glyph-splitting.svg @@ -0,0 +1,17 @@ + + Ligatures handling in mixed fonts (2) + + This is a very resvg-specific test to make sure we're corrently + handling ligatures in text with multiple fonts + + + + + + H नमस्ते + + + + + diff --git a/crates/usvg/src/text/layout.rs b/crates/usvg/src/text/layout.rs index 3e9ced0e4..719e5f735 100644 --- a/crates/usvg/src/text/layout.rs +++ b/crates/usvg/src/text/layout.rs @@ -1,7 +1,7 @@ // Copyright 2022 the Resvg Authors // SPDX-License-Identifier: Apache-2.0 OR MIT -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::num::NonZeroU16; use std::sync::Arc; @@ -905,16 +905,23 @@ fn process_chunk( } // Overwrite span's glyphs. + let mut positions = HashSet::new(); let mut iter = tmp_glyphs.into_iter(); while let Some(new_glyph) = iter.next() { if !span_contains(span, new_glyph.byte_idx) { continue; } - let Some(idx) = glyphs.iter().position(|g| g.byte_idx == new_glyph.byte_idx) else { + let Some(idx) = glyphs + .iter() + .position(|g| g.byte_idx == new_glyph.byte_idx) + .filter(|pos| !positions.contains(pos)) + else { continue; }; + positions.insert(idx); + let prev_cluster_len = glyphs[idx].cluster_len; if prev_cluster_len < new_glyph.cluster_len { // If the new font represents the same cluster with fewer glyphs From 9793bf91018e6f5c5e3675b1e8248a491e6265e7 Mon Sep 17 00:00:00 2001 From: arnaud Date: Mon, 14 Jul 2025 11:16:08 -0500 Subject: [PATCH 2/2] Instantiate positions HashSet only once --- crates/usvg/src/text/layout.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/usvg/src/text/layout.rs b/crates/usvg/src/text/layout.rs index 719e5f735..2261f66bb 100644 --- a/crates/usvg/src/text/layout.rs +++ b/crates/usvg/src/text/layout.rs @@ -882,6 +882,11 @@ fn process_chunk( // but some can use `fi` (U+FB01) instead. // Meaning that during merging we have to overwrite not individual glyphs, but clusters. + // Glyph splitting assigns distinct glyphs to the same index in the original text, we need to + // store previously used indices to make sure we do not re-use the same index while overwriting + // span glyphs. + let mut positions = HashSet::new(); + let mut glyphs = Vec::new(); for span in &chunk.spans { let font = match fonts_cache.get(&span.font) { @@ -904,8 +909,9 @@ fn process_chunk( continue; } + positions.clear(); + // Overwrite span's glyphs. - let mut positions = HashSet::new(); let mut iter = tmp_glyphs.into_iter(); while let Some(new_glyph) = iter.next() { if !span_contains(span, new_glyph.byte_idx) {