From ecf738b96470edddc3ef82112c8621901c87e4bf Mon Sep 17 00:00:00 2001 From: ReadRoberts Date: Thu, 21 Mar 2019 10:06:47 -0700 Subject: [PATCH 1/5] Add support for building VVAR table from vmtx and VORG tables. Add test case. --- Lib/fontTools/varLib/__init__.py | 140 ++++++++++++++---- Tests/varLib/data/TestVVAR.designspace | 36 +++++ .../data/master_vvar_cff2/TestVVAR.0.otf | Bin 0 -> 4276 bytes .../data/master_vvar_cff2/TestVVAR.1.otf | Bin 0 -> 4332 bytes Tests/varLib/data/test_results/TestVVAR.ttx | 59 ++++++++ Tests/varLib/varLib_test.py | 14 ++ 6 files changed, 223 insertions(+), 26 deletions(-) create mode 100644 Tests/varLib/data/TestVVAR.designspace create mode 100644 Tests/varLib/data/master_vvar_cff2/TestVVAR.0.otf create mode 100644 Tests/varLib/data/master_vvar_cff2/TestVVAR.1.otf create mode 100644 Tests/varLib/data/test_results/TestVVAR.ttx diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 0543ee375..cceabcb6e 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -463,46 +463,134 @@ def _merge_TTHinting(font, masterModel, master_ttfs, tolerance=0.5): var = TupleVariation(support, delta) cvar.variations.append(var) -def _add_HVAR(font, masterModel, master_ttfs, axisTags): +hvar_fields = { 'table_tag': 'HVAR', + 'metrics_tag': 'hmtx', + 'sb1': 'LsbMap', + 'sb2': 'RsbMap', + 'mapping_name': 'AdvWidthMap', + } +vvar_fields = { 'table_tag': 'VVAR', + 'metrics_tag': 'vmtx', + 'bearing1': 'TsbMap', + 'bearing2': 'BsbMap', + 'mapping': 'AdvHeightMap', + } +MetricsFields = namedtuple('MetricsFields', + ['table_tag', 'metrics_tag', 'sb1', 'sb2', 'adv_mapping', 'vorig_mapping']) - log.info("Generating HVAR") +hvar_fields = MetricsFields(table_tag='HVAR', metrics_tag='hmtx', sb1='LsbMap', + sb2='RsbMap', adv_mapping='AdvWidthMap', vorig_mapping=None) + +vvar_fields = MetricsFields(table_tag='VVAR', metrics_tag='vmtx', sb1='TsbMap', + sb2='BsbMap', adv_mapping='AdvHeightMap', vorig_mapping='VOrgMap') + +def _add_HVAR(font, masterModel, master_ttfs, axisTags): + _add_VHVAR(font, masterModel, master_ttfs, axisTags, hvar_fields) + +def _add_VVAR(font, masterModel, master_ttfs, axisTags): + _add_VHVAR(font, masterModel, master_ttfs, axisTags, vvar_fields) + + +def _add_VHVAR(font, masterModel, master_ttfs, axisTags, table_fields): + + table_tag = table_fields.table_tag + assert table_tag not in font + log.info("Generating " + table_tag) + VHVAR = newTable(table_tag) + table_class = getattr(ot, table_tag) + vhvar = VHVAR.table = table_class() + vhvar.Version = 0x00010000 glyphOrder = font.getGlyphOrder() - hAdvanceDeltasAndSupports = {} - metricses = [m["hmtx"].metrics for m in master_ttfs] - for glyph in glyphOrder: - hAdvances = [metrics[glyph][0] if glyph in metrics else None for metrics in metricses] - hAdvanceDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(hAdvances) + # Build list of source font advance widths for each glyph + metrics_tag = table_fields.metrics_tag + adv_metricses = [m[metrics_tag].metrics for m in master_ttfs] - singleModel = models.allEqual(id(v[1]) for v in hAdvanceDeltasAndSupports.values()) + # Build list of source font vertical origin coords for each glyph + if table_tag == 'VVAR' and 'VORG' in master_ttfs[0]: + v_orig_metricses = [m['VORG'].VOriginRecords for m in master_ttfs] + default_y_origs = [m['VORG'].defaultVertOriginY for m in master_ttfs] + v_orig_metricses = list(zip(v_orig_metricses, default_y_origs)) + else: + v_orig_metricses = None + + metricsStore, advanceMapping, vOrigMapping = _get_advance_metrics(font, + masterModel, master_ttfs, axisTags, glyphOrder, adv_metricses, + v_orig_metricses) + + vhvar.VarStore = metricsStore + if advanceMapping is None: + setattr(vhvar, table_fields.adv_mapping, None) + else: + setattr(vhvar, table_fields.adv_mapping, advanceMapping) + if vOrigMapping is not None: + setattr(vhvar, table_fields.vorig_mapping, vOrigMapping) + setattr(vhvar, table_fields.sb1, None) + setattr(vhvar, table_fields.sb2, None) + + font[table_tag] = VHVAR + return + +def _get_advance_metrics(font, masterModel, master_ttfs, + axisTags, glyphOrder, adv_metricses, v_orig_metricses=None): + + vhAdvanceDeltasAndSupports = {} + vOrigDeltasAndSupports = {} + for glyph in glyphOrder: + vhAdvances = [metrics[glyph][0] if glyph in metrics else None for metrics in adv_metricses] + vhAdvanceDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(vhAdvances) + + singleModel = models.allEqual(id(v[1]) for v in vhAdvanceDeltasAndSupports.values()) + + if v_orig_metricses: + singleModel = False + for glyph in glyphOrder: + # We need to supply a vOrigs tuple with non-None default values + # for each glyph. v_orig_metricses contains values only for those + # glyphs which have a non-default vOrig. + vOrigs = [metrics[glyph] if glyph in metrics else defaultVOrig + for metrics, defaultVOrig in v_orig_metricses] + print(glyph, vOrigs) + vOrigDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(vOrigs) directStore = None if singleModel: # Build direct mapping - - supports = next(iter(hAdvanceDeltasAndSupports.values()))[1][1:] + supports = next(iter(vhAdvanceDeltasAndSupports.values()))[1][1:] varTupleList = builder.buildVarRegionList(supports, axisTags) varTupleIndexes = list(range(len(supports))) varData = builder.buildVarData(varTupleIndexes, [], optimize=False) for glyphName in glyphOrder: - varData.addItem(hAdvanceDeltasAndSupports[glyphName][0]) + varData.addItem(vhAdvanceDeltasAndSupports[glyphName][0]) varData.optimize() directStore = builder.buildVarStore(varTupleList, [varData]) # Build optimized indirect mapping storeBuilder = varStore.OnlineVarStoreBuilder(axisTags) - mapping = {} + adv_mapping = {} for glyphName in glyphOrder: - deltas,supports = hAdvanceDeltasAndSupports[glyphName] + deltas, supports = vhAdvanceDeltasAndSupports[glyphName] storeBuilder.setSupports(supports) - mapping[glyphName] = storeBuilder.storeDeltas(deltas) + adv_mapping[glyphName] = storeBuilder.storeDeltas(deltas) + + if v_orig_metricses: + v_orig_mapping_i = {} + for glyphName in glyphOrder: + deltas, supports = vOrigDeltasAndSupports[glyphName] + storeBuilder.setSupports(supports) + v_orig_mapping_i[glyphName] = storeBuilder.storeDeltas(deltas) + indirectStore = storeBuilder.finish() mapping2 = indirectStore.optimize() - mapping = [mapping2[mapping[g]] for g in glyphOrder] - advanceMapping = builder.buildVarIdxMap(mapping, glyphOrder) + adv_mapping = [mapping2[adv_mapping[g]] for g in glyphOrder] + advanceMapping = builder.buildVarIdxMap(adv_mapping, glyphOrder) + + if v_orig_metricses: + v_orig_mapping_i = [mapping2[v_orig_mapping_i[g]] for g in glyphOrder] use_direct = False + vOrigMapping = None if directStore: # Compile both, see which is more compact @@ -517,18 +605,16 @@ def _add_HVAR(font, masterModel, master_ttfs, axisTags): use_direct = directSize < indirectSize - # Done; put it all together. - assert "HVAR" not in font - HVAR = font["HVAR"] = newTable('HVAR') - hvar = HVAR.table = ot.HVAR() - hvar.Version = 0x00010000 - hvar.LsbMap = hvar.RsbMap = None if use_direct: - hvar.VarStore = directStore - hvar.AdvWidthMap = None + metricsStore = directStore + advanceMapping = None else: - hvar.VarStore = indirectStore - hvar.AdvWidthMap = advanceMapping + metricsStore = indirectStore + if v_orig_metricses: + vOrigMapping = builder.buildVarIdxMap(v_orig_mapping_i, glyphOrder) + + return metricsStore, advanceMapping, vOrigMapping + def _add_MVAR(font, masterModel, master_ttfs, axisTags): @@ -840,6 +926,8 @@ def build(designspace, master_finder=lambda s:s, exclude=[], optimize=True): _add_MVAR(vf, model, master_fonts, axisTags) if 'HVAR' not in exclude: _add_HVAR(vf, model, master_fonts, axisTags) + if 'VVAR' not in exclude and 'vmtx' in vf: + _add_VVAR(vf, model, master_fonts, axisTags) if 'GDEF' not in exclude or 'GPOS' not in exclude: _merge_OTL(vf, model, master_fonts, axisTags) if 'gvar' not in exclude and 'glyf' in vf: diff --git a/Tests/varLib/data/TestVVAR.designspace b/Tests/varLib/data/TestVVAR.designspace new file mode 100644 index 000000000..162f7385c --- /dev/null +++ b/Tests/varLib/data/TestVVAR.designspace @@ -0,0 +1,36 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/data/master_vvar_cff2/TestVVAR.0.otf b/Tests/varLib/data/master_vvar_cff2/TestVVAR.0.otf new file mode 100644 index 0000000000000000000000000000000000000000..9749ab5b046a0d034b0dbdab2016d8f0dd01e982 GIT binary patch literal 4276 zcmb7H2~dwfR` z_g){>6Q7U}Py9#`(U7E>F)`B5=CXZ+_-YAZjZx;fS%eT3A$^x3XS2);XN*5MX(b{4 zzo4EPZ=PV>xy)LJCeNXslo)TCr8(fg9>=etE+uDM?1`~epAzD;7WMRWt0g5caIi1R ze@6x4gtRch5AHA~nF&m0_HhBAe$N+N0D>IZYdgUI(pM33=dy=u_ zo>%Tg0#$>&azEm$8jne-v^@~vYSl!qtRhC$A+M|^eyUquSwo^!_q?+9F&j!!)JAW= z57Dc)du4|5Ij`JdU|r`b8vW?$vVNK1D~Et1g?94<(K>Ua|`jjZSyD>=6?t6ciNbEpkcp z$+m2CcUc`FWG1l@J6S~>B!i@pbmGGO)`Dat1`&ZBj=3;LO(JS^@Bn!FeQAF9&gg#Cr2g%1)wlrhZJk*m>mq;wJkR0S@h07j^_=`k8 zi6oF{>`zc)q@hn1+BguM9x)n72$@0_k~w4%nU9hgBR!1qFhYVdw~H(%lSn8TPYTe> zq^*oLBA z*#C*>@oJm@XSGdeVM7!)oOwQG;lvfpAgi%PnTT#4k`u|LAhn=PELJoDfALt$I3lI> z*?KoKx#A5R3BnxZs%Buj0xK-%lB(n`?+81N*l^YIupTaNwMHwu#f4)LWCH#Ql%F2` zAMO{uQoU?14tdu&aJ4*_p?o0*d2f@!@+nqyAJQAS=l;`@=_vIk3~ywuVMb!tilXkJ z-I3kh@_ngjlB``l)6%29gnUEJc%#Yw^5?yByN~0V{Kluk%bKWEs)4_A3gxju?oq@Y zq}dbx>+>C7Qa;O{&*@h)nC%L{t4+oH`wtmDHh9vsnQ`%^8qM!-5qyQDz~A ziAH2$#&CHoG%O}6Im9q2c(UA1Heeeb945CDLy|*+!;HbnAx1-JFy0#f1q-X9dbt5#5?A5G6$wq_wP5ddR(f8Yxk z@O-Y}QG5b0{v_NG{vhHQh(Yn=mIKRx_04tn*&*(vc{_4z($LWdLPdWNLm7m%Eq)W& zyKt=a;3upSXvySedn>Sy;Ny1S(=qToJ%37!M4u=S0U{nmIR?()R1D@rfSrc7>e^bF z)4ffXCq~n?NnPwFvNcabZ-C^_Kua*g51`%k{$v2 z3ExTiHlWwVPAWd+Z|IKm@wDmNSFbhRmN7LM8={(wtWAukVl7ZVi7(@UItaSD;v--e z;Jp(kPqEEFw?xnbK!JZx;r_Dkh60O%=ohlG*(?s5Prfu$L{2j+pn{V9hLQ!TUlBK70-zPUBybeuk3u;K3o(v@L{{TrisA; zIxbfXFBJWNu447MlutQ)gKK~zMD90@YsGM27MQYO&&bRcrTu-gRU+LYfE1)2KHikCZtrbtuJleuz4dpRlc`k z_g1!h+qPQ3mG0=I*~%`O*%6>*8sy0oS%b(^B0um%MwTY3bdW>caoE#LJCX6l?v5tjbD%p3VY!tub$mtL7tp2aE05}? zU0SSdSF;Z6)cq}v?4zB?{5mG=)oo~?OAF#R%)oOu6+|GO#bLOt&+w7FKgR>iW8q(V zWkE+pM8^YL0;?MuK4YhN1kI|+tX_(4V{n6y2K1VJ-g+L`bvWI0;tbmYv^f4~K^w5| z;D?X#fMQfh^!{1M*U35%Q^imab8%ly8jpt0~-E zcvVQi#)CdR)ye7|^jPDU2XA6g0-<$YMJy9-g`$rIjO-8|@5^_O^22*Dp#gO23)3CZ zz#?H3FoOCg>`%=%G!YZF&tS9R+;uc0O(oD{rQvZ-IDzy$A*7fQgkE+nBpHT323N z_Tr`zwpctpMn7yei?PgGohYBo+XeA!e$Vm<=I;M0UXwIbB#KhRU6{nGN~($~Ut)ay zDgH9=`8`i#g*6XPb-U2$cG>9P8hO71yjKe=-A6^>N--cStdP}lJZ*TDeWsuk$apvp z$0PIO0sR9WRbOq z2qoDyo@9|N5hlgvw#Nn;Z{+{L^dETAe;|j#o2)>0()LbrQ!Z=RNgewZ9a*}cQbeA_x3BVM!v8})#8+KI7*Y?Uo=ClrdLzj%te*Im ziy*3~n7Ik~uF}fSB79HD|9DkIoo{uxh+n?LWh8-0k36iBM=+|N;;$loyH*PoZ;nz>jOo=M`Fx~uS7-|WaKduGk}n(y?pHy zHJ_j*l%~!uf^ftWE!qo>|jo&j7GY>`F0sy4Ju2;~L#q^3yPbZkawX!+>^^?)Klt z|ILFeH^bPVT@Rs<9h40)1QrBku9uNW7f^tuYfwrI= z27Pq{gK{K<>f)(ZI%5w4TCE!slywlNI~tVrVA5R=$_ALKYYWQ8-}x}e*N+R@Lm)xF zFDP>`>pu$0J>WKjF(~(hT*IKC90tP-4+iB(Fc}U_a(k-ET#E~RYLgmgv5rzx^4*J^ zs<+DPbC!73Np5qjI^CCVR#S?L)hx}?t7bX9&aw(;zS-t3FUxbP8IDrb<|y^5+1V*s zs(rG}p5Cba@gSY)B_^c{-kz1YN@)o+P%ZnU$ zIlK;^qsXCVq|lzunxYP?aFn?mi;A6UfxFbFddl4S<#|2=bw{Dk=NU6%L}g{A*`XE2 zoaZi~aG$d*0VaVPJWvH?;DW_a2tH7ui6{;%V1-d6r9eLUECMIVUh?#kj}uCWtAS`U z#F8|ftodLL_!mPl@vPr6YMxmX<0UGC3Tib^8)YpgJ&$x1GQa_)#BBktMP$=2h2m7O z!(_7C134B`OfmVC5jrzrv_L#0!z`Evvtb5FHp=vCj$bp_17rJOA&i7X7!H*bt3_IZ z8L|VoyoA>s!0`Lbg=F&)My;oUuypsC0~toscqN3Th~hP`0^&=FwCb5@Br_yYn+T(c z|3A2XUwPaAtb7)ExCx7!#-BlR@zUFLK{eH@h%nwol|h1b*3mxhreVPoW`MS{$uMzApNU48KvFN@bV?g76SF%P zt?Y{GO6ls-&d`9SYTA2en|kbi1pW?ZgW0rr?a@Y|r|WroH-GTx=l}N3w`Ptq64<@EYh#8Sv2Jn~q(21wx5jWmCIL111?4&7~S$XB9uC%l`OEtD8^D^o74+cnki@=%DyrO zLe?nKnaD(TNTxHHDb6cfKV-F+_gt-O(LmLX-5Xaso^PGVWF#_^DiVcN!PwI&*O8yW zm!Eq61 zi|8E+zI5U_ zb)qkmeez=-lCj8V;*>3(ja+oeTa|Ubhvz@y;~v>>(Wu<%JU450p$q8^K2nEG9A7_r z{hbs1sroYqE+PLCPo(2$j&X&D|;}JedLUYLNSQIBuFlF@;2ntaQc1oeY9Ba z9xm@hNqtc-FSvmGBmCg_ODFl;?_3f6pBL$T)4saXoA+=T(X3VYmG*fG$_VC9&?*|i zI*4zT6=Vx%;tkCf&LnYhi$6rcjtSkR_fKGwUcu^)YDF3g*hQK1gNzmlseISdM_xJj z1eZf!i4qZWL_8P61Xr+vIc25nBZo+vjn|hp-o1S>7bPvUk`rDNXL<5dta3H;A?vs- z2Pg-rkXH_T*LV|ET*`Nd4+B8Oo^GI$_-EoS1v`&3ynprT2iEZiuo%7T z`A%1@eS?di!=p7#d-$Qfk2T|)9NUjG85USn+3hz{COs2E%cN(L_->h{U=$OV$@CMG z#C)=nybR25)VvI=gShDH??**QV?s0f)0otZA@Yn;SG*^8%e}ni6kBQfYvlyrz3)Fu~D$QG32=?IV$Y3Yv_ z$%6%r-h!_`_WnM8hiFr(#<-!ZtGfZYEPwYpH92| zasPQH^GMj)ah{0@$j*vLCK`~PlaWjw7t56W$JqXJ2jAQG8m~dtV0&WnejDGnkkPTj zOrlDJE13G>Oxi~L5?*Y0wTW+}ZNGrgnMXxhMRZ%t1sR7t1}(mEE`ELld!ymp&R3CN z!RM-KTwLy35j6*6xiD{6u=!MjNZG`3I&&<|sh)#;F1og112>6If4lPNvb7a!m+=*2 zs-tl@&AV5l3_X<2MV}{`>q==0ikmrJt^3!WOT1=B&CX4`xQwFxUY)LNP}#|~SJr5y zj78zvDh51G`yH6H7Rw>4GdII?5Aq^h zu?_cdeCFM^zIlqDto!TUR?0>P`#pF!$GazwmvgH5hj%cszOjWq0B!WHB4zZaat89T zIC{m@YA%x&(zarOl`!(H#q^@D;}?hDKFq&7@#D9zivE1_?p?AKtCM3D9K!tL$nrhwASPys1GFM91&e)&Nh2x&#FsW4-o>BZwe1N!&C%b=q@t~&jea`CMlk(F3%VY(IGyit7e8oD5iEQyCP5)x>6LT?e&WYP)xpJ_)xwy89Up>n1 z+g;e`*~3Q_u#T<%-gE*@>@Jr73iE$SnL4g8aVzomiLA2?*~c9xv%HW|}R;YdpaH1YW zJ&D4IdJ$>2u%2||8w0x3wCQ%bdl|_Bbm!9kDeFLA;VkoksiMpm2Ymw;ty`z{P*zhQ zUI)E{T+@4zT}Ko`9-3c>7C?_S61C*o_6yzl2x%nrHoX5^PoSF`ZfG@ls5ggbm7!G^ YOFWgRKhb2GV=CQOTj>57Nq2krH^q2&(*OVf literal 0 HcmV?d00001 diff --git a/Tests/varLib/data/test_results/TestVVAR.ttx b/Tests/varLib/data/test_results/TestVVAR.ttx new file mode 100644 index 000000000..48ca40858 --- /dev/null +++ b/Tests/varLib/data/test_results/TestVVAR.ttx @@ -0,0 +1,59 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/varLib/varLib_test.py b/Tests/varLib/varLib_test.py index 32e7ab5e2..c7e6a65d9 100644 --- a/Tests/varLib/varLib_test.py +++ b/Tests/varLib/varLib_test.py @@ -442,6 +442,20 @@ class BuildTest(unittest.TestCase): mvar_tags = [vr.ValueTag for vr in varfont["MVAR"].table.ValueRecord] assert all(tag in mvar_tags for tag in fontTools.varLib.mvar.MVAR_ENTRIES) + def test_varlib_build_VVAR_CFF2(self): + ds_path = self.get_test_input('TestVVAR.designspace') + suffix = '.otf' + expected_ttx_name = 'TestVVAR' + tables = ["VVAR"] + + finder = lambda s: s.replace('.ufo', suffix) + varfont, model, _ = build(ds_path, finder) + varfont = reload_font(varfont) + + expected_ttx_path = self.get_test_output(expected_ttx_name + '.ttx') + self.expect_ttx(varfont, expected_ttx_path, tables) + self.check_ttx_dump(varfont, expected_ttx_path, tables, suffix) + def test_load_masters_layerName_without_required_font(): ds = DesignSpaceDocument() From 1fe0348bad7f6d1caf44fa16df7c815da792ab95 Mon Sep 17 00:00:00 2001 From: ReadRoberts Date: Thu, 21 Mar 2019 10:48:56 -0700 Subject: [PATCH 2/5] Remove old debug pdb.set_trace(). This has already been fixed on another branch that has not yet been merged, but I need to fix it here for the VVAR test to pass. --- Lib/fontTools/varLib/cff.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Lib/fontTools/varLib/cff.py b/Lib/fontTools/varLib/cff.py index a000dd48f..b6febe2e0 100644 --- a/Lib/fontTools/varLib/cff.py +++ b/Lib/fontTools/varLib/cff.py @@ -469,11 +469,6 @@ class CFF2CharStringMergePen(T2CharStringPen): arg[0] = round_func(arg[0]) program.append(arg[0]) for arg in blendlist: - # for each coordinate tuple, append the region deltas - if len(arg) != 3: - print(arg) - import pdb - pdb.set_trace() deltas = var_model.getDeltas(arg) if round_func: deltas = [round_func(delta) for delta in deltas] From c06c5c508709039d06696dd4e2a0877b9e325f35 Mon Sep 17 00:00:00 2001 From: ReadRoberts Date: Tue, 26 Mar 2019 10:45:04 -0700 Subject: [PATCH 3/5] Code clean-up of items pointed out in review. removed unused dicts Remove print statement Rename v_orig_mapping_i to v_orig_mapping. The suffix was left over from an earlier pass, when there was a mapping for the direct store and another one for the indirect store. --- Lib/fontTools/varLib/__init__.py | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index cceabcb6e..79ba293c0 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -463,18 +463,6 @@ def _merge_TTHinting(font, masterModel, master_ttfs, tolerance=0.5): var = TupleVariation(support, delta) cvar.variations.append(var) -hvar_fields = { 'table_tag': 'HVAR', - 'metrics_tag': 'hmtx', - 'sb1': 'LsbMap', - 'sb2': 'RsbMap', - 'mapping_name': 'AdvWidthMap', - } -vvar_fields = { 'table_tag': 'VVAR', - 'metrics_tag': 'vmtx', - 'bearing1': 'TsbMap', - 'bearing2': 'BsbMap', - 'mapping': 'AdvHeightMap', - } MetricsFields = namedtuple('MetricsFields', ['table_tag', 'metrics_tag', 'sb1', 'sb2', 'adv_mapping', 'vorig_mapping']) @@ -551,7 +539,6 @@ def _get_advance_metrics(font, masterModel, master_ttfs, # glyphs which have a non-default vOrig. vOrigs = [metrics[glyph] if glyph in metrics else defaultVOrig for metrics, defaultVOrig in v_orig_metricses] - print(glyph, vOrigs) vOrigDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(vOrigs) directStore = None @@ -575,11 +562,11 @@ def _get_advance_metrics(font, masterModel, master_ttfs, adv_mapping[glyphName] = storeBuilder.storeDeltas(deltas) if v_orig_metricses: - v_orig_mapping_i = {} + v_orig_mapping = {} for glyphName in glyphOrder: deltas, supports = vOrigDeltasAndSupports[glyphName] storeBuilder.setSupports(supports) - v_orig_mapping_i[glyphName] = storeBuilder.storeDeltas(deltas) + v_orig_mapping[glyphName] = storeBuilder.storeDeltas(deltas) indirectStore = storeBuilder.finish() mapping2 = indirectStore.optimize() @@ -587,7 +574,7 @@ def _get_advance_metrics(font, masterModel, master_ttfs, advanceMapping = builder.buildVarIdxMap(adv_mapping, glyphOrder) if v_orig_metricses: - v_orig_mapping_i = [mapping2[v_orig_mapping_i[g]] for g in glyphOrder] + v_orig_mapping = [mapping2[v_orig_mapping[g]] for g in glyphOrder] use_direct = False vOrigMapping = None @@ -611,7 +598,7 @@ def _get_advance_metrics(font, masterModel, master_ttfs, else: metricsStore = indirectStore if v_orig_metricses: - vOrigMapping = builder.buildVarIdxMap(v_orig_mapping_i, glyphOrder) + vOrigMapping = builder.buildVarIdxMap(v_orig_mapping, glyphOrder) return metricsStore, advanceMapping, vOrigMapping From ff0716f7b5c574121ff77a241d54fd4848516471 Mon Sep 17 00:00:00 2001 From: ReadRoberts Date: Tue, 26 Mar 2019 13:24:35 -0700 Subject: [PATCH 4/5] Code clean-up of items pointed out in review. re-named variables from snake-case to camel-case throughout functions, except for 'master_ttfs' (which is ugly when camel-cased, and is the lone snake-cased in other functions) and the function names (which follows the precedent set in almost all of the rest of the module). --- Lib/fontTools/varLib/__init__.py | 98 ++++++++++++++++---------------- 1 file changed, 48 insertions(+), 50 deletions(-) diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 79ba293c0..389a74381 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -464,81 +464,80 @@ def _merge_TTHinting(font, masterModel, master_ttfs, tolerance=0.5): cvar.variations.append(var) MetricsFields = namedtuple('MetricsFields', - ['table_tag', 'metrics_tag', 'sb1', 'sb2', 'adv_mapping', 'vorig_mapping']) + ['tableTag', 'metricsTag', 'sb1', 'sb2', 'advMapping', 'vOrigMapping']) -hvar_fields = MetricsFields(table_tag='HVAR', metrics_tag='hmtx', sb1='LsbMap', - sb2='RsbMap', adv_mapping='AdvWidthMap', vorig_mapping=None) - -vvar_fields = MetricsFields(table_tag='VVAR', metrics_tag='vmtx', sb1='TsbMap', - sb2='BsbMap', adv_mapping='AdvHeightMap', vorig_mapping='VOrgMap') +hvarFields = MetricsFields(tableTag='HVAR', metricsTag='hmtx', sb1='LsbMap', + sb2='RsbMap', advMapping='AdvWidthMap', vOrigMapping=None) + +vvarFields = MetricsFields(tableTag='VVAR', metricsTag='vmtx', sb1='TsbMap', + sb2='BsbMap', advMapping='AdvHeightMap', vOrigMapping='VOrgMap') def _add_HVAR(font, masterModel, master_ttfs, axisTags): - _add_VHVAR(font, masterModel, master_ttfs, axisTags, hvar_fields) + _add_VHVAR(font, masterModel, master_ttfs, axisTags, hvarFields) def _add_VVAR(font, masterModel, master_ttfs, axisTags): - _add_VHVAR(font, masterModel, master_ttfs, axisTags, vvar_fields) + _add_VHVAR(font, masterModel, master_ttfs, axisTags, vvarFields) +def _add_VHVAR(font, masterModel, master_ttfs, axisTags, tableFields): -def _add_VHVAR(font, masterModel, master_ttfs, axisTags, table_fields): - - table_tag = table_fields.table_tag - assert table_tag not in font - log.info("Generating " + table_tag) - VHVAR = newTable(table_tag) - table_class = getattr(ot, table_tag) + tableTag = tableFields.tableTag + assert tableTag not in font + log.info("Generating " + tableTag) + VHVAR = newTable(tableTag) + table_class = getattr(ot, tableTag) vhvar = VHVAR.table = table_class() vhvar.Version = 0x00010000 glyphOrder = font.getGlyphOrder() # Build list of source font advance widths for each glyph - metrics_tag = table_fields.metrics_tag - adv_metricses = [m[metrics_tag].metrics for m in master_ttfs] + metricsTag = tableFields.metricsTag + advMetricses = [m[metricsTag].metrics for m in master_ttfs] # Build list of source font vertical origin coords for each glyph - if table_tag == 'VVAR' and 'VORG' in master_ttfs[0]: - v_orig_metricses = [m['VORG'].VOriginRecords for m in master_ttfs] - default_y_origs = [m['VORG'].defaultVertOriginY for m in master_ttfs] - v_orig_metricses = list(zip(v_orig_metricses, default_y_origs)) + if tableTag == 'VVAR' and 'VORG' in master_ttfs[0]: + vOrigMetricses = [m['VORG'].VOriginRecords for m in master_ttfs] + defaultYOrigs = [m['VORG'].defaultVertOriginY for m in master_ttfs] + vOrigMetricses = list(zip(vOrigMetricses, defaultYOrigs)) else: - v_orig_metricses = None + vOrigMetricses = None metricsStore, advanceMapping, vOrigMapping = _get_advance_metrics(font, - masterModel, master_ttfs, axisTags, glyphOrder, adv_metricses, - v_orig_metricses) + masterModel, master_ttfs, axisTags, glyphOrder, advMetricses, + vOrigMetricses) vhvar.VarStore = metricsStore if advanceMapping is None: - setattr(vhvar, table_fields.adv_mapping, None) + setattr(vhvar, tableFields.advMapping, None) else: - setattr(vhvar, table_fields.adv_mapping, advanceMapping) + setattr(vhvar, tableFields.advMapping, advanceMapping) if vOrigMapping is not None: - setattr(vhvar, table_fields.vorig_mapping, vOrigMapping) - setattr(vhvar, table_fields.sb1, None) - setattr(vhvar, table_fields.sb2, None) + setattr(vhvar, tableFields.vOrigMapping, vOrigMapping) + setattr(vhvar, tableFields.sb1, None) + setattr(vhvar, tableFields.sb2, None) - font[table_tag] = VHVAR + font[tableTag] = VHVAR return def _get_advance_metrics(font, masterModel, master_ttfs, - axisTags, glyphOrder, adv_metricses, v_orig_metricses=None): + axisTags, glyphOrder, advMetricses, vOrigMetricses=None): vhAdvanceDeltasAndSupports = {} vOrigDeltasAndSupports = {} for glyph in glyphOrder: - vhAdvances = [metrics[glyph][0] if glyph in metrics else None for metrics in adv_metricses] + vhAdvances = [metrics[glyph][0] if glyph in metrics else None for metrics in advMetricses] vhAdvanceDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(vhAdvances) singleModel = models.allEqual(id(v[1]) for v in vhAdvanceDeltasAndSupports.values()) - if v_orig_metricses: + if vOrigMetricses: singleModel = False for glyph in glyphOrder: # We need to supply a vOrigs tuple with non-None default values - # for each glyph. v_orig_metricses contains values only for those + # for each glyph. vOrigMetricses contains values only for those # glyphs which have a non-default vOrig. vOrigs = [metrics[glyph] if glyph in metrics else defaultVOrig - for metrics, defaultVOrig in v_orig_metricses] + for metrics, defaultVOrig in vOrigMetricses] vOrigDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(vOrigs) directStore = None @@ -555,28 +554,28 @@ def _get_advance_metrics(font, masterModel, master_ttfs, # Build optimized indirect mapping storeBuilder = varStore.OnlineVarStoreBuilder(axisTags) - adv_mapping = {} + advMapping = {} for glyphName in glyphOrder: deltas, supports = vhAdvanceDeltasAndSupports[glyphName] storeBuilder.setSupports(supports) - adv_mapping[glyphName] = storeBuilder.storeDeltas(deltas) + advMapping[glyphName] = storeBuilder.storeDeltas(deltas) - if v_orig_metricses: - v_orig_mapping = {} + if vOrigMetricses: + vOrigMap = {} for glyphName in glyphOrder: deltas, supports = vOrigDeltasAndSupports[glyphName] storeBuilder.setSupports(supports) - v_orig_mapping[glyphName] = storeBuilder.storeDeltas(deltas) + vOrigMap[glyphName] = storeBuilder.storeDeltas(deltas) indirectStore = storeBuilder.finish() mapping2 = indirectStore.optimize() - adv_mapping = [mapping2[adv_mapping[g]] for g in glyphOrder] - advanceMapping = builder.buildVarIdxMap(adv_mapping, glyphOrder) + advMapping = [mapping2[advMapping[g]] for g in glyphOrder] + advanceMapping = builder.buildVarIdxMap(advMapping, glyphOrder) - if v_orig_metricses: - v_orig_mapping = [mapping2[v_orig_mapping[g]] for g in glyphOrder] + if vOrigMetricses: + vOrigMap = [mapping2[vOrigMap[g]] for g in glyphOrder] - use_direct = False + useDirect = False vOrigMapping = None if directStore: # Compile both, see which is more compact @@ -590,19 +589,18 @@ def _get_advance_metrics(font, masterModel, master_ttfs, advanceMapping.compile(writer, font) indirectSize = len(writer.getAllData()) - use_direct = directSize < indirectSize + useDirect = directSize < indirectSize - if use_direct: + if useDirect: metricsStore = directStore advanceMapping = None else: metricsStore = indirectStore - if v_orig_metricses: - vOrigMapping = builder.buildVarIdxMap(v_orig_mapping, glyphOrder) + if vOrigMetricses: + vOrigMapping = builder.buildVarIdxMap(vOrigMap, glyphOrder) return metricsStore, advanceMapping, vOrigMapping - def _add_MVAR(font, masterModel, master_ttfs, axisTags): log.info("Generating MVAR") From 22288588a7e2cefc8d973585063189a18feee997 Mon Sep 17 00:00:00 2001 From: ReadRoberts Date: Tue, 2 Apr 2019 09:12:14 -0700 Subject: [PATCH 5/5] Clean up code formatting - re-name variables --- Lib/fontTools/varLib/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 389a74381..38ee201c0 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -484,8 +484,8 @@ def _add_VHVAR(font, masterModel, master_ttfs, axisTags, tableFields): assert tableTag not in font log.info("Generating " + tableTag) VHVAR = newTable(tableTag) - table_class = getattr(ot, tableTag) - vhvar = VHVAR.table = table_class() + tableClass = getattr(ot, tableTag) + vhvar = VHVAR.table = tableClass() vhvar.Version = 0x00010000 glyphOrder = font.getGlyphOrder()