[go: nahoru, domu]

Fix bug causing multiple underlines to be applied in certain situations.

In certain situations (e.g. when mixing two different scripts in the
same Text element), two underlines could be renderered instead of one if
the element was set to underline. This has been fixed by using
UnderlineSpan instead of setting the "underline" bit on the TextPaint.

Bug: 195388378
Test: New screenshot tests added.
Relnote: "Fix bug causing multiple underlines to be applied in certain
situations."

Change-Id: Ib6712e68987be272f471e8c229840f57cd6f3845
diff --git a/wear/tiles/tiles-renderer/src/androidTest/java/androidx/wear/tiles/renderer/test/TileRendererGoldenTest.java b/wear/tiles/tiles-renderer/src/androidTest/java/androidx/wear/tiles/renderer/test/TileRendererGoldenTest.java
index 433bec7..e3c0260 100644
--- a/wear/tiles/tiles-renderer/src/androidTest/java/androidx/wear/tiles/renderer/test/TileRendererGoldenTest.java
+++ b/wear/tiles/tiles-renderer/src/androidTest/java/androidx/wear/tiles/renderer/test/TileRendererGoldenTest.java
@@ -93,6 +93,7 @@
                     {"line_in_arc"},
                     {"line_multi_height"},
                     {"long_text"},
+                    {"mixed_language_text"},
                     {"multi_line_text_alignment"},
                     {"row_column_space_test"},
                     {"row_with_alignment"},
diff --git a/wear/tiles/tiles-renderer/src/androidTest/res/raw/mixed_language_text.textproto b/wear/tiles/tiles-renderer/src/androidTest/res/raw/mixed_language_text.textproto
new file mode 100644
index 0000000..27891c3
--- /dev/null
+++ b/wear/tiles/tiles-renderer/src/androidTest/res/raw/mixed_language_text.textproto
@@ -0,0 +1,73 @@
+box {
+  modifiers {
+    background {
+      color {
+        argb: 0xFF000000
+      }
+    }
+  }
+  width {
+    expanded_dimension: {}
+  }
+  height {
+    expanded_dimension: {}
+  }
+  vertical_alignment {
+    value: VERTICAL_ALIGN_CENTER
+  }
+  horizontal_alignment {
+    value: HORIZONTAL_ALIGN_CENTER
+  }
+  contents {
+    column {
+      horizontal_alignment {
+        value: HORIZONTAL_ALIGN_LEFT
+      }
+      contents {
+        text {
+          text {
+            value: "Latin Only"
+          }
+          font_style {
+            size { value: 16 }
+            underline { value: true }
+          }
+        }
+      }
+      contents {
+        text {
+          text {
+            value: "Underline 안녕하세요"
+          }
+          font_style {
+            size { value: 16 }
+            underline { value: true }
+          }
+        }
+      }
+      contents {
+        text {
+          text {
+            value: "Italic 안녕하세요"
+          }
+          font_style {
+            size { value: 16 }
+            italic { value: true }
+          }
+        }
+      }
+      contents {
+        text {
+          text {
+            value: "Italic Underline 안녕하세요"
+          }
+          font_style {
+            size { value: 16 }
+            italic { value: true }
+            underline { value: true }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/wear/tiles/tiles-renderer/src/main/java/androidx/wear/tiles/renderer/internal/TileRendererInternal.java b/wear/tiles/tiles-renderer/src/main/java/androidx/wear/tiles/renderer/internal/TileRendererInternal.java
index 869a322..647c94a 100644
--- a/wear/tiles/tiles-renderer/src/main/java/androidx/wear/tiles/renderer/internal/TileRendererInternal.java
+++ b/wear/tiles/tiles-renderer/src/main/java/androidx/wear/tiles/renderer/internal/TileRendererInternal.java
@@ -539,22 +539,14 @@
     }
 
     private void applyFontStyle(FontStyle style, TextView textView) {
+        // Note: Underline must be applied as a Span to work correctly (as opposed to using
+        // TextPaint#setTextUnderline). This is applied in the caller instead.
+
         // Need to supply typefaceStyle when creating the typeface (will select specialist
         // bold/italic typefaces), *and* when setting the typeface (will set synthetic bold/italic
         // flags in Paint if they're not supported by the given typeface).
         textView.setTypeface(createTypeface(style), fontStyleToTypefaceStyle(style));
 
-        int currentPaintFlags = textView.getPaintFlags();
-
-        // Remove the bits we're setting
-        currentPaintFlags &= ~Paint.UNDERLINE_TEXT_FLAG;
-
-        if (style.hasUnderline() && style.getUnderline().getValue()) {
-            currentPaintFlags |= Paint.UNDERLINE_TEXT_FLAG;
-        }
-
-        textView.setPaintFlags(currentPaintFlags);
-
         if (style.hasSize()) {
             textView.setTextSize(TypedValue.COMPLEX_UNIT_SP, style.getSize().getValue());
         }
@@ -1026,7 +1018,18 @@
 
         LayoutParams layoutParams = generateDefaultLayoutParams();
 
-        textView.setText(text.getText().getValue());
+        // Underlines are applied using a Spannable here, rather than setting paint bits (or using
+        // Paint#setTextUnderline). When multiple fonts are mixed on the same line (especially when
+        // mixing anything with NotoSans-CJK), multiple underlines can appear. Using UnderlineSpan
+        // instead though causes the correct behaviour to happen (only a single underline).
+        SpannableStringBuilder ssb = new SpannableStringBuilder();
+        ssb.append(text.getText().getValue());
+
+        if (text.getFontStyle().getUnderline().getValue()) {
+            ssb.setSpan(new UnderlineSpan(), 0, ssb.length(), Spanned.SPAN_MARK_MARK);
+        }
+
+        textView.setText(ssb);
 
         textView.setEllipsize(textTruncationToEllipsize(text.getOverflow()));
         textView.setGravity(textAlignToAndroidGravity(text.getMultilineAlignment()));