Draw a single underline if multiple fonts are used in a single line.

Bug: 297336724
Test: atest hwui_unit_tests (with flag on/off)
Change-Id: I48018abd68c767c9c7ad08722a8974efc1bfe67c
diff --git a/libs/hwui/Android.bp b/libs/hwui/Android.bp
index e1dd145..ff1eedb 100644
--- a/libs/hwui/Android.bp
+++ b/libs/hwui/Android.bp
@@ -143,6 +143,7 @@
                 "libcrypto",
                 "libsync",
                 "libui",
+                "aconfig_text_flags_c_lib",
             ],
             static_libs: [
                 "libEGL_blobCache",
@@ -712,11 +713,13 @@
     ],
 
     static_libs: [
+        "libflagtest",
         "libgmock",
         "libhwui_static",
     ],
     shared_libs: [
         "libmemunreachable",
+        "server_configurable_flags",
     ],
     srcs: [
         "tests/unit/main.cpp",
@@ -756,6 +759,7 @@
         "tests/unit/TestUtilsTests.cpp",
         "tests/unit/ThreadBaseTests.cpp",
         "tests/unit/TypefaceTests.cpp",
+        "tests/unit/UnderlineTest.cpp",
         "tests/unit/VectorDrawableTests.cpp",
         "tests/unit/WebViewFunctorManagerTests.cpp",
     ],
diff --git a/libs/hwui/FeatureFlags.h b/libs/hwui/FeatureFlags.h
new file mode 100644
index 0000000..ffb329d
--- /dev/null
+++ b/libs/hwui/FeatureFlags.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ANDROID_HWUI_FEATURE_FLAGS_H
+#define ANDROID_HWUI_FEATURE_FLAGS_H
+
+#ifdef __ANDROID__
+#include <com_android_text_flags.h>
+#endif  // __ANDROID__
+
+namespace android {
+
+namespace text_feature {
+
+inline bool fix_double_underline() {
+#ifdef __ANDROID__
+    return com_android_text_flags_fix_double_underline();
+#else
+    return true;
+#endif  // __ANDROID__
+}
+
+}  // namespace text_feature
+
+}  // namespace android
+
+#endif  // ANDROID_HWUI_FEATURE_FLAGS_H
diff --git a/libs/hwui/SkiaCanvas.cpp b/libs/hwui/SkiaCanvas.cpp
index 8394c3c..31fc929 100644
--- a/libs/hwui/SkiaCanvas.cpp
+++ b/libs/hwui/SkiaCanvas.cpp
@@ -47,6 +47,7 @@
 #include <utility>
 
 #include "CanvasProperty.h"
+#include "FeatureFlags.h"
 #include "Mesh.h"
 #include "NinePatchUtils.h"
 #include "VectorDrawable.h"
@@ -795,7 +796,9 @@
     sk_sp<SkTextBlob> textBlob(builder.make());
 
     applyLooper(&paintCopy, [&](const SkPaint& p) { mCanvas->drawTextBlob(textBlob, 0, 0, p); });
-    drawTextDecorations(x, y, totalAdvance, paintCopy);
+    if (!text_feature::fix_double_underline()) {
+        drawTextDecorations(x, y, totalAdvance, paintCopy);
+    }
 }
 
 void SkiaCanvas::drawLayoutOnPath(const minikin::Layout& layout, float hOffset, float vOffset,
diff --git a/libs/hwui/hwui/Canvas.cpp b/libs/hwui/hwui/Canvas.cpp
index 2351797..80b6c03 100644
--- a/libs/hwui/hwui/Canvas.cpp
+++ b/libs/hwui/hwui/Canvas.cpp
@@ -16,17 +16,18 @@
 
 #include "Canvas.h"
 
+#include <SkFontMetrics.h>
+#include <SkRRect.h>
+
+#include "FeatureFlags.h"
 #include "MinikinUtils.h"
 #include "Paint.h"
 #include "Properties.h"
 #include "RenderNode.h"
 #include "Typeface.h"
-#include "pipeline/skia/SkiaRecordingCanvas.h"
-
+#include "hwui/DrawTextFunctor.h"
 #include "hwui/PaintFilter.h"
-
-#include <SkFontMetrics.h>
-#include <SkRRect.h>
+#include "pipeline/skia/SkiaRecordingCanvas.h"
 
 namespace android {
 
@@ -34,13 +35,6 @@
     return new uirenderer::skiapipeline::SkiaRecordingCanvas(renderNode, width, height);
 }
 
-static inline void drawStroke(SkScalar left, SkScalar right, SkScalar top, SkScalar thickness,
-                              const Paint& paint, Canvas* canvas) {
-    const SkScalar strokeWidth = fmax(thickness, 1.0f);
-    const SkScalar bottom = top + strokeWidth;
-    canvas->drawRect(left, top, right, bottom, paint);
-}
-
 void Canvas::drawTextDecorations(float x, float y, float length, const Paint& paint) {
     // paint has already been filtered by our caller, so we can ignore any filter
     const bool strikeThru = paint.isStrikeThru();
@@ -72,73 +66,6 @@
     }
 }
 
