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