Merge pull request #943 from googlefonts/copy-shared-panose

instantiator: preserve instance openTypeOS2Panose when values shared by all sources

Author: anthrotype

Lib/ufo2ft/instantiator.py

@@ -121,7 +121,6 @@
 # - openTypeNameUniqueID
 # - openTypeNameWWSFamilyName
 # - openTypeNameWWSSubfamilyName
-# - openTypeOS2Panose
 # - postscriptFullName
 # - postscriptUniqueID
 # - woffMetadataUniqueID
@@ -152,6 +151,7 @@
     "openTypeNameVersion",
     "openTypeOS2CodePageRanges",
     "openTypeOS2FamilyClass",
+    "openTypeOS2Panose",
     "openTypeOS2Selection",
     "openTypeOS2Type",
     "openTypeOS2UnicodeRanges",
@@ -467,6 +467,29 @@ def from_designspace(
             else {}
         )
         copy_info: Optional[Info] = default_font.info if do_info else None
+        if copy_info is not None and copy_info.openTypeOS2Panose is not None:
+            # only copy panose values that are shared by all the designspace.sources,
+            # and set to 0 those that differ
+            panose_sources = [
+                source.font.info.openTypeOS2Panose
+                for source in designspace.sources
+                if source.font.info.openTypeOS2Panose is not None
+            ]
+            shared_panose = [
+                values[0] if len(set(values)) == 1 else 0
+                for values in zip(*panose_sources)
+            ]
+            # defcon already validates openTypeOS2Panose length, while ufoLib2 doesn't;
+            # to be safe, ensure we have exactly 10 values (pad with zeros or truncate)
+            if len(shared_panose) != 10:
+                logger.warning(
+                    "openTypeOS2Panose values in designspace sources have invalid "
+                    "length, they will be padded/truncated to 10 values"
+                )
+                shared_panose = (shared_panose + [0] * 10)[:10]
+            copy_info = copy.copy(copy_info)
+            copy_info.openTypeOS2Panose = shared_panose if any(shared_panose) else None
+
         copy_lib: Mapping[str, Any] = default_font.lib if do_info else {}
 
         # The list of glyphs-not-to-export-and-decompose-where-used-as-a-component is

tests/instantiator_test.py

@@ -1011,8 +1011,8 @@ def test_data_independence(ufo_module, data_dir):
 
     assert generator.copy_info.openTypeOS2Panose == [2, 11, 5, 4, 2, 2, 2, 2, 2, 4]
     generator.copy_info.openTypeOS2Panose.append(1000)
-    assert instance_font1.info.openTypeOS2Panose is None
-    assert instance_font2.info.openTypeOS2Panose is None
+    assert instance_font1.info.openTypeOS2Panose == [2, 11, 5, 4, 2, 2, 2, 2, 2, 4]
+    assert instance_font2.info.openTypeOS2Panose == [2, 11, 5, 4, 2, 2, 2, 2, 2, 4]
 
     # copy_feature_text not tested because it is a(n immutable) string
 
@@ -1039,7 +1039,6 @@ def test_skipped_fontinfo_attributes():
         "openTypeNameUniqueID",
         "openTypeNameWWSFamilyName",
         "openTypeNameWWSSubfamilyName",
-        "openTypeOS2Panose",
         "postscriptFontName",
         "postscriptFullName",
         "postscriptUniqueID",
@@ -1070,6 +1069,237 @@ def test_designspace_v5_discrete_axis_raises_error(data_dir):
         ufo2ft.instantiator.Instantiator.from_designspace(designspace)
 
 