-static void simplifyPaint(int color, Paint* paint) {
-    paint->setColor(color);
-    paint->setShader(nullptr);
-    paint->setColorFilter(nullptr);
-    paint->setLooper(nullptr);
-    paint->setStrokeWidth(4 + 0.04 * paint->getSkFont().getSize());
-    paint->setStrokeJoin(SkPaint::kRound_Join);
-    paint->setLooper(nullptr);
-}
-
-class DrawTextFunctor {
-public:
-    DrawTextFunctor(const minikin::Layout& layout, Canvas* canvas, const Paint& paint, float x,
-                    float y, float totalAdvance)
-            : layout(layout)
-            , canvas(canvas)
-            , paint(paint)
-            , x(x)
-            , y(y)
-            , totalAdvance(totalAdvance) {}
-
-    void operator()(size_t start, size_t end) {
-        auto glyphFunc = [&](uint16_t* text, float* positions) {
-            for (size_t i = start, textIndex = 0, posIndex = 0; i < end; i++) {
-                text[textIndex++] = layout.getGlyphId(i);
-                positions[posIndex++] = x + layout.getX(i);
-                positions[posIndex++] = y + layout.getY(i);
-            }
-        };
-
-        size_t glyphCount = end - start;
-
-        if (CC_UNLIKELY(canvas->isHighContrastText() && paint.getAlpha() != 0)) {
-            // high contrast draw path
-            int color = paint.getColor();
-            int channelSum = SkColorGetR(color) + SkColorGetG(color) + SkColorGetB(color);
-            bool darken = channelSum < (128 * 3);
-
-            // outline
-            gDrawTextBlobMode = DrawTextBlobMode::HctOutline;
-            Paint outlinePaint(paint);
-            simplifyPaint(darken ? SK_ColorWHITE : SK_ColorBLACK, &outlinePaint);
-            outlinePaint.setStyle(SkPaint::kStrokeAndFill_Style);
-            canvas->drawGlyphs(glyphFunc, glyphCount, outlinePaint, x, y, totalAdvance);
-
-            // inner
-            gDrawTextBlobMode = DrawTextBlobMode::HctInner;
-            Paint innerPaint(paint);
-            simplifyPaint(darken ? SK_ColorBLACK : SK_ColorWHITE, &innerPaint);
-            innerPaint.setStyle(SkPaint::kFill_Style);
-            canvas->drawGlyphs(glyphFunc, glyphCount, innerPaint, x, y, totalAdvance);
-            gDrawTextBlobMode = DrawTextBlobMode::Normal;
-        } else {
-            // standard draw path
-            canvas->drawGlyphs(glyphFunc, glyphCount, paint, x, y, totalAdvance);
-        }
-    }
-
-private:
-    const minikin::Layout& layout;
-    Canvas* canvas;
-    const Paint& paint;
-    float x;
-    float y;
-    float totalAdvance;
-};
-
 void Canvas::drawGlyphs(const minikin::Font& font, const int* glyphIds, const float* positions,
                         int glyphCount, const Paint& paint) {
     // Minikin modify skFont for auto-fakebold/auto-fakeitalic.
@@ -182,6 +109,31 @@
 
     DrawTextFunctor f(layout, this, paint, x, y, layout.getAdvance());
     MinikinUtils::forFontRun(layout, &paint, f);
+
+    if (text_feature::fix_double_underline()) {
+        Paint copied(paint);
+        PaintFilter* filter = getPaintFilter();
+        if (filter != nullptr) {
+            filter->filterFullPaint(&copied);
+        }
+        const bool isUnderline = copied.isUnderline();
+        const bool isStrikeThru = copied.isStrikeThru();
+        if (isUnderline || isStrikeThru) {
+            const SkScalar left = x;
+            const SkScalar right = x + layout.getAdvance();
+            if (isUnderline) {
+                const SkScalar top = y + f.getUnderlinePosition();
+                drawStroke(left, right, top, f.getUnderlineThickness(), copied, this);
+            }
+            if (isStrikeThru) {
+                float textSize = paint.getSkFont().getSize();
+                const float position = textSize * Paint::kStdStrikeThru_Top;
+                const SkScalar thickness = textSize * Paint::kStdStrikeThru_Thickness;
+                const SkScalar top = y + position;
+                drawStroke(left, right, top, thickness, copied, this);
+            }
+        }
+    }
 }
 
 void Canvas::drawDoubleRoundRectXY(float outerLeft, float outerTop, float outerRight,
diff --git a/libs/hwui/hwui/Canvas.h b/libs/hwui/hwui/Canvas.h
index 44ee31d..9ec023b 100644
--- a/libs/hwui/hwui/Canvas.h
+++ b/libs/hwui/hwui/Canvas.h
@@ -285,7 +285,7 @@
      * totalAdvance: used to define width of text decorations (underlines, strikethroughs).
      */
     virtual void drawGlyphs(ReadGlyphFunc glyphFunc, int count, const Paint& paint, float x,
-                            float y,float totalAdvance) = 0;
+                            float y, float totalAdvance) = 0;
     virtual void drawLayoutOnPath(const minikin::Layout& layout, float hOffset, float vOffset,
                                   const Paint& paint, const SkPath& path, size_t start,
                                   size_t end) = 0;
