From a19800195aefbdcf7e20450ec23bb6b47a74522b Mon Sep 17 00:00:00 2001 From: Fini Jastrow Date: Wed, 22 Dec 2021 14:29:34 +0100 Subject: [PATCH 1/4] font-patcher: Copy selection instead of continuously regenerating [why] This is a TODO item. Well, two in fact. The symbolFont's selection is used for two things - the main loop to iterate over all glyphs to insert - to select the one glyph that is actually copied over Because the main loop uses iterators on the selection. The iterator is not 'stable' but invalidates if the selection is changed. The current code therefor restores the old selection before the loop jumps to the head again. This design is not very robust. [how] We need the selection to copy the symbol glyph. But we can rewrite the loop that it does not need the selection at every iteration, but that the selection is copied into a list, and we loop over that list - which is independent on a later selection state. Signed-off-by: Fini Jastrow --- font-patcher | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/font-patcher b/font-patcher index 49e2ad79b..ac706c3af 100755 --- a/font-patcher +++ b/font-patcher @@ -669,15 +669,14 @@ class font_patcher: symbolFont.selection.select((str("ranges"), str("unicode")), symbolFontStart, symbolFontEnd) self.sourceFont.selection.select((str("ranges"), str("unicode")), sourceFontStart, sourceFontEnd) - # Get number of selected non-empty glyphs @TODO FIXME - for index, sym_glyph in enumerate(symbolFont.selection.byGlyphs): - glyphSetLength += 1 - # end for + # Get number of selected non-empty glyphs + symbolFontSelection = list(symbolFont.selection.byGlyphs) + glyphSetLength = len(symbolFontSelection) if self.args.quiet is False: sys.stdout.write("Adding " + str(max(1, glyphSetLength)) + " Glyphs from " + setName + " Set \n") - for index, sym_glyph in enumerate(symbolFont.selection.byGlyphs): + for index, sym_glyph in enumerate(symbolFontSelection): index = max(1, index) try: @@ -822,12 +821,6 @@ class font_patcher: # does not overlap the bearings (edges) self.remove_glyph_neg_bearings(self.sourceFont[currentSourceFontGlyph]) - # reset selection so iteration works properly @TODO fix? rookie misunderstanding? - # This is likely needed because the selection was changed when the glyph was copy/pasted - if symbolFontStart == 0: - symbolFont.selection.all() - else: - symbolFont.selection.select((str("ranges"), str("unicode")), symbolFontStart, symbolFontEnd) # end for if self.args.quiet is False or self.args.progressbars: From 355a7c55a8331ca8f6e719b9cc65d4caa9623167 Mon Sep 17 00:00:00 2001 From: Fini Jastrow Date: Wed, 22 Dec 2021 14:59:03 +0100 Subject: [PATCH 2/4] font-patcher: Remove unneeded code [why] The selection is never used. Later in the code we use a sourceFont selection to paste the glyphs into it, but that is selected just there, before, and locally not totally. Signed-off-by: Fini Jastrow --- font-patcher | 2 -- 1 file changed, 2 deletions(-) diff --git a/font-patcher b/font-patcher index ac706c3af..18cc0655e 100755 --- a/font-patcher +++ b/font-patcher @@ -663,11 +663,9 @@ class font_patcher: # and only copy those that are not already contained in the source font if symbolFontStart == 0: symbolFont.selection.all() - self.sourceFont.selection.all() careful = True else: symbolFont.selection.select((str("ranges"), str("unicode")), symbolFontStart, symbolFontEnd) - self.sourceFont.selection.select((str("ranges"), str("unicode")), sourceFontStart, sourceFontEnd) # Get number of selected non-empty glyphs symbolFontSelection = list(symbolFont.selection.byGlyphs) From 7e92302c12dd26e03ca066240d4f0a803eb0ae07 Mon Sep 17 00:00:00 2001 From: Fini Jastrow Date: Wed, 22 Dec 2021 15:00:39 +0100 Subject: [PATCH 3/4] font-patcher: Be explicit what shall be transformed [why] In the patch loop we use two methods to do something with the glyphs - the glyph object directly (i.e. font[codepoint]) - a selection of glyphs in the font Using the selection is a bit anonymous and depends on lines far away (where the selection is set); direct glyph access seems to be easier understood and is used almost everywhere. Direct glyph access can not be used for copy() and paste(), but for everything else. [how] The direct glyph addressing is already in use almost for everything, but not for the transform() calls. This is changed. Now we transform a specific glyph and not 'all selected glyphs' (with selection = that specific glyph). Signed-off-by: Fini Jastrow --- font-patcher | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/font-patcher b/font-patcher index 18cc0655e..c4aaf324a 100755 --- a/font-patcher +++ b/font-patcher @@ -776,7 +776,7 @@ class font_patcher: if 'overlap' in sym_attr['params']: scale_ratio_x *= 1 + sym_attr['params']['overlap'] scale_ratio_y *= 1 + sym_attr['params']['overlap'] - self.sourceFont.transform(psMat.scale(scale_ratio_x, scale_ratio_y)) + self.sourceFont[currentSourceFontGlyph].transform(psMat.scale(scale_ratio_x, scale_ratio_y)) # Use the dimensions from the newly pasted and stretched glyph sym_dim = get_glyph_dimensions(self.sourceFont[currentSourceFontGlyph]) @@ -807,7 +807,7 @@ class font_patcher: x_align_distance += overlap_width align_matrix = psMat.translate(x_align_distance, y_align_distance) - self.sourceFont.transform(align_matrix) + self.sourceFont[currentSourceFontGlyph].transform(align_matrix) # Needed for setting 'advance width' on each glyph so they do not overlap, # also ensures the font is considered monospaced on Windows by setting the From c728079b61dadca12b6c1d63464737ec96739c0c Mon Sep 17 00:00:00 2001 From: Fini Jastrow Date: Wed, 22 Dec 2021 21:27:35 +0100 Subject: [PATCH 4/4] font-patcher: Simplify output code [why] The code around `currentSourceFontGlyph` and `copiedToSlot` is needlessly complex and checks for conditions that are impossible to occur (possibly because the algorithm was different in the past). It becomes rather hard to follow which variable holds what kind of value and when. [how] Drop all the string-that-contain-hex-numbers stuff and use a regular integer list/variable. Format the string when it is output. Drop obsolete checks. Signed-off-by: Fini Jastrow --- font-patcher | 28 +++++++--------------------- 1 file changed, 7 insertions(+), 21 deletions(-) diff --git a/font-patcher b/font-patcher index c4aaf324a..31fd2c7b5 100755 --- a/font-patcher +++ b/font-patcher @@ -647,10 +647,8 @@ class font_patcher: careful = True if exactEncoding is False: - sourceFontList = [] + sourceFontList = list(range(sourceFontStart, sourceFontEnd + 1)) sourceFontCounter = 0 - for i in range(sourceFontStart, sourceFontEnd + 1): - sourceFontList.append(format(i, 'X')) scale_factor = 0 if scaleGlyph: @@ -685,25 +683,16 @@ class font_patcher: if exactEncoding: # use the exact same hex values for the source font as for the symbol font currentSourceFontGlyph = sym_glyph.encoding - - # Save as a hex string without the '0x' prefix - copiedToSlot = format(sym_glyph.unicode, 'X') else: # use source font defined hex values based on passed in start and end - # convince that this string really is a hex: - currentSourceFontGlyph = int("0x" + sourceFontList[sourceFontCounter], 16) - copiedToSlot = sourceFontList[sourceFontCounter] + currentSourceFontGlyph = sourceFontList[sourceFontCounter] sourceFontCounter += 1 - if int(copiedToSlot, 16) < 0: - print("Found invalid glyph slot number. Skipping.") - continue - if self.args.quiet is False: if self.args.progressbars: update_progress(round(float(index + 1) / glyphSetLength, 2)) else: - progressText = "\nUpdating glyph: " + str(sym_glyph) + " " + str(sym_glyph.glyphname) + " putting at: " + copiedToSlot + progressText = "\nUpdating glyph: {} {} putting at: {:X}".format(sym_glyph, sym_glyph.glyphname, currentSourceFontGlyph) sys.stdout.write(progressText) sys.stdout.flush() @@ -711,20 +700,17 @@ class font_patcher: sym_dim = get_glyph_dimensions(sym_glyph) # check if a glyph already exists in this location - if copiedToSlot.startswith("uni"): - copiedToSlot = copiedToSlot[3:] - codepoint = int("0x" + copiedToSlot, 16) if careful or 'careful' in sym_attr['params']: - if codepoint in self.sourceFont: + if currentSourceFontGlyph in self.sourceFont: if self.args.quiet is False: - print(" Found existing Glyph at {}. Skipping...".format(copiedToSlot)) + print(" Found existing Glyph at {:X}. Skipping...".format(currentSourceFontGlyph)) # We don't want to touch anything so move to next Glyph continue else: # If we overwrite an existing glyph all subtable entries regarding it will be wrong # (Probably; at least if we add a symbol and do not substitude a ligature or such) - if codepoint in self.sourceFont: - self.sourceFont[codepoint].removePosSub("*") + if currentSourceFontGlyph in self.sourceFont: + self.sourceFont[currentSourceFontGlyph].removePosSub("*") # Select and copy symbol from its encoding point # We need to do this select after the careful check, this way we don't