+def test_opentype_os2_panose_merging(ufo_module):
+    """Test that openTypeOS2Panose values are properly merged across sources.
+
+    Only panose values that are identical across all sources should be copied
+    to instances. Different values should be set to 0.
+    """
+    d = designspaceLib.DesignSpaceDocument()
+    d.addAxisDescriptor(
+        name="Weight", tag="wght", minimum=300, default=300, maximum=900
+    )
+
+    font1 = ufo_module.Font()
+    font1.info.openTypeOS2Panose = [2, 11, 5, 2, 4, 5, 4, 2, 2, 4]  # default source
+
+    font2 = ufo_module.Font()
+    font2.info.openTypeOS2Panose = [
+        2,
+        11,
+        8,  # different at index 2
+        2,
+        4,
+        5,
+        4,
+        2,
+        2,
+        4,
+    ]
+
+    font3 = ufo_module.Font()
+    font3.info.openTypeOS2Panose = [
+        2,
+        11,
+        5,
+        3,  # different at index 3
+        4,
+        5,
+        4,
+        2,
+        2,
+        4,
+    ]
+
+    d.addSourceDescriptor(location={"Weight": 300}, font=font1)
+    d.addSourceDescriptor(location={"Weight": 600}, font=font2)
+    d.addSourceDescriptor(location={"Weight": 900}, font=font3)
+    d.addInstanceDescriptor(styleName="Regular", location={"Weight": 400})
+    d.addInstanceDescriptor(styleName="Bold", location={"Weight": 700})
+
+    generator = ufo2ft.instantiator.Instantiator.from_designspace(d)
+
+    instance1 = generator.generate_instance(d.instances[0])
+    instance2 = generator.generate_instance(d.instances[1])
+
+    # Indices 2 and 3 differ across sources, so they should be 0
+    # All other indices are the same across sources
+    expected_panose = [2, 11, 0, 0, 4, 5, 4, 2, 2, 4]
+
+    assert instance1.info.openTypeOS2Panose == expected_panose
+    assert instance2.info.openTypeOS2Panose == expected_panose
+
+
+def test_opentype_os2_panose_all_different(ufo_module):
+    """Test that when all panose values differ, the attribute is deleted."""
+    d = designspaceLib.DesignSpaceDocument()
+    d.addAxisDescriptor(
+        name="Weight", tag="wght", minimum=300, default=300, maximum=900
+    )
+
+    # Create fonts with completely different panose values
+    font1 = ufo_module.Font()
+    font1.info.openTypeOS2Panose = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
+
+    font2 = ufo_module.Font()
+    font2.info.openTypeOS2Panose = [10, 9, 8, 7, 6, 5, 4, 3, 2, 1]
+
+    d.addSourceDescriptor(location={"Weight": 300}, font=font1)
+    d.addSourceDescriptor(location={"Weight": 900}, font=font2)
+    d.addInstanceDescriptor(styleName="Regular", location={"Weight": 600})
+
+    generator = ufo2ft.instantiator.Instantiator.from_designspace(d)
+    instance = generator.generate_instance(d.instances[0])
+
+    # All values differ, so openTypeOS2Panose should be unset
+    assert instance.info.openTypeOS2Panose is None
+
+
+def test_opentype_os2_panose_missing_in_some_sources(ufo_module):
+    """Test handling when some sources don't have panose values."""
+    d = designspaceLib.DesignSpaceDocument()
+    d.addAxisDescriptor(
+        name="Weight", tag="wght", minimum=300, default=300, maximum=900
+    )
+
+    font1 = ufo_module.Font()
+    font1.info.openTypeOS2Panose = [2, 11, 5, 2, 4, 5, 4, 2, 2, 4]
+
+    # Second font doesn't have panose, doesn't contribute
+    font2 = ufo_module.Font()
+    assert font2.info.openTypeOS2Panose is None
+
+    font3 = ufo_module.Font()
+    font3.info.openTypeOS2Panose = [2, 11, 5, 2, 4, 5, 4, 2, 2, 4]
+
+    d.addSourceDescriptor(location={"Weight": 300}, font=font1)
+    d.addSourceDescriptor(location={"Weight": 600}, font=font2)
+    d.addSourceDescriptor(location={"Weight": 900}, font=font3)
+    d.addInstanceDescriptor(styleName="Regular", location={"Weight": 400})
+
+    generator = ufo2ft.instantiator.Instantiator.from_designspace(d)
+    instance = generator.generate_instance(d.instances[0])
+
+    # The default source's panose should be copied since font2 has None and
+    # font3 has same panose as font1
+    assert instance.info.openTypeOS2Panose == [2, 11, 5, 2, 4, 5, 4, 2, 2, 4]
+
+
+def test_opentype_os2_panose_single_source(ufo_module):
+    """Test that panose values are preserved as-is when there's only one source.
+
+    Cf. https://github.com/googlefonts/fontc/issues/1609
+    """
+    d = designspaceLib.DesignSpaceDocument()
+    d.addAxisDescriptor(
+        name="Weight", tag="wght", minimum=300, default=400, maximum=900
+    )
+
+    font1 = ufo_module.Font()
+    font1.info.openTypeOS2Panose = [2, 11, 5, 2, 4, 5, 4, 2, 2, 4]
+
+    d.addSourceDescriptor(location={"Weight": 400}, font=font1)
+    d.addInstanceDescriptor(styleName="Regular", location={"Weight": 400})
+
+    generator = ufo2ft.instantiator.Instantiator.from_designspace(d)
+
+    instance = generator.generate_instance(d.instances[0])
+
+    # With only one source, panose should be preserved as-is
+    assert instance.info.openTypeOS2Panose == [2, 11, 5, 2, 4, 5, 4, 2, 2, 4]
+
+
+def test_opentype_os2_panose_no_mutation_of_default(ufo_module):
+    """Test that processing panose values doesn't mutate the default source."""
+    d = designspaceLib.DesignSpaceDocument()
+    d.addAxisDescriptor(
+        name="Weight", tag="wght", minimum=300, default=300, maximum=900
+    )
+
+    font1 = ufo_module.Font()
+    original_panose = [2, 11, 5, 2, 4, 5, 4, 2, 2, 4]
+    font1.info.openTypeOS2Panose = original_panose.copy()
+
+    font2 = ufo_module.Font()
+    font2.info.openTypeOS2Panose = [
+        2,
+        11,
+        8,  # different at index 2
+        2,
+        4,
+        5,
+        4,
+        2,
+        2,
+        4,
+    ]
+
+    d.addSourceDescriptor(location={"Weight": 300}, font=font1)
+    d.addSourceDescriptor(location={"Weight": 900}, font=font2)
+    d.addInstanceDescriptor(styleName="Regular", location={"Weight": 600})
+
+    generator = ufo2ft.instantiator.Instantiator.from_designspace(d)
+    instance = generator.generate_instance(d.instances[0])
+
+    # The instance should have merged panose with 0 at index 2
+    assert instance.info.openTypeOS2Panose == [2, 11, 0, 2, 4, 5, 4, 2, 2, 4]
+
+    # The default source should remain unchanged
+    assert font1.info.openTypeOS2Panose == original_panose
+    assert d.default.font.info.openTypeOS2Panose == original_panose
+
+
+@pytest.mark.parametrize(
+    "panose_data,expected",
+    [
+        pytest.param(
+            [[2, 11, 5], [2, 11, 8, 2, 4, 5, 4, 2, 2, 4, 99, 88, 77, 66, 55]],
+            [2, 11, 0, 0, 0, 0, 0, 0, 0, 0],
+            id="zero-padded",  # At least one source has length < 10 (will be padded)
+        ),
+        pytest.param(
+            [
+                [2, 11, 5, 2, 4, 5, 4, 2, 2, 4, 99, 88],
+                [2, 11, 8, 2, 4, 5, 4, 2, 2, 4, 77, 66, 55],
+            ],
+            [2, 11, 0, 2, 4, 5, 4, 2, 2, 4],
+            id="truncated",  # All sources have length > 10 (will be truncated)
+        ),
+    ],
+)
+def test_opentype_os2_panose_malformed_lengths(panose_data, expected, caplog):
+    """Test handling of panose values with incorrect lengths (ufoLib2 only)."""
+    ufo_module = pytest.importorskip("ufoLib2")
+
+    d = designspaceLib.DesignSpaceDocument()
+    d.addAxisDescriptor(
+        name="Weight", tag="wght", minimum=300, default=300, maximum=900
+    )
+
+    fonts = []
+    for i, panose_values in enumerate(panose_data):
+        font = ufo_module.Font()
+        font.info.openTypeOS2Panose = panose_values
+        fonts.append(font)
+        d.addSourceDescriptor(location={"Weight": 300 + i * 600}, font=font)
+
+    d.addInstanceDescriptor(styleName="Regular", location={"Weight": 600})
+
+    with caplog.at_level(logging.WARNING):
+        generator = ufo2ft.instantiator.Instantiator.from_designspace(d)
+        instance = generator.generate_instance(d.instances[0])
+
+    # Should log a warning about invalid length
+    assert (
+        "openTypeOS2Panose values in designspace sources have invalid length"
+        in caplog.text
+    )
+
+    # Should produce a valid result with exactly 10 values
+    assert instance.info.openTypeOS2Panose == expected
+    assert len(instance.info.openTypeOS2Panose) == 10
+
+
 def test_strict_math_glyph(ufo_module, data_dir):
     designspace = designspaceLib.DesignSpaceDocument.fromfile(
         data_dir / "InstantiatorStrictMathGlyph" / "StrictMathGlyph.designspace"