diff --git a/libs/hwui/hwui/DrawTextFunctor.h b/libs/hwui/hwui/DrawTextFunctor.h
new file mode 100644
index 0000000..2e6e976
--- /dev/null
+++ b/libs/hwui/hwui/DrawTextFunctor.h
@@ -0,0 +1,141 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <SkFontMetrics.h>
+#include <SkRRect.h>
+
+#include "Canvas.h"
+#include "FeatureFlags.h"
+#include "MinikinUtils.h"
+#include "Paint.h"
+#include "Properties.h"
+#include "RenderNode.h"
+#include "Typeface.h"
+#include "hwui/PaintFilter.h"
+#include "pipeline/skia/SkiaRecordingCanvas.h"
+
+namespace android {
+
+static inline void drawStroke(SkScalar left, SkScalar right, SkScalar top, SkScalar thickness,
+                              const Paint& paint, Canvas* canvas) {
+    const SkScalar strokeWidth = fmax(thickness, 1.0f);
+    const SkScalar bottom = top + strokeWidth;
+    canvas->drawRect(left, top, right, bottom, paint);
+}
+
+static void simplifyPaint(int color, Paint* paint) {
+    paint->setColor(color);
+    paint->setShader(nullptr);
+    paint->setColorFilter(nullptr);
+    paint->setLooper(nullptr);
+    paint->setStrokeWidth(4 + 0.04 * paint->getSkFont().getSize());
+    paint->setStrokeJoin(SkPaint::kRound_Join);
+    paint->setLooper(nullptr);
+}
+
+class DrawTextFunctor {
+public:
+    DrawTextFunctor(const minikin::Layout& layout, Canvas* canvas, const Paint& paint, float x,
+                    float y, float totalAdvance)
+            : layout(layout)
+            , canvas(canvas)
+            , paint(paint)
+            , x(x)
+            , y(y)
+            , totalAdvance(totalAdvance)
+            , underlinePosition(0)
+            , underlineThickness(0) {}
+
+    void operator()(size_t start, size_t end) {
+        auto glyphFunc = [&](uint16_t* text, float* positions) {
+            for (size_t i = start, textIndex = 0, posIndex = 0; i < end; i++) {
+                text[textIndex++] = layout.getGlyphId(i);
+                positions[posIndex++] = x + layout.getX(i);
+                positions[posIndex++] = y + layout.getY(i);
+            }
+        };
+
+        size_t glyphCount = end - start;
+
+        if (CC_UNLIKELY(canvas->isHighContrastText() && paint.getAlpha() != 0)) {
+            // high contrast draw path
+            int color = paint.getColor();
+            int channelSum = SkColorGetR(color) + SkColorGetG(color) + SkColorGetB(color);
+            bool darken = channelSum < (128 * 3);
+
+            // outline
+            gDrawTextBlobMode = DrawTextBlobMode::HctOutline;
+            Paint outlinePaint(paint);
+            simplifyPaint(darken ? SK_ColorWHITE : SK_ColorBLACK, &outlinePaint);
+            outlinePaint.setStyle(SkPaint::kStrokeAndFill_Style);
+            canvas->drawGlyphs(glyphFunc, glyphCount, outlinePaint, x, y, totalAdvance);
+
+            // inner
+            gDrawTextBlobMode = DrawTextBlobMode::HctInner;
+            Paint innerPaint(paint);
+            simplifyPaint(darken ? SK_ColorBLACK : SK_ColorWHITE, &innerPaint);
+            innerPaint.setStyle(SkPaint::kFill_Style);
+            canvas->drawGlyphs(glyphFunc, glyphCount, innerPaint, x, y, totalAdvance);
+            gDrawTextBlobMode = DrawTextBlobMode::Normal;
+        } else {
+            // standard draw path
+            canvas->drawGlyphs(glyphFunc, glyphCount, paint, x, y, totalAdvance);
+        }
+
+        if (text_feature::fix_double_underline()) {
+            // Extract underline position and thickness.
+            if (paint.isUnderline()) {
+                SkFontMetrics metrics;
+                paint.getSkFont().getMetrics(&metrics);
+                const float textSize = paint.getSkFont().getSize();
+                SkScalar position;
+                if (!metrics.hasUnderlinePosition(&position)) {
+                    position = textSize * Paint::kStdUnderline_Top;
+                }
+                SkScalar thickness;
+                if (!metrics.hasUnderlineThickness(&thickness)) {
+                    thickness = textSize * Paint::kStdUnderline_Thickness;
+                }
+
+                // If multiple fonts are used, use the most bottom position and most thick stroke
+                // width as the underline position. This follows the CSS standard:
+                // https://www.w3.org/TR/css-text-decor-3/#text-underline-position-property
+                // <quote>
+                // The exact position and thickness of line decorations is UA-defined in this level.
+                // However, for underlines and overlines the UA must use a single thickness and
+                // position on each line for the decorations deriving from a single decorating box.
+                // </quote>
+                underlinePosition = std::max(underlinePosition, position);
+                underlineThickness = std::max(underlineThickness, thickness);
+            }
+        }
+    }
+
+    float getUnderlinePosition() const { return underlinePosition; }
+    float getUnderlineThickness() const { return underlineThickness; }
+
+private:
+    const minikin::Layout& layout;
+    Canvas* canvas;
+    const Paint& paint;
+    float x;
+    float y;
+    float totalAdvance;
+    float underlinePosition;
+    float underlineThickness;
+};
+
+}  // namespace android
diff --git a/libs/hwui/tests/unit/UnderlineTest.cpp b/libs/hwui/tests/unit/UnderlineTest.cpp
new file mode 100644
index 0000000..db2be20
--- /dev/null
+++ b/libs/hwui/tests/unit/UnderlineTest.cpp
@@ -0,0 +1,153 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <fcntl.h>
+#include <flag_macros.h>
+#include <gtest/gtest.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <utils/Log.h>
+
+#include "SkAlphaType.h"
+#include "SkBitmap.h"
+#include "SkData.h"
+#include "SkFontMgr.h"
+#include "SkImageInfo.h"
+#include "SkRefCnt.h"
+#include "SkStream.h"
+#include "SkTypeface.h"
+#include "SkiaCanvas.h"
+#include "hwui/Bitmap.h"
+#include "hwui/DrawTextFunctor.h"
+#include "hwui/MinikinSkia.h"
+#include "hwui/MinikinUtils.h"
+#include "hwui/Paint.h"
+#include "hwui/Typeface.h"
+
+using namespace android;
+
+namespace {
+
+constexpr char kRobotoVariable[] = "/system/fonts/Roboto-Regular.ttf";
+constexpr char kJPFont[] = "/system/fonts/NotoSansCJK-Regular.ttc";
+
+// The underline position and thickness are cames from post table.
+constexpr float ROBOTO_POSITION_EM = 150.0 / 2048.0;
+constexpr float ROBOTO_THICKNESS_EM = 100.0 / 2048.0;
+constexpr float NOTO_CJK_POSITION_EM = 125.0 / 1000.0;
+constexpr float NOTO_CJK_THICKNESS_EM = 50.0 / 1000.0;
+
+void unmap(const void* ptr, void* context) {
+    void* p = const_cast<void*>(ptr);
+    size_t len = reinterpret_cast<size_t>(context);
+    munmap(p, len);
+}
+
+// Create a font family from a single font file.
+std::shared_ptr<minikin::FontFamily> buildFamily(const char* fileName) {
+    int fd = open(fileName, O_RDONLY);
+    LOG_ALWAYS_FATAL_IF(fd == -1, "Failed to open file %s", fileName);
+    struct stat st = {};
+    LOG_ALWAYS_FATAL_IF(fstat(fd, &st) == -1, "Failed to stat file %s", fileName);
+    void* data = mmap(nullptr, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
+    sk_sp<SkData> skData =
+            SkData::MakeWithProc(data, st.st_size, unmap, reinterpret_cast<void*>(st.st_size));
+    std::unique_ptr<SkStreamAsset> fontData(new SkMemoryStream(skData));
+    sk_sp<SkFontMgr> fm(SkFontMgr::RefDefault());
+    sk_sp<SkTypeface> typeface(fm->makeFromStream(std::move(fontData)));
+    LOG_ALWAYS_FATAL_IF(typeface == nullptr, "Failed to make typeface from %s", fileName);
+    std::shared_ptr<minikin::MinikinFont> font =
+            std::make_shared<MinikinFontSkia>(std::move(typeface), 0, data, st.st_size, fileName, 0,
+                                              std::vector<minikin::FontVariation>());
+    std::vector<std::shared_ptr<minikin::Font>> fonts;
+    fonts.push_back(minikin::Font::Builder(font).build());
+    return minikin::FontFamily::create(std::move(fonts));
+}
+
+// Create a typeface from roboto and NotoCJK.
+Typeface* makeTypeface() {
+    return Typeface::createFromFamilies(
+            std::vector<std::shared_ptr<minikin::FontFamily>>(
+                    {buildFamily(kRobotoVariable), buildFamily(kJPFont)}),
+            RESOLVE_BY_FONT_TABLE, RESOLVE_BY_FONT_TABLE, nullptr /* fallback */);
+}
+
+// Execute a text layout.
+minikin::Layout doLayout(const std::vector<uint16_t> text, Paint paint, Typeface* typeface) {
+    return MinikinUtils::doLayout(&paint, minikin::Bidi::LTR, typeface, text.data(), text.size(),
+                                  0 /* start */, text.size(), 0, text.size(), nullptr);
+}
+
+DrawTextFunctor processFunctor(const std::vector<uint16_t>& text, Paint* paint) {
+    // Create canvas
+    SkImageInfo info = SkImageInfo::Make(1, 1, kN32_SkColorType, kOpaque_SkAlphaType);
+    sk_sp<Bitmap> bitmap = Bitmap::allocateHeapBitmap(info);
+    SkBitmap skBitmap;
+    bitmap->getSkBitmap(&skBitmap);
+    SkiaCanvas canvas(skBitmap);
+
+    // Create minikin::Layout
+    std::unique_ptr<Typeface> typeface(makeTypeface());
+    minikin::Layout layout = doLayout(text, *paint, typeface.get());
+
+    DrawTextFunctor f(layout, &canvas, *paint, 0, 0, layout.getAdvance());
+    MinikinUtils::forFontRun(layout, paint, f);
+    return f;
+}
+
+TEST_WITH_FLAGS(UnderlineTest, Roboto,
+                REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::text::flags,
+                                                    fix_double_underline))) {
+    float textSize = 100;
+    Paint paint;
+    paint.getSkFont().setSize(textSize);
+    paint.setUnderline(true);
+    // the text is "abc"
+    DrawTextFunctor functor = processFunctor({0x0061, 0x0062, 0x0063}, &paint);
+
+    EXPECT_EQ(ROBOTO_POSITION_EM * textSize, functor.getUnderlinePosition());
+    EXPECT_EQ(ROBOTO_THICKNESS_EM * textSize, functor.getUnderlineThickness());
+}
+
+TEST_WITH_FLAGS(UnderlineTest, NotoCJK,
+                REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::text::flags,
+                                                    fix_double_underline))) {
+    float textSize = 100;
+    Paint paint;
+    paint.getSkFont().setSize(textSize);
+    paint.setUnderline(true);
+    // The text is 恂恄恆 in Japanese
+    DrawTextFunctor functor = processFunctor({0x3042, 0x3044, 0x3046}, &paint);
+
+    EXPECT_EQ(NOTO_CJK_POSITION_EM * textSize, functor.getUnderlinePosition());
+    EXPECT_EQ(NOTO_CJK_THICKNESS_EM * textSize, functor.getUnderlineThickness());
+}
+
+TEST_WITH_FLAGS(UnderlineTest, Mixture,
+                REQUIRES_FLAGS_ENABLED(ACONFIG_FLAG(com::android::text::flags,
+                                                    fix_double_underline))) {
+    float textSize = 100;
+    Paint paint;
+    paint.getSkFont().setSize(textSize);
+    paint.setUnderline(true);
+    // The text is a恄c. The only middle of the character is Japanese.
+    DrawTextFunctor functor = processFunctor({0x0061, 0x3044, 0x0063}, &paint);
+
+    // We use the bottom, thicker line as underline. Here, use Noto's one.
+    EXPECT_EQ(NOTO_CJK_POSITION_EM * textSize, functor.getUnderlinePosition());
+    EXPECT_EQ(NOTO_CJK_THICKNESS_EM * textSize, functor.getUnderlineThickness());
+}
+}  // namespace