From 5ef0ba4171f4f1f730ab8cadf8b3ae7ecc372850 Mon Sep 17 00:00:00 2001
From: Denise LaFayette <dlafayette@netflix.com>
Date: Tue, 27 Apr 2021 11:29:37 -0700
Subject: [PATCH] Fix bug in TTML inherited text alignment

Fix bug where child does not correctly inherit ancestor's text
alignment setting.

Make it so that alignment is only applied on a P node.
https://www.w3.org/TR/2018/REC-ttml2-20181108/#style-attribute-textAlign
---
 .../exoplayer2/text/ttml/TtmlNode.java        | 24 +++++++++++--------
 .../exoplayer2/text/ttml/TtmlDecoderTest.java |  8 +++++++
 .../src/test/assets/media/ttml/text_align.xml |  6 +++++
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/library/core/src/main/java/com/google/android/exoplayer2/text/ttml/TtmlNode.java b/library/core/src/main/java/com/google/android/exoplayer2/text/ttml/TtmlNode.java
index 96c2dbe5f4..a66c609a86 100644
--- a/library/core/src/main/java/com/google/android/exoplayer2/text/ttml/TtmlNode.java
+++ b/library/core/src/main/java/com/google/android/exoplayer2/text/ttml/TtmlNode.java
@@ -409,17 +409,21 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
     if (resolvedStyle != null) {
       TtmlRenderUtil.applyStylesToSpan(
           text, start, end, resolvedStyle, parent, globalStyles, verticalType);
-      if (resolvedStyle.getShearPercentage() != TtmlStyle.UNSPECIFIED_SHEAR && TAG_P.equals(tag)) {
-        // Shear style should only be applied to P nodes
-        // https://www.w3.org/TR/2018/REC-ttml2-20181108/#style-attribute-shear
-        // The spec doesn't specify the coordinate system to use for block shear
-        // however the spec shows examples of how different values are expected to be rendered.
-        // See: https://www.w3.org/TR/2018/REC-ttml2-20181108/#style-attribute-shear
-        // https://www.w3.org/TR/2018/REC-ttml2-20181108/#style-attribute-fontShear
-        // This maps the shear percentage to shear angle in graphics coordinates
-        regionOutput.setShearDegrees((resolvedStyle.getShearPercentage() * -90) / 100);
+      if (TAG_P.equals(tag)) {
+        if (resolvedStyle.getShearPercentage() != TtmlStyle.UNSPECIFIED_SHEAR) {
+          // Shear style should only be applied to P nodes
+          // https://www.w3.org/TR/2018/REC-ttml2-20181108/#style-attribute-shear
+          // The spec doesn't specify the coordinate system to use for block shear
+          // however the spec shows examples of how different values are expected to be rendered.
+          // See: https://www.w3.org/TR/2018/REC-ttml2-20181108/#style-attribute-shear
+          // https://www.w3.org/TR/2018/REC-ttml2-20181108/#style-attribute-fontShear
+          // This maps the shear percentage to shear angle in graphics coordinates
+          regionOutput.setShearDegrees((resolvedStyle.getShearPercentage() * -90) / 100);
+        }
+        if (resolvedStyle.getTextAlign() != null) {
+          regionOutput.setTextAlignment(resolvedStyle.getTextAlign());
+        }
       }
-      regionOutput.setTextAlignment(resolvedStyle.getTextAlign());
     }
   }
 
diff --git a/library/core/src/test/java/com/google/android/exoplayer2/text/ttml/TtmlDecoderTest.java b/library/core/src/test/java/com/google/android/exoplayer2/text/ttml/TtmlDecoderTest.java
index 2e908f3325..2b77ab7243 100644
--- a/library/core/src/test/java/com/google/android/exoplayer2/text/ttml/TtmlDecoderTest.java
+++ b/library/core/src/test/java/com/google/android/exoplayer2/text/ttml/TtmlDecoderTest.java
@@ -607,6 +607,14 @@ public final class TtmlDecoderTest {
     Cue seventhCue = getOnlyCueAtTimeUs(subtitle, 70_000_000);
     assertThat(seventhCue.text.toString()).isEqualTo("No textAlign property");
     assertThat(seventhCue.textAlignment).isNull();
+
+    Cue eighthCue = getOnlyCueAtTimeUs(subtitle, 80_000_000);
+    assertThat(eighthCue.text.toString()).isEqualTo("Ancestor start alignment");
+    assertThat(eighthCue.textAlignment).isEqualTo(Layout.Alignment.ALIGN_NORMAL);
+
+    Cue ninthCue = getOnlyCueAtTimeUs(subtitle, 90_000_000);
+    assertThat(ninthCue.text.toString()).isEqualTo("Not a P node");
+    assertThat(ninthCue.textAlignment).isNull();
   }
 
   @Test
diff --git a/testdata/src/test/assets/media/ttml/text_align.xml b/testdata/src/test/assets/media/ttml/text_align.xml
index 7244a7c292..c9648d5647 100644
--- a/testdata/src/test/assets/media/ttml/text_align.xml
+++ b/testdata/src/test/assets/media/ttml/text_align.xml
@@ -24,5 +24,11 @@
     <div>
       <p begin="70s" end="78s">No textAlign property</p>
     </div>
+    <div>
+      <p begin="80s" end="88s" tts:textAlign="start"><span tts:fontSize="50%">Ancestor start</span> alignment</p>
+    </div>
+    <div>
+      <p begin="90s" end="98s"><span tts:textAlign="start">Not a P node</span></p>
+    </div>
   </body>
 </tt>