OCTO-11559

docs/changelog.rst

@@ -1,5 +1,26 @@
 Changelog
 ---------
+2.3.6
+^^^^^^
+  - Fix ``fit_to_screen()`` producing negative or zero extent when origin
+    is at or beyond 90% horizontal / 95% vertical. An early-return guard
+    now preserves the layout unchanged for these edge-of-screen origins
+    instead of computing ``90 - origin`` (which yielded 0 or negative).
+
+  - Fix ``Size.from_string()`` rejecting negative values (e.g. ``"-5%"``).
+    The regex now accepts an optional leading minus sign.
+
+  - Add ``VW`` and ``VH`` viewport units to ``UnitEnum``. DFXP/TTML files
+    using ``tts:origin="10vw 10vh"`` or ``tts:extent="80vw 50vh"`` now
+    parse without error. ``as_percentage_of()`` treats them as equivalent
+    to percentages; the default ``relativize=True`` writer path converts
+    them to ``%`` so they never leak into output.
+
+  - DFXP writer: relativize and fit-to-screen the language-level
+    ``layout_info`` (the ``<div region="...">`` layout), not just
+    per-caption layouts. Previously the language-level layout was written
+    raw, causing duplicate regions in output.
+
 2.3.5
 ^^^^^^
   - Fix SCC positioning lost on conversion to VTT/DFXP/SAMI. The

pycaption/dfxp/writer.py

@@ -93,6 +93,10 @@ def _relativize_layouts(self, caption_set, langs):
         :type langs: list[str]
         """
         for lang in langs:
+            caption_set.set_layout_info(
+                lang,
+                self._relativize_and_fit_to_screen(caption_set.get_layout_info(lang)),
+            )
             for caption in caption_set.get_captions(lang):
                 caption.layout_info = self._relativize_and_fit_to_screen(
                     caption.layout_info

pycaption/geometry.py

@@ -29,6 +29,8 @@ class UnitEnum(Enum):
     PERCENT = "%"
     CELL = "c"
     PT = "pt"
+    VW = "vw"
+    VH = "vh"
 
 
 class VerticalAlignmentEnum(Enum):
@@ -95,10 +97,7 @@ def __hash__(self):
     def __eq__(self, other):
         if not isinstance(other, Alignment):
             return NotImplemented
-        return (
-            self.horizontal == other.horizontal
-            and self.vertical == other.vertical
-        )
+        return self.horizontal == other.horizontal and self.vertical == other.vertical
 
     def __repr__(self):
         return f"<Alignment ({self.horizontal} {self.vertical})>"
@@ -181,10 +180,7 @@ def serialized(self):
     def __eq__(self, other):
         if not isinstance(other, Stretch):
             return NotImplemented
-        return (
-            self.horizontal == other.horizontal
-            and self.vertical == other.vertical
-        )
+        return self.horizontal == other.horizontal and self.vertical == other.vertical
 
     def __hash__(self):
         return hash(hash(self.horizontal) * 59 + hash(self.vertical) * 61 + 67)
@@ -200,9 +196,8 @@ def to_xml_attribute(self, **kwargs):
 
     def is_relative(self):
         """Return True if all dimensions are expressed as percentages."""
-        return (
-            (not self.horizontal or self.horizontal.is_relative())
-            and (not self.vertical or self.vertical.is_relative())
+        return (not self.horizontal or self.horizontal.is_relative()) and (
+            not self.vertical or self.vertical.is_relative()
         )
 
     def as_percentage_of(self, video_width, video_height):
@@ -240,9 +235,8 @@ def add_stretch(self, stretch):
 
     def is_relative(self):
         """Return True if all dimensions are expressed as percentages."""
-        return (
-            (not self.x or self.x.is_relative())
-            and (not self.y or self.y.is_relative())
+        return (not self.x or self.x.is_relative()) and (
+            not self.y or self.y.is_relative()
         )
 
     def as_percentage_of(self, video_width, video_height):
@@ -357,6 +351,9 @@ def as_percentage_of(self, video_width=None, video_height=None):
         if unit == UnitEnum.PERCENT:
             return self  # Nothing to do here
 
+        if unit in (UnitEnum.VW, UnitEnum.VH):
+            return Size(value, UnitEnum.PERCENT)
+
         # The input must be valid so that any conversion can be done
         if not (video_width or video_height):
             raise RelativizationError(
@@ -401,7 +398,7 @@ def from_string(cls, string):
         :rtype: Size
         """
         size_pattern = re.compile(
-            r"^(((?P<value>\d+(\.\d+)?)(?P<unit>"
+            r"^(((?P<value>-?\d+(\.\d+)?)(?P<unit>"
             rf"{'|'.join([unit.value for unit in UnitEnum])}))|0)$"
         )
         match = size_pattern.search(string)
