From f8fe1d9687a91c9a4a3572d104d483d0dc7964ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Wed, 14 Jan 2026 20:12:45 -0300 Subject: [PATCH] Add multi-thread protection to: ModelSelection. Make UIAbstractView::notifySelectionChange thread-safe. Added new UI Test (FontRendering.UITextTest). --- .../fontrendering/eepp-ui-text-test.webp | Bin 0 -> 7950 bytes .../assets/layouts/ui_text_test.xml | 65 ++++++++++++++++++ include/eepp/ui/models/modelselection.hpp | 19 ++++- src/eepp/ui/abstract/uiabstractview.cpp | 6 ++ src/eepp/ui/models/modelselection.cpp | 8 +++ src/tests/unit_tests/fontrendering.cpp | 30 ++++++++ 6 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 bin/unit_tests/assets/fontrendering/eepp-ui-text-test.webp create mode 100644 bin/unit_tests/assets/layouts/ui_text_test.xml diff --git a/bin/unit_tests/assets/fontrendering/eepp-ui-text-test.webp b/bin/unit_tests/assets/fontrendering/eepp-ui-text-test.webp new file mode 100644 index 0000000000000000000000000000000000000000..ce72650c7691615758a4f0610f9348b252355e09 GIT binary patch literal 7950 zcmb_=Wmr^Q7dAbVqzKX=AcBMf(v2Y95)vv32qG~9Lk=C%jpWcJ(mAArG(#iO5<@vd zgVcP3KJWXyKfmv~zWw9uv)8%TeXqUN-e+bXJym680v0SRqo)cw&vnE!&Y7{W1kj4P zSTr#|>m4$j&~?C8nR~BsUY8j!RN7CH#M{-4HG~u2mU(yr$hfT@e<;f&l%D$E_RGrJ zdZT^Cuv;Lvdb_p!)6B*A5UGb}{7yvbO5^Z{8tJsDN5j!~7q?Qcp7RL}9npJ50`1-U zNcb937gB1DNJ%8DnKSRsSw^Eca8%Q;Q`>*xCDgJ#|AKC#Q?2Gut>@DMVKE(2(de(S zHk+FcPjvdOy_u`ujmnA~8k&7)y`eFLX>YADt(@uubX9us!oeKWMq{9!(L&75#xH|w zILa6B`fD5(R!IELo9HhhT|yEkA=kd9cq&*O3gbNOFT5J69p>r{vmEB0JIpDga|Gkp zo^2mqBV}R6dDfwG7uS|!zU(+s*r}I;C(=b7YiJ{@^Q`o`ueb+?zCOAxr6Cc%h~HcZ zGXotOTnPJ#exfwJjPFiqCmRTTZ6zQY zBi>mQ($I$%kKxWEZp?6trz5}3SN8i4lW9k5yb)IxqA~T<$k6r|L#_Jry*PqQ8vD94 z$y;mYc@@FgPNVHDNm8;KHCY!H5a{Kreki3jbyg8P=eo=@4mCRl?a>;O(zY~?sbm5A zhPr#OwK%k7GedmEeH^d_narHEe9CDtF+wgzu8WMsAX0ghOyooGQ`g2&--DjEdnl#O zab365-t39Y0IOYf6~wg;h}CZ=JN&}3k0|u9BYLh0&eAOVG4rTws6au`SJrYFcnVkn z;*(bvCiD+zVru{l=K+e!Vu@>PGbVHCJrTtf=4W0HB`(_{x{r(pubvc1+-L$}5K~T|T=Q%Feb002Bs3 zIPqFlCvTL=zsZD-C3sKj#BJre)-B+KO`sfcG2)&@Rl({sq0TG@{>ZihSd!ULhR0x) zWBgL4npU&grd;Yt?g# zW||N?C~k|-je)Em)ZQvVyjPh>Ef{@Lm{l{c{^u?~t7c(N(H5-bd0=NCb}ukRa+3k+1NgJ_rW>^HqsA;1%sBAeJqF!n%rC!R zn66$dlPj*;j84?<|2&#?xO{lx3@==@`I-R5jDwH%F19nGuB(6lzb?bL!F@J@_D0{9@cRk59l*|ZJ9J5VGHW2Zru_WnWtI_o@OO%C zoWOPLaOk{$U*m+p!t3h<|9ct$eJL3-%O9vaSDbdId^wk{0C=9P5VLF)6MFt9m*mJ9 zllKGv^u?WrOY%l%Hl3D8uhC63wi(6^Vj{lTIA#e$X1gs>mBNq|Xt6PHGqMsha?901 z%Yqp#voY6v6q@F$HxGS5mU^oa6}u^m8*HqRZ1ZR~Nx3#EzMkzkW7bw_j{6;h9y~Sb zkLl5I#+U{D*b}uMA(KFBuVzYzynF`*X&doBHiAiDVcs8;oV1O^AJexG@fy7-&qb|qggzx3 zC%RUr6NK^5=vFx zaO=CCDM)UW_gJbZ%ir0v#s%lZjgJuWe1CAogse!sNa74;HgE=``F{0=`q*0AT5Oxb z)JQI;OZVz-G%RGL81OQ8u`I)DwP67ieHh-f{9DO6X^fX!x(wuAy;UpY)V2lBu@U1n zYUGeSGSkgOufsLketL-HNW>e*OU4evc`Sgj+qF*K$@`~u?V1|(D14spzY9PYsc>iw zPO+-ecPOnwj`cPvAKhf{m#@kjLGv#~Ht+_TwIS-tMVhJ$+wGB+H!gK93Hx~^{SvNL9=M@P_rF= z?0gEp61xz6tmN8;`n=Y;!J=9m&5^uD_T~`PkQQiw2)+;g25*UEeC69doE@+Q%w6&z z;q|l2?{sk~OI08tICi$rj>GTMu)yK1*mHo?RZ>j{nFGIKS-05Ty4|VUGzu)I+E)3W zhzx3AM-j*+{fE1)d=TTUy*CpNF1;sE&Kvb}j?F?!uAkMH)~{_7b(mCn`WO7)PYYq9 zunAUto-&OGd~$XlK8Ev2nWqeQ;-R2<$RBa0Y=#y*i^Y)KA1XO(Blhu>q%RFirsdce zlSrfyJSC8jQkq7a&eCZ*^^?N1i&fh5`Kzzf-G`ot(4^>;o?uKC5Z3_6lqeN*SbXt^ zc6(Zql)vI7?}Ij2r%YBzb)t3TwaTzctL{Z+3;yEj<~L?(P?o54T_%|5F?htJBx6i* zHwqhC_5wH9shx;FrH$O+@s+7)j*XL{eH5FQK74OAdEr#TkT>#YXc9kbh9m5vlT-Wd zwckzv^x|IZ(DwK-u}CFZ2?w~Sn)2^-r^Ca8iCtdfgM{^7-7dZ>fxELh{R~haJ&(mX zdhlr^w1}*Q>+XB3&uR?>0Z!XQ0<_o$E>{8RjGK2&r#!8C7Yz>J=u0@sHNpM>tP+NT z-d5i4Qlsraq_>^-)h;(k`Wz0koQBz7ix_-sU!eg(x)|QN=m27|va2QcYQl{e^Cfs0 zp>HqiQ>MvhowUvVOh1m8H%MW4^ezCC_I%tIDjzE8DX)bpxDV5pp2Y{^=dc>#;TG@1 zL@0tL47=9HPyN-3S(^I&v#JcgNGtDZicD;Q&NsH~p-xC(wA)oM4yO7zXWkPrtt_G& zZW$bly;m?^8yR>2@8i=hlrZ=QaF0vH)rkm;r#bd3X0M1na|L;BE6rS@WrS(wQRg8^ zW2cqGbeOte0weZM%Ml{m{wGN&W5?v!w)TbHPOYX!S%M7*Jet^(n*%5Y&U`;K^`i0X z*I_HlyFC&@jH*1Puf(?bNL5-hSBl-}s%Qhhc1^Hq+|rxL4H}}U789WkH1GqRMK85) z5A3Yzr09ct{hb{M%|Fv_l$%Io6t$*0%0&g9msrX+qUMdHnjBtFKXRp5X4(s)VdvVQ zDFKZ|W~gmmeUSyDp}gK<#)(`S+LW0AV?gl6BTI(TP`u549$m-OM~3mY<|26H+|v-% z^4s*!USb7{H{r$-Cz7?Vg}%RzSOAH;g6ZgtLPzU7Um?>9x=engUl2@+eTLScrtUnK z<$m-@g7B2gqjQ&r`Y_x8W~;2ldDNEO7Tt*MsOWq8uJh>j*aJFgu#G&p?Fsjyw$aDV z$dcP#C%MGTSp=`_vS7+obY?lsiDXO5*D_w4F54qCw~G}wO`() zruh_iK1tB@QNIy*_&h<4cDS!3Xc1u6EO5pgBj=)teKkhcmts{M|8QVtvv-0LPWk{EO)QYZ;ADA<=C5480lS z-l<1OjYcADkAU6a9y`6Q_`6I4o6?ZW-{@WEGjo15>cS%KJLxKLq8+C4ThaLyh|G?t zKeI_*6TB&%4i0hbJ!5MIj0`P$dVa}<++2%^n_DMz)XFz1B z+7`19cJYM@)sn^Er@vhSVp1J;b-VgWYgEErsX_my=y6Pz(3F!y_cIB@fxj}$bcM<# z_8iy*eYzQG;W<2n!b}!>0L&KUYS;Sj3MruV-^P9CsC!GJL|+v}SCL3^dUopr(@d?~ zi=SW9`y4NG(aUGhFp_LF&Ts=yW0~D{H;DGXk~In|KXjzK>u^dF8T227{CX-ab~CkS zIHa+8f@0KNhHH4^D=A(O>77(B>Dlao655W@h9^Pg;}L$x`?U?l1o@NVvhGDq(r#Kq zoaI}KXLmYnT40e}Hev$BEDB~=hCE(R6V%AVeSh>T=qK`MXj2{K>f;N4gZtSl9#hP? z;N0YtYVag?8y0ARKAxEiykc6(Iy98Y%hETp1izoP*v$#j@Pc!{U)Yq!0UrGv(&(!_ zjSTRr=n!oEj@hK-`K&KH#3yTyGZfHxNeQs;cI@-w52Ch9e0}7sOy&HKJ7P8I4hq6m zf7Rt3xVy%-Cr*->5WLm6&pRi(lAvf}(MzlNN#ffC%3|7E~-xyWl5_A{V~2IKnHxkDPae;R)UPacjOOxr-3uLMPPw z^}3R-kw|1&n~Nz*egY(#_9e^7Zcg%9*8)C9;T@NE*gpy>JX1vjl-^j9*cYUr(Y2=6 z-qMUe|D`{`tGK@rx5+E!M6@;q|JeNO>mwlJE_sB1x(@F3oAEI^VQp|lQ)L>oC6J8_ zoH9oGF~zzQK$yRyH)V@#4yZ11k&QfUus6j zj2gdRMYe_25A0{(%(O!`iwVn&?s>iaY5ou5M8*}b|E1BY6g#5CQe3;T~ek|aM4J@R3 zp>^*JYaY8}wbGFRQ%pjrhQ#eaf;eD&d*PNHm9_E%C%i~SME+eWfsr5qmS-v@cGzNm z=ERLDnn|nJyZpXXJB?(?+9|y=pI?=B=j?kZEim$K)@{HIa43u?6 zX7{%tpGem3lt~pme#(=~#erEJsOz~`Msd@Yiv`F|YabKQd3(jwN|2#xiu%^Atf2}% zhU-%{TRKQH)~g*;Bth9sigfe}Hy&possQtsIjedOn>o7FMr`}$2^+WcT4rLn!n*s+ zZ*T3B_+(9ioVf}oO=r{Qnx9UInVLntu3tTNC-IRkS}n?S-k~P;!>p(IpBS~N=M;A5 zn08sAR8D!9oCpN)7>V-Oev;Xup-1D*-bN=#TYXgQPT_;FSJ&SV)+9^hbsMqNem|Qc zp@zbN`u(fO*~sUeokA0uw>u{!in3b57uJCj zHR@i+A0ARFVFzn>8{rMjj(QMbBp~`F!kYXKibfg8i|^O(sxuXU{k7$CTo`Q;UezhJ z8lko?&R%jz@M(orQ|H4*rQ9}f6n~(lW6b_abk)Um*Hc`$A^bhHI-(9zGN2Vf|Dnfv zzA>VIV73jKjdLM%h&Qda8*H1@b)h{Yz0K3xJc8jjvh7!WE*}x-`F2|U@OvDIf}3Yb z6PcjPaNh9&E0}afnR-s_n14@b6;?1ekXM}5i3*d7VM-pUo@i9QJ-RXG=@Ba#^3Lj` z1LK7oeQ_w>JhyNE@86)VGtzf^7XqMIoQ)Pab5oXbzQpjv2uVc6P6XS9p5{mM<&*&B z@nCYPFC=qWDTc8>8|eu$v;^Q*p#?R&!WESGOU~1#B7IC#J+-5Ma$+OdhxiZqq-1C* z1=ml-C9HwkHpx?lZ>LY!#a^_z%}KO547W{Q0z=hzaoX{`%_Ptr6R-J5P4M}mOIEVC zjt->Uj3oxO4r|3o+b}7&i7b)crZ9FpIvz4yd zLBAiihLvWnaY)Z(#pXQ6n2wLYN3Ir}gPb|z1989Ry^}5%u|w`vfJq@@#DOlmK0QJF zzG9^;9@0MT%%m#VbkSldn@biAWodJ(cvOj;)-Cqy6nAy8iTg3r2st<|-#hhi z#jbKVOaf@7LEE7G!Xx>6Va*4~JvJ$jW@&a^B_k-uGXCJ5&$W4*gd!RBmY^DD0ud7n z>!Xq>1(>?@H4F)K1l#_nD(e(pi4K7HD)A0t+r&j0kGnXp=Qc=i(p$6u1rwu<6l>o+ z-eECZjmU|A_@@|ys;dkg=pRnUMN8|Sh1SIVHAD?cRNw9tE|KJb`PI{%XBODn?2N8( zh(E`F!U7vbwWyHE>C`hlyEFXtm3zRA2KW4o9$nLK1#jeDe>RAoo;F}!!6FS;r5&-x z-Fr)6CZG5mYSrA?E9!@?TiMobADpr7ll1$)(HYh8Wi=57gsyz_Zsk3AtmCep=s7n! zT*S7#r1YztH1$%TqFwP);!c{rV+->8=#Yx9QbPbpLqEmAXc&*haRi^3tWYG$Ur zPY}+pv%#IfavHODvqdlV-KkW?9AQ^zqc0BxL_qE|0&m^bJ||Wk&qNmlNW+q){3}R+UJd$}X9vM@deXNG#LRXU746^WLFYTu;!-&J;hL7^jZRV6XUMtLqfC}AlLjY zCHxiRz>oNWg2)SFYBKh%CokJvZM^X|)%_Z99^NNH$FXG%2XhwsOvNtgD2Ce0;SK^{ z`w}dIKbOCS;Jx55 z0L={{o9Q?Wt{|A#Yn94rW$h^6-^zBQS(TCj)h17F$4$@uM$TP}H+m6$fQEW`m8)dV z9DbOBa3_81GVG0JZI&30f!;K5aAHpbl$7i@YM|>oXU;T zuUWmr)Oat44K<$}QsPm*2g*PCf9WAW&Y(}f2?N6Fn__Qz!nh`D`99$@%#keF485=dI7?VqI!VCap4W9j zzDw36`9mv`&KW4FS%`*8GMfeXle34}rhR4vG7D?dIf)oiZb;c?2`<}}^V6NN282~N zZ42(n<59i^CRN!j2NJRmZ17mg;<6}F=sq!8h`90QPXCN_xUo1tF`^-m31j{FzMO~D zn?k;O*39sRgfK{akje#cVNL|vI7c_F%F`HM?E0HVoSHp&@3SjKns=JP{DMW2ylYe1 zh|C@;tz&5n zRMTU}7dMPGr)EU+H=wjuL>eXLmAW$f=jHmrW8Qw?;naN9qJw&|gQv6&-qD8_3J%n} z;<(x1IVwAbS8x$DZR=dJ&K#*%`F`GwEgfTn{DIxuZQ6+W;~6A9bGbC#>_AjET#2BU z^=#nFslR(-HesWWs^^@1(v_7iZ|h>yIMMa)Uu0AzA`RXC^dErz<>o?tkmOr(^#TF6uYCW(PPqTW1KazDCrbK; j?GN;iGTrh`EYw%`&$W-Bo3dlRxo9Qy$NwJ&{^kDzHn;Wf literal 0 HcmV?d00001 diff --git a/bin/unit_tests/assets/layouts/ui_text_test.xml b/bin/unit_tests/assets/layouts/ui_text_test.xml new file mode 100644 index 000000000..f74a33fae --- /dev/null +++ b/bin/unit_tests/assets/layouts/ui_text_test.xml @@ -0,0 +1,65 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/include/eepp/ui/models/modelselection.hpp b/include/eepp/ui/models/modelselection.hpp index 6a9a86a11..2ae6f38ef 100644 --- a/include/eepp/ui/models/modelselection.hpp +++ b/include/eepp/ui/models/modelselection.hpp @@ -2,6 +2,8 @@ #define EE_UI_MODEL_MODELSELECTION_HPP #include +#include +#include #include #include @@ -17,12 +19,20 @@ class EE_API ModelSelection { public: ModelSelection( UIAbstractView* view ) : mView( view ) {} - int size() const { return mIndexes.size(); } - bool isEmpty() const { return mIndexes.empty(); } + int size() const { + Lock l( mMutex ); + return mIndexes.size(); + } + bool isEmpty() const { + Lock l( mMutex ); + return mIndexes.empty(); + } bool contains( const ModelIndex& index ) const { + Lock l( mMutex ); return std::find( mIndexes.begin(), mIndexes.end(), index ) != mIndexes.end(); } bool containsRow( int row ) const { + Lock l( mMutex ); for ( auto& index : mIndexes ) { if ( index.row() == row ) return true; @@ -48,13 +58,16 @@ class EE_API ModelSelection { } std::vector indexes() const { + Lock l( mMutex ); std::vector indexes; + indexes.reserve( mIndexes.size() ); for ( auto& index : mIndexes ) indexes.push_back( index ); return indexes; } ModelIndex first() const { + Lock l( mMutex ); if ( mIndexes.empty() ) return {}; return *mIndexes.begin(); @@ -79,6 +92,8 @@ class EE_API ModelSelection { bool mDisableNotify{ false }; bool mNotifyPending{ false }; void notifySelectionChanged(); + + mutable Mutex mMutex; }; }}} // namespace EE::UI::Models diff --git a/src/eepp/ui/abstract/uiabstractview.cpp b/src/eepp/ui/abstract/uiabstractview.cpp index b68c0f2ad..3cfe3cb0b 100644 --- a/src/eepp/ui/abstract/uiabstractview.cpp +++ b/src/eepp/ui/abstract/uiabstractview.cpp @@ -151,6 +151,12 @@ void UIAbstractView::onModelSelectionChange() { } void UIAbstractView::notifySelectionChange() { + if ( !Engine::isMainThread() ) { + debounce( [this] { notifySelectionChange(); }, Time::Zero, + String::hash( "notifySelectionChange" ) ); + return; + } + onModelSelectionChange(); sendCommonEvent( Event::OnSelectionChanged ); if ( mOnSelectionChange ) diff --git a/src/eepp/ui/models/modelselection.cpp b/src/eepp/ui/models/modelselection.cpp index ff6b81071..d93081497 100644 --- a/src/eepp/ui/models/modelselection.cpp +++ b/src/eepp/ui/models/modelselection.cpp @@ -5,6 +5,7 @@ namespace EE { namespace UI { namespace Models { void ModelSelection::removeAllMatching( std::function filter ) { + Lock l( mMutex ); std::vector notMatching; for ( auto& index : mIndexes ) { if ( !filter( index ) ) @@ -18,6 +19,7 @@ void ModelSelection::removeAllMatching( std::function void ModelSelection::set( const ModelIndex& index ) { eeASSERT( index.isValid() ); + Lock l( mMutex ); if ( mIndexes.size() == 1 && contains( index ) ) return; mIndexes.clear(); @@ -30,6 +32,7 @@ void ModelSelection::set( const std::vector& indexes, bool notify ) for ( auto& index : indexes ) eeASSERT( index.isValid() ); #endif + Lock l( mMutex ); mIndexes.clear(); mIndexes = indexes; if ( notify ) @@ -38,6 +41,7 @@ void ModelSelection::set( const std::vector& indexes, bool notify ) void ModelSelection::add( const ModelIndex& index ) { eeASSERT( index.isValid() ); + Lock l( mMutex ); auto contains = std::find( mIndexes.begin(), mIndexes.end(), index ); if ( contains == mIndexes.end() ) return; @@ -47,6 +51,7 @@ void ModelSelection::add( const ModelIndex& index ) { void ModelSelection::toggle( const ModelIndex& index ) { eeASSERT( index.isValid() ); + Lock l( mMutex ); auto contains = std::find( mIndexes.begin(), mIndexes.end(), index ); if ( contains != mIndexes.end() ) mIndexes.erase( contains ); @@ -57,6 +62,7 @@ void ModelSelection::toggle( const ModelIndex& index ) { bool ModelSelection::remove( const ModelIndex& index ) { eeASSERT( index.isValid() ); + Lock l( mMutex ); auto contains = std::find( mIndexes.begin(), mIndexes.end(), index ); if ( contains == mIndexes.end() ) return false; @@ -66,6 +72,7 @@ bool ModelSelection::remove( const ModelIndex& index ) { } void ModelSelection::clear( bool notify ) { + Lock l( mMutex ); if ( mIndexes.empty() ) return; mIndexes.clear(); @@ -74,6 +81,7 @@ void ModelSelection::clear( bool notify ) { } void ModelSelection::notifySelectionChanged() { + Lock l( mMutex ); if ( !mDisableNotify ) { mView->notifySelectionChange(); mNotifyPending = false; diff --git a/src/tests/unit_tests/fontrendering.cpp b/src/tests/unit_tests/fontrendering.cpp index ba92fcf60..e11ef2feb 100644 --- a/src/tests/unit_tests/fontrendering.cpp +++ b/src/tests/unit_tests/fontrendering.cpp @@ -795,3 +795,33 @@ UTEST( FontRendering, textSetFillColor ) { Engine::destroySingleton(); } + +UTEST( FontRendering, UITextTest ) { + const auto runTest = [&]() { + UIApplication app( + WindowSettings( 1024, 650, "eepp - UI Text Test", WindowStyle::Default, + WindowBackend::Default, 32, {}, 1, false, true ), + UIApplication::Settings( Sys::getProcessPath() + ".." + FileSystem::getOSSlash(), 1 ) ); + FileSystem::changeWorkingDirectory( Sys::getProcessPath() ); + app.getUI()->loadLayoutFromFile( "assets/layouts/ui_text_test.xml" ); + SceneManager::instance()->update(); + SceneManager::instance()->draw(); + compareImages( utest_state, utest_result, app.getWindow(), "eepp-ui-text-test" ); + }; + + UTEST_PRINT_STEP( "Text Shaper disabled" ); + { + BoolScopedOp op( Text::TextShaperEnabled, false ); + runTest(); + } + + UTEST_PRINT_STEP( "Text Shaper enabled" ); + { + BoolScopedOp op( Text::TextShaperEnabled, true ); + runTest(); + + UTEST_PRINT_STEP( "Text Shaper enabled w/o optimizations" ); + BoolScopedOp op2( Text::TextShaperOptimizations, false ); + runTest(); + } +}