Skip to content

SDK / exoplayer

Go to a project
Commit ad115a5a by ibaker Committed by Oliver Woodman
Options

Fix some boolean logic in TtmlStyle#inherit 

I got confused copying the hasBackgroundColor logic in
https://github.com/google/ExoPlayer/commit/3aa52c231720eaed88cdf27eff0f97d4bcf7625f

Add tests to confirm I got it right this time

PiperOrigin-RevId: 292898421
parent 49fa6d63
Inline Side-by-side
Showing with 119 additions and 44 deletions
library/core/src/main/java/com/google/android/exoplayer2/text/ttml/TtmlStyle.java
...@@ -24,7 +24,6 @@ import com.google.android.exoplayer2.text.Cue.VerticalType; ...@@ -24,7 +24,6 @@ import com.google.android.exoplayer2.text.Cue.VerticalType;
import java.lang.annotation.Documented; import java.lang.annotation.Documented;
import java.lang.annotation.Retention; import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy; import java.lang.annotation.RetentionPolicy;
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
/** /**
* Style object of a <code>TtmlNode</code> * Style object of a <code>TtmlNode</code>
...@@ -62,7 +61,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ...@@ -62,7 +61,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
private static final int OFF = 0; private static final int OFF = 0;
private static final int ON = 1; private static final int ON = 1;
private @MonotonicNonNull String fontFamily; @Nullable private String fontFamily;
private int fontColor; private int fontColor;
private boolean hasFontColor; private boolean hasFontColor;
private int backgroundColor; private int backgroundColor;
...@@ -73,8 +72,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ...@@ -73,8 +72,8 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
@OptionalBoolean private int italic; @OptionalBoolean private int italic;
@FontSizeUnit private int fontSizeUnit; @FontSizeUnit private int fontSizeUnit;
private float fontSize; private float fontSize;
private @MonotonicNonNull String id; @Nullable private String id;
private Layout.@MonotonicNonNull Alignment textAlign; @Nullable private Layout.Alignment textAlign;
@OptionalBoolean private int textCombine; @OptionalBoolean private int textCombine;
@Cue.VerticalType private int verticalType; @Cue.VerticalType private int verticalType;
...@@ -84,6 +83,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ...@@ -84,6 +83,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
bold = UNSPECIFIED; bold = UNSPECIFIED;
italic = UNSPECIFIED; italic = UNSPECIFIED;
fontSizeUnit = UNSPECIFIED; fontSizeUnit = UNSPECIFIED;
textCombine = UNSPECIFIED;
verticalType = Cue.TYPE_UNSET; verticalType = Cue.TYPE_UNSET;
} }
...@@ -134,7 +134,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ...@@ -134,7 +134,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
return fontFamily; return fontFamily;
} }
public TtmlStyle setFontFamily(String fontFamily) { public TtmlStyle setFontFamily(@Nullable String fontFamily) {
this.fontFamily = fontFamily; this.fontFamily = fontFamily;
return this; return this;
} }
...@@ -228,14 +228,14 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ...@@ -228,14 +228,14 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
if (chaining && !hasBackgroundColor && ancestor.hasBackgroundColor) { if (chaining && !hasBackgroundColor && ancestor.hasBackgroundColor) {
setBackgroundColor(ancestor.backgroundColor); setBackgroundColor(ancestor.backgroundColor);
} }
if (chaining && verticalType != Cue.TYPE_UNSET && ancestor.verticalType == Cue.TYPE_UNSET) { if (chaining && verticalType == Cue.TYPE_UNSET) {
setVerticalType(ancestor.verticalType); verticalType = ancestor.verticalType;
} }
} }
return this; return this;
} }
public TtmlStyle setId(String id) { public TtmlStyle setId(@Nullable String id) {
this.id = id; this.id = id;
return this; return this;
} }
...@@ -250,7 +250,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull; ...@@ -250,7 +250,7 @@ import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
return textAlign; return textAlign;
} }
public TtmlStyle setTextAlign(Layout.Alignment textAlign) { public TtmlStyle setTextAlign(@Nullable Layout.Alignment textAlign) {
this.textAlign = textAlign; this.textAlign = textAlign;
return this; return this;
} }
......
library/core/src/test/java/com/google/android/exoplayer2/text/ttml/TtmlStyleTest.java
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
package com.google.android.exoplayer2.text.ttml; package com.google.android.exoplayer2.text.ttml;
import static android.graphics.Color.BLACK; import static android.graphics.Color.BLACK;
import static android.graphics.Color.WHITE;
import static com.google.android.exoplayer2.text.ttml.TtmlStyle.STYLE_BOLD; import static com.google.android.exoplayer2.text.ttml.TtmlStyle.STYLE_BOLD;
import static com.google.android.exoplayer2.text.ttml.TtmlStyle.STYLE_BOLD_ITALIC; import static com.google.android.exoplayer2.text.ttml.TtmlStyle.STYLE_BOLD_ITALIC;
import static com.google.android.exoplayer2.text.ttml.TtmlStyle.STYLE_ITALIC; import static com.google.android.exoplayer2.text.ttml.TtmlStyle.STYLE_ITALIC;
...@@ -26,8 +25,10 @@ import static com.google.common.truth.Truth.assertThat; ...@@ -26,8 +25,10 @@ import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage; import static com.google.common.truth.Truth.assertWithMessage;
import android.graphics.Color; import android.graphics.Color;
import android.text.Layout;
import androidx.annotation.ColorInt;
import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.ext.junit.runners.AndroidJUnit4;
import org.junit.Before; import com.google.android.exoplayer2.text.Cue;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
...@@ -35,58 +36,83 @@ import org.junit.runner.RunWith; ...@@ -35,58 +36,83 @@ import org.junit.runner.RunWith;
@RunWith(AndroidJUnit4.class) @RunWith(AndroidJUnit4.class)
public final class TtmlStyleTest { public final class TtmlStyleTest {
private static final String FONT_FAMILY = "serif";
private static final String ID = "id"; private static final String ID = "id";
public static final int FOREGROUND_COLOR = Color.WHITE; private static final String FONT_FAMILY = "serif";
public static final int BACKGROUND_COLOR = Color.BLACK; @ColorInt private static final int FONT_COLOR = Color.WHITE;
private TtmlStyle style; private static final float FONT_SIZE = 12.5f;
@TtmlStyle.FontSizeUnit private static final int FONT_SIZE_UNIT = TtmlStyle.FONT_SIZE_UNIT_EM;
@Before @ColorInt private static final int BACKGROUND_COLOR = Color.BLACK;
public void setUp() throws Exception { private static final Layout.Alignment TEXT_ALIGN = Layout.Alignment.ALIGN_CENTER;
style = new TtmlStyle(); private static final boolean TEXT_COMBINE = true;
} @Cue.VerticalType private static final int VERTICAL_TYPE = Cue.VERTICAL_TYPE_RL;
private final TtmlStyle populatedStyle =
new TtmlStyle()
.setId(ID)
.setItalic(true)
.setBold(true)
.setBackgroundColor(BACKGROUND_COLOR)
.setFontColor(FONT_COLOR)
.setLinethrough(true)
.setUnderline(true)
.setFontFamily(FONT_FAMILY)
.setFontSize(FONT_SIZE)
.setFontSizeUnit(FONT_SIZE_UNIT)
.setTextAlign(TEXT_ALIGN)
.setTextCombine(TEXT_COMBINE)
.setVerticalType(VERTICAL_TYPE);
@Test @Test
public void testInheritStyle() { public void testInheritStyle() {
style.inherit(createAncestorStyle()); TtmlStyle style = new TtmlStyle();
style.inherit(populatedStyle);
assertWithMessage("id must not be inherited").that(style.getId()).isNull(); assertWithMessage("id must not be inherited").that(style.getId()).isNull();
assertThat(style.isUnderline()).isTrue(); assertThat(style.isUnderline()).isTrue();
assertThat(style.isLinethrough()).isTrue(); assertThat(style.isLinethrough()).isTrue();
assertThat(style.getStyle()).isEqualTo(STYLE_BOLD_ITALIC); assertThat(style.getStyle()).isEqualTo(STYLE_BOLD_ITALIC);
assertThat(style.getFontFamily()).isEqualTo(FONT_FAMILY); assertThat(style.getFontFamily()).isEqualTo(FONT_FAMILY);
assertThat(style.getFontColor()).isEqualTo(WHITE); assertThat(style.getFontColor()).isEqualTo(FONT_COLOR);
assertWithMessage("do not inherit backgroundColor").that(style.hasBackgroundColor()).isFalse(); assertThat(style.getFontSize()).isEqualTo(FONT_SIZE);
assertThat(style.getFontSizeUnit()).isEqualTo(FONT_SIZE_UNIT);
assertThat(style.getTextAlign()).isEqualTo(TEXT_ALIGN);
assertThat(style.getTextCombine()).isEqualTo(TEXT_COMBINE);
assertWithMessage("backgroundColor should not be inherited")
.that(style.hasBackgroundColor())
.isFalse();
assertWithMessage("verticalType should not be inherited")
.that(style.getVerticalType())
.isEqualTo(Cue.TYPE_UNSET);
} }
@Test @Test
public void testChainStyle() { public void testChainStyle() {
style.chain(createAncestorStyle()); TtmlStyle style = new TtmlStyle();
style.chain(populatedStyle);
assertWithMessage("id must not be inherited").that(style.getId()).isNull(); assertWithMessage("id must not be inherited").that(style.getId()).isNull();
assertThat(style.isUnderline()).isTrue(); assertThat(style.isUnderline()).isTrue();
assertThat(style.isLinethrough()).isTrue(); assertThat(style.isLinethrough()).isTrue();
assertThat(style.getStyle()).isEqualTo(STYLE_BOLD_ITALIC); assertThat(style.getStyle()).isEqualTo(STYLE_BOLD_ITALIC);
assertThat(style.getFontFamily()).isEqualTo(FONT_FAMILY); assertThat(style.getFontFamily()).isEqualTo(FONT_FAMILY);
assertThat(style.getFontColor()).isEqualTo(FOREGROUND_COLOR); assertThat(style.getFontColor()).isEqualTo(FONT_COLOR);
// do inherit backgroundColor when chaining assertThat(style.getFontSize()).isEqualTo(FONT_SIZE);
assertWithMessage("do not inherit backgroundColor when chaining") assertThat(style.getFontSizeUnit()).isEqualTo(FONT_SIZE_UNIT);
.that(style.getBackgroundColor()).isEqualTo(BACKGROUND_COLOR); assertThat(style.getTextAlign()).isEqualTo(TEXT_ALIGN);
} assertThat(style.getTextCombine()).isEqualTo(TEXT_COMBINE);
assertWithMessage("backgroundColor should be chained")
private static TtmlStyle createAncestorStyle() { .that(style.getBackgroundColor())
TtmlStyle ancestor = new TtmlStyle(); .isEqualTo(BACKGROUND_COLOR);
ancestor.setId(ID); assertWithMessage("verticalType should be chained")
ancestor.setItalic(true); .that(style.getVerticalType())
ancestor.setBold(true); .isEqualTo(VERTICAL_TYPE);
ancestor.setBackgroundColor(BACKGROUND_COLOR);
ancestor.setFontColor(FOREGROUND_COLOR);
ancestor.setLinethrough(true);
ancestor.setUnderline(true);
ancestor.setFontFamily(FONT_FAMILY);
return ancestor;
} }
@Test @Test
public void testStyle() { public void testStyle() {
TtmlStyle style = new TtmlStyle();
assertThat(style.getStyle()).isEqualTo(UNSPECIFIED); assertThat(style.getStyle()).isEqualTo(UNSPECIFIED);
style.setItalic(true); style.setItalic(true);
assertThat(style.getStyle()).isEqualTo(STYLE_ITALIC); assertThat(style.getStyle()).isEqualTo(STYLE_ITALIC);
...@@ -100,6 +126,8 @@ public final class TtmlStyleTest { ...@@ -100,6 +126,8 @@ public final class TtmlStyleTest {
@Test @Test
public void testLinethrough() { public void testLinethrough() {
TtmlStyle style = new TtmlStyle();
assertThat(style.isLinethrough()).isFalse(); assertThat(style.isLinethrough()).isFalse();
style.setLinethrough(true); style.setLinethrough(true);
assertThat(style.isLinethrough()).isTrue(); assertThat(style.isLinethrough()).isTrue();
...@@ -109,6 +137,8 @@ public final class TtmlStyleTest { ...@@ -109,6 +137,8 @@ public final class TtmlStyleTest {
@Test @Test
public void testUnderline() { public void testUnderline() {
TtmlStyle style = new TtmlStyle();
assertThat(style.isUnderline()).isFalse(); assertThat(style.isUnderline()).isFalse();
style.setUnderline(true); style.setUnderline(true);
assertThat(style.isUnderline()).isTrue(); assertThat(style.isUnderline()).isTrue();
...@@ -118,6 +148,8 @@ public final class TtmlStyleTest { ...@@ -118,6 +148,8 @@ public final class TtmlStyleTest {
@Test @Test
public void testFontFamily() { public void testFontFamily() {
TtmlStyle style = new TtmlStyle();
assertThat(style.getFontFamily()).isNull(); assertThat(style.getFontFamily()).isNull();
style.setFontFamily(FONT_FAMILY); style.setFontFamily(FONT_FAMILY);
assertThat(style.getFontFamily()).isEqualTo(FONT_FAMILY); assertThat(style.getFontFamily()).isEqualTo(FONT_FAMILY);
...@@ -126,23 +158,47 @@ public final class TtmlStyleTest { ...@@ -126,23 +158,47 @@ public final class TtmlStyleTest {
} }
@Test @Test
public void testColor() { public void testFontColor() {
TtmlStyle style = new TtmlStyle();
assertThat(style.hasFontColor()).isFalse(); assertThat(style.hasFontColor()).isFalse();
style.setFontColor(Color.BLACK); style.setFontColor(Color.BLACK);
assertThat(style.getFontColor()).isEqualTo(BLACK);
assertThat(style.hasFontColor()).isTrue(); assertThat(style.hasFontColor()).isTrue();
assertThat(style.getFontColor()).isEqualTo(BLACK);
}
@Test
public void testFontSize() {
TtmlStyle style = new TtmlStyle();
assertThat(style.getFontSize()).isEqualTo(0);
style.setFontSize(10.5f);
assertThat(style.getFontSize()).isEqualTo(10.5f);
}
@Test
public void testFontSizeUnit() {
TtmlStyle style = new TtmlStyle();
assertThat(style.getFontSizeUnit()).isEqualTo(UNSPECIFIED);
style.setFontSizeUnit(TtmlStyle.FONT_SIZE_UNIT_EM);
assertThat(style.getFontSizeUnit()).isEqualTo(TtmlStyle.FONT_SIZE_UNIT_EM);
} }
@Test @Test
public void testBackgroundColor() { public void testBackgroundColor() {
TtmlStyle style = new TtmlStyle();
assertThat(style.hasBackgroundColor()).isFalse(); assertThat(style.hasBackgroundColor()).isFalse();
style.setBackgroundColor(Color.BLACK); style.setBackgroundColor(Color.BLACK);
assertThat(style.getBackgroundColor()).isEqualTo(BLACK);
assertThat(style.hasBackgroundColor()).isTrue(); assertThat(style.hasBackgroundColor()).isTrue();
assertThat(style.getBackgroundColor()).isEqualTo(BLACK);
} }
@Test @Test
public void testId() { public void testId() {
TtmlStyle style = new TtmlStyle();
assertThat(style.getId()).isNull(); assertThat(style.getId()).isNull();
style.setId(ID); style.setId(ID);
assertThat(style.getId()).isEqualTo(ID); assertThat(style.getId()).isEqualTo(ID);
...@@ -150,4 +206,23 @@ public final class TtmlStyleTest { ...@@ -150,4 +206,23 @@ public final class TtmlStyleTest {
assertThat(style.getId()).isNull(); assertThat(style.getId()).isNull();
} }
@Test
public void testTextAlign() {
TtmlStyle style = new TtmlStyle();
assertThat(style.getTextAlign()).isNull();
style.setTextAlign(Layout.Alignment.ALIGN_OPPOSITE);
assertThat(style.getTextAlign()).isEqualTo(Layout.Alignment.ALIGN_OPPOSITE);
style.setTextAlign(null);
assertThat(style.getTextAlign()).isNull();
}
@Test
public void testTextCombine() {
TtmlStyle style = new TtmlStyle();
assertThat(style.getTextCombine()).isFalse();
style.setTextCombine(true);
assertThat(style.getTextCombine()).isTrue();
}
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment