fix(high contrast text): fix solid rectangle background obscuring other text in the same layout pass
This fixes right-aligned or center-aligned text drawing bounds all the
way to the end, even if there's no glyphs there. It also fixes problems
with emojis interspersed with normal text, since the emojis are drawn
in a separate pass with a separate layout, causing their background rect
to be drawn over any normal text drawn before.
The issue was the layout bounds encompasses more than just the glyphs:
it draws from the start of the text bounds all the way to the last
glyph, even if there is empty space between the text start and the first
glyph.
Reverts parts of Ib2ef3925acc89b01c71addb63194d2a403c99cb6, since we no
longer need a layout.
Fix: 322344265
Flag: com.android.graphics.hwui.flags.high_contrast_text_small_text_rect
Test: manual
1. adb shell setenforce 0 && adb shell setprop persist.device_config.aconfig_flags.accessibility.com.android.graphics.hwui.flags.high_contrast_text_small_text_rect true && adb shell stop && adb shell start
2. Settings -> Accessibility -> Display Size and Text
3. Turn on High Contrast Text
Change-Id: Ia4d74bbdc4cb98fc7f5d25d4b9e940d43aff2bb0
diff --git a/libs/hwui/hwui/Canvas.cpp b/libs/hwui/hwui/Canvas.cpp
index e9f4b81c..80b6c03 100644
--- a/libs/hwui/hwui/Canvas.cpp
+++ b/libs/hwui/hwui/Canvas.cpp
@@ -18,7 +18,6 @@
#include <SkFontMetrics.h>
#include <SkRRect.h>
-#include <minikin/MinikinRect.h>
#include "FeatureFlags.h"
#include "MinikinUtils.h"
@@ -108,13 +107,7 @@
// care of all alignment.
paint.setTextAlign(Paint::kLeft_Align);
- minikin::MinikinRect bounds;
- // We only need the bounds to draw a rectangular background in high contrast mode. Let's save
- // the cycles otherwise.
- if (flags::high_contrast_text_small_text_rect() && isHighContrastText()) {
- MinikinUtils::getBounds(&paint, bidiFlags, typeface, text, textSize, &bounds);
- }
- DrawTextFunctor f(layout, this, paint, x, y, layout.getAdvance(), bounds);
+ DrawTextFunctor f(layout, this, paint, x, y, layout.getAdvance());
MinikinUtils::forFontRun(layout, &paint, f);
if (text_feature::fix_double_underline()) {
diff --git a/libs/hwui/hwui/DrawTextFunctor.h b/libs/hwui/hwui/DrawTextFunctor.h
index ba65439..02bf0d8 100644
--- a/libs/hwui/hwui/DrawTextFunctor.h
+++ b/libs/hwui/hwui/DrawTextFunctor.h
@@ -16,6 +16,7 @@
#include <SkFontMetrics.h>
#include <SkRRect.h>
+#include <SkTextBlob.h>
#include <com_android_graphics_hwui_flags.h>
#include "../utils/Color.h"
@@ -66,7 +67,7 @@
* @param bounds bounds of the text. Only required if high contrast text mode is enabled.
*/
DrawTextFunctor(const minikin::Layout& layout, Canvas* canvas, const Paint& paint, float x,
- float y, float totalAdvance, const minikin::MinikinRect& bounds)
+ float y, float totalAdvance)
: layout(layout)
, canvas(canvas)
, paint(paint)
@@ -74,8 +75,7 @@
, y(y)
, totalAdvance(totalAdvance)
, underlinePosition(0)
- , underlineThickness(0)
- , bounds(bounds) {}
+ , underlineThickness(0) {}
void operator()(size_t start, size_t end) {
auto glyphFunc = [&](uint16_t* text, float* positions) {
@@ -106,12 +106,32 @@
simplifyPaint(darken ? SK_ColorWHITE : SK_ColorBLACK, &outlinePaint);
outlinePaint.setStyle(SkPaint::kStrokeAndFill_Style);
if (flags::high_contrast_text_small_text_rect()) {
- auto bgBounds(bounds);
- auto padding = kHighContrastTextBorderWidth + 0.1f * paint.getSkFont().getSize();
- bgBounds.offset(x, y);
- canvas->drawRect(bgBounds.mLeft - padding, bgBounds.mTop - padding,
- bgBounds.mRight + padding, bgBounds.mBottom + padding,
- outlinePaint);
+ const SkFont& font = paint.getSkFont();
+ auto padding = kHighContrastTextBorderWidth + 0.1f * font.getSize();
+
+ // Draw the background only behind each glyph's bounds. We do this instead of using
+ // the bounds of the entire layout, because the layout includes alignment whitespace
+ // etc which can obscure other text from separate passes (e.g. emojis).
+ // Merge all the glyph bounds into one rect for this line, since drawing a rect for
+ // each glyph is expensive.
+ SkRect glyphBounds;
+ SkRect bgBounds;
+ for (size_t i = start; i < end; i++) {
+ auto glyph = layout.getGlyphId(i);
+
+ font.getBounds(reinterpret_cast<const SkGlyphID*>(&glyph), 1, &glyphBounds,
+ &paint);
+ glyphBounds.offset(layout.getX(i), layout.getY(i));
+
+ bgBounds.join(glyphBounds);
+ }
+
+ if (!bgBounds.isEmpty()) {
+ bgBounds.offset(x, y);
+ bgBounds.outset(padding, padding);
+ canvas->drawRect(bgBounds.fLeft, bgBounds.fTop, bgBounds.fRight,
+ bgBounds.fBottom, outlinePaint);
+ }
} else {
canvas->drawGlyphs(glyphFunc, glyphCount, outlinePaint, x, y, totalAdvance);
}
@@ -169,7 +189,6 @@
float totalAdvance;
float underlinePosition;
float underlineThickness;
- const minikin::MinikinRect& bounds;
};
} // namespace android
diff --git a/libs/hwui/tests/unit/UnderlineTest.cpp b/libs/hwui/tests/unit/UnderlineTest.cpp
index 9911bfa..c70a304 100644
--- a/libs/hwui/tests/unit/UnderlineTest.cpp
+++ b/libs/hwui/tests/unit/UnderlineTest.cpp
@@ -103,9 +103,8 @@
// Create minikin::Layout
std::unique_ptr<Typeface> typeface(makeTypeface());
minikin::Layout layout = doLayout(text, *paint, typeface.get());
- minikin::MinikinRect bounds;
- DrawTextFunctor f(layout, &canvas, *paint, 0, 0, layout.getAdvance(), bounds);
+ DrawTextFunctor f(layout, &canvas, *paint, 0, 0, layout.getAdvance());
MinikinUtils::forFontRun(layout, paint, f);
return f;
}