From f2df149db367b3b5274e7fd032df25af77cb639f Mon Sep 17 00:00:00 2001
From: Cosimo Lupo <clupo@google.com>
Date: Fri, 14 May 2021 10:33:12 +0100
Subject: [PATCH] COLR: set {BaseGlyph,Layer}RecordArray to None when no
 colorGlyphsV0 in populateCOLRv0

Same when LayerV1List is empty. All these fields are optional in v1
---
 Lib/fontTools/colorLib/builder.py | 30 +++++++++++++++---------------
 Tests/colorLib/builder_test.py    |  5 ++---
 2 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/Lib/fontTools/colorLib/builder.py b/Lib/fontTools/colorLib/builder.py
index 821244af0..d880dab23 100644
--- a/Lib/fontTools/colorLib/builder.py
+++ b/Lib/fontTools/colorLib/builder.py
@@ -142,7 +142,7 @@ def populateCOLRv0(
     Args:
         table: a raw otTables.COLR() object (not ttLib's table_C_O_L_R_).
         colorGlyphsV0: map of base glyph names to lists of (layer glyph names,
-            color palette index) tuples.
+            color palette index) tuples. Can be empty.
         glyphMap: a map from glyph names to glyph indices, as returned from
             TTFont.getReverseGlyphMap(), to optionally sort base records by GID.
     """
@@ -167,11 +167,14 @@ def populateCOLRv0(
             layerRec.PaletteIndex = paletteIndex
             layerRecords.append(layerRec)
 
+    table.BaseGlyphRecordArray = table.LayerRecordArray = None
+    if baseGlyphRecords:
+        table.BaseGlyphRecordArray = ot.BaseGlyphRecordArray()
+        table.BaseGlyphRecordArray.BaseGlyphRecord = baseGlyphRecords
+    if layerRecords:
+        table.LayerRecordArray = ot.LayerRecordArray()
+        table.LayerRecordArray.LayerRecord = layerRecords
     table.BaseGlyphRecordCount = len(baseGlyphRecords)
-    table.BaseGlyphRecordArray = ot.BaseGlyphRecordArray()
-    table.BaseGlyphRecordArray.BaseGlyphRecord = baseGlyphRecords
-    table.LayerRecordArray = ot.LayerRecordArray()
-    table.LayerRecordArray.LayerRecord = layerRecords
     table.LayerRecordCount = len(layerRecords)
 
 
@@ -209,18 +212,13 @@ def buildCOLR(
     else:
         # unless explicitly requested for v1 or have variations, in which case
         # we encode all color glyph as v1
-        colorGlyphsV0, colorGlyphsV1 = None, colorGlyphs
+        colorGlyphsV0, colorGlyphsV1 = {}, colorGlyphs
 
     colr = ot.COLR()
 
-    if colorGlyphsV0:
-        populateCOLRv0(colr, colorGlyphsV0, glyphMap)
-    else:
-        colr.BaseGlyphRecordCount = colr.LayerRecordCount = 0
-        colr.BaseGlyphRecordArray = colr.LayerRecordArray = None
+    populateCOLRv0(colr, colorGlyphsV0, glyphMap)
 
-    if colorGlyphsV1:
-        colr.LayerV1List, colr.BaseGlyphV1List = buildColrV1(colorGlyphsV1, glyphMap)
+    colr.LayerV1List, colr.BaseGlyphV1List = buildColrV1(colorGlyphsV1, glyphMap)
 
     if version is None:
         version = 1 if (varStore or colorGlyphsV1) else 0
@@ -538,7 +536,9 @@ class LayerV1ListBuilder:
     def buildPaint(self, paint: _PaintInput) -> ot.Paint:
         return self.tableBuilder.build(ot.Paint, paint)
 
-    def build(self) -> ot.LayerV1List:
+    def build(self) -> Optional[ot.LayerV1List]:
+        if not self.layers:
+            return None
         layers = ot.LayerV1List()
         layers.LayerCount = len(self.layers)
         layers.Paint = self.layers
@@ -564,7 +564,7 @@ def _format_glyph_errors(errors: Mapping[str, Exception]) -> str:
 def buildColrV1(
     colorGlyphs: _ColorGlyphsDict,
     glyphMap: Optional[Mapping[str, int]] = None,
-) -> Tuple[ot.LayerV1List, ot.BaseGlyphV1List]:
+) -> Tuple[Optional[ot.LayerV1List], ot.BaseGlyphV1List]:
     if glyphMap is not None:
         colorGlyphItems = sorted(
             colorGlyphs.items(), key=lambda item: glyphMap[item[0]]
diff --git a/Tests/colorLib/builder_test.py b/Tests/colorLib/builder_test.py
index 28af165fb..ff246646e 100644
--- a/Tests/colorLib/builder_test.py
+++ b/Tests/colorLib/builder_test.py
@@ -1144,8 +1144,7 @@ def test_build_layerv1list_empty():
     # 2 v1 glyphs, none in LayerV1List
     assert colr.table.BaseGlyphV1List.BaseGlyphCount == 2
     assert len(colr.table.BaseGlyphV1List.BaseGlyphV1Record) == 2
-    assert colr.table.LayerV1List.LayerCount == 0
-    assert len(colr.table.LayerV1List.Paint) == 0
+    assert colr.table.LayerV1List is None
 
 
 def _paint_names(paints) -> List[str]:
@@ -1528,7 +1527,7 @@ class BuildCOLRTest(object):
             },
         )
 
-        assert len(colr.table.LayerV1List.Paint) == 0, "PaintColrLayers should be gone"
+        assert colr.table.LayerV1List is None, "PaintColrLayers should be gone"
         assert colr.table.BaseGlyphV1List.BaseGlyphCount == 1
         paint = colr.table.BaseGlyphV1List.BaseGlyphV1Record[0].Paint
         assert paint.Format == ot.PaintFormat.PaintGlyph