Merge pull request #6724 from nnoury:fix/subtitles-outline-color

PiperOrigin-RevId: 292316767

RELEASENOTES.md

@@ -35,8 +35,11 @@
     *   Add `showScrubber` and `hideScrubber` methods to `DefaultTimeBar`.
     *   Update `TrackSelectionDialogBuilder` to use AndroidX Compat Dialog
         ([#7357](https://github.com/google/ExoPlayer/issues/7357)).
-*   Text: Use anti-aliasing and bitmap filtering when displaying bitmap
+*   Text:
-    subtitles.
+    *   Use anti-aliasing and bitmap filtering when displaying bitmap
+        subtitles.
+    *   Fix `SubtitlePainter` to render `EDGE_TYPE_OUTLINE` using the correct
+        color.
 *   Cronet extension: Default to using the Cronet implementation in Google Play
     Services rather than Cronet Embedded. This allows Cronet to be used with a
     negligible increase in application size, compared to approximately 8MB when

library/ui/src/main/java/com/google/android/exoplayer2/ui/SubtitlePainter.java

@@ -33,6 +33,7 @@ import android.text.TextPaint;
 import android.text.TextUtils;
 import android.text.style.AbsoluteSizeSpan;
 import android.text.style.BackgroundColorSpan;
+import android.text.style.ForegroundColorSpan;
 import android.text.style.RelativeSizeSpan;
 import android.util.DisplayMetrics;
 import androidx.annotation.Nullable;
@@ -99,6 +100,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
 
   // Derived drawing variables.
   private @MonotonicNonNull StaticLayout textLayout;
+  private @MonotonicNonNull StaticLayout edgeLayout;
   private int textLeft;
   private int textTop;
   private int textPaddingX;
@@ -291,11 +293,38 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
       }
     }
 
+    // Remove embedded font color to not destroy edges, otherwise it overrides edge color.
+    SpannableStringBuilder cueTextEdge = new SpannableStringBuilder(cueText);
+    if (edgeType == CaptionStyleCompat.EDGE_TYPE_OUTLINE) {
+      int cueLength = cueTextEdge.length();
+      ForegroundColorSpan[] foregroundColorSpans =
+          cueTextEdge.getSpans(0, cueLength, ForegroundColorSpan.class);
+      for (ForegroundColorSpan foregroundColorSpan : foregroundColorSpans) {
+        cueTextEdge.removeSpan(foregroundColorSpan);
+      }
+    }
+
+    // EDGE_TYPE_NONE & EDGE_TYPE_DROP_SHADOW both paint in one pass, they ignore cueTextEdge.
+    // In other cases we use two painters and we need to apply the background in the first one only,
+    // otherwise the background color gets drawn in front of the edge color
+    // (https://github.com/google/ExoPlayer/pull/6724#issuecomment-564650572).
     if (Color.alpha(backgroundColor) > 0) {
-      SpannableStringBuilder newCueText = new SpannableStringBuilder(cueText);
+      if (edgeType == CaptionStyleCompat.EDGE_TYPE_NONE
-      newCueText.setSpan(
+          || edgeType == CaptionStyleCompat.EDGE_TYPE_DROP_SHADOW) {
-          new BackgroundColorSpan(backgroundColor), 0, newCueText.length(), Spanned.SPAN_PRIORITY);
+        SpannableStringBuilder newCueText = new SpannableStringBuilder(cueText);
-      cueText = newCueText;
+        newCueText.setSpan(
+            new BackgroundColorSpan(backgroundColor),
+            0,
+            newCueText.length(),
+            Spanned.SPAN_PRIORITY);
+        cueText = newCueText;
+      } else {
+        cueTextEdge.setSpan(
+            new BackgroundColorSpan(backgroundColor),
+            0,
+            cueTextEdge.length(),
+            Spanned.SPAN_PRIORITY);
+      }
     }
 
     Alignment textAlignment = cueTextAlignment == null ? Alignment.ALIGN_CENTER : cueTextAlignment;
@@ -371,6 +400,9 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
     // Update the derived drawing variables.
     this.textLayout = new StaticLayout(cueText, textPaint, textWidth, textAlignment, spacingMult,
         spacingAdd, true);
+    this.edgeLayout =
+        new StaticLayout(
+            cueTextEdge, textPaint, textWidth, textAlignment, spacingMult, spacingAdd, true);
     this.textLeft = textLeft;
     this.textTop = textTop;
     this.textPaddingX = textPaddingX;
@@ -410,8 +442,9 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
   }
 
   private void drawTextLayout(Canvas canvas) {
-    StaticLayout layout = textLayout;
+    StaticLayout textLayout = this.textLayout;
-    if (layout == null) {
+    StaticLayout edgeLayout = this.edgeLayout;
+    if (textLayout == null || edgeLayout == null) {
       // Nothing to draw.
       return;
     }
@@ -434,7 +467,7 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
       textPaint.setStrokeWidth(outlineWidth);
       textPaint.setColor(edgeColor);
       textPaint.setStyle(Style.FILL_AND_STROKE);
-      layout.draw(canvas);
+      edgeLayout.draw(canvas);
     } else if (edgeType == CaptionStyleCompat.EDGE_TYPE_DROP_SHADOW) {
       textPaint.setShadowLayer(shadowRadius, shadowOffset, shadowOffset, edgeColor);
     } else if (edgeType == CaptionStyleCompat.EDGE_TYPE_RAISED
@@ -446,13 +479,13 @@ import org.checkerframework.checker.nullness.qual.RequiresNonNull;
       textPaint.setColor(foregroundColor);
       textPaint.setStyle(Style.FILL);
       textPaint.setShadowLayer(shadowRadius, -offset, -offset, colorUp);
-      layout.draw(canvas);
+      edgeLayout.draw(canvas);
       textPaint.setShadowLayer(shadowRadius, offset, offset, colorDown);
     }
 
     textPaint.setColor(foregroundColor);
     textPaint.setStyle(Style.FILL);
-    layout.draw(canvas);
+    textLayout.draw(canvas);
     textPaint.setShadowLayer(0, 0, 0, 0);
 
     canvas.restoreToCount(saveCount);