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

3 files changed, 31 insertions, 20 deletions

diff --git a/libs/hwui/hwui/Canvas.cpp b/libs/hwui/hwui/Canvas.cpp
index e9f4b81c7624..80b6c0385fca 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 @@ void Canvas::drawText(const uint16_t* text, int textSize, int start, int count,
     // 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 ba6543988a7b..02bf0d8d5e95 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 @@ public:
      * @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 @@ public:
             , 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 @@ public:
             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 @@ private:
     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 9911bfa70443..c70a30477ecf 100644
--- a/libs/hwui/tests/unit/UnderlineTest.cpp
+++ b/libs/hwui/tests/unit/UnderlineTest.cpp
@@ -103,9 +103,8 @@ DrawTextFunctor processFunctor(const std::vector<uint16_t>& text, Paint* paint)
     // 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;
 }