@@ -416,7 +413,6 @@ def from_string(cls, string):
             value = match.group("value")
             return cls(value, UnitEnum(unit))
         else:
-            # If the unit is missing, the only accepted alternative is zero
             return cls(match.group(0), UnitEnum.PIXEL)
 
     def __repr__(self):
@@ -447,7 +443,6 @@ def __hash__(self):
         return hash(hash(self.value) * 41 + hash(self.unit) * 43 + 47)
 
 
-
 class Padding:
     """Represents padding information. Consists of 4 Size objects, representing
     padding from (in this order): before (up), after (down), start (left) and
@@ -647,14 +642,16 @@ def __init__(
                     setattr(self, attr_name, getattr(inherit_from, attr_name))
 
     def __bool__(self):
-        return any((
-            self.origin,
-            self.extent,
-            self.padding,
-            self.alignment,
-            self.webvtt_positioning,
-            self.writing_direction,
-        ))
+        return any(
+            (
+                self.origin,
+                self.extent,
+                self.padding,
+                self.alignment,
+                self.webvtt_positioning,
+                self.writing_direction,
+            )
+        )
 
     def __repr__(self):
         return (
@@ -729,6 +726,15 @@ def fit_to_screen(self):
         if not self.origin:
             return self
 
+        if (
+            self.origin.x.unit != UnitEnum.PERCENT
+            or self.origin.y.unit != UnitEnum.PERCENT
+        ):
+            return self
+
+        if self.origin.x.value >= 90 or self.origin.y.value >= 95:
+            return self
+
         diff_horizontal = Size(90 - self.origin.x.value, UnitEnum.PERCENT)
         diff_vertical = Size(95 - self.origin.y.value, UnitEnum.PERCENT)
 
@@ -749,8 +755,10 @@ def _corrected_extent(self, diff_horizontal, diff_vertical):
         """Return extent clamped so origin + extent doesn't exceed the screen."""
         bottom_right = self.origin.add_stretch(self.extent)
 
-        if (bottom_right.x.unit != UnitEnum.PERCENT
-                or bottom_right.y.unit != UnitEnum.PERCENT):
+        if (
+            bottom_right.x.unit != UnitEnum.PERCENT
+            or bottom_right.y.unit != UnitEnum.PERCENT
+        ):
             raise ValueError(
                 "Units must be relativized before extent "
                 "can be calculated based on origin."

tests/fixtures/dfxp.py

@@ -1359,13 +1359,12 @@ def sample_dfxp_empty_cue_output():
       <style tts:color="white" tts:fontFamily="monospace" tts:fontSize="1c" xml:id="default"/>
    </styling>
    <layout>
-      <region tts:displayAlign="after" tts:origin="10% 10%" tts:textAlign="start" xml:id="r0"/>
-      <region tts:displayAlign="after" tts:extent="80% 85%" tts:origin="10% 10%" tts:textAlign="start" xml:id="r1"/>
+      <region tts:displayAlign="after" tts:extent="80% 85%" tts:origin="10% 10%" tts:textAlign="start" xml:id="r0"/>
    </layout>
  </head>
  <body>
    <div region="r0" xml:lang="en-US">
-     <p begin="00:00:01.209" end="00:00:02.312" region="r1" style="default">
+     <p begin="00:00:01.209" end="00:00:02.312" region="r0" style="default">
        abc
      </p>
    </div>

tests/test_geometry.py

@@ -1,6 +1,6 @@
 import pytest
 
-from pycaption import CaptionReadSyntaxError
+from pycaption import CaptionReadSyntaxError, DFXPReader, DFXPWriter, WebVTTReader
 from pycaption.geometry import (
     Alignment,
     HorizontalAlignmentEnum,
@@ -139,6 +139,32 @@ def test_valid_size_from_string(self, string, value, unit):
         assert size.value == value
         assert size.unit == unit
 
+    @pytest.mark.parametrize(
+        "string, value, unit",
+        [
+            ("-5%", -5.0, UnitEnum.PERCENT),
+            ("-2.5px", -2.5, UnitEnum.PIXEL),
+        ],
+    )
+    def test_negative_size_from_string(self, string, value, unit):
+        size = Size.from_string(string)
+
+        assert size.value == value
+        assert size.unit == unit
+
+    @pytest.mark.parametrize(
+        "string, value, unit",
+        [
+            ("80vw", 80.0, UnitEnum.VW),
+            ("50vh", 50.0, UnitEnum.VH),
+        ],
+    )
+    def test_viewport_units_from_string(self, string, value, unit):
+        size = Size.from_string(string)
+
+        assert size.value == value
+        assert size.unit == unit
+
     @pytest.mark.parametrize("string", ["10", "11,1px", "12xx", "%", "o1pt"])
     def test_invalid_size_from_string(self, string):
         with pytest.raises(CaptionReadSyntaxError) as exc_info:
@@ -159,9 +185,7 @@ class TestAlignmentFromHorizontalAndVertical:
         ],
     )
     def test_horizontal_mapping(self, text_align, expected):
