From 2c671b45e7f89d4ba166a51239bcdf42985a7757 Mon Sep 17 00:00:00 2001 From: OlteanuRares Date: Mon, 10 Aug 2026 17:23:37 +0300 Subject: [PATCH] OCTO-11559 Fix fit_to_screen crash for edge-of-screen origins, support negative sizes and viewport units --- docs/changelog.rst | 21 +++++++ pycaption/dfxp/writer.py | 4 ++ pycaption/geometry.py | 62 +++++++++++--------- tests/fixtures/dfxp.py | 5 +- tests/test_geometry.py | 121 +++++++++++++++++++++++++++++++++++++-- 5 files changed, 179 insertions(+), 34 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 7289b04f..4278a8a0 100644 --- a/docs/changelog.rst +++ b/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 ``
`` 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 diff --git a/pycaption/dfxp/writer.py b/pycaption/dfxp/writer.py index 8c533371..4731b49e 100644 --- a/pycaption/dfxp/writer.py +++ b/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 diff --git a/pycaption/geometry.py b/pycaption/geometry.py index 51ec12f8..17ea673a 100644 --- a/pycaption/geometry.py +++ b/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"" @@ -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\d+(\.\d+)?)(?P" + r"^(((?P-?\d+(\.\d+)?)(?P" 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." diff --git a/tests/fixtures/dfxp.py b/tests/fixtures/dfxp.py index 6d98f901..0ab1a5e5 100644 --- a/tests/fixtures/dfxp.py +++ b/tests/fixtures/dfxp.py @@ -1359,13 +1359,12 @@ def sample_dfxp_empty_cue_output():