Repository: fonttools/ufoLib2

Issue:
`bounds` and `controlPointBounds` property for Glyph and Contour?
May be useful to have? Can use the available pens and `draw*` methods.

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=h1) Report
> Merging [#53](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=desc) into [master](https://codecov.io/gh/fonttools/ufoLib2/commit/5f56f622297848a65931c4a2514a9134a28f9118?src=pr&el=desc) will **increase** coverage by `4.05%`.
> The diff coverage is `89.01%`.

[![Impacted file tree graph](https://codecov.io/gh/fonttools/ufoLib2/pull/53/graphs/tree.svg?width=650&token=DaKFpWdMDw&height=150&src=pr)](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   71.95%   76.01%   +4.05%     
==========================================
  Files          19       19              
  Lines        1077     1155      +78     
  Branches      160      184      +24     
==========================================
+ Hits          775      878     +103     
+ Misses        258      229      -29     
- Partials       44       48       +4
```


| [Impacted Files](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/ufoLib2/objects/glyph.py](https://codecov.io/gh/fonttools/ufoLib2/pull/53/diff?src=pr&el=tree#diff-c3JjL3Vmb0xpYjIvb2JqZWN0cy9nbHlwaC5weQ==) | `74.86% <83.67%> (+7.5%)` | :arrow_up: |
| [src/ufoLib2/objects/layer.py](https://codecov.io/gh/fonttools/ufoLib2/pull/53/diff?src=pr&el=tree#diff-c3JjL3Vmb0xpYjIvb2JqZWN0cy9sYXllci5weQ==) | `85.61% <95.23%> (+26.48%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=footer). Last update [5f56f62...89f18ff](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

j0hnsmith: # [Codecov](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=h1) Report
> Merging [#53](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=desc) into [master](https://codecov.io/gh/fonttools/ufoLib2/commit/c16a568d7105c4d05d0bfc00d68448b56f88ae29&el=desc) will **increase** coverage by `1.38%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/fonttools/ufoLib2/pull/53/graphs/tree.svg?width=650&height=150&src=pr&token=DaKFpWdMDw)](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   75.36%   76.75%   +1.38%     
==========================================
  Files          19       19              
  Lines        1108     1200      +92     
  Branches      172      192      +20     
==========================================
+ Hits          835      921      +86     
+ Misses        230      229       -1     
- Partials       43       50       +7     
```


| [Impacted Files](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/ufoLib2/objects/glyph.py](https://codecov.io/gh/fonttools/ufoLib2/pull/53/diff?src=pr&el=tree#diff-c3JjL3Vmb0xpYjIvb2JqZWN0cy9nbHlwaC5weQ==) | `77.47% <0.00%> (+8.03%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=footer). Last update [c16a568...c162dea](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov-io: # [Codecov](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=h1) Report
> Merging [#53](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=desc) into [master](https://codecov.io/gh/fonttools/ufoLib2/commit/c16a568d7105c4d05d0bfc00d68448b56f88ae29?src=pr&el=desc) will **increase** coverage by `1.38%`.
> The diff coverage is `86.95%`.

[![Impacted file tree graph](https://codecov.io/gh/fonttools/ufoLib2/pull/53/graphs/tree.svg?width=650&token=DaKFpWdMDw&height=150&src=pr)](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   75.36%   76.75%   +1.38%     
==========================================
  Files          19       19              
  Lines        1108     1200      +92     
  Branches      172      192      +20     
==========================================
+ Hits          835      921      +86     
+ Misses        230      229       -1     
- Partials       43       50       +7
```


| [Impacted Files](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/ufoLib2/objects/contour.py](https://codecov.io/gh/fonttools/ufoLib2/pull/53/diff?src=pr&el=tree#diff-c3JjL3Vmb0xpYjIvb2JqZWN0cy9jb250b3VyLnB5) | `61.7% <100%> (+2.61%)` | :arrow_up: |
| [src/ufoLib2/objects/component.py](https://codecov.io/gh/fonttools/ufoLib2/pull/53/diff?src=pr&el=tree#diff-c3JjL3Vmb0xpYjIvb2JqZWN0cy9jb21wb25lbnQucHk=) | `77.27% <100%> (+3.58%)` | :arrow_up: |
| [src/ufoLib2/objects/point.py](https://codecov.io/gh/fonttools/ufoLib2/pull/53/diff?src=pr&el=tree#diff-c3JjL3Vmb0xpYjIvb2JqZWN0cy9wb2ludC5weQ==) | `93.33% <100%> (+2.42%)` | :arrow_up: |
| [src/ufoLib2/objects/anchor.py](https://codecov.io/gh/fonttools/ufoLib2/pull/53/diff?src=pr&el=tree#diff-c3JjL3Vmb0xpYjIvb2JqZWN0cy9hbmNob3IucHk=) | `100% <100%> (ø)` | :arrow_up: |
| [src/ufoLib2/objects/glyph.py](https://codecov.io/gh/fonttools/ufoLib2/pull/53/diff?src=pr&el=tree#diff-c3JjL3Vmb0xpYjIvb2JqZWN0cy9nbHlwaC5weQ==) | `77.47% <84.61%> (+8.03%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=footer). Last update [c16a568...0c82f1f](https://codecov.io/gh/fonttools/ufoLib2/pull/53?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/ufoLib2/objects/anchor.py b/src/ufoLib2/objects/anchor.py
index 9522302..075624b 100644
--- a/src/ufoLib2/objects/anchor.py
+++ b/src/ufoLib2/objects/anchor.py
@@ -12,3 +12,8 @@ class Anchor(AttrDictMixin):
     name = attr.ib(default=None, type=Optional[str])
     color = attr.ib(default=None, type=Optional[str])
     identifier = attr.ib(default=None, type=Optional[str])
+
+    def move(self, delta):
+        x, y = delta
+        self.x += x
+        self.y += y
diff --git a/src/ufoLib2/objects/component.py b/src/ufoLib2/objects/component.py
index 6eac951..c7dd47f 100644
--- a/src/ufoLib2/objects/component.py
+++ b/src/ufoLib2/objects/component.py
@@ -16,6 +16,10 @@ class Component:
     )
     identifier = attr.ib(default=None, type=Optional[str])
 
+    def move(self, delta):
+        x, y = delta
+        self.transformation = self.transformation.translate(x, y)
+
     # -----------
     # Pen methods
     # -----------
diff --git a/src/ufoLib2/objects/contour.py b/src/ufoLib2/objects/contour.py
index 81a875a..3e9b054 100644
--- a/src/ufoLib2/objects/contour.py
+++ b/src/ufoLib2/objects/contour.py
@@ -45,6 +45,10 @@ class Contour(MutableSequence):
             return True
         return self.points[0].type == "move"
 
+    def move(self, delta):
+        for point in self.points:
+            point.move(delta)
+
     # -----------
     # Pen methods
     # -----------
diff --git a/src/ufoLib2/objects/glyph.py b/src/ufoLib2/objects/glyph.py
index ad4bca8..f132af0 100644
--- a/src/ufoLib2/objects/glyph.py
+++ b/src/ufoLib2/objects/glyph.py
@@ -1,9 +1,11 @@
+from collections import namedtuple
 from copy import deepcopy
 from typing import Any, Dict, List, Optional, Union
 
 import attr
 from fontTools.misc.transform import Transform
 from fontTools.pens.pointPen import PointToSegmentPen, SegmentToPointPen
+from fontTools.pens.boundsPen import BoundsPen, ControlBoundsPen
 
 from ufoLib2.objects.anchor import Anchor
 from ufoLib2.objects.contour import Contour
@@ -186,6 +188,14 @@ class Glyph:
         pointPen = self.getPointPen()
         glyph.drawPoints(pointPen)
 
+    def move(self, delta):
+        for contour in self.contours:
+            contour.move(delta)
+        for component in self.components:
+            component.move(delta)
+        for anchor in self.anchors:
+            anchor.move(delta)
+
     # -----------
     # Pen methods
     # -----------
@@ -231,3 +241,96 @@ class Glyph:
             self.lib["public.verticalOrigin"] = value
         elif "public.verticalOrigin" in self.lib:
             del self.lib["public.verticalOrigin"]
+
+    # bounds and side-bearings
+
+    BoundingBox = namedtuple("BoundingBox", "xMin yMin xMax yMax")
+
+    def getBounds(self, layer=None):
+        if layer is None and self.components:
+            raise TypeError("layer is required to compute bounds of components")
+
+        pen = BoundsPen(layer)
+        self.draw(pen)
+        return pen.bounds if pen.bounds is None else self.BoundingBox(*pen.bounds)
+
+    def getControlBounds(self, layer=None):
+        if layer is None and self.components:
+            raise TypeError("layer is required to compute bounds of components")
+
+        pen = ControlBoundsPen(layer)
+        self.draw(pen)
+        return pen.bounds if pen.bounds is None else self.BoundingBox(*pen.bounds)
+
+    def getLeftMargin(self, layer=None):
+        bounds = self.getBounds(layer)
+        if bounds is None:
+            return None
+        return bounds.xMin
+
+    def setLeftMargin(self, value, layer=None):
+        bounds = self.getBounds(layer)
+        if bounds is None:
+            return None
+        diff = value - bounds.xMin
+        if diff:
+            self.width += diff
+            self.move((diff, 0))
+
+    def getRightMargin(self, layer=None):
+        bounds = self.getBounds(layer)
+        if bounds is None:
+            return None
+        return self.width - bounds.xMax
+
+    def setRightMargin(self, value, layer=None):
+        bounds = self.getBounds(layer)
+        if bounds is None:
+            return None
+        self.width = bounds.xMax + value
+
+    def getBottomMargin(self, layer=None):
+        bounds = self.getBounds(layer)
+        if bounds is None:
+            return None
+        if self.verticalOrigin is None:
+            return bounds.yMin
+        else:
+            return bounds.yMin - (self.verticalOrigin - self.height)
+
+    def setBottomMargin(self, value, layer=None):
+        bounds = self.getBounds(layer)
+        if bounds is None:
+            return None
+        # blindly copied from defcon Glyph._set_bottomMargin; not sure it's correct
+        if self.verticalOrigin is None:
+            oldValue = bounds.yMin
+            self.verticalOrigin = self.height
+        else:
+            oldValue = bounds.yMin - (self.verticalOrigin - self.height)
+        diff = value - oldValue
+        if diff:
+            self.height += diff
+
+    def getTopMargin(self, layer=None):
+        bounds = self.getBounds(layer)
+        if bounds is None:
+            return None
+        if self.verticalOrigin is None:
+            return self.height - bounds.yMax
+        else:
+            return self.verticalOrigin - bounds.yMax
+
+    def setTopMargin(self, value, layer=None):
+        bounds = self.getBounds(layer)
+        if bounds is None:
+            return
+        if self.verticalOrigin is None:
+            oldValue = self.height - bounds.yMax
+        else:
+            oldValue = self.verticalOrigin - bounds.yMax
+        diff = value - oldValue
+        if oldValue != value:
+            # Is this still correct when verticalOrigin was not previously set?
+            self.verticalOrigin = bounds.yMax + value
+            self.height += diff
diff --git a/src/ufoLib2/objects/point.py b/src/ufoLib2/objects/point.py
index a8d08c8..aa235ec 100644
--- a/src/ufoLib2/objects/point.py
+++ b/src/ufoLib2/objects/point.py
@@ -16,3 +16,8 @@ class Point:
     def segmentType(self):
         # alias for backward compatibility with defcon API
         return self.type
+
+    def move(self, delta):
+        x, y = delta
+        self.x += x
+        self.y += y


Test patch:
diff --git a/tests/objects/test_glyph.py b/tests/objects/test_glyph.py
index f643313..45aa96f 100644
--- a/tests/objects/test_glyph.py
+++ b/tests/objects/test_glyph.py
@@ -1,4 +1,13 @@
-from ufoLib2.objects import Anchor, Contour, Component, Glyph, Guideline, Image, Point
+from ufoLib2.objects import (
+    Anchor,
+    Contour,
+    Component,
+    Glyph,
+    Guideline,
+    Image,
+    Layer,
+    Point,
+)
 
 import pytest
 
@@ -101,3 +110,234 @@ def test_glyph_repr():
 
     g = Glyph("a")
     assert repr(g) == f"<ufoLib2.objects.glyph.Glyph 'a' at {hex(id(g))}>"
+
+
+def test_glyph_get_bounds():
+    a = Glyph("a")
+    pen = a.getPen()
+    pen.moveTo((0, 0))
+    pen.curveTo((10, 10), (10, 20), (0, 20))
+    pen.closePath()
+
+    b = Glyph("b", components=[Component("a", (1, 0, 0, 1, -50, 100))])
+
+    layer = Layer(glyphs=[a, b])
+
+    assert a.getBounds(layer) == Glyph.BoundingBox(xMin=0, yMin=0, xMax=7.5, yMax=20)
+
+    assert a.getControlBounds(layer) == Glyph.BoundingBox(
+        xMin=0, yMin=0, xMax=10, yMax=20
+    )
+
+    with pytest.raises(
+        TypeError, match="layer is required to compute bounds of components"
+    ):
+        b.getBounds()
+    with pytest.raises(
+        TypeError, match="layer is required to compute bounds of components"
+    ):
+        b.getControlBounds()
+
+    assert b.getBounds(layer) == (-50, 100, -42.5, 120)  # namedtuple is a tuple
+    assert b.getControlBounds(layer) == (-50, 100, -40, 120)
+
+
+def test_glyph_get_bounds_empty():
+    g = Glyph()
+    assert g.getBounds() is None
+    assert g.getControlBounds() is None
+
+
+@pytest.fixture
+def layer():
+    a = Glyph("a")
+    pen = a.getPen()
+    pen.moveTo((8, 0))
+    pen.lineTo((18, 0))
+    pen.lineTo((18, 20))
+    pen.lineTo((8, 20))
+    pen.closePath()
+    a.width = 30
+    a.appendAnchor({"x": 10, "y": 30, "name": "top"})
+
+    b = Glyph("b", width=a.width, components=[Component("a", (1, 0, 0, 1, 2, -5))])
+
+    layer = Layer(glyphs=[a, b])
+    return layer
+
+
+def test_glyph_get_margins(layer):
+    a = layer["a"]
+
+    # for simple contour glyphs without components, layer is optional/unused
+    assert a.getLeftMargin() == 8
+    assert a.getLeftMargin(layer) == 8
+    assert a.getRightMargin() == 12
+    assert a.getRightMargin(layer) == 12
+    assert a.getBottomMargin() == 0
+    assert a.getBottomMargin(layer) == 0
+    assert a.getTopMargin() == -20
+    assert a.getTopMargin(layer) == -20
+
+    a.verticalOrigin = 20
+    assert a.getBottomMargin() == -20
+    assert a.getBottomMargin(layer) == -20
+    assert a.getTopMargin() == 0
+    assert a.getTopMargin(layer) == 0
+
+    b = layer["b"]
+    # for composite glyphs, layer is required
+    for m in ("Left", "Right", "Top", "Bottom"):
+        with pytest.raises(TypeError, match="layer is required to compute bounds"):
+            getattr(b, f"get{m}Margin")()
+
+    assert b.getLeftMargin(layer) == 10
+    assert b.getRightMargin(layer) == 10
+    assert b.getBottomMargin(layer) == -5
+    assert b.getTopMargin(layer) == -15
+
+    b.verticalOrigin = 15
+    assert b.getBottomMargin(layer) == -20
+    assert b.getTopMargin(layer) == 0
+
+    c = Glyph()  # empty glyph
+    assert c.getLeftMargin() is None
+    assert c.getRightMargin() is None
+    assert c.getBottomMargin() is None
+    assert c.getTopMargin() is None
+
+
+def test_simple_glyph_set_left_margins(layer):
+    a = layer["a"]
+    b = layer["b"]  # same width, has component 'a' shifted +2 horizontally
+
+    assert a.getLeftMargin() == 8
+    assert b.getLeftMargin(layer) == 10
+    assert a.width == 30
+    assert b.width == 30
+    assert a.anchors[0].x, a.anchors[0].y == (10, 20)
+
+    a.setLeftMargin(8)  # no change
+    assert a.getLeftMargin() == 8
+    assert a.width == 30
+
+    a.setLeftMargin(10)  # +2
+    assert a.getLeftMargin() == 10
+    assert a.width == 32
+    # anchors were shifted
+    assert a.anchors[0].x, a.anchors[0].y == (12, 20)
+    # composite glyph "b" also shifts, but keeps original width
+    assert b.getLeftMargin(layer) == 12
+    assert b.width == 30
+
+    a.setLeftMargin(-2)  # -12
+    assert a.getLeftMargin(-2)
+    assert a.width == 20
+
+
+def test_composite_glyph_set_left_margins(layer):
+    b = layer["b"]
+
+    assert b.getLeftMargin(layer) == 10
+    assert b.width == 30
+
+    b.setLeftMargin(12, layer)  # +2
+    assert b.getLeftMargin(layer) == 12
+    assert b.width == 32
+
+
+def test_simple_glyph_set_right_margins(layer):
+    a = layer["a"]
+    b = layer["b"]  # same width, has component 'a' shifted +2 horizontally
+
+    assert a.getRightMargin() == 12
+    assert b.getRightMargin(layer) == 10
+    assert a.width == 30
+    assert b.width == 30
+    assert a.anchors[0].x, a.anchors[0].y == (10, 20)
+
+    a.setRightMargin(12)  # no change
+    assert a.getRightMargin() == 12
+    assert a.width == 30
+
+    a.setRightMargin(10)  # -2
+    assert a.getRightMargin() == 10
+    # only width changes, anchors stay same
+    assert a.width == 28
+    assert a.anchors[0].x, a.anchors[0].y == (10, 20)
+    # composite glyph "b" does _not_ change when "a" RSB changes
+    assert b.getRightMargin(layer) == 10
+    assert b.width == 30
+
+    a.setRightMargin(-2)  # -12
+    assert a.getRightMargin() == -2
+    assert a.width == 16
+
+
+def test_composite_glyph_set_right_margins(layer):
+    b = layer["b"]
+
+    assert b.getRightMargin(layer) == 10
+    assert b.width == 30
+
+    b.setRightMargin(12, layer)  # +2
+    assert b.getRightMargin(layer) == 12
+    assert b.width == 32
+
+
+def test_simple_glyph_set_bottom_margins(layer):
+    a = layer["a"]
+    b = layer["b"]  # same height/origin, has component 'a' shifted -5 vertically
+    a.verticalOrigin = b.verticalOrigin = a.height = b.height = 30
+
+    assert a.getBottomMargin() == 0
+    assert b.getBottomMargin(layer) == -5
+
+    a.setBottomMargin(-10)
+    assert a.getBottomMargin(layer) == -10
+    assert a.height == 20
+    assert a.verticalOrigin == 30
+    # composite glyph "b" does not change
+    assert b.getBottomMargin(layer) == -5
+    assert b.height == b.verticalOrigin == 30
+
+
+def test_composite_glyph_set_bottom_margins(layer):
+    b = layer["b"]
+    b.verticalOrigin = b.height = 30
+
+    assert b.getBottomMargin(layer) == -5
+    assert b.height == 30
+
+    b.setBottomMargin(0, layer)  # +5
+    assert b.getBottomMargin(layer) == 0
+    assert b.height == 35
+
+
+def test_simple_glyph_set_top_margins(layer):
+    a = layer["a"]
+    b = layer["b"]  # same height/origin, has component 'a' shifted -5 vertically
+    a.verticalOrigin = b.verticalOrigin = a.height = b.height = 30
+
+    assert a.getTopMargin() == 10
+    assert b.getTopMargin(layer) == 15
+
+    a.setTopMargin(-10)
+    assert a.getTopMargin() == -10
+    assert a.height == 10
+    assert a.verticalOrigin == 10
+    # composite glyph "b" does not change
+    assert b.getTopMargin(layer) == 15
+    assert b.height == b.verticalOrigin == 30
+
+
+def test_composite_glyph_set_top_margins(layer):
+    b = layer["b"]
+    b.verticalOrigin = b.height = 30
+
+    assert b.getTopMargin(layer) == 15
+    assert b.height == 30
+
+    b.setTopMargin(10, layer)  # -5
+    assert b.getTopMargin(layer) == 10
+    assert b.height == 25