fix(force invert): fix white-on-white text when text is drawn against a fill rect in the same RenderNode

This fixes issues with custom Views that draw background and text in the
same View/RenderNode. Instead of trying to force the individual paints
to be dark, we simply invert the whole view.

Adds a new hasFill() member to DisplayList and Canvas so that we record
when the RenderNode has a call that fills the background.

Also adds a new Container usage hint type, for this exact situation.

Bug: 282821643
Bug: 293883260
Test: RenderNodeTests.cpp:
  mmm -j8 frameworks/base/libs/hwui && adb push $ANDROID_PRODUCT_OUT/data/nativetest/hwui_unit_tests/hwui_unit_tests     /data/nativetest/hwui_unit_tests/hwui_unit_tests && adb shell /data/nativetest/hwui_unit_tests/hwui_unit_tests

Change-Id: Ia0fe9c739bf07c1f3a8508d3f295a1dc54ee48f9

8 files changed, 94 insertions, 4 deletions

diff --git a/libs/hwui/CanvasTransform.cpp b/libs/hwui/CanvasTransform.cpp
index cd4fae86aa52..b667daf9c850 100644
--- a/libs/hwui/CanvasTransform.cpp
+++ b/libs/hwui/CanvasTransform.cpp
@@ -80,6 +80,19 @@ SkColor transformColorInverse(ColorTransform transform, SkColor color) {
 static void applyColorTransform(ColorTransform transform, SkPaint& paint) {
     if (transform == ColorTransform::None) return;
 
+    if (transform == ColorTransform::Invert) {
+        auto filter = SkHighContrastFilter::Make(
+                {/* grayscale= */ false, SkHighContrastConfig::InvertStyle::kInvertLightness,
+                 /* contrast= */ 0.0f});
+
+        if (paint.getColorFilter()) {
+            paint.setColorFilter(SkColorFilters::Compose(filter, paint.refColorFilter()));
+        } else {
+            paint.setColorFilter(filter);
+        }
+        return;
+    }
+
     SkColor newColor = transformColor(transform, paint.getColor());
     paint.setColor(newColor);
 
diff --git a/libs/hwui/CanvasTransform.h b/libs/hwui/CanvasTransform.h
index 291f4cf7193b..288dca4de5c1 100644
--- a/libs/hwui/CanvasTransform.h
+++ b/libs/hwui/CanvasTransform.h
@@ -29,12 +29,15 @@ enum class UsageHint {
     Unknown = 0,
     Background = 1,
     Foreground = 2,
+    // Contains foreground (usually text), like a button or chip
+    Container = 3
 };
 
 enum class ColorTransform {
     None,
     Light,
     Dark,
+    Invert
 };
 
 // True if the paint was modified, false otherwise
diff --git a/libs/hwui/DisplayList.h b/libs/hwui/DisplayList.h
index 8c180da9c84f..b1c5bf49ede5 100644
--- a/libs/hwui/DisplayList.h
+++ b/libs/hwui/DisplayList.h
@@ -145,6 +145,8 @@ public:
         return mImpl && mImpl->hasText();
     }
 
+    [[nodiscard]] bool hasFill() const { return mImpl && mImpl->hasFill(); }
+
     void applyColorTransform(ColorTransform transform) {
         if (mImpl) {
             mImpl->applyColorTransform(transform);
diff --git a/libs/hwui/RecordingCanvas.cpp b/libs/hwui/RecordingCanvas.cpp
index ff0d8d74831c..3b694c5d399b 100644
--- a/libs/hwui/RecordingCanvas.cpp
+++ b/libs/hwui/RecordingCanvas.cpp
@@ -718,6 +718,27 @@ static constexpr inline bool is_power_of_two(int value) {
     return (value & (value - 1)) == 0;
 }
 
+template <typename T>
+constexpr bool doesPaintHaveFill(T& paint) {
+    using T1 = std::remove_cv_t<T>;
+    if constexpr (std::is_same_v<T1, SkPaint>) {
+        return paint.getStyle() != SkPaint::Style::kStroke_Style;
+    } else if constexpr (std::is_same_v<T1, SkPaint&>) {
+        return paint.getStyle() != SkPaint::Style::kStroke_Style;
+    } else if constexpr (std::is_same_v<T1, SkPaint*>) {
+        return paint && paint->getStyle() != SkPaint::Style::kStroke_Style;
+    } else if constexpr (std::is_same_v<T1, const SkPaint*>) {
+        return paint && paint->getStyle() != SkPaint::Style::kStroke_Style;
+    }
+
+    return false;
+}
+
+template <typename... Args>
+constexpr bool hasPaintWithFill(Args&&... args) {
+    return (... || doesPaintHaveFill(args));
+}
+
 template <typename T, typename... Args>
 void* DisplayListData::push(size_t pod, Args&&... args) {
     size_t skip = SkAlignPtr(sizeof(T) + pod);
@@ -736,6 +757,14 @@ void* DisplayListData::push(size_t pod, Args&&... args) {
     new (op) T{std::forward<Args>(args)...};
     op->type = (uint32_t)T::kType;
     op->skip = skip;
+
+    // check if this is a fill op or not, in case we need to avoid messing with it with force invert
+    if constexpr (!std::is_same_v<T, DrawTextBlob>) {
+        if (hasPaintWithFill(args...)) {
+            mHasFill = true;
+        }
+    }
+
     return op + 1;
 }
 
diff --git a/libs/hwui/RecordingCanvas.h b/libs/hwui/RecordingCanvas.h
index 4f54ee286a56..afadbfda7471 100644
--- a/libs/hwui/RecordingCanvas.h
+++ b/libs/hwui/RecordingCanvas.h
@@ -110,7 +110,7 @@ class RecordingCanvas;
 
 class DisplayListData final {
 public:
-    DisplayListData() : mHasText(false) {}
+    DisplayListData() : mHasText(false), mHasFill(false) {}
     ~DisplayListData();
 
     void draw(SkCanvas* canvas) const;
@@ -121,6 +121,7 @@ public:
     void applyColorTransform(ColorTransform transform);
 
     bool hasText() const { return mHasText; }
+    bool hasFill() const { return mHasFill; }
     size_t usedSize() const { return fUsed; }
     size_t allocatedSize() const { return fReserved; }
 
@@ -192,6 +193,7 @@ private:
     size_t fReserved = 0;
 
     bool mHasText : 1;
+    bool mHasFill : 1;
 };
 
 class RecordingCanvas final : public SkCanvasVirtualEnforcer<SkNoDrawCanvas> {
diff --git a/libs/hwui/RenderNode.cpp b/libs/hwui/RenderNode.cpp
index 3e131bc44d39..0b42c88aa448 100644
--- a/libs/hwui/RenderNode.cpp
+++ b/libs/hwui/RenderNode.cpp
@@ -427,7 +427,13 @@ void RenderNode::handleForceDark(android::uirenderer::TreeInfo *info) {
         children.push_back(node);
     });
     if (mDisplayList.hasText()) {
-        usage = UsageHint::Foreground;
+        if (mDisplayList.hasFill()) {
+            // Handle a special case for custom views that draw both text and background in the
+            // same RenderNode, which would otherwise be altered to white-on-white text.
+            usage = UsageHint::Container;
+        } else {
+            usage = UsageHint::Foreground;
+        }
     }
     if (usage == UsageHint::Unknown) {
         if (children.size() > 1) {
@@ -453,8 +459,13 @@ void RenderNode::handleForceDark(android::uirenderer::TreeInfo *info) {
             drawn.join(bounds);
         }
     }
-    mDisplayList.applyColorTransform(
-            usage == UsageHint::Background ? ColorTransform::Dark : ColorTransform::Light);
+
+    if (usage == UsageHint::Container) {
+        mDisplayList.applyColorTransform(ColorTransform::Invert);
+    } else {
+        mDisplayList.applyColorTransform(usage == UsageHint::Background ? ColorTransform::Dark
+                                                                        : ColorTransform::Light);
+    }
 }
 
 void RenderNode::pushStagingDisplayListChanges(TreeObserver& observer, TreeInfo& info) {
diff --git a/libs/hwui/pipeline/skia/SkiaDisplayList.h b/libs/hwui/pipeline/skia/SkiaDisplayList.h
index e5bd5c9b2a3b..b9dc1c49f09e 100644
--- a/libs/hwui/pipeline/skia/SkiaDisplayList.h
+++ b/libs/hwui/pipeline/skia/SkiaDisplayList.h
@@ -96,6 +96,8 @@ public:
 
     bool hasText() const { return mDisplayList.hasText(); }
 
+    bool hasFill() const { return mDisplayList.hasFill(); }
+
     /**
      * Attempts to reset and reuse this DisplayList.
      *
diff --git a/libs/hwui/tests/unit/RenderNodeTests.cpp b/libs/hwui/tests/unit/RenderNodeTests.cpp
index 8273524167f9..e727ea899098 100644
--- a/libs/hwui/tests/unit/RenderNodeTests.cpp
+++ b/libs/hwui/tests/unit/RenderNodeTests.cpp
@@ -331,3 +331,31 @@ RENDERTHREAD_TEST(DISABLED_RenderNode, prepareTree_HwLayer_AVD_enqueueDamage) {
     EXPECT_EQ(uirenderer::Rect(0, 0, 200, 400), info.layerUpdateQueue->entries().at(0).damage);
     canvasContext->destroy();
 }
+
+TEST(RenderNode, hasNoFill) {
+    auto rootNode =
+            TestUtils::createNode(0, 0, 200, 400, [](RenderProperties& props, Canvas& canvas) {
+                Paint paint;
+                paint.setStyle(SkPaint::Style::kStroke_Style);
+                canvas.drawRect(10, 10, 100, 100, paint);
+            });
+
+    TestUtils::syncHierarchyPropertiesAndDisplayList(rootNode);
+
+    EXPECT_FALSE(rootNode.get()->getDisplayList().hasFill());
+    EXPECT_FALSE(rootNode.get()->getDisplayList().hasText());
+}
+
+TEST(RenderNode, hasFill) {
+    auto rootNode =
+            TestUtils::createNode(0, 0, 200, 400, [](RenderProperties& props, Canvas& canvas) {
+                Paint paint;
+                paint.setStyle(SkPaint::kStrokeAndFill_Style);
+                canvas.drawRect(10, 10, 100, 100, paint);
+            });
+
+    TestUtils::syncHierarchyPropertiesAndDisplayList(rootNode);
+
+    EXPECT_TRUE(rootNode.get()->getDisplayList().hasFill());
+    EXPECT_FALSE(rootNode.get()->getDisplayList().hasText());
+}