From f9e0c2fef33e874dc6ba40ed8c5cfd43b412cda2 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 10 Mar 2021 15:48:20 +0000 Subject: [PATCH 1/9] set default width to one based on NotoColorEmoji --- src/nanoemoji/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nanoemoji/config.py b/src/nanoemoji/config.py index 01835d94..c774d77c 100644 --- a/src/nanoemoji/config.py +++ b/src/nanoemoji/config.py @@ -112,7 +112,7 @@ class FontConfig(NamedTuple): output_file: str = "AnEmojiFamily.ttf" color_format: str = "glyf_colr_1" upem: int = 1024 - width: int = 1024 + width: int = 1275 # default based on Noto Emoji ascent: int = 950 # default based on Noto Emoji descent: int = -250 # default based on Noto Emoji transform: Affine2D = Affine2D.identity() From 9c34b3f6a5c353b1eb0c3199498f4993abc49f39 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 10 Mar 2021 16:13:38 +0000 Subject: [PATCH 2/9] scale height to (ascender-descender), preserve aspect, shift so width is centered Fixes #2 and #230 --- src/nanoemoji/color_glyph.py | 96 +++++++++++++++++++--------------- tests/color_glyph_test.py | 38 +++++++++----- tests/colr_to_svg.py | 48 +++++++++-------- tests/one_rect_transformed.ttx | 20 +++---- tests/test_helper.py | 2 + 5 files changed, 118 insertions(+), 86 deletions(-) diff --git a/src/nanoemoji/color_glyph.py b/src/nanoemoji/color_glyph.py index dfd633b6..ce129c5d 100644 --- a/src/nanoemoji/color_glyph.py +++ b/src/nanoemoji/color_glyph.py @@ -37,52 +37,58 @@ import ufoLib2 -def _scale_viewbox_to_emsquare(view_box: Rect, upem: int) -> Tuple[float, float]: - # scale to font upem - return (upem / view_box.w, upem / view_box.h) - - -def _shift_origin_0_0( - view_box: Rect, x_scale: float, y_scale: float -) -> Tuple[float, float]: - # shift so origin is 0,0 - return (-view_box.x * x_scale, -view_box.y * y_scale) +def _scale_viewbox_to_font_metrics( + view_box: Rect, ascender: int, descender: int, width: int +): + # scale height to (ascender - descender) + scale = (ascender - descender) / view_box.h + # shift so width is centered + dx = (width - scale * view_box.w) / 2 + return Affine2D.compose_ltr( + ( + # first normalize viewbox origin + Affine2D(1, 0, 0, 1, -view_box.x, -view_box.y), + Affine2D(scale, 0, 0, scale, dx, 0), + ) + ) -def map_viewbox_to_font_emsquare( - view_box: Rect, upem: int, user_transform: Affine2D +def map_viewbox_to_font_space( + view_box: Rect, ascender: int, descender: int, width: int, user_transform: Affine2D ) -> Affine2D: - x_scale, y_scale = _scale_viewbox_to_emsquare(view_box, upem) - # flip y axis - y_scale = -y_scale - # shift so things are in the right place - dx, dy = _shift_origin_0_0(view_box, x_scale, y_scale) - dy = dy + upem - affine = Affine2D(x_scale, 0, 0, y_scale, dx, dy) - return Affine2D.compose_ltr((affine, user_transform)) + return Affine2D.compose_ltr( + [ + _scale_viewbox_to_font_metrics(view_box, ascender, descender, width), + # flip y axis and shift so things are in the right place + Affine2D(1, 0, 0, -1, 0, ascender), + user_transform, + ] + ) # https://docs.microsoft.com/en-us/typography/opentype/spec/svg#coordinate-systems-and-glyph-metrics -def map_viewbox_to_otsvg_emsquare( - view_box: Rect, upem: int, user_transform: Affine2D +def map_viewbox_to_otsvg_space( + view_box: Rect, ascender: int, descender: int, width: int, user_transform: Affine2D ) -> Affine2D: - x_scale, y_scale = _scale_viewbox_to_emsquare(view_box, upem) - dx, dy = _shift_origin_0_0(view_box, x_scale, y_scale) - - # shift so things are in the right place - dy = dy - upem - affine = Affine2D(x_scale, 0, 0, y_scale, dx, dy) - return Affine2D.compose_ltr((affine, user_transform)) + return Affine2D.compose_ltr( + [ + _scale_viewbox_to_font_metrics(view_box, ascender, descender, width), + # shift things in the [+x,-y] quadrant where OT-SVG expects them + Affine2D(1, 0, 0, 1, 0, -ascender), + user_transform, + ] + ) def _get_gradient_transform( - upem: int, - user_transform: Affine2D, + config: FontConfig, grad_el: etree.Element, shape_bbox: Rect, view_box: Rect, ) -> Affine2D: - transform = map_viewbox_to_font_emsquare(view_box, upem, user_transform) + transform = map_viewbox_to_font_space( + view_box, config.ascent, config.descent, config.width, config.transform + ) gradient_units = grad_el.attrib.get("gradientUnits", "objectBoundingBox") if gradient_units == "objectBoundingBox": @@ -112,9 +118,7 @@ def _parse_linear_gradient( # Set P2 to P1 rotated 90 degrees counter-clockwise around P0 p2 = p0 + (p1 - p0).perpendicular() - transform = _get_gradient_transform( - config.upem, config.transform, grad_el, shape_bbox, view_box - ) + transform = _get_gradient_transform(config, grad_el, shape_bbox, view_box) p0 = transform.map_point(p0) p1 = transform.map_point(p1) @@ -140,7 +144,9 @@ def _parse_radial_gradient( c1 = Point(gradient.cx, gradient.cy) r1 = gradient.r - transform = map_viewbox_to_font_emsquare(view_box, config.upem, config.transform) + transform = map_viewbox_to_font_space( + view_box, config.ascent, config.descent, config.width, config.transform + ) gradient_units = grad_el.attrib.get("gradientUnits", "objectBoundingBox") if gradient_units == "objectBoundingBox": @@ -150,7 +156,7 @@ def _parse_radial_gradient( assert transform[1:3] == (0, 0), ( f"{transform} contains unexpected skew/rotation:" - " upem, view_box, shape_bbox are all rectangles" + " (ascender-descender), view_box, shape_bbox are all rectangles" ) # if viewBox is not square or if gradientUnits="objectBoundingBox" and the bbox @@ -373,16 +379,24 @@ def transform_for_font_space(self): """Creates a Transform to map SVG coords to font coords""" if not self._has_viewbox_for_transform(): return Affine2D.identity() - return map_viewbox_to_font_emsquare( - self.svg.view_box(), self.ufo.info.unitsPerEm, self.user_transform + return map_viewbox_to_font_space( + self.svg.view_box(), + self.ufo.info.ascender, + self.ufo.info.descender, + self.ufo[self.glyph_name].width, + self.user_transform, ) def transform_for_otsvg_space(self): """Creates a Transform to map SVG coords OT-SVG coords""" if not self._has_viewbox_for_transform(): return Affine2D.identity() - return map_viewbox_to_otsvg_emsquare( - self.svg.view_box(), self.ufo.info.unitsPerEm, self.user_transform + return map_viewbox_to_otsvg_space( + self.svg.view_box(), + self.ufo.info.ascender, + self.ufo.info.descender, + self.ufo[self.glyph_name].width, + self.user_transform, ) def paints(self): diff --git a/tests/color_glyph_test.py b/tests/color_glyph_test.py index ecfd5442..819fd6be 100644 --- a/tests/color_glyph_test.py +++ b/tests/color_glyph_test.py @@ -26,9 +26,11 @@ # TODO test _glyph_name obeys codepoint order -def _ufo(upem): +def _ufo(config): ufo = ufoLib2.Font() - ufo.info.unitsPerEm = upem + ufo.info.unitsPerEm = config.upem + ufo.info.ascender = config.ascent + ufo.info.descender = config.descent return ufo @@ -41,32 +43,44 @@ def _nsvg(filename): @pytest.mark.parametrize( - "view_box, upem, expected_transform", + "view_box, upem, width, ascender, descender, expected_transform", [ # same upem, flip y - ("0 0 1024 1024", 1024, Affine2D(1, 0, 0, -1, 0, 1024)), + ("0 0 1024 1024", 1024, 1024, 1024, 0, Affine2D(1, 0, 0, -1, 0, 1024)), # noto emoji norm. scale, flip y - ("0 0 128 128", 1024, Affine2D(8, 0, 0, -8, 0, 1024)), + ("0 0 128 128", 1024, 1024, 1024, 0, Affine2D(8, 0, 0, -8, 0, 1024)), # noto emoji emoji_u26be.svg viewBox. Scale, flip y and translate - ("-151 297 128 128", 1024, Affine2D(8, 0, 0, -8, 1208, 3400)), + ("-151 297 128 128", 1024, 1024, 1024, 0, Affine2D(8, 0, 0, -8, 1208, 3400)), # made up example. Scale, translate, flip y ( "10 11 20 21", 100, - Affine2D(a=5.0, b=0, c=0, d=-4.761905, e=-50.0, f=152.380952), + 100, + 100, + 0, + Affine2D(a=4.761905, b=0, c=0, d=-4.761905, e=-45.238095, f=152.380952), + ), + # noto emoji width, ascender, descender + ( + "0 0 1024 1024", + 1024, + 1275, + 950, + -250, + Affine2D(1.171875, 0, 0, -1.171875, 37.5, 950), ), ], ) -def test_transform(view_box, upem, expected_transform): +def test_transform(view_box, upem, width, ascender, descender, expected_transform): svg_str = ( '" ) - config = FontConfig(upem=upem) + config = FontConfig(upem=upem, width=width, ascent=ascender, descent=descender) color_glyph = ColorGlyph.create( - config, _ufo(config.upem), "duck", 1, [0x0042], SVG.fromstring(svg_str) + config, _ufo(config), "duck", 1, [0x0042], SVG.fromstring(svg_str) ) assert color_glyph.transform_for_font_space() == pytest.approx(expected_transform) @@ -280,9 +294,9 @@ def _round_gradient_coordinates(paint, prec=6): ], ) def test_paint_from_shape(svg_in, expected_paints): - config = FontConfig(upem=1000) + config = FontConfig(upem=1000, ascent=1000, descent=0, width=1000) color_glyph = ColorGlyph.create( - config, _ufo(config.upem), "duck", 1, [0x0042], _nsvg(svg_in) + config, _ufo(config), "duck", 1, [0x0042], _nsvg(svg_in) ) assert { _round_gradient_coordinates(paint) for paint in color_glyph.paints() diff --git a/tests/colr_to_svg.py b/tests/colr_to_svg.py index 92949514..593ce2d3 100644 --- a/tests/colr_to_svg.py +++ b/tests/colr_to_svg.py @@ -49,11 +49,11 @@ _GRADIENT_PAINT_FORMATS = (PaintLinearGradient.format, PaintRadialGradient.format) -def _map_font_emsquare_to_viewbox( - view_box: Rect, upem: int, user_transform: Affine2D +def _map_font_space_to_viewbox( + view_box: Rect, ascender: int, descender: int, width: int ) -> Affine2D: - return color_glyph.map_viewbox_to_font_emsquare( - view_box, upem, user_transform + return color_glyph.map_viewbox_to_font_space( + view_box, ascender, descender, width, Affine2D.identity() ).inverse() @@ -71,12 +71,12 @@ def _draw_svg_path( svg_path: etree.Element, glyph_set: ttLib.ttFont._TTGlyphSet, glyph_name: str, - upem_to_vbox: Affine2D, + font_to_vbox: Affine2D, ): # use glyph set to resolve references in composite glyphs svg_pen = SVGPathPen(glyph_set) # wrap svg pen with "filter" pen mapping coordinates from UPEM to SVG space - transform_pen = transformPen.TransformPen(svg_pen, upem_to_vbox) + transform_pen = transformPen.TransformPen(svg_pen, font_to_vbox) glyph = glyph_set[glyph_name] glyph.draw(transform_pen) @@ -140,17 +140,17 @@ def _apply_gradient_ot_paint( svg_defs: etree.Element, svg_path: etree.Element, ttfont: ttLib.TTFont, - upem_to_vbox: Affine2D, + font_to_vbox: Affine2D, ot_paint: otTables.Paint, transform: Affine2D = Affine2D.identity(), ): paint = _gradient_paint(ttfont, ot_paint) # Gradient paint coordinates are in UPEM space, we want them in SVG viewBox - paint = _map_gradient_coordinates(paint, upem_to_vbox) + paint = _map_gradient_coordinates(paint, font_to_vbox) # Likewise PaintTransforms refer to UPEM so they must be adjusted for SVG if transform != Affine2D.identity(): transform = Affine2D.product( - upem_to_vbox.inverse(), Affine2D.product(transform, upem_to_vbox) + font_to_vbox.inverse(), Affine2D.product(transform, font_to_vbox) ) _apply_gradient_paint(svg_defs, svg_path, paint, transform=transform) @@ -162,15 +162,16 @@ def _colr_v0_glyph_to_svg( glyph_name: str, ) -> etree.Element: svg_root = _svg_root(view_box) - upem_to_vbox = _map_font_emsquare_to_viewbox( - view_box, ttfont["head"].unitsPerEm, Affine2D.identity() - ) + ascender = ttfont["OS/2"].sTypoAscender + descender = ttfont["OS/2"].sTypoDescender + width = ttfont["hmtx"][glyph_name][0] + font_to_vbox = _map_font_space_to_viewbox(view_box, ascender, descender, width) for glyph_layer in ttfont["COLR"].ColorLayers[glyph_name]: svg_path = etree.SubElement(svg_root, "path") paint = PaintSolid(_color(ttfont, glyph_layer.colorID)) _apply_solid_paint(svg_path, paint) - _draw_svg_path(svg_path, glyph_set, glyph_layer.name, upem_to_vbox) + _draw_svg_path(svg_path, glyph_set, glyph_layer.name, font_to_vbox) return svg_root @@ -179,7 +180,7 @@ def _colr_v1_paint_to_svg( ttfont: ttLib.TTFont, glyph_set: Mapping[str, Any], svg_root: etree.Element, - upem_to_vbox: Affine2D, + font_to_vbox: Affine2D, ot_paint: otTables.Paint, svg_path: Optional[etree.Element] = None, transform: Affine2D = Affine2D.identity(), @@ -191,7 +192,7 @@ def _colr_v1_paint_to_svg( assert svg_path is not None svg_defs = svg_root[0] _apply_gradient_ot_paint( - svg_defs, svg_path, ttfont, upem_to_vbox, ot_paint, transform + svg_defs, svg_path, ttfont, font_to_vbox, ot_paint, transform ) elif ot_paint.Format == PaintGlyph.format: assert svg_path is None, "recursive PaintGlyph is unsupported" @@ -204,12 +205,12 @@ def _colr_v1_paint_to_svg( ttfont, glyph_set, svg_root, - upem_to_vbox, + font_to_vbox, ot_paint.Paint, svg_path, ) - _draw_svg_path(svg_path, glyph_set, layer_glyph, upem_to_vbox) + _draw_svg_path(svg_path, glyph_set, layer_glyph, font_to_vbox) elif ot_paint.Format == PaintTransform.format: transform = Affine2D.product( ( @@ -230,7 +231,7 @@ def _colr_v1_paint_to_svg( ttfont, glyph_set, svg_root, - upem_to_vbox, + font_to_vbox, ot_paint.Paint, svg_path, transform=transform, @@ -245,7 +246,7 @@ def _colr_v1_paint_to_svg( ttfont, glyph_set, svg_root, - upem_to_vbox, + font_to_vbox, child_paint, svg_path, transform=transform, @@ -262,10 +263,11 @@ def _colr_v1_glyph_to_svg( ) -> etree.Element: glyph_set = ttfont.getGlyphSet() svg_root = _svg_root(view_box) - upem_to_vbox = _map_font_emsquare_to_viewbox( - view_box, ttfont["head"].unitsPerEm, Affine2D.identity() - ) - _colr_v1_paint_to_svg(ttfont, glyph_set, svg_root, upem_to_vbox, glyph.Paint) + ascender = ttfont["OS/2"].sTypoAscender + descender = ttfont["OS/2"].sTypoDescender + width = ttfont["hmtx"][glyph.BaseGlyph][0] + font_to_vbox = _map_font_space_to_viewbox(view_box, ascender, descender, width) + _colr_v1_paint_to_svg(ttfont, glyph_set, svg_root, font_to_vbox, glyph.Paint) return svg_root diff --git a/tests/one_rect_transformed.ttx b/tests/one_rect_transformed.ttx index 38a3751d..56ceb93d 100644 --- a/tests/one_rect_transformed.ttx +++ b/tests/one_rect_transformed.ttx @@ -12,8 +12,8 @@ - - + + @@ -41,20 +41,20 @@ - + - - + + - + - - - - + + + + diff --git a/tests/test_helper.py b/tests/test_helper.py index e57fcf3c..e455a91d 100644 --- a/tests/test_helper.py +++ b/tests/test_helper.py @@ -57,6 +57,8 @@ def color_font_config(config_overrides, svgs, tmp_dir=None): ._replace( family="UnitTest", upem=100, + ascent=100, + descent=0, width=100, keep_glyph_names=True, fea_file=fea_file, From ce0ee1aac9373276e86ff41ff580688df4524d4f Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Wed, 10 Mar 2021 17:07:37 +0000 Subject: [PATCH 3/9] non-square aspect ratio == proportional width; square == monospace --- src/nanoemoji/color_glyph.py | 8 +++++- tests/color_glyph_test.py | 48 ++++++++++++++++++++++++++++++------ 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/nanoemoji/color_glyph.py b/src/nanoemoji/color_glyph.py index ce129c5d..73b2e503 100644 --- a/src/nanoemoji/color_glyph.py +++ b/src/nanoemoji/color_glyph.py @@ -339,7 +339,13 @@ def create( logging.debug(" ColorGlyph for %s (%s)", filename, codepoints) glyph_name = glyph.glyph_name(codepoints) base_glyph = ufo.newGlyph(glyph_name) - base_glyph.width = font_config.width + + # non-square aspect ratio == proportional width; square == monospace + view_box = svg.view_box() + if view_box is not None: + base_glyph.width = round(font_config.width * view_box.w / view_box.h) + else: + base_glyph.width = font_config.width # Setup direct access to the glyph if possible if len(codepoints) == 1: diff --git a/tests/color_glyph_test.py b/tests/color_glyph_test.py index 819fd6be..e89971b9 100644 --- a/tests/color_glyph_test.py +++ b/tests/color_glyph_test.py @@ -43,14 +43,22 @@ def _nsvg(filename): @pytest.mark.parametrize( - "view_box, upem, width, ascender, descender, expected_transform", + "view_box, upem, width, ascender, descender, expected_transform, expected_width", [ # same upem, flip y - ("0 0 1024 1024", 1024, 1024, 1024, 0, Affine2D(1, 0, 0, -1, 0, 1024)), + ("0 0 1024 1024", 1024, 1024, 1024, 0, Affine2D(1, 0, 0, -1, 0, 1024), 1024), # noto emoji norm. scale, flip y - ("0 0 128 128", 1024, 1024, 1024, 0, Affine2D(8, 0, 0, -8, 0, 1024)), + ("0 0 128 128", 1024, 1024, 1024, 0, Affine2D(8, 0, 0, -8, 0, 1024), 1024), # noto emoji emoji_u26be.svg viewBox. Scale, flip y and translate - ("-151 297 128 128", 1024, 1024, 1024, 0, Affine2D(8, 0, 0, -8, 1208, 3400)), + ( + "-151 297 128 128", + 1024, + 1024, + 1024, + 0, + Affine2D(8, 0, 0, -8, 1208, 3400), + 1024, + ), # made up example. Scale, translate, flip y ( "10 11 20 21", @@ -58,7 +66,8 @@ def _nsvg(filename): 100, 100, 0, - Affine2D(a=4.761905, b=0, c=0, d=-4.761905, e=-45.238095, f=152.380952), + Affine2D(a=4.761905, b=0, c=0, d=-4.761905, e=-47.738095, f=152.380952), + 95, ), # noto emoji width, ascender, descender ( @@ -68,10 +77,33 @@ def _nsvg(filename): 950, -250, Affine2D(1.171875, 0, 0, -1.171875, 37.5, 950), + 1275, + ), + # viewbox wider than tall + ( + "0 0 20 10", + 100, + 100, + 100, + 0, + Affine2D(a=10, b=0, c=0, d=-10, e=0, f=100), + 200, + ), + # viewbox taller than wide + ( + "0 0 10 20", + 100, + 100, + 100, + 0, + Affine2D(a=5, b=0, c=0, d=-5, e=0, f=100), + 50, ), ], ) -def test_transform(view_box, upem, width, ascender, descender, expected_transform): +def test_transform_and_width( + view_box, upem, width, ascender, descender, expected_transform, expected_width +): svg_str = ( '" ) config = FontConfig(upem=upem, width=width, ascent=ascender, descent=descender) + ufo = _ufo(config) color_glyph = ColorGlyph.create( - config, _ufo(config), "duck", 1, [0x0042], SVG.fromstring(svg_str) + config, ufo, "duck", 1, [0x0042], SVG.fromstring(svg_str) ) assert color_glyph.transform_for_font_space() == pytest.approx(expected_transform) + assert ufo[color_glyph.glyph_name].width == expected_width def _round_gradient_coordinates(paint, prec=6): From 60f8fe142d2d8adc29b363b9dbea1c6aefc5b3a4 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 11 Mar 2021 12:19:41 +0000 Subject: [PATCH 4/9] minor changes as per review --- src/nanoemoji/color_glyph.py | 20 +++++++------------- tests/color_glyph_test.py | 4 ++-- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/nanoemoji/color_glyph.py b/src/nanoemoji/color_glyph.py index 73b2e503..627b6843 100644 --- a/src/nanoemoji/color_glyph.py +++ b/src/nanoemoji/color_glyph.py @@ -40,6 +40,7 @@ def _scale_viewbox_to_font_metrics( view_box: Rect, ascender: int, descender: int, width: int ): + assert descender <= 0 # scale height to (ascender - descender) scale = (ascender - descender) / view_box.h # shift so width is centered @@ -381,11 +382,10 @@ def _has_viewbox_for_transform(self) -> bool: ) return view_box is not None - def transform_for_font_space(self): - """Creates a Transform to map SVG coords to font coords""" + def _transform(self, map_fn): if not self._has_viewbox_for_transform(): return Affine2D.identity() - return map_viewbox_to_font_space( + return map_fn( self.svg.view_box(), self.ufo.info.ascender, self.ufo.info.descender, @@ -394,16 +394,10 @@ def transform_for_font_space(self): ) def transform_for_otsvg_space(self): - """Creates a Transform to map SVG coords OT-SVG coords""" - if not self._has_viewbox_for_transform(): - return Affine2D.identity() - return map_viewbox_to_otsvg_space( - self.svg.view_box(), - self.ufo.info.ascender, - self.ufo.info.descender, - self.ufo[self.glyph_name].width, - self.user_transform, - ) + return self._transform(map_viewbox_to_otsvg_space) + + def transform_for_font_space(self): + return self._transform(map_viewbox_to_font_space) def paints(self): """Set of Paint used by this glyph.""" diff --git a/tests/color_glyph_test.py b/tests/color_glyph_test.py index e89971b9..b5455a73 100644 --- a/tests/color_glyph_test.py +++ b/tests/color_glyph_test.py @@ -79,7 +79,7 @@ def _nsvg(filename): Affine2D(1.171875, 0, 0, -1.171875, 37.5, 950), 1275, ), - # viewbox wider than tall + # wider than tall: uniformly scale by height and stretch advance width to fit ( "0 0 20 10", 100, @@ -89,7 +89,7 @@ def _nsvg(filename): Affine2D(a=10, b=0, c=0, d=-10, e=0, f=100), 200, ), - # viewbox taller than wide + # taller than wide: uniformly scale by height and shrink advance width to fit ( "0 0 10 20", 100, From 20e45db98fb9674298c261a85393d3c42d0714f8 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 11 Mar 2021 15:31:50 +0000 Subject: [PATCH 5/9] add (failing) test parsing gradients for non-square viewbox looks like I'm only using the default --width when parsing gradients, and not taking into account a potentially scaled glyph advance widths (by aspect ratio) that we apply for the outlines when the viewbox is non-square. I'll fix that in a follow-up commit. --- tests/color_glyph_test.py | 18 ++++++++++++++++++ tests/gradient_non_square_viewbox.svg | 10 ++++++++++ 2 files changed, 28 insertions(+) create mode 100644 tests/gradient_non_square_viewbox.svg diff --git a/tests/color_glyph_test.py b/tests/color_glyph_test.py index b5455a73..e1cb154a 100644 --- a/tests/color_glyph_test.py +++ b/tests/color_glyph_test.py @@ -325,6 +325,24 @@ def _round_gradient_coordinates(paint, prec=6): ), }, ), + ( + # viewBox="0 0 10 8" (w > h), with a linearGradient from (1, 1) to (9, 1). + # The default advance width gets scaled by aspect ratio 1000 * 10/8 == 1250. + # Test that linearGradient p0 and p1 are centered horizontally relative to + # the scaled advance width (and not relative to the default advance width). + "gradient_non_square_viewbox.svg", + { + PaintLinearGradient( + stops=( + ColorStop(stopOffset=0.1, color=Color.fromstring("blue")), + ColorStop(stopOffset=0.9, color=Color.fromstring("cyan")), + ), + p0=Point(125.0, 875.0), + p1=Point(1125.0, 875.0), + p2=Point(125.0, 125.0), + ), + }, + ), ], ) def test_paint_from_shape(svg_in, expected_paints): diff --git a/tests/gradient_non_square_viewbox.svg b/tests/gradient_non_square_viewbox.svg new file mode 100644 index 00000000..7a6bc3d2 --- /dev/null +++ b/tests/gradient_non_square_viewbox.svg @@ -0,0 +1,10 @@ + + + + + + + + + From 09c8af04e1b33763675f78ceb16d01cc901798d8 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 11 Mar 2021 15:36:00 +0000 Subject: [PATCH 6/9] use proportional glyph width when mapping svg gradient to font space Fixes the previously failing test --- src/nanoemoji/color_glyph.py | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/nanoemoji/color_glyph.py b/src/nanoemoji/color_glyph.py index 627b6843..c1f830f9 100644 --- a/src/nanoemoji/color_glyph.py +++ b/src/nanoemoji/color_glyph.py @@ -86,9 +86,10 @@ def _get_gradient_transform( grad_el: etree.Element, shape_bbox: Rect, view_box: Rect, + glyph_width: int, ) -> Affine2D: transform = map_viewbox_to_font_space( - view_box, config.ascent, config.descent, config.width, config.transform + view_box, config.ascent, config.descent, glyph_width, config.transform ) gradient_units = grad_el.attrib.get("gradientUnits", "objectBoundingBox") @@ -109,6 +110,7 @@ def _parse_linear_gradient( grad_el: etree.Element, shape_bbox: Rect, view_box: Rect, + glyph_width: int, shape_opacity: float = 1.0, ): gradient = SVGLinearGradient.from_element(grad_el, view_box) @@ -119,7 +121,9 @@ def _parse_linear_gradient( # Set P2 to P1 rotated 90 degrees counter-clockwise around P0 p2 = p0 + (p1 - p0).perpendicular() - transform = _get_gradient_transform(config, grad_el, shape_bbox, view_box) + transform = _get_gradient_transform( + config, grad_el, shape_bbox, view_box, glyph_width + ) p0 = transform.map_point(p0) p1 = transform.map_point(p1) @@ -136,6 +140,7 @@ def _parse_radial_gradient( grad_el: etree.Element, shape_bbox: Rect, view_box: Rect, + glyph_width: int, shape_opacity: float = 1.0, ): gradient = SVGRadialGradient.from_element(grad_el, view_box) @@ -146,7 +151,7 @@ def _parse_radial_gradient( r1 = gradient.r transform = map_viewbox_to_font_space( - view_box, config.ascent, config.descent, config.width, config.transform + view_box, config.ascent, config.descent, glyph_width, config.transform ) gradient_units = grad_el.attrib.get("gradientUnits", "objectBoundingBox") @@ -244,7 +249,9 @@ def shape_cache_key(self): return (self.path, self.reuses) -def _paint(debug_hint: str, config: FontConfig, picosvg: SVG, shape: SVGPath) -> Paint: +def _paint( + debug_hint: str, config: FontConfig, picosvg: SVG, shape: SVGPath, glyph_width: int +) -> Paint: if shape.fill.startswith("url("): el = picosvg.resolve_url(shape.fill, "*") try: @@ -253,6 +260,7 @@ def _paint(debug_hint: str, config: FontConfig, picosvg: SVG, shape: SVGPath) -> el, shape.bounding_box(), picosvg.view_box(), + glyph_width, shape.opacity, ) except ValueError as e: @@ -264,12 +272,12 @@ def _paint(debug_hint: str, config: FontConfig, picosvg: SVG, shape: SVGPath) -> def _in_glyph_reuse_key( - debug_hint: str, config: FontConfig, picosvg: SVG, shape: SVGPath + debug_hint: str, config: FontConfig, picosvg: SVG, shape: SVGPath, glyph_width: int ) -> Tuple[Paint, SVGPath]: """Within a glyph reuse shapes only when painted consistently. paint+normalized shape ensures this.""" return ( - _paint(debug_hint, config, picosvg, shape), + _paint(debug_hint, config, picosvg, shape, glyph_width), normalize(shape, config.reuse_tolerance), ) @@ -278,18 +286,21 @@ def _painted_layers( debug_hint: str, config: FontConfig, picosvg: SVG, + glyph_width: int, ) -> Generator[PaintedLayer, None, None]: if config.reuse_tolerance < 0: # shape reuse disabled for path in picosvg.shapes(): - yield PaintedLayer(_paint(debug_hint, config, picosvg, path), path.d) + yield PaintedLayer( + _paint(debug_hint, config, picosvg, path, glyph_width), path.d + ) return # Don't sort; we only want to find groups that are consecutive in the picosvg # to ensure we don't mess up layer order for (paint, normalized), paths in groupby( picosvg.shapes(), - key=lambda s: _in_glyph_reuse_key(debug_hint, config, picosvg, s), + key=lambda s: _in_glyph_reuse_key(debug_hint, config, picosvg, s, glyph_width), ): paths = list(paths) transforms = () @@ -360,6 +371,7 @@ def create( filename, font_config, svg, + base_glyph.width, ) ) From 524f02bf81e3be90940fa54e7e33f4f188ba0e0a Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 11 Mar 2021 16:25:01 +0000 Subject: [PATCH 7/9] add FontConfig.validate() so we don't need to assert the obvious later I deliberately did not use absl.flags validators because those would not check if an invalid value was passed via config file, as opposed to CLI flags. There's probably a cleverer way, but at least this works for both. --- src/nanoemoji/config.py | 13 ++++++++++++- tests/color_glyph_test.py | 4 +++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/nanoemoji/config.py b/src/nanoemoji/config.py index c774d77c..bfdee40a 100644 --- a/src/nanoemoji/config.py +++ b/src/nanoemoji/config.py @@ -137,6 +137,17 @@ def output_format(self): def has_picosvgs(self): return not self.color_format.startswith("untouchedsvg") + def validate(self): + for attr_name in ("upem", "width", "ascent", "version_major", "version_minor"): + value = getattr(self, attr_name) + if value < 0: + raise ValueError(f"'{attr_name}' must be zero or positive") + + if self.descent > 0: + raise ValueError("'descent' must be zero or negative") + + return self + def write(dest: Path, config: FontConfig): toml_cfg = { @@ -316,4 +327,4 @@ def load(config_file: Path = None, additional_srcs: Tuple[Path] = None) -> FontC axes=tuple(axes), masters=tuple(masters), source_names=tuple(sorted(source_names)), - ) + ).validate() diff --git a/tests/color_glyph_test.py b/tests/color_glyph_test.py index e1cb154a..7bbb1e44 100644 --- a/tests/color_glyph_test.py +++ b/tests/color_glyph_test.py @@ -110,7 +110,9 @@ def test_transform_and_width( f' viewBox="{view_box}"' "/>" ) - config = FontConfig(upem=upem, width=width, ascent=ascender, descent=descender) + config = FontConfig( + upem=upem, width=width, ascent=ascender, descent=descender + ).validate() ufo = _ufo(config) color_glyph = ColorGlyph.create( config, ufo, "duck", 1, [0x0042], SVG.fromstring(svg_str) From 8e2b310f94193e418b20fedc4595f57587e4eb1c Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 11 Mar 2021 17:31:04 +0000 Subject: [PATCH 8/9] only use proportional advance width when larger than default width Previously, I was simply scaling the FontConfig.width by the aspect ratio. But that can produce unintended consequences if the default width != font height (ascender-descender): e.g. if one wanted to pad glyphs on both sides (like we do with noto-emoji) by setting a wider default width than the font height, the impact of non-square viewbox on that padding would be even greater as the viewbox gets wider. It's better to do this: given the font height and the viewbox aspect ratio, scale font heigth by aspect ratio to obtain the scaled advance width; ise the latter if it's larger than the default width, otherwise use the default width (and place things centered). A user can play with the default --width (even set it to 0) until they like it. This effectively becomes the minimum default advance width, to be used only used when the proportional advance width that is computed from the viewbox width (relative to its height which is fixed to ascender-descender) is smaller. Yeah, I think I like that. --- src/nanoemoji/color_glyph.py | 9 ++++++++- tests/color_glyph_test.py | 12 ++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/nanoemoji/color_glyph.py b/src/nanoemoji/color_glyph.py index c1f830f9..12e41798 100644 --- a/src/nanoemoji/color_glyph.py +++ b/src/nanoemoji/color_glyph.py @@ -329,6 +329,13 @@ def _painted_layers( yield PaintedLayer(paint, path.d) +def _color_glyph_advance_width(view_box: Rect, config: FontConfig) -> int: + # Scale advance width proportionally to viewbox aspect ratio. + # Use the default advance width if it's larger than the proportional one. + font_height = config.ascent - config.descent # descent <= 0 + return max(config.width, round(font_height * view_box.w / view_box.h)) + + class ColorGlyph(NamedTuple): ufo: ufoLib2.Font filename: str @@ -355,7 +362,7 @@ def create( # non-square aspect ratio == proportional width; square == monospace view_box = svg.view_box() if view_box is not None: - base_glyph.width = round(font_config.width * view_box.w / view_box.h) + base_glyph.width = _color_glyph_advance_width(view_box, font_config) else: base_glyph.width = font_config.width diff --git a/tests/color_glyph_test.py b/tests/color_glyph_test.py index 7bbb1e44..64e0e409 100644 --- a/tests/color_glyph_test.py +++ b/tests/color_glyph_test.py @@ -59,15 +59,15 @@ def _nsvg(filename): Affine2D(8, 0, 0, -8, 1208, 3400), 1024, ), - # made up example. Scale, translate, flip y + # made up example. Scale, translate, flip y, center horizontally ( "10 11 20 21", 100, 100, 100, 0, - Affine2D(a=4.761905, b=0, c=0, d=-4.761905, e=-47.738095, f=152.380952), - 95, + Affine2D(a=4.761905, b=0, c=0, d=-4.761905, e=-45.238095, f=152.380952), + 100, ), # noto emoji width, ascender, descender ( @@ -89,15 +89,15 @@ def _nsvg(filename): Affine2D(a=10, b=0, c=0, d=-10, e=0, f=100), 200, ), - # taller than wide: uniformly scale by height and shrink advance width to fit + # taller than wide: uniformly scale by height, center within advance width ( "0 0 10 20", 100, 100, 100, 0, - Affine2D(a=5, b=0, c=0, d=-5, e=0, f=100), - 50, + Affine2D(a=5, b=0, c=0, d=-5, e=25, f=100), + 100, ), ], ) From 40fff1b3dbb480dbafaea6dff58cbde6c9dbf0cc Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 11 Mar 2021 17:52:40 +0000 Subject: [PATCH 9/9] rename ascent->ascender, descent->descender The ascender/descender forms are more common in typography. Also UFO and the Oxford English Dictionary agree with me. --- src/nanoemoji/color_glyph.py | 6 +++--- src/nanoemoji/config.py | 32 +++++++++++++++++++------------- src/nanoemoji/write_font.py | 4 ++-- tests/color_glyph_test.py | 8 ++++---- tests/test_helper.py | 4 ++-- 5 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/nanoemoji/color_glyph.py b/src/nanoemoji/color_glyph.py index 12e41798..6524c6d5 100644 --- a/src/nanoemoji/color_glyph.py +++ b/src/nanoemoji/color_glyph.py @@ -89,7 +89,7 @@ def _get_gradient_transform( glyph_width: int, ) -> Affine2D: transform = map_viewbox_to_font_space( - view_box, config.ascent, config.descent, glyph_width, config.transform + view_box, config.ascender, config.descender, glyph_width, config.transform ) gradient_units = grad_el.attrib.get("gradientUnits", "objectBoundingBox") @@ -151,7 +151,7 @@ def _parse_radial_gradient( r1 = gradient.r transform = map_viewbox_to_font_space( - view_box, config.ascent, config.descent, glyph_width, config.transform + view_box, config.ascender, config.descender, glyph_width, config.transform ) gradient_units = grad_el.attrib.get("gradientUnits", "objectBoundingBox") @@ -332,7 +332,7 @@ def _painted_layers( def _color_glyph_advance_width(view_box: Rect, config: FontConfig) -> int: # Scale advance width proportionally to viewbox aspect ratio. # Use the default advance width if it's larger than the proportional one. - font_height = config.ascent - config.descent # descent <= 0 + font_height = config.ascender - config.descender # descender <= 0 return max(config.width, round(font_height * view_box.w / view_box.h)) diff --git a/src/nanoemoji/config.py b/src/nanoemoji/config.py index bfdee40a..2f6939e4 100644 --- a/src/nanoemoji/config.py +++ b/src/nanoemoji/config.py @@ -52,8 +52,8 @@ flags.DEFINE_string("config", None, "Config file.") flags.DEFINE_integer("upem", None, "Units per em.") flags.DEFINE_integer("width", None, "Width.") -flags.DEFINE_integer("ascent", None, "Ascender") -flags.DEFINE_integer("descent", None, "Descender.") +flags.DEFINE_integer("ascender", None, "Ascender") +flags.DEFINE_integer("descender", None, "Descender.") flags.DEFINE_string("transform", None, "User transform, in font coordinates.") flags.DEFINE_integer("version_major", None, "Major version.") flags.DEFINE_integer("version_minor", None, "Minor version.") @@ -113,8 +113,8 @@ class FontConfig(NamedTuple): color_format: str = "glyf_colr_1" upem: int = 1024 width: int = 1275 # default based on Noto Emoji - ascent: int = 950 # default based on Noto Emoji - descent: int = -250 # default based on Noto Emoji + ascender: int = 950 # default based on Noto Emoji + descender: int = -250 # default based on Noto Emoji transform: Affine2D = Affine2D.identity() version_major: int = 1 version_minor: int = 0 @@ -138,13 +138,19 @@ def has_picosvgs(self): return not self.color_format.startswith("untouchedsvg") def validate(self): - for attr_name in ("upem", "width", "ascent", "version_major", "version_minor"): + for attr_name in ( + "upem", + "width", + "ascender", + "version_major", + "version_minor", + ): value = getattr(self, attr_name) if value < 0: raise ValueError(f"'{attr_name}' must be zero or positive") - if self.descent > 0: - raise ValueError("'descent' must be zero or negative") + if self.descender > 0: + raise ValueError("'descender' must be zero or negative") return self @@ -156,8 +162,8 @@ def write(dest: Path, config: FontConfig): "color_format": config.color_format, "upem": config.upem, "width": config.width, - "ascent": config.ascent, - "descent": config.descent, + "ascender": config.ascender, + "descender": config.descender, "transform": config.transform.tostring(), "version_major": config.version_major, "version_minor": config.version_minor, @@ -235,8 +241,8 @@ def load(config_file: Path = None, additional_srcs: Tuple[Path] = None) -> FontC color_format = _pop_flag(config, "color_format") upem = int(_pop_flag(config, "upem")) width = int(_pop_flag(config, "width")) - ascent = int(_pop_flag(config, "ascent")) - descent = int(_pop_flag(config, "descent")) + ascender = int(_pop_flag(config, "ascender")) + descender = int(_pop_flag(config, "descender")) transform = _pop_flag(config, "transform") if not isinstance(transform, Affine2D): assert isinstance(transform, str) @@ -312,8 +318,8 @@ def load(config_file: Path = None, additional_srcs: Tuple[Path] = None) -> FontC color_format=color_format, upem=upem, width=width, - ascent=ascent, - descent=descent, + ascender=ascender, + descender=descender, transform=transform, version_major=version_major, version_minor=version_minor, diff --git a/src/nanoemoji/write_font.py b/src/nanoemoji/write_font.py index 094df84f..e5ac880f 100644 --- a/src/nanoemoji/write_font.py +++ b/src/nanoemoji/write_font.py @@ -145,8 +145,8 @@ def _ufo(config: FontConfig) -> ufoLib2.Font: ufo.info.familyName = config.family # set various font metadata; see the full list of fontinfo attributes at # https://unifiedfontobject.org/versions/ufo3/fontinfo.plist/#generic-dimension-information - ufo.info.ascender = config.ascent - ufo.info.descender = config.descent + ufo.info.ascender = config.ascender + ufo.info.descender = config.descender ufo.info.unitsPerEm = config.upem # version ufo.info.versionMajor = config.version_major diff --git a/tests/color_glyph_test.py b/tests/color_glyph_test.py index 64e0e409..ade7dcae 100644 --- a/tests/color_glyph_test.py +++ b/tests/color_glyph_test.py @@ -29,8 +29,8 @@ def _ufo(config): ufo = ufoLib2.Font() ufo.info.unitsPerEm = config.upem - ufo.info.ascender = config.ascent - ufo.info.descender = config.descent + ufo.info.ascender = config.ascender + ufo.info.descender = config.descender return ufo @@ -111,7 +111,7 @@ def test_transform_and_width( "/>" ) config = FontConfig( - upem=upem, width=width, ascent=ascender, descent=descender + upem=upem, width=width, ascender=ascender, descender=descender ).validate() ufo = _ufo(config) color_glyph = ColorGlyph.create( @@ -348,7 +348,7 @@ def _round_gradient_coordinates(paint, prec=6): ], ) def test_paint_from_shape(svg_in, expected_paints): - config = FontConfig(upem=1000, ascent=1000, descent=0, width=1000) + config = FontConfig(upem=1000, ascender=1000, descender=0, width=1000) color_glyph = ColorGlyph.create( config, _ufo(config), "duck", 1, [0x0042], _nsvg(svg_in) ) diff --git a/tests/test_helper.py b/tests/test_helper.py index e455a91d..67a42447 100644 --- a/tests/test_helper.py +++ b/tests/test_helper.py @@ -57,8 +57,8 @@ def color_font_config(config_overrides, svgs, tmp_dir=None): ._replace( family="UnitTest", upem=100, - ascent=100, - descent=0, + ascender=100, + descender=0, width=100, keep_glyph_names=True, fea_file=fea_file,