JadePrince / noto

Automatically exported from code.google.com/p/noto
0 stars 0 forks source link

Code review request #252

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Branch name:

dev_dougfelt_svgfont

Purpose of code changes on this branch:

Some tools for building a font using SVG images for glyphs.

When reviewing my code changes, please focus on:

Anything non-pythonic, anything where I should be using existing tools instead 
and/or differently.

Notes FYI

This is a lot of code, I was trying to solve lots of problems and the structure 
kept changing.  Sorry about that, it should have been reviewed in stages.

The general goal is to replicate the functionaliy of add_glyphs.py, using  SVG 
instead of PNG for color emoji glyphs.  The driver is add_svg_glyphs.py and it 
uses svg_builder and svg_cleaner.  generate_test_html was a small utility to 
generate an .html file so I could look at the SVG glyphs (supported by firefox 
but not chrome at the moment).

I tried to structure this in a way that seemed cleaner to me, using classes and 
file scoping instead of lots of global functions, but that's probably my java 
bias and it might be inappropriate.  It also may be that some of the 
functionality I implemented is in other of the nototools files, I didn't read 
through them all before diving in to this.

I found that I needed to do more manipulations on the SVG source than simple 
regex made convenient (creating/removing a number of attributes) and so wrote 
svg_cleaner to do this. The tree representation it generates is based somewhat 
on the fontTools XMLReader code, basically it is pretty simple.  Where 
XMLReader uses tuples and strings, I use objects, because after the tree is 
built I want to go in and mutate individual objects as well as the tree.  I 
might have been able to use an existing DOM implementation for some of this, at 
the time I was concerned about needing to read/write CDATA and most of the 
built-in python xml tools don't seem support CDATA.

add_glyphs is currently used in the Makefile, I have made no modifications to 
the Makefile for add_svg glyphs.  Unlike the PNG code I do not need to call out 
to C tools to generate waved flag images, nor do I need to call code to build 
the font data structures as this code has been added to fontTools since the 
original code that used add_glyphs was written.  So add_svg_glyphs.py code can 
generate a complete .ttx file ready for use (sans conversion to woff, which is 
straightforward). This is different from the old add_glyphs code which 
addressed a smaller portion of the problem.  add_svg_glyphs is basically the 
driver loop from the old add_glyphs and calls out to svg_builder to manipulate 
the font, this structure could be used to build using the PNG data as well and 
I designed the code a bit with this in mind.

There's probably tools to manipulate ttx font table objects directly like this 
code needs to, but I didn't look hard enough for it.  I took some of the 
existing code from add_glyphs and packed it into a utility class for this, to 
which I added some other utilities for working with the svg tables. This code 
all depends on the 'internals' (if such a thing exists in python) of the 
fontTools code, so it's fragile.

Note that some of the SVG files (at least one) breaks the svg engine in firefox 
and stops portions of otherwise valid svg fonts from rendering.  Currently the 
SVG for 1f301 does this, so building a font with this image in it will cause 
subsequent svg glyphs to not work. I do not yet know why this is.

After the review, I'll merge this branch into:

no immediate plans to merge yet, since I assume it will need a number of 
reviews.

Original issue reported on code.google.com by dougf...@google.com on 14 Feb 2015 at 1:05

GoogleCodeExporter commented 9 years ago

Original comment by dougf...@google.com on 5 Mar 2015 at 1:46