-        alignment = Alignment.from_horizontal_and_vertical_align(
-            text_align=text_align
-        )
+        alignment = Alignment.from_horizontal_and_vertical_align(text_align=text_align)
 
         assert alignment.horizontal == expected
         assert alignment.vertical is None
@@ -200,3 +224,92 @@ def test_unknown_values_return_none(self):
 
     def test_no_args_returns_none(self):
         assert Alignment.from_horizontal_and_vertical_align() is None
+
+
+class TestFitToScreen:
+    def test_origin_at_90_horizontal_returns_unchanged(self):
+        origin = Point(Size(90, UnitEnum.PERCENT), Size(50, UnitEnum.PERCENT))
+        extent = Stretch(Size(40, UnitEnum.PERCENT), Size(20, UnitEnum.PERCENT))
+        layout = Layout(origin=origin, extent=extent)
+
+        result = layout.fit_to_screen()
+
+        assert result.extent == extent
+
+    def test_origin_beyond_95_vertical_returns_unchanged(self):
+        origin = Point(Size(50, UnitEnum.PERCENT), Size(95, UnitEnum.PERCENT))
+        extent = Stretch(Size(40, UnitEnum.PERCENT), Size(20, UnitEnum.PERCENT))
+        layout = Layout(origin=origin, extent=extent)
+
+        result = layout.fit_to_screen()
+
+        assert result.extent == extent
+
+    def test_origin_below_thresholds_still_adjusts(self):
+        origin = Point(Size(80, UnitEnum.PERCENT), Size(80, UnitEnum.PERCENT))
+        extent = Stretch(Size(40, UnitEnum.PERCENT), Size(40, UnitEnum.PERCENT))
+        layout = Layout(origin=origin, extent=extent)
+
+        result = layout.fit_to_screen()
+
+        assert result.extent.horizontal == Size(10, UnitEnum.PERCENT)
+        assert result.extent.vertical == Size(15, UnitEnum.PERCENT)
+
+    def test_non_percent_units_returns_unchanged(self):
+        origin = Point(Size(10, UnitEnum.VW), Size(10, UnitEnum.VH))
+        extent = Stretch(Size(80, UnitEnum.VW), Size(50, UnitEnum.VH))
+        layout = Layout(origin=origin, extent=extent)
+
+        result = layout.fit_to_screen()
+
+        assert result is layout
+
+
+class TestViewportUnitConversion:
+    def test_vw_converts_to_percent(self):
+        size = Size(80, UnitEnum.VW)
+
+        result = size.as_percentage_of(video_width=1920)
+
+        assert result.value == 80.0
+        assert result.unit == UnitEnum.PERCENT
+
+    def test_vh_converts_to_percent(self):
+        size = Size(50, UnitEnum.VH)
+
+        result = size.as_percentage_of(video_height=1080)
+
+        assert result.value == 50.0
+        assert result.unit == UnitEnum.PERCENT
+
+
+class TestGeometryRoundTrips:
+    def test_vtt_extreme_position_to_dfxp_roundtrip(self):
+        vtt = (
+            "WEBVTT\n\n"
+            "00:00:01.000 --> 00:00:03.000 position:95% line:95% size:40%\n"
+            "Hello world\n"
+        )
+        caption_set = WebVTTReader().read(vtt)
+        dfxp_output = DFXPWriter().write(caption_set)
+        DFXPReader().read(dfxp_output)
+
+    def test_dfxp_viewport_units_write_no_leak(self):
+        dfxp = (
+            '<?xml version="1.0" encoding="utf-8"?>\n'
+            '<tt xml:lang="en" xmlns="http://www.w3.org/ns/ttml"'
+            ' xmlns:tts="http://www.w3.org/ns/ttml#styling">\n'
+            "<head><layout>\n"
+            '<region xml:id="r1" tts:origin="10vw 10vh"'
+            ' tts:extent="80vw 50vh"/>\n'
+            "</layout></head>\n"
+            '<body><div><p begin="00:00:01.000" end="00:00:03.000"'
+            ' region="r1">Hello</p></div></body></tt>\n'
+        )
+        caption_set = DFXPReader().read(dfxp)
+        output = DFXPWriter(relativize=True, video_width=1920, video_height=1080).write(
+            caption_set
+        )
+
+        assert "vw" not in output
+        assert "vh